[Rt-commit] rt branch, 4.4/loop-merge-error, created. rt-4.4.1-95-ga99b566

[Shawn Moore]

The branch, 4.4/loop-merge-error has been created
        at  a99b566793518b051577d685432115140bec85d0 (commit)

- Log -----------------------------------------------------------------
commit a99b566793518b051577d685432115140bec85d0
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Fri Aug 5 18:01:07 2016 +0000

    Avoid treating an error message as a principal ID
    
    CanonicalizePrincipal in most cases returns a scalar value: the
    RT::Principal object. However, under error circumstances,
    CanonicalizePrincipal would return a pair of (ok, msg), which the code
    calling CanonicalizePrincipal was not prepared to handle. This lead to the
    error message being used in place of the RT::Principal object, causing
    nonsensical error messages like:
    
        Can't locate object method "Id" via package "recipient at ourdomain.tld is
        an address RT receives mail at. Adding it as a 'Requestor' would create
        a mail loop" (perhaps you forgot to load "recipient at ourdomain.tld is an
        address RT receives mail at. Adding it as a 'Requestor' would create a
        mail loop"?)
    
    This fix instead propagates failures upwards directly from
    RT::Record::Role::Roles->AddRoleMember. RT::Ticket->AddWatcher is the only
    other caller of this method and it handles the (ok, msg) style.
    
    Fixes: I#32237

diff --git a/lib/RT/Record/Role/Roles.pm b/lib/RT/Record/Role/Roles.pm
index 3a4dada..284c6c0 100644
--- a/lib/RT/Record/Role/Roles.pm
+++ b/lib/RT/Record/Role/Roles.pm
@@ -461,7 +461,8 @@ sub AddRoleMember {
     my $self = shift;
     my %args = (@_);
 
-    my $principal = $self->CanonicalizePrincipal(%args);
+    my ($principal, $msg) = $self->CanonicalizePrincipal(%args);
+    return (0, $msg) if !$principal;
 
     my $type = delete $args{Type};
     return (0, $self->loc("That role is invalid for this object"))
@@ -486,7 +487,7 @@ sub AddRoleMember {
     return (0, $self->loc('[_1] cannot be a group', $group->Label) )
                 if $group->SingleMemberRoleGroup and $principal->IsGroup;
 
-    my ( $ok, $msg ) = $group->_AddMember( %args, PrincipalId => $principal->Id, RecordTransaction => !$args{Silent} );
+    ( (my $ok), $msg ) = $group->_AddMember( %args, PrincipalId => $principal->Id, RecordTransaction => !$args{Silent} );
     unless ($ok) {
         $RT::Logger->error("Failed to add principal ".$principal->Id." as a member of group ".$group->Id.": ".$msg);
 

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