[Rt-commit] rt branch, 4.0/uninitialized-warning-rt-principal-hasright, created. rt-4.0.7-58-ga772541

[Jim Brandt]

The branch, 4.0/uninitialized-warning-rt-principal-hasright has been created
        at  a772541a70a508b8e808b850e3828f03216157f2 (commit)

- Log -----------------------------------------------------------------
commit a772541a70a508b8e808b850e3828f03216157f2
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Tue Sep 18 13:14:06 2012 -0400

    Fix uninitialized value warning in RT::Principal::HasRight
    
    HasRight was storing the cananicalized value of a passed-in
    Right argument back in the argument hash, so the debug log output
    on an invalid right had an uninitialized value rather
    than the invalid right argument.
    
    Store the cananicalized right in a new variable to allow the debug
    message to report on the invalid right passed in.
    Also, avoid putting a modified argument back in the arg hash
    which can be misleading.

diff --git a/lib/RT/Principal.pm b/lib/RT/Principal.pm
index 0ee03f1..4d09b46 100644
--- a/lib/RT/Principal.pm
+++ b/lib/RT/Principal.pm
@@ -263,14 +263,14 @@ sub HasRight {
         return 1;
     }
 
-    $args{'Right'} = RT::ACE->CanonicalizeRightName( $args{'Right'} );
-    unless ( $args{'Right'} ) {
+    my $right = RT::ACE->CanonicalizeRightName( $args{'Right'} );
+    unless ( $right ) {
         $RT::Logger->error(
                "Invalid right. Couldn't canonicalize right '$args{'Right'}'");
         return undef;
     }
 
-    return undef if $args{'Right'} eq 'ExecuteCode'
+    return undef if $right eq 'ExecuteCode'
         and RT->Config->Get('DisallowExecuteCode');
 
     $args{'EquivObjects'} = [ @{ $args{'EquivObjects'} } ]
@@ -280,7 +280,7 @@ sub HasRight {
         $RT::Logger->debug(   "Disabled User #"
                             . $self->id
                             . " failed access check for "
-                            . $args{'Right'} );
+                            . $right );
         return (undef);
     }
 
@@ -295,7 +295,7 @@ sub HasRight {
         my $cached = $_ACL_CACHE->fetch(
             $self->id .';:;'. ref($args{'Object'}) .'-'. $args{'Object'}->id
         );
-        return $cached->{'SuperUser'} || $cached->{ $args{'Right'} }
+        return $cached->{'SuperUser'} || $cached->{ $right }
             if $cached;
     }
 
@@ -312,12 +312,12 @@ sub HasRight {
 # 1) full_hashkey - key for any result and for full combination of uid, right and objects
 # 2) short_hashkey - one key for each object to store positive results only, it applies
 # only to direct group rights and partly to role rights
-    my $full_hashkey = join (";:;", $self->id, $args{'Right'});
+    my $full_hashkey = join (";:;", $self->id, $right);
     foreach ( @{ $args{'EquivObjects'} } ) {
         my $ref_id = $self->_ReferenceId($_);
         $full_hashkey .= ";:;".$ref_id;
 
-        my $short_hashkey = join(";:;", $self->id, $args{'Right'}, $ref_id);
+        my $short_hashkey = join(";:;", $self->id, $right, $ref_id);
         my $cached_answer = $_ACL_CACHE->fetch($short_hashkey);
         return $cached_answer > 0 if defined $cached_answer;
     }
@@ -330,7 +330,7 @@ sub HasRight {
     my ( $hitcount, $via_obj ) = $self->_HasRight(%args);
 
     $_ACL_CACHE->set( $full_hashkey => $hitcount ? 1 : -1 );
-    $_ACL_CACHE->set( join(';:;',  $self->id, $args{'Right'},$via_obj) => 1 )
+    $_ACL_CACHE->set( join(';:;',  $self->id, $right,$via_obj) => 1 )
         if $via_obj && $hitcount;
 
     return ($hitcount);
diff --git a/t/api/rights.t b/t/api/rights.t
index a1795ca..68f0aff 100644
--- a/t/api/rights.t
+++ b/t/api/rights.t
@@ -47,11 +47,13 @@
 # 
 # END BPS TAGGED BLOCK }}}
 
-use RT::Test nodata => 1, tests => 30;
+use RT::Test nodata => 1, tests => 34;
 
 use strict;
 use warnings;
 
+use Test::Warn;
+
 # clear all global right
 {
     my $acl = RT::ACL->new(RT->SystemUser);
@@ -195,3 +197,20 @@ my $ticket2;
         "user is not AdminCc and can't modify ticket2 (same question different answer)"
     );
 }
+
+my $queue2 = RT::Test->load_or_create_queue( Name => 'Rights' );
+ok $queue2 && $queue2->id, 'loaded or created queue';
+
+my $user2 = RT::Test->load_or_create_user(
+    Name => 'user2', Password => 'password',
+);
+ok $user2 && $user2->id, 'Created user: ' . $user2->Name . ' with id ' . $user2->Id;
+
+{
+    ok( !$user2->HasRight( Right => 'Foo', Object => $queue2 ),
+        "HasRight false for invalid right Foo"
+    );
+    warning_like {$user2->HasRight( Right => 'Foo', Object => $queue2 )}
+                qr/Invalid right\. Couldn't canonicalize right 'Foo'/,
+                  'Got warning on invalid right';
+}

-----------------------------------------------------------------------