[Rt-commit] rt branch 5.0/use-bind-values created. rt-5.0.1-611-gb1bd4d1d9d
BPS Git Server
git at git.bestpractical.com
Wed Sep 1 14:50:12 UTC 2021
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "rt".
The branch, 5.0/use-bind-values has been created
at b1bd4d1d9daea0f339d0b7683d6022692ce0509e (commit)
- Log -----------------------------------------------------------------
commit b1bd4d1d9daea0f339d0b7683d6022692ce0509e
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed Sep 1 05:27:49 2021 +0800
Refactor _AddSubClause calls to use ordinary Limit calls to use bind values
This is because DBIx::SearchBuilder doesn't extract bind values from
subclauses added via _AddSubClause.
diff --git a/lib/RT/Groups.pm b/lib/RT/Groups.pm
index 80a200a8b6..b48e08ad48 100644
--- a/lib/RT/Groups.pm
+++ b/lib/RT/Groups.pm
@@ -343,9 +343,36 @@ sub ForWhichCurrentUserHasRight {
# ...which are the target object of an ACL with that right, or
# where the target is the system object (a global right)
my $acl = $self->_JoinACL( %args );
- $self->_AddSubClause(
- ACLObjects => "( (main.id = $acl.ObjectId AND $acl.ObjectType = 'RT::Group')"
- . " OR $acl.ObjectType = 'RT::System')");
+ $self->_OpenParen('ACL');
+ $self->Limit(
+ ALIAS => $acl,
+ FIELD => 'ObjectType',
+ OPERATOR => '=',
+ VALUE => 'RT::Group',
+ ENTRYAGGREGATOR => 'OR',
+ SUBCLAUSE => 'ACL',
+ );
+ $self->Limit(
+ ALIAS => $acl,
+ FIELD => 'ObjectId',
+ OPERATOR => '=',
+ VALUE => 'main.id',
+ QUOTEVALUE => 0,
+ ENTRYAGGREGATOR => 'AND',
+ SUBCLAUSE => 'ACL',
+ );
+ $self->_CloseParen('ACL');
+
+ $self->_OpenParen('ACL');
+ $self->Limit(
+ ALIAS => $acl,
+ FIELD => 'ObjectType',
+ OPERATOR => '=',
+ VALUE => 'RT::System',
+ ENTRYAGGREGATOR => 'OR',
+ SUBCLAUSE => 'ACL',
+ );
+ $self->_CloseParen('ACL');
# ...and where that right is granted to any group..
my $member = $self->Join(
diff --git a/lib/RT/Users.pm b/lib/RT/Users.pm
index 990a387e17..fa417c3852 100644
--- a/lib/RT/Users.pm
+++ b/lib/RT/Users.pm
@@ -469,7 +469,11 @@ sub WhoHaveRoleRight
my @roles = RT::Principal->RolesWithRight( %args );
unless ( @roles ) {
- $self->_AddSubClause( "WhichRole", "(main.id = 0)" );
+ $self->Limit(
+ FIELD => 'id',
+ OPERATOR => '=',
+ VALUE => 0,
+ );
return;
}
@@ -482,13 +486,46 @@ sub WhoHaveRoleRight
VALUE => RT->SystemUser->id
);
- $self->_AddSubClause( "WhichRole", "(". join( ' OR ',
- map $RT::Handle->__MakeClauseCaseInsensitive("$groups.Name", '=', "'$_'"), @roles
- ) .")" );
+ $self->_OpenParen('WhichRole');
+ for my $role ( @roles ) {
+ $self->Limit(
+ ALIAS => $groups,
+ FIELD => 'Name',
+ OPERATOR => '=',
+ VALUE => $role,
+ SUBCLAUSE => 'WhichRole',
+ ENTRYAGGREGATOR => 'OR',
+ CASESENSITIVE => 0,
+ );
+ }
+ $self->_CloseParen('WhichRole');
- my @groups_clauses = $self->_RoleClauses( $groups, @objects );
- $self->_AddSubClause( "WhichObject", "(". join( ' OR ', @groups_clauses ) .")" )
- if @groups_clauses;
+ if (@objects) {
+ foreach my $obj (@objects) {
+ $self->_OpenParen('WhichObject');
+ my $type = ref($obj) ? ref($obj) : $obj;
+ $self->Limit(
+ ALIAS => $groups,
+ FIELD => 'Domain',
+ OPERATOR => '=',
+ VALUE => "$type-Role",
+ CASESENSITIVE => 0,
+ ENTRYAGGREGATOR => 'OR',
+ SUBCLAUSE => 'WhichObject',
+ );
+ if ( my $id = eval { $obj->id } ) {
+ $self->Limit(
+ ALIAS => $groups,
+ FIELD => 'Instance',
+ OPERATOR => '=',
+ VALUE => $id,
+ ENTRYAGGREGATOR => 'AND',
+ SUBCLAUSE => 'WhichObject',
+ );
+ }
+ $self->_CloseParen('WhichObject');
+ }
+ }
return;
}
@@ -544,30 +581,45 @@ sub WhoHaveGroupRight
# the one we're looking up or _possibly_ superuser
my $acl = $self->_JoinACL( %args );
- my ($check_objects) = ('');
my @objects = $self->_GetEquivObjects( %args );
my %seen;
if ( @objects ) {
- my @object_clauses;
foreach my $obj ( @objects ) {
my $type = ref($obj)? ref($obj): $obj;
my $id = 0;
$id = $obj->id if ref($obj) && UNIVERSAL::can($obj, 'id') && $obj->id;
next if $seen{"$type-$id"}++;
- my $object_clause = "$acl.ObjectType = '$type'";
- $object_clause .= " AND $acl.ObjectId = $id" if $id;
- push @object_clauses, "($object_clause)";
+ $self->_OpenParen("ACL");
+ $self->Limit(
+ ALIAS => $acl,
+ FIELD => 'ObjectType',
+ OPERATOR => '=',
+ VALUE => $type,
+ ENTRYAGGREGATOR => 'OR',
+ SUBCLAUSE => 'ACL',
+ );
+ if ($id) {
+ $self->Limit(
+ ALIAS => $acl,
+ FIELD => 'ObjectId',
+ OPERATOR => '=',
+ VALUE => $id,
+ ENTRYAGGREGATOR => 'AND',
+ SUBCLAUSE => 'ACL',
+ );
+ }
+ $self->_CloseParen('ACL');
}
-
- $check_objects = join ' OR ', @object_clauses;
} else {
- if( !$args{'IncludeSystemRights'} ) {
- $check_objects = "($acl.ObjectType != 'RT::System')";
- }
+ $self->Limit(
+ ALIAS => $acl,
+ FIELD => 'ObjectType',
+ OPERATOR => '!=',
+ VALUE => 'RT::System',
+ );
}
- $self->_AddSubClause( "WhichObject", "($check_objects)" );
my $group_members = $self->_JoinGroupMembersForGroupRights( %args, ACLAlias => $acl );
# Find only members of groups that have the right.
commit 84772abc6a370db71d84c0a98b13bf201a7b38be
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed Sep 1 05:23:32 2021 +0800
Refactor bare select queries to use bind values
diff --git a/lib/RT/Principal.pm b/lib/RT/Principal.pm
index a49f63aa0e..953867ad70 100644
--- a/lib/RT/Principal.pm
+++ b/lib/RT/Principal.pm
@@ -418,12 +418,10 @@ sub HasRights {
my %res = ();
{
- my $query
- = "SELECT DISTINCT ACL.RightName "
- . $self->_HasGroupRightQuery(
- EquivObjects => $args{'EquivObjects'}
- );
- my $rights = $RT::Handle->dbh->selectcol_arrayref($query);
+ my ( $query, @bind_values ) = $self->_HasGroupRightQuery( EquivObjects => $args{'EquivObjects'} );
+ $query = "SELECT DISTINCT ACL.RightName $query";
+
+ my $rights = $RT::Handle->dbh->selectcol_arrayref($query, undef, @bind_values);
unless ($rights) {
$RT::Logger->warning( $RT::Handle->dbh->errstr );
return ();
@@ -432,12 +430,9 @@ sub HasRights {
}
my $roles;
{
- my $query
- = "SELECT DISTINCT Groups.Name "
- . $self->_HasRoleRightQuery(
- EquivObjects => $args{'EquivObjects'}
- );
- $roles = $RT::Handle->dbh->selectcol_arrayref($query);
+ my ( $query, @bind_values ) = $self->_HasRoleRightQuery( EquivObjects => $args{'EquivObjects'} );
+ $query = "SELECT DISTINCT Groups.Name $query";
+ $roles = $RT::Handle->dbh->selectcol_arrayref($query, undef, @bind_values);
unless ($roles) {
$RT::Logger->warning( $RT::Handle->dbh->errstr );
return ();
@@ -468,14 +463,11 @@ sub HasRights {
if ( @enabled_roles ) {
- my $query
- = "SELECT DISTINCT ACL.RightName "
- . $self->_RolesWithRightQuery(
- EquivObjects => $args{'EquivObjects'}
- )
- . ' AND ('. join( ' OR ', map "PrincipalType = '$_'", @enabled_roles ) .')'
- ;
- my $rights = $RT::Handle->dbh->selectcol_arrayref($query);
+ my ( $query, @bind_values ) = $self->_RolesWithRightQuery( EquivObjects => $args{'EquivObjects'} );
+ $query = "SELECT DISTINCT ACL.RightName $query";
+ $query .= ' AND ('. join( ' OR ', ("PrincipalType = ?") x @enabled_roles ) .')';
+ push @bind_values, @enabled_roles;
+ my $rights = $RT::Handle->dbh->selectcol_arrayref($query, undef, @bind_values);
unless ($rights) {
$RT::Logger->warning( $RT::Handle->dbh->errstr );
return ();
@@ -520,12 +512,11 @@ sub _HasGroupRight {
@_
);
- my $query
- = "SELECT ACL.id, ACL.ObjectType, ACL.ObjectId "
- . $self->_HasGroupRightQuery( %args );
+ my ( $query, @bind_values ) = $self->_HasGroupRightQuery(%args);
+ $query = "SELECT ACL.id, ACL.ObjectType, ACL.ObjectId $query";
$self->_Handle->ApplyLimits( \$query, 1 );
- my ( $hit, $obj, $id ) = $self->_Handle->FetchResult($query);
+ my ( $hit, $obj, $id ) = $self->_Handle->FetchResult($query, @bind_values);
return (0) unless $hit;
$obj .= "-$id" if $id;
@@ -540,30 +531,33 @@ sub _HasGroupRightQuery {
@_
);
+ my @bind_values;
my $query
= "FROM ACL, Principals, CachedGroupMembers WHERE "
# Never find disabled groups.
. "Principals.id = ACL.PrincipalId "
- . "AND Principals.PrincipalType = 'Group' "
- . "AND Principals.Disabled = 0 "
-
+ . "AND Principals.PrincipalType = ? " # 'Group'
+ . "AND Principals.Disabled = ? " # 0
# See if the principal is a member of the group recursively or _is the rightholder_
# never find recursively disabled group members
# also, check to see if the right is being granted _directly_ to this principal,
# as is the case when we want to look up group rights
. "AND CachedGroupMembers.GroupId = ACL.PrincipalId "
. "AND CachedGroupMembers.GroupId = Principals.id "
- . "AND CachedGroupMembers.MemberId = ". $self->Id . " "
- . "AND CachedGroupMembers.Disabled = 0 ";
+ . "AND CachedGroupMembers.MemberId = ? " # $self->Id
+ . "AND CachedGroupMembers.Disabled = ? "; # 0
+ push @bind_values, 'Group', 0, $self->Id, 0;
my @clauses;
foreach my $obj ( @{ $args{'EquivObjects'} } ) {
my $type = ref($obj) || $obj;
- my $clause = "ACL.ObjectType = '$type'";
+ my $clause = "ACL.ObjectType = ?"; # $type
+ push @bind_values, $type;
if ( defined eval { $obj->id } ) { # it might be 0
- $clause .= " AND ACL.ObjectId = " . $obj->id;
+ $clause .= " AND ACL.ObjectId = ?"; # $obj->id;
+ push @bind_values, $obj->id;
}
push @clauses, "($clause)";
@@ -574,13 +568,15 @@ sub _HasGroupRightQuery {
if ( my $right = $args{'Right'} ) {
# Only find superuser or rights with the name $right
if ( $right eq 'SuperUser' ) {
- $query .= " AND ACL.RightName = 'SuperUser' "
+ $query .= " AND ACL.RightName = ? "; # 'SuperUser'
+ push @bind_values, 'SuperUser';
}
else {
- $query .= " AND ACL.RightName IN ('SuperUser', '$right')";
+ $query .= " AND ACL.RightName IN (?, ?)"; # 'SuperUser', $right
+ push @bind_values, 'SuperUser', $right;
}
}
- return $query;
+ return ( $query, @bind_values );
}
sub _HasRoleRight {
@@ -593,11 +589,11 @@ sub _HasRoleRight {
my @roles = $self->RolesWithRight(%args);
return 0 unless @roles;
- my $query = "SELECT Groups.id "
- . $self->_HasRoleRightQuery( %args, Roles => \@roles );
+ my ( $query, @bind_values ) = $self->_HasRoleRightQuery( %args, Roles => \@roles );
+ $query = "SELECT Groups.id $query";
$self->_Handle->ApplyLimits( \$query, 1 );
- my ($hit) = $self->_Handle->FetchResult($query);
+ my ($hit) = $self->_Handle->FetchResult($query, @bind_values);
return (1) if $hit;
return 0;
@@ -619,30 +615,44 @@ sub _HasRoleRightQuery {
" FROM Groups, Principals, CachedGroupMembers WHERE "
# Never find disabled things
- . "Principals.Disabled = 0 " . "AND CachedGroupMembers.Disabled = 0 "
+ . "Principals.Disabled = ? " . "AND CachedGroupMembers.Disabled = ? " # 0, 0
# We always grant rights to Groups
. "AND Principals.id = Groups.id "
- . "AND Principals.PrincipalType = 'Group' "
+ . "AND Principals.PrincipalType = ? " # 'Group'
# See if the principal is a member of the group recursively or _is the rightholder_
# never find recursively disabled group members
# also, check to see if the right is being granted _directly_ to this principal,
# as is the case when we want to look up group rights
. "AND Principals.id = CachedGroupMembers.GroupId "
- . "AND CachedGroupMembers.MemberId IN (" . ( join ',', $self->Id, map { $_->id } @{ $groups->ItemsArrayRef } ) . ") "
+ . "AND CachedGroupMembers.MemberId IN (" . ( join ',', ('?') x (1 + @{ $groups->ItemsArrayRef } ) ) . ") "
;
+ my @bind_values = ( 0, 0, 'Group', $self->Id, map { $_->id } @{ $groups->ItemsArrayRef } );
if ( $args{'Roles'} ) {
- $query .= "AND (" . join( ' OR ',
- map $RT::Handle->__MakeClauseCaseInsensitive('Groups.Name', '=', "'$_'"),
- @{ $args{'Roles'} }
- ) . ")";
+ $query .= "AND ("
+ . join( ' OR ',
+ ( $RT::Handle->__MakeClauseCaseInsensitive( 'Groups.Name', '=', '?' ) )
+ x @{ $args{'Roles'} } )
+ . ")";
+ push @bind_values, map { lc $_ } @{ $args{'Roles'} };
}
- my @object_clauses = RT::Users->_RoleClauses( Groups => @{ $args{'EquivObjects'} } );
+ my @object_clauses;
+ foreach my $obj ( @{ $args{'EquivObjects'} } ) {
+ my $type = ref($obj) ? ref($obj) : $obj;
+
+ my $role_clause = $RT::Handle->__MakeClauseCaseInsensitive( "Groups.Domain", '=', '?' );
+ push @bind_values, lc "$type-Role";
+ if ( my $id = eval { $obj->id } ) {
+ $role_clause .= " AND Groups.Instance = ?";
+ push @bind_values, $id;
+ }
+ push @object_clauses, "($role_clause)";
+ }
$query .= " AND (" . join( ' OR ', @object_clauses ) . ")";
- return $query;
+ return ( $query, @bind_values );
}
=head2 RolesWithRight
@@ -672,10 +682,10 @@ sub RolesWithRight {
return () if $args{'Right'} eq 'ExecuteCode'
and RT->Config->Get('DisallowExecuteCode');
- my $query = "SELECT DISTINCT PrincipalType "
- . $self->_RolesWithRightQuery( %args );
+ my ($query, @bind_values) = $self->_RolesWithRightQuery( %args );
+ $query = "SELECT DISTINCT PrincipalType $query";
- my $roles = $RT::Handle->dbh->selectcol_arrayref($query);
+ my $roles = $RT::Handle->dbh->selectcol_arrayref($query, undef, @bind_values);
unless ($roles) {
$RT::Logger->warning( $RT::Handle->dbh->errstr );
return ();
@@ -715,32 +725,39 @@ sub _RolesWithRightQuery {
@_
);
+ my @bind_values;
my $query = " FROM ACL WHERE"
# we need only roles
- . " PrincipalType != 'Group'";
+ . " PrincipalType != ?"; # 'Group'
+ push @bind_values, 'Group';
if ( my $right = $args{'Right'} ) {
if ( $args{'IncludeSuperusers'} && $right ne 'SuperUser' ) {
- $query .= " AND RightName IN ('SuperUser', '$right')";
+ $query .= " AND RightName IN (?, ?)"; # 'SuperUser', $right
+ push @bind_values, 'SuperUser', $right;
}
else {
- $query .= " AND RightName = '$right'";
+ $query .= " AND RightName = ?"; # $right
+ push @bind_values, $right;
}
}
# skip rights granted on system level if we were asked to
unless ( $args{'IncludeSystemRights'} ) {
- $query .= " AND ObjectType != 'RT::System'";
+ $query .= " AND ObjectType != ?"; # 'RT::System'
+ push @bind_values, 'RT::System';
}
my (@object_clauses);
foreach my $obj ( @{ $args{'EquivObjects'} } ) {
my $type = ref($obj) ? ref($obj) : $obj;
- my $object_clause = "ObjectType = '$type'";
+ my $object_clause = "ObjectType = ?"; # $type
+ push @bind_values, $type;
if ( my $id = eval { $obj->id } ) {
- $object_clause .= " AND ObjectId = $id";
+ $object_clause .= " AND ObjectId = ?"; # $id
+ push @bind_values, $id;
}
push @object_clauses, "($object_clause)";
}
@@ -749,7 +766,7 @@ sub _RolesWithRightQuery {
$query .= " AND (" . join( ' OR ', @object_clauses ) . ")"
if @object_clauses;
- return $query;
+ return ( $query, @bind_values );
}
commit 76e8a1041ac96909ba25420b026ef9f02d5759cd
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Aug 31 05:58:20 2021 +0800
Update tests to force BuildSelectQuery to not use bind values
Later versions of DBIx::SearchBuilder use bind values by default.
diff --git a/t/api/sql.t b/t/api/sql.t
index 7241e6fb3c..7a39cd65f9 100644
--- a/t/api/sql.t
+++ b/t/api/sql.t
@@ -7,7 +7,7 @@ use RT::Test tests => undef;
my $users = RT::Users->new( RT->SystemUser );
$users->WhoHaveGroupRight( Right => 'OwnTicket', Object => RT->System, IncludeSuperusers => 1 );
like(
- $users->BuildSelectQuery,
+ $users->BuildSelectQuery(PreferBind => 0),
qr{RightName IN \('SuperUser', 'OwnTicket'\)},
'RightName check in WhoHaveGroupRight uses IN'
);
@@ -31,7 +31,7 @@ my %ticketsql = (
my $tickets = RT::Tickets->new( RT->SystemUser );
for my $query ( sort keys %ticketsql ) {
$tickets->FromSQL($query);
- like( $tickets->BuildSelectQuery, $ticketsql{$query}, qq{TicketSQL "$query" uses IN} );
+ like( $tickets->BuildSelectQuery(PreferBind => 0), $ticketsql{$query}, qq{TicketSQL "$query" uses IN} );
}
done_testing;
diff --git a/t/ticket/priority.t b/t/ticket/priority.t
index 6a40a5b7cb..327a968e2b 100644
--- a/t/ticket/priority.t
+++ b/t/ticket/priority.t
@@ -91,11 +91,11 @@ diag "Ticket/Transaction search";
for my $field (qw/Priority InitialPriority FinalPriority/) {
my $tickets = RT::Tickets->new( RT->SystemUser );
$tickets->FromSQL("Queue = 'General' AND $field = 'Low'");
- like( $tickets->BuildSelectQuery, qr/$field = '20'/, "$field is translated properly" );
+ like( $tickets->BuildSelectQuery(PreferBind => 0), qr/$field = '20'/, "$field is translated properly" );
my $txns = RT::Transactions->new( RT->SystemUser );
$txns->FromSQL("TicketQueue = 'General' AND Ticket$field = 'Low'");
- like( $txns->BuildSelectQuery, qr/$field = '20'/, "Ticket$field is translated properly" );
+ like( $txns->BuildSelectQuery(PreferBind => 0), qr/$field = '20'/, "Ticket$field is translated properly" );
}
my $tickets = RT::Tickets->new( RT->SystemUser );
-----------------------------------------------------------------------
hooks/post-receive
--
rt
More information about the rt-commit
mailing list