Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd21fca0

Browse files
committed
Fix misbehavior of DROP OWNED BY with duplicate polroles entries.
Ordinarily, a pg_policy.polroles array wouldn't list the same rolemore than once; but CREATE POLICY does not prevent that. If weperform DROP OWNED BY on a role that is listed more than once,RemoveRoleFromObjectPolicy either suffered an assertion failureor encountered a tuple-updated-by-self error. Rewrite it to copecorrectly with duplicate entries, and add a CommandCounterIncrementcall to prevent the other problem.Per discussion, there's other cleanup that ought to happen here,but this seems like the minimum essential fix.Per bug #17062 from Alexander Lakhin. It's been broken all along,so back-patch to all supported branches.Discussion:https://postgr.es/m/17062-11f471ae3199ca23@postgresql.org
1 parent84bee96 commitd21fca0

File tree

3 files changed

+46
-23
lines changed

3 files changed

+46
-23
lines changed

‎src/backend/commands/policy.c

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include"access/relation.h"
1919
#include"access/sysattr.h"
2020
#include"access/table.h"
21+
#include"access/xact.h"
2122
#include"catalog/catalog.h"
2223
#include"catalog/dependency.h"
2324
#include"catalog/indexing.h"
@@ -425,10 +426,9 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
425426
Oidrelid;
426427
Relationrel;
427428
ArrayType*policy_roles;
428-
intnum_roles;
429429
Datumroles_datum;
430430
boolattr_isnull;
431-
boolnoperm= true;
431+
boolkeep_policy= true;
432432

433433
Assert(classid==PolicyRelationId);
434434

@@ -481,31 +481,20 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
481481

482482
policy_roles=DatumGetArrayTypePCopy(roles_datum);
483483

484-
/* We should be removing exactly one entry from the roles array */
485-
num_roles=ARR_DIMS(policy_roles)[0]-1;
486-
487-
Assert(num_roles >=0);
488-
489484
/* Must own relation. */
490-
if (pg_class_ownercheck(relid,GetUserId()))
491-
noperm= false;/* user is allowed to modify this policy */
492-
else
485+
if (!pg_class_ownercheck(relid,GetUserId()))
493486
ereport(WARNING,
494487
(errcode(ERRCODE_WARNING_PRIVILEGE_NOT_REVOKED),
495488
errmsg("role \"%s\" could not be removed from policy \"%s\" on \"%s\"",
496489
GetUserNameFromId(roleid, false),
497490
NameStr(((Form_pg_policy)GETSTRUCT(tuple))->polname),
498491
RelationGetRelationName(rel))));
499-
500-
/*
501-
* If multiple roles exist on this policy, then remove the one we were
502-
* asked to and leave the rest.
503-
*/
504-
if (!noperm&&num_roles>0)
492+
else
505493
{
506494
inti,
507495
j;
508496
Oid*roles= (Oid*)ARR_DATA_PTR(policy_roles);
497+
intnum_roles=ARR_DIMS(policy_roles)[0];
509498
Datum*role_oids;
510499
char*qual_value;
511500
Node*qual_expr;
@@ -582,16 +571,22 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
582571
else
583572
with_check_qual=NULL;
584573

585-
/* Rebuild the roles array to then update the pg_policy tuple with */
574+
/*
575+
* Rebuild the roles array, without any mentions of the target role.
576+
* Ordinarily there'd be exactly one, but we must cope with duplicate
577+
* mentions, since CREATE/ALTER POLICY historically have allowed that.
578+
*/
586579
role_oids= (Datum*)palloc(num_roles*sizeof(Datum));
587-
for (i=0,j=0;i<ARR_DIMS(policy_roles)[0];i++)
588-
/* Copy over all of the roles which are not the one being removed */
580+
for (i=0,j=0;i<num_roles;i++)
581+
{
589582
if (roles[i]!=roleid)
590583
role_oids[j++]=ObjectIdGetDatum(roles[i]);
584+
}
585+
num_roles=j;
591586

592-
/*We should have only removed theone role */
593-
Assert(j==num_roles);
594-
587+
/*If any roles remain, update thepolicy entry. */
588+
if (num_roles>0)
589+
{
595590
/* This is the array for the new tuple */
596591
role_ids=construct_array(role_oids,num_roles,OIDOID,
597592
sizeof(Oid), true,TYPALIGN_INT);
@@ -646,8 +641,17 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
646641

647642
heap_freetuple(new_tuple);
648643

644+
/* Make updates visible */
645+
CommandCounterIncrement();
646+
649647
/* Invalidate Relation Cache */
650648
CacheInvalidateRelcache(rel);
649+
}
650+
else
651+
{
652+
/* No roles would remain, so drop the policy instead */
653+
keep_policy= false;
654+
}
651655
}
652656

653657
/* Clean up. */
@@ -657,7 +661,7 @@ RemoveRoleFromObjectPolicy(Oid roleid, Oid classid, Oid policy_id)
657661

658662
table_close(pg_policy_rel,RowExclusiveLock);
659663

660-
return(noperm||num_roles>0);
664+
returnkeep_policy;
661665
}
662666

663667
/*

‎src/test/regress/expected/rowsecurity.out

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3886,6 +3886,15 @@ ERROR: policy "p1" for table "dob_t1" does not exist
38863886
CREATE POLICY p1 ON dob_t1 TO regress_rls_dob_role1,regress_rls_dob_role2 USING (true);
38873887
DROP OWNED BY regress_rls_dob_role1;
38883888
DROP POLICY p1 ON dob_t1; -- should succeed
3889+
-- same cases with duplicate polroles entries
3890+
CREATE POLICY p1 ON dob_t1 TO regress_rls_dob_role1,regress_rls_dob_role1 USING (true);
3891+
DROP OWNED BY regress_rls_dob_role1;
3892+
DROP POLICY p1 ON dob_t1; -- should fail, already gone
3893+
ERROR: policy "p1" for table "dob_t1" does not exist
3894+
CREATE POLICY p1 ON dob_t1 TO regress_rls_dob_role1,regress_rls_dob_role1,regress_rls_dob_role2 USING (true);
3895+
DROP OWNED BY regress_rls_dob_role1;
3896+
DROP POLICY p1 ON dob_t1; -- should succeed
3897+
-- partitioned target
38893898
CREATE POLICY p1 ON dob_t2 TO regress_rls_dob_role1,regress_rls_dob_role2 USING (true);
38903899
DROP OWNED BY regress_rls_dob_role1;
38913900
DROP POLICY p1 ON dob_t2; -- should succeed

‎src/test/regress/sql/rowsecurity.sql

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1757,6 +1757,16 @@ CREATE POLICY p1 ON dob_t1 TO regress_rls_dob_role1,regress_rls_dob_role2 USING
17571757
DROP OWNED BY regress_rls_dob_role1;
17581758
DROP POLICY p1ON dob_t1;-- should succeed
17591759

1760+
-- same cases with duplicate polroles entries
1761+
CREATE POLICY p1ON dob_t1 TO regress_rls_dob_role1,regress_rls_dob_role1 USING (true);
1762+
DROP OWNED BY regress_rls_dob_role1;
1763+
DROP POLICY p1ON dob_t1;-- should fail, already gone
1764+
1765+
CREATE POLICY p1ON dob_t1 TO regress_rls_dob_role1,regress_rls_dob_role1,regress_rls_dob_role2 USING (true);
1766+
DROP OWNED BY regress_rls_dob_role1;
1767+
DROP POLICY p1ON dob_t1;-- should succeed
1768+
1769+
-- partitioned target
17601770
CREATE POLICY p1ON dob_t2 TO regress_rls_dob_role1,regress_rls_dob_role2 USING (true);
17611771
DROP OWNED BY regress_rls_dob_role1;
17621772
DROP POLICY p1ON dob_t2;-- should succeed

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp