commiting rt 3.8.10 to HEAD

18 files changed, 181 insertions, 102 deletions

diff --git a/rt/lib/RT/Action.pm b/rt/lib/RT/Action.pm
index cb1eeccc8..963b454ac 100755
--- a/rt/lib/RT/Action.pm
+++ b/rt/lib/RT/Action.pm
@@ -230,9 +230,6 @@ sub DESTROY {
 
 # }}}
 
-eval "require RT::Action_Vendor";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/Action_Vendor.pm});
-eval "require RT::Action_Local";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/Action_Local.pm});
+RT::Base->_ImportOverlays();
 
 1;
diff --git a/rt/lib/RT/Action/CreateTickets.pm b/rt/lib/RT/Action/CreateTickets.pm
index 0a7eca3d8..5a1693569 100644
--- a/rt/lib/RT/Action/CreateTickets.pm
+++ b/rt/lib/RT/Action/CreateTickets.pm
@@ -1258,10 +1258,7 @@ sub Options {
   )
 }
 
-eval "require RT::Action::CreateTickets_Vendor";
-die $@ if ( $@ && $@ !~ qr{^Can't locate RT/Action/CreateTickets_Vendor.pm} );
-eval "require RT::Action::CreateTickets_Local";
-die $@ if ( $@ && $@ !~ qr{^Can't locate RT/Action/CreateTickets_Local.pm} );
+RT::Base->_ImportOverlays();
 
 1;
 
diff --git a/rt/lib/RT/Action/EscalatePriority.pm b/rt/lib/RT/Action/EscalatePriority.pm
index 94d6e76f0..e15e50c84 100644
--- a/rt/lib/RT/Action/EscalatePriority.pm
+++ b/rt/lib/RT/Action/EscalatePriority.pm
@@ -163,9 +163,6 @@ sub Commit {
    }
 }
 
-eval "require RT::Action::EscalatePriority_Vendor";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/Action/EscalatePriority_Vendor.pm});
-eval "require RT::Action::EscalatePriority_Local";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/Action/EscalatePriority_Local.pm});
+RT::Base->_ImportOverlays();
 
 1;
diff --git a/rt/lib/RT/Condition.pm b/rt/lib/RT/Condition.pm
index 2774fe823..2e0a94b92 100755
--- a/rt/lib/RT/Condition.pm
+++ b/rt/lib/RT/Condition.pm
@@ -238,9 +238,6 @@ sub DESTROY {
 
 # }}}
 
-eval "require RT::Condition_Vendor";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/Condition_Vendor.pm});
-eval "require RT::Condition_Local";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/Condition_Local.pm});
+RT::Base->_ImportOverlays();
 
 1;
diff --git a/rt/lib/RT/Config.pm b/rt/lib/RT/Config.pm
index e3bdbe90b..008afba7c 100644
--- a/rt/lib/RT/Config.pm
+++ b/rt/lib/RT/Config.pm
@@ -908,10 +908,11 @@ sub Meta {
 sub Sections {
     my $self = shift;
     my %seen;
-    return sort
+    my @sections = sort
         grep !$seen{$_}++,
         map $_->{'Section'} || 'General',
         values %META;
+    return @sections;
 }
 
 sub Options {
@@ -940,14 +941,6 @@ sub Options {
     return @res;
 }
 
-eval "require RT::Config_Vendor";
-if ($@ && $@ !~ qr{^Can't locate RT/Config_Vendor.pm}) {
-    die $@;
-};
-
-eval "require RT::Config_Local";
-if ($@ && $@ !~ qr{^Can't locate RT/Config_Local.pm}) {
-    die $@;
-};
+RT::Base->_ImportOverlays();
 
 1;
diff --git a/rt/lib/RT/CustomField.pm b/rt/lib/RT/CustomField.pm
index e2563481f..c018356b2 100644
--- a/rt/lib/RT/CustomField.pm
+++ b/rt/lib/RT/CustomField.pm
@@ -388,24 +388,7 @@ sub _CoreAccessible {
  }
 };
 
-
-        eval "require RT::CustomField_Overlay";
-        if ($@ && $@ !~ qr{^Can't locate RT/CustomField_Overlay.pm}) {
-            die $@;
-        };
-
-        eval "require RT::CustomField_Vendor";
-        if ($@ && $@ !~ qr{^Can't locate RT/CustomField_Vendor.pm}) {
-            die $@;
-        };
-
-        eval "require RT::CustomField_Local";
-        if ($@ && $@ !~ qr{^Can't locate RT/CustomField_Local.pm}) {
-            die $@;
-        };
-
-
-
+RT::Base->_ImportOverlays();
 
 =head1 SEE ALSO
 
diff --git a/rt/lib/RT/Date.pm b/rt/lib/RT/Date.pm
index e68526c07..384b74abc 100644
--- a/rt/lib/RT/Date.pm
+++ b/rt/lib/RT/Date.pm
@@ -1099,9 +1099,6 @@ sub Timezone {
 }
 
 
-eval "require RT::Date_Vendor";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/Date_Vendor.pm});
-eval "require RT::Date_Local";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/Date_Local.pm});
+RT::Base->_ImportOverlays();
 
 1;
diff --git a/rt/lib/RT/Interface/Web.pm b/rt/lib/RT/Interface/Web.pm
index 8dcacb14f..ff9d9b635 100644
--- a/rt/lib/RT/Interface/Web.pm
+++ b/rt/lib/RT/Interface/Web.pm
@@ -195,6 +195,8 @@ sub HandleRequest {
     # Process session-related callbacks before any auth attempts
     $HTML::Mason::Commands::m->callback( %$ARGS, CallbackName => 'Session', CallbackPage => '/autohandler' );
 
+    MaybeRejectPrivateComponentRequest();
+
     MaybeShowNoAuthPage($ARGS);
 
     AttemptExternalAuth($ARGS) if RT->Config->Get('WebExternalAuthContinuous') or not _UserLoggedIn();
@@ -412,6 +414,37 @@ sub MaybeShowNoAuthPage {
     $m->abort;
 }
 
+=head2 MaybeRejectPrivateComponentRequest
+
+This function will reject calls to private components, like those under
+C</Elements>. If the requested path is a private component then we will
+abort with a C<403> error.
+
+=cut
+
+sub MaybeRejectPrivateComponentRequest {
+    my $m = $HTML::Mason::Commands::m;
+    my $path = $m->request_comp->path;
+
+    # We do not check for dhandler here, because requesting our dhandlers
+    # directly is okay. Mason will invoke the dhandler with a dhandler_arg of
+    # 'dhandler'.
+
+    if ($path =~ m{
+            / # leading slash
+            ( Elements    |
+              _elements   | # mobile UI
+              Widgets     |
+              autohandler | # requesting this directly is suspicious
+              l           ) # loc component
+            ( $ | / ) # trailing slash or end of path
+        }xi) {
+            $m->abort(403);
+    }
+
+    return;
+}
+
 =head2 ShowRequestedPage  \%ARGS
 
 This function, called exclusively by RT's autohandler, dispatches
@@ -796,8 +829,15 @@ sub SendStaticFile {
         }
         $type ||= "application/octet-stream";
     }
+
+    # CGI.pm version 3.51 and 3.52 bang charset=iso-8859-1 onto our JS
+    # since we don't specify a charset
+    if ( $type =~ m{application/javascript} &&
+         $type !~ m{charset=([\w-]+)$} ) {
+         $type .= "; charset=utf-8";
+    }
     $HTML::Mason::Commands::r->content_type($type);
-    open my $fh, "<$file" or die "couldn't open file: $!";
+    open( my $fh, '<', $file ) or die "couldn't open file: $!";
     binmode($fh);
     {
         local $/ = \16384;
@@ -841,8 +881,13 @@ sub StripContent {
     # Check for plaintext sig
     return '' if not $html and $content =~ /^(--)?\Q$sig\E$/;
 
-    # Check for html-formatted sig
-    RT::Interface::Web::EscapeUTF8( \$sig );
+    # Check for html-formatted sig; we don't use EscapeUTF8 here
+    # because we want to precisely match the escaping that FCKEditor
+    # uses. see also 311223f5, which fixed this for 4.0
+    $sig =~ s/&/&amp;/g;
+    $sig =~ s/</&lt;/g;
+    $sig =~ s/>/&gt;/g;
+
     return ''
       if $html
           and $content =~ m{^(?:<p>)?(--)?\Q$sig\E(?:</p>)?$}s;
@@ -2272,9 +2317,6 @@ sub _parse_saved_search {
     return ( _load_container_object( $obj_type, $obj_id ), $search_id );
 }
 
-eval "require RT::Interface::Web_Vendor";
-die $@ if ( $@ && $@ !~ qr{^Can't locate RT/Interface/Web_Vendor.pm} );
-eval "require RT::Interface::Web_Local";
-die $@ if ( $@ && $@ !~ qr{^Can't locate RT/Interface/Web_Local.pm} );
+RT::Base->_ImportOverlays();
 
 1;
diff --git a/rt/lib/RT/Principal_Overlay.pm b/rt/lib/RT/Principal_Overlay.pm
index f46525269..a8e8f3c42 100644
--- a/rt/lib/RT/Principal_Overlay.pm
+++ b/rt/lib/RT/Principal_Overlay.pm
@@ -220,9 +220,9 @@ sub RevokeRight {
 
 # }}}
 
-# {{{ sub _CleanupInvalidDelegations
+# {{{ sub CleanupInvalidDelegations
 
-=head2 sub _CleanupInvalidDelegations { InsideTransaction => undef }
+=head2 sub CleanupInvalidDelegations { InsideTransaction => undef }
 
 Revokes all ACE entries delegated by this principal which are
 inconsistent with this principal's current delegation rights.  Does
@@ -244,15 +244,19 @@ and logs an internal error if the deletion fails (should not happen).
 # This is currently just a stub for the methods of the same name in
 # RT::User and RT::Group.
 
-sub _CleanupInvalidDelegations {
+# backcompat for 3.8.8 and before
+*_CleanupInvalidDelegations = \&CleanupInvalidDelegations;
+
+sub CleanupInvalidDelegations {
     my $self = shift;
     unless ( $self->Id ) {
 	$RT::Logger->warning("Principal not loaded.");
 	return (undef);
     }
-    return ($self->Object->_CleanupInvalidDelegations(@_));
+    return ($self->Object->CleanupInvalidDelegations(@_));
 }
 
+
 # }}}
 
 # {{{ sub HasRight
diff --git a/rt/lib/RT/Record.pm b/rt/lib/RT/Record.pm
index ce46a90a6..121c08686 100755
--- a/rt/lib/RT/Record.pm
+++ b/rt/lib/RT/Record.pm
@@ -1988,9 +1988,6 @@ sub WikiBase {
     return RT->Config->Get('WebPath'). "/index.html?q=";
 }
 
-eval "require RT::Record_Vendor";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/Record_Vendor.pm});
-eval "require RT::Record_Local";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/Record_Local.pm});
+RT::Base->_ImportOverlays();
 
 1;
diff --git a/rt/lib/RT/Search/Googleish.pm b/rt/lib/RT/Search/Googleish.pm
index 4232b434b..2e26a8da6 100644
--- a/rt/lib/RT/Search/Googleish.pm
+++ b/rt/lib/RT/Search/Googleish.pm
@@ -204,9 +204,6 @@ sub Prepare  {
 }
 # }}}
 
-eval "require RT::Search::Googleish_Vendor";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/Search/Googleish_Vendor.pm});
-eval "require RT::Search::Googleish_Local";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/Search/Googleish_Local.pm});
+RT::Base->_ImportOverlays();
 
 1;
diff --git a/rt/lib/RT/SearchBuilder.pm b/rt/lib/RT/SearchBuilder.pm
index da542ea4e..ec4a223c0 100644
--- a/rt/lib/RT/SearchBuilder.pm
+++ b/rt/lib/RT/SearchBuilder.pm
@@ -85,6 +85,17 @@ sub _Init  {
     $self->SUPER::_Init( 'Handle' => $RT::Handle);
 }
 
+sub OrderByCols {
+    my $self = shift;
+    my @sort;
+    for my $s (@_) {
+        next if defined $s->{FIELD} and $s->{FIELD} =~ /\W/;
+        $s->{FIELD} = $s->{FUNCTION} if $s->{FUNCTION};
+        push @sort, $s;
+    }
+    return $self->SUPER::OrderByCols( @sort );
+}
+
 =head2 LimitToEnabled
 
 Only find items that haven't been disabled
@@ -274,14 +285,47 @@ This Limit sub calls SUPER::Limit, but defaults "CASESENSITIVE" to 1, thus
 making sure that by default lots of things don't do extra work trying to 
 match lower(colname) agaist lc($val);
 
+We also force VALUE to C<NULL> when the OPERATOR is C<IS> or C<IS NOT>.
+This ensures that we don't pass invalid SQL to the database or allow SQL
+injection attacks when we pass through user specified values.
+
 =cut
 
 sub Limit {
     my $self = shift;
-    my %args = ( CASESENSITIVE => 1,
-                 @_ );
+    my %ARGS = (
+        CASESENSITIVE => 1,
+        OPERATOR => '=',
+        @_,
+    );
 
-    return $self->SUPER::Limit(%args);
+    # We use the same regex here that DBIx::SearchBuilder uses to exclude
+    # values from quoting
+    if ( $ARGS{'OPERATOR'} =~ /IS/i ) {
+        # Don't pass anything but NULL for IS and IS NOT
+        $ARGS{'VALUE'} = 'NULL';
+    }
+
+    if ($ARGS{FUNCTION}) {
+        ($ARGS{ALIAS}, $ARGS{FIELD}) = split /\./, delete $ARGS{FUNCTION}, 2;
+        $self->SUPER::Limit(%ARGS);
+    } elsif ($ARGS{FIELD} =~ /\W/
+          or $ARGS{OPERATOR} !~ /^(=|<|>|!=|<>|<=|>=
+                                  |(NOT\s*)?LIKE
+                                  |(NOT\s*)?(STARTS|ENDS)WITH
+                                  |(NOT\s*)?MATCHES
+                                  |IS(\s*NOT)?
+                                  |IN)$/ix) {
+        $RT::Logger->crit("Possible SQL injection attack: $ARGS{FIELD} $ARGS{OPERATOR}");
+        $self->SUPER::Limit(
+            %ARGS,
+            FIELD    => 'id',
+            OPERATOR => '<',
+            VALUE    => '0',
+        );
+    } else {
+        $self->SUPER::Limit(%ARGS);
+    }
 }
 
 =head2 ItemsOrderBy
@@ -345,9 +389,6 @@ sub _DoCount {
     return $self->SUPER::_DoCount(@_);
 }
 
-eval "require RT::SearchBuilder_Vendor";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/SearchBuilder_Vendor.pm});
-eval "require RT::SearchBuilder_Local";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/SearchBuilder_Local.pm});
+RT::Base->_ImportOverlays();
 
 1;
diff --git a/rt/lib/RT/System.pm b/rt/lib/RT/System.pm
index 8e5f96368..588be3e0d 100644
--- a/rt/lib/RT/System.pm
+++ b/rt/lib/RT/System.pm
@@ -211,9 +211,6 @@ sub QueueCacheNeedsUpdate {
     }
 }
 
-eval "require RT::System_Vendor";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/System_Vendor.pm});
-eval "require RT::System_Local";
-die $@ if ($@ && $@ !~ qr{^Can't locate RT/System_Local.pm});
+RT::Base->_ImportOverlays();
 
 1;
diff --git a/rt/lib/RT/Test.pm b/rt/lib/RT/Test.pm
index 9954ec729..f7f3bf9a4 100644
--- a/rt/lib/RT/Test.pm
+++ b/rt/lib/RT/Test.pm
@@ -75,7 +75,7 @@ wrap 'HTTP::Request::Common::form_data',
    };
 
 
-our @EXPORT = qw(is_empty);
+our @EXPORT = qw(is_empty parse_mail);
 our ($port, $dbname);
 our @SERVERS;
 
@@ -217,7 +217,7 @@ sub bootstrap_config {
     $tmp{'config'}{'RT'} = File::Spec->catfile(
         "$tmp{'directory'}", 'RT_SiteConfig.pm'
     );
-    open my $config, '>', $tmp{'config'}{'RT'}
+    open( my $config, '>', $tmp{'config'}{'RT'} )
         or die "Couldn't open $tmp{'config'}{'RT'}: $!";
 
     print $config qq{
@@ -246,7 +246,7 @@ Set( \$RTAddressRegexp , qr/^bad_re_that_doesnt_match\$/);
 Set( \$MailCommand, sub {
     my \$MIME = shift;
 
-    open my \$handle, '>>', '$mail_catcher'
+    open( my \$handle, '>>', '$mail_catcher' )
         or die "Unable to open '$mail_catcher' for appending: \$!";
 
     \$MIME->print(\$handle);
@@ -272,7 +272,7 @@ sub bootstrap_logging {
     $tmp{'log'}{'RT'} = File::Spec->catfile(
         "$tmp{'directory'}", 'rt.debug.log'
     );
-    open my $fh, '>', $tmp{'log'}{'RT'}
+    open( my $fh, '>', $tmp{'log'}{'RT'} )
         or die "Couldn't open $tmp{'config'}{'RT'}: $!";
     # make world writable so apache under different user
     # can write into it
@@ -303,7 +303,7 @@ sub set_config_wrapper {
                 SCALAR => '$',
             );
             my $sigil = $sigils{$type} || $sigils{'SCALAR'};
-            open my $fh, '>>', $tmp{'config'}{'RT'}
+            open( my $fh, '>>', $tmp{'config'}{'RT'} )
                 or die "Couldn't open config file: $!";
             require Data::Dumper;
             my $dump = Data::Dumper::Dumper([@_[2 .. $#_]]);
@@ -774,7 +774,7 @@ sub open_mailgate_ok {
     my $baseurl = shift;
     my $queue   = shift || 'general';
     my $action  = shift || 'correspond';
-    Test::More::ok(open(my $mail, "|$RT::BinPath/rt-mailgate --url $baseurl --queue $queue --action $action"), "Opened the mailgate - $!");
+    Test::More::ok(open(my $mail, '|-', "$RT::BinPath/rt-mailgate --url $baseurl --queue $queue --action $action"), "Opened the mailgate - $!");
     return $mail;
 }
 
@@ -1072,7 +1072,7 @@ sub start_apache_server {
     my %info = $self->apache_server_info( variant => $variant );
 
     Test::More::diag(do {
-        open my $fh, '<', $tmp{'config'}{'RT'};
+        open( my $fh, '<', $tmp{'config'}{'RT'} ) or die $!;
         local $/;
         <$fh>
     });
@@ -1118,7 +1118,7 @@ sub start_apache_server {
         }
         Test::More::BAIL_OUT("Couldn't start apache server, no pid file")
             unless -e $opt{'pid_file'};
-        open my $pid_fh, '<', $opt{'pid_file'}
+        open( my $pid_fh, '<', $opt{'pid_file'} )
             or Test::More::BAIL_OUT("Couldn't open pid file: $!");
         my $pid = <$pid_fh>;
         chomp $pid;
@@ -1230,7 +1230,7 @@ sub file_content {
 
     Test::More::diag "reading content of '$path'" if $ENV{'TEST_VERBOSE'};
 
-    open my $fh, "<:raw", $path
+    open( my $fh, "<:raw", $path )
         or do {
             warn "couldn't open file '$path': $!" unless $args{noexist};
             return ''
@@ -1289,7 +1289,7 @@ sub process_in_file {
         ($out_fh, $out_conf) = tempfile();
     } else {
         $out_conf = $args{'out'};
-        open $out_fh, '>', $out_conf
+        open( $out_fh, '>', $out_conf )
             or die "couldn't open '$out_conf': $!";
     }
     print $out_fh $text;
@@ -1298,6 +1298,14 @@ sub process_in_file {
     return ($out_fh, $out_conf);
 }
 
+sub parse_mail {
+    my $mail = shift;
+    require RT::EmailParser;
+    my $parser = RT::EmailParser->new;
+    $parser->ParseMIMEEntityFromScalar( $mail );
+    return $parser->Entity;
+}
+
 END {
     my $Test = RT::Test->builder;
     return if $Test->{Original_Pid} != $$;
diff --git a/rt/lib/RT/Ticket_Overlay.pm b/rt/lib/RT/Ticket_Overlay.pm
index 3ab31d3c2..2feed28dd 100644
--- a/rt/lib/RT/Ticket_Overlay.pm
+++ b/rt/lib/RT/Ticket_Overlay.pm
@@ -1014,15 +1014,14 @@ sub Import {
 
     $self->OwnerGroup->_AddMember( PrincipalId => $Owner->PrincipalId );
 
-    my $watcher;
-    foreach $watcher ( @{ $args{'Cc'} } ) {
+    foreach my $watcher ( @{ $args{'Cc'} } ) {
         $self->_AddWatcher( Type => 'Cc', Email => $watcher, Silent => 1 );
     }
-    foreach $watcher ( @{ $args{'AdminCc'} } ) {
+    foreach my $watcher ( @{ $args{'AdminCc'} } ) {
         $self->_AddWatcher( Type => 'AdminCc', Email => $watcher,
             Silent => 1 );
     }
-    foreach $watcher ( @{ $args{'Requestor'} } ) {
+    foreach my $watcher ( @{ $args{'Requestor'} } ) {
         $self->_AddWatcher( Type => 'Requestor', Email => $watcher,
             Silent => 1 );
     }
diff --git a/rt/lib/RT/Tickets_Overlay.pm b/rt/lib/RT/Tickets_Overlay.pm
index ffbbc8539..c6cb9ab7e 100644
--- a/rt/lib/RT/Tickets_Overlay.pm
+++ b/rt/lib/RT/Tickets_Overlay.pm
@@ -150,6 +150,13 @@ our %FIELD_METADATA = (
     Tagnum           => [ 'FREESIDEFIELD', 'cust_tag' ],
 );
 
+our %SEARCHABLE_SUBFIELDS = (
+    User => [qw(
+        EmailAddress Name RealName Nickname Organization Address1 Address2
+        WorkPhone HomePhone MobilePhone PagerPhone id
+    )],
+);
+
 # Mapping of Field Type to Function
 our %dispatch = (
     ENUM            => \&_EnumLimit,
@@ -837,6 +844,13 @@ sub _WatcherLimit {
     my $type = $meta->[1] || '';
     my $class = $meta->[2] || 'Ticket';
 
+    # Bail if the subfield is not allowed
+    if (    $rest{SUBKEY}
+        and not grep { $_ eq $rest{SUBKEY} } @{$SEARCHABLE_SUBFIELDS{'User'}})
+    {
+        die "Invalid watcher subfield: '$rest{SUBKEY}'";
+    }
+
     # Owner was ENUM field, so "Owner = 'xxx'" allowed user to
     # search by id and Name at the same time, this is workaround
     # to preserve backward compatibility
@@ -1235,7 +1249,7 @@ Try and turn a CF descriptor into (cfid, cfname) object pair.
 sub _CustomFieldDecipher {
     my ($self, $string) = @_;
 
-    my ($queue, $field, $column) = ($string =~ /^(?:(.+?)\.)?{(.+)}(?:\.(.+))?$/);
+    my ($queue, $field, $column) = ($string =~ /^(?:(.+?)\.)?{(.+)}(?:\.(Content|LargeContent))?$/);
     $field ||= ($string =~ /^{(.*?)}$/)[0] || $string;
 
     my $cf;
@@ -1767,9 +1781,20 @@ sub OrderByCols {
            foreach my $uid ( $self->CurrentUser->Id, $RT::Nobody->Id ) {
                if ( RT->Config->Get('DatabaseType') eq 'Oracle' ) {
                    my $f = ($row->{'ALIAS'} || 'main') .'.Owner';
-                   push @res, { %$row, ALIAS => '', FIELD => "CASE WHEN $f=$uid THEN 1 ELSE 0 END", ORDER => $order } ;
+                   push @res, {
+                       %$row,
+                       FIELD => undef,
+                       ALIAS => '',
+                       FUNCTION => "CASE WHEN $f=$uid THEN 1 ELSE 0 END",
+                       ORDER => $order
+                   };
                } else {
-                   push @res, { %$row, FIELD => "Owner=$uid", ORDER => $order } ;
+                   push @res, {
+                       %$row,
+                       FIELD => undef,
+                       FUNCTION => "Owner=$uid",
+                       ORDER => $order
+                   };
                }
            }
 
@@ -3300,9 +3325,9 @@ is a description of the purpose of that TicketRestriction
 sub DescribeRestrictions {
     my $self = shift;
 
-    my ( $row, %listing );
+    my %listing;
 
-    foreach $row ( keys %{ $self->{'TicketRestrictions'} } ) {
+    foreach my $row ( keys %{ $self->{'TicketRestrictions'} } ) {
         $listing{$row} = $self->{'TicketRestrictions'}{$row}{'DESCRIPTION'};
     }
     return (%listing);
@@ -3377,9 +3402,8 @@ sub DeleteRestriction {
 sub _RestrictionsToClauses {
     my $self = shift;
 
-    my $row;
     my %clause;
-    foreach $row ( keys %{ $self->{'TicketRestrictions'} } ) {
+    foreach my $row ( keys %{ $self->{'TicketRestrictions'} } ) {
         my $restriction = $self->{'TicketRestrictions'}{$row};
 
         # We need to reimplement the subclause aggregation that SearchBuilder does.
diff --git a/rt/lib/RT/User_Overlay.pm b/rt/lib/RT/User_Overlay.pm
index 17e9645de..37d138901 100644
--- a/rt/lib/RT/User_Overlay.pm
+++ b/rt/lib/RT/User_Overlay.pm
@@ -1807,7 +1807,7 @@ sub WatchedQueues {
 
 }
 
-=head2 _CleanupInvalidDelegations { InsideTransaction => undef }
+=head2 CleanupInvalidDelegations { InsideTransaction => undef }
 
 Revokes all ACE entries delegated by this user which are inconsistent
 with their current delegation rights.  Does not perform permission
@@ -1821,12 +1821,15 @@ and logs an internal error if the deletion fails (should not happen).
 
 =cut
 
-# XXX Currently there is a _CleanupInvalidDelegations method in both
+# XXX Currently there is a CleanupInvalidDelegations method in both
 # RT::User and RT::Group.  If the recursive cleanup call for groups is
 # ever unrolled and merged, this code will probably want to be
 # factored out into RT::Principal.
 
-sub _CleanupInvalidDelegations {
+# backcompat for 3.8.8 and before
+*_CleanupInvalidDelegations = \&CleanupInvalidDelegations;
+
+sub CleanupInvalidDelegations {
     my $self = shift;
     my %args = ( InsideTransaction => undef,
           @_ );
diff --git a/rt/lib/RT/Users_Overlay.pm b/rt/lib/RT/Users_Overlay.pm
index 4d03b6056..16ec5ed87 100644
--- a/rt/lib/RT/Users_Overlay.pm
+++ b/rt/lib/RT/Users_Overlay.pm
@@ -406,6 +406,12 @@ sub WhoHaveRoleRight
     );
 
     my @objects = $self->_GetEquivObjects( %args );
+
+    # RT::Principal->RolesWithRight only expects EquivObjects, so we need to
+    # fill it.  At the very least it needs $args{Object}, which
+    # _GetEquivObjects above does for us.
+    unshift @{$args{'EquivObjects'}}, @objects;
+
     my @roles = RT::Principal->RolesWithRight( %args );
     unless ( @roles ) {
         $self->_AddSubClause( "WhichRole", "(main.id = 0)" );