Bug #20345 for CGI-Application-Plugin-Authorization: [PATCH] Avoid warnings in SimpleGroup.pm

Fri Jul 07 10:48:14 2006 Guest - Ticket created

Subject:

[PATCH] Avoid warnings in SimpleGroup.pm

This should squelch the annoying "Use of uninitialized value in string eq" warning messages from the authorize_user routine. --- Orig-CGI-Application-Plugin-Authorization-0.05/lib/CGI/Application/Plugin/ Authorization/Driver/SimpleGroup.pm Fri Jun 16 19:50:45 2006 +++ CGI-Application-Plugin-Authorization-0.05/lib/CGI/Application/Plugin/ Authorization/Driver/SimpleGroup.pm Fri Jul 7 10:43:47 2006 @@ -79,7 +79,18 @@ my @groups = @_; foreach my $group (@groups) { - return 1 if ($username eq $group); + # Allow undef == undef (we need this check to avoid warnings) + if ((not defined $username) and (not defined $group)) { + return 1; + } + # But don't allow if only one of the two is undef + # (again, we need this check to avoid warnings) + elsif ((not defined $username) or (not defined $group)) { + return 0; + } + elsif ($username eq $group) { + return 1; + } } return 0; }

Fri Jul 07 12:51:38 2006 cees-perl [...] crtconsulting.ca - Correspondence added

On Fri Jul 07 10:48:14 2006, guest wrote: Show quoted text

> This should squelch the annoying "Use of uninitialized value in string > eq" warning > messages from the authorize_user routine. > > > --- Orig-CGI-Application-Plugin-Authorization- > 0.05/lib/CGI/Application/Plugin/ > Authorization/Driver/SimpleGroup.pm Fri Jun 16 19:50:45 2006 > +++ CGI-Application-Plugin-Authorization- > 0.05/lib/CGI/Application/Plugin/ > Authorization/Driver/SimpleGroup.pm Fri Jul 7 10:43:47 2006 > @@ -79,7 +79,18 @@ > my @groups = @_; > > foreach my $group (@groups) { > - return 1 if ($username eq $group); > + # Allow undef == undef (we need this check to avoid warnings) > + if ((not defined $username) and (not defined $group)) { > + return 1; > + } > + # But don't allow if only one of the two is undef > + # (again, we need this check to avoid warnings) > + elsif ((not defined $username) or (not defined $group)) { > + return 0; > + } > + elsif ($username eq $group) { > + return 1; > + } > } > return 0; > }

If there are uninitialized errors popping up, then I definately want to get rid of them. However, I am not sure if this is the best way to do it. I am confused as to why you want to return a successful authorization if no username or group is passed in (will that situation ever even occur?). Also, with the second test, return 0 if a group is not defined means that any subsequent groups are not tested at all. How about this for a compromise? Does it suffice your needs? sub authorize_user { my $self = shift; my $username = shift; my @groups = @_; return 0 unless defined $username; foreach my $group (@groups) { next unless defined $group; return 1 if ($username eq $group); } return 0; } Cheers, Cees

Fri Jul 07 12:51:39 2006 The RT System itself - Status changed from 'new' to 'open'

Fri Jul 07 16:28:59 2006 matt [...] summersault.com - Correspondence added

Subject:	Re: [rt.cpan.org #20345] [PATCH] Avoid warnings in SimpleGroup.pm
Date:	Fri, 7 Jul 2006 16:28:28 -0400
To:	bug-CGI-Application-Plugin-Authorization [...] rt.cpan.org
From:	Matt Christian <matt [...] summersault.com>

On Friday, July 7, 2006, at 12:51 PM, via RT wrote: Show quoted text

> I am confused as to why you want to return a successful authorization > if > no username or group is passed in (will that situation ever even > occur?).

We thought there were places (in a public "authorize everyone" zone, for example) where the username and group would both be undef and we'd want to return success. Upon further review, however, it turns out we make a separate check for this case before authorize is called, meaning that we never do need the undef == undef case after all. Show quoted text

> Also, with the second test, return 0 if a group is not defined > means that any subsequent groups are not tested at all.

Yep, another good point. Show quoted text

> How about this for a compromise? Does it suffice your needs?

The compromise solution looks great and should suffice nicely. Thanks! --Matt

Sun Mar 02 00:22:29 2008 cees-perl [...] crtconsulting.ca - Correspondence added

Patch has been applied in version 0.07

Sun Mar 02 00:22:31 2008 cees-perl [...] crtconsulting.ca - Status changed from 'open' to 'resolved'