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

Commit152bfc0

Browse files
committed
Fix bugs in manipulation of large objects.
In v16 and up (since commitafbfc02), large object ownershipchecking has been broken because object_ownercheck() didn't take careof the discrepancy between our object-address representation of largeobjects (classId == LargeObjectRelationId) and the catalog where theirownership info is actually stored (LargeObjectMetadataRelationId).This resulted in failures such as "unrecognized class ID: 2613"when trying to update blob properties as a non-superuser.Poking around for related bugs, I found that AlterObjectOwner_internalwould pass the wrong classId to the PostAlterHook in the no-op codepath where the large object already has the desired owner. Also,recordExtObjInitPriv checked for the wrong classId; that bug is onlylatent because the stanza is dead code anyway, but as long as we'recarrying it around it should be less wrong. These bugs are quite old.In HEAD, we can reduce the scope for future bugs of this ilk bychanging AlterObjectOwner_internal's API to let the translation happeninside that function, rather than requiring callers to know about it.A more bulletproof fix, perhaps, would be to start usingLargeObjectMetadataRelationId as the dependency and object-addressclassId for blobs. However that has substantial risk of breakingthird-party code; even within our own code, it'd create hasslesfor pg_dump which would have to cope with a version-dependentrepresentation. For now, keep the status quo.Discussion:https://postgr.es/m/2650449.1702497209@sss.pgh.pa.us
1 parentdb69101 commit152bfc0

File tree

8 files changed

+50
-17
lines changed

8 files changed

+50
-17
lines changed

‎src/backend/catalog/aclchk.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3967,9 +3967,14 @@ object_ownercheck(Oid classid, Oid objectid, Oid roleid)
39673967
if (superuser_arg(roleid))
39683968
return true;
39693969

3970+
/* For large objects, the catalog to consult is pg_largeobject_metadata */
3971+
if (classid==LargeObjectRelationId)
3972+
classid=LargeObjectMetadataRelationId;
3973+
39703974
cacheid=get_object_catcache_oid(classid);
39713975
if (cacheid!=-1)
39723976
{
3977+
/* we can get the object's tuple from the syscache */
39733978
HeapTupletuple;
39743979

39753980
tuple=SearchSysCache1(cacheid,ObjectIdGetDatum(objectid));
@@ -3986,7 +3991,6 @@ object_ownercheck(Oid classid, Oid objectid, Oid roleid)
39863991
else
39873992
{
39883993
/* for catalogs without an appropriate syscache */
3989-
39903994
Relationrel;
39913995
ScanKeyDataentry[1];
39923996
SysScanDescscan;
@@ -4306,9 +4310,9 @@ recordExtObjInitPriv(Oid objoid, Oid classoid)
43064310

43074311
ReleaseSysCache(tuple);
43084312
}
4309-
/* pg_largeobject_metadata */
4310-
elseif (classoid==LargeObjectMetadataRelationId)
4313+
elseif (classoid==LargeObjectRelationId)
43114314
{
4315+
/* For large objects, we must consult pg_largeobject_metadata */
43124316
DatumaclDatum;
43134317
boolisNull;
43144318
HeapTupletuple;

‎src/backend/catalog/pg_shdepend.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1618,6 +1618,10 @@ shdepReassignOwned(List *roleids, Oid newrole)
16181618
OidclassId=sdepForm->classid;
16191619
Relationcatalog;
16201620

1621+
/*
1622+
* For large objects, the catalog to modify is
1623+
* pg_largeobject_metadata
1624+
*/
16211625
if (classId==LargeObjectRelationId)
16221626
classId=LargeObjectMetadataRelationId;
16231627

‎src/backend/commands/alter.c

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -929,9 +929,8 @@ ExecAlterOwnerStmt(AlterOwnerStmt *stmt)
929929
classId=address.classId;
930930

931931
/*
932-
* XXX - get_object_address returns Oid of pg_largeobject
933-
* catalog for OBJECT_LARGEOBJECT because of historical
934-
* reasons. Fix up it here.
932+
* For large objects, the catalog to modify is
933+
* pg_largeobject_metadata
935934
*/
936935
if (classId==LargeObjectRelationId)
937936
classId=LargeObjectMetadataRelationId;
@@ -1075,16 +1074,31 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId)
10751074
/* Perform actual update */
10761075
CatalogTupleUpdate(rel,&newtup->t_self,newtup);
10771076

1078-
/* Update owner dependency reference */
1077+
/*
1078+
* Update owner dependency reference. When working on a large object,
1079+
* we have to translate back to the OID conventionally used for LOs'
1080+
* classId.
1081+
*/
10791082
if (classId==LargeObjectMetadataRelationId)
10801083
classId=LargeObjectRelationId;
1084+
10811085
changeDependencyOnOwner(classId,objectId,new_ownerId);
10821086

10831087
/* Release memory */
10841088
pfree(values);
10851089
pfree(nulls);
10861090
pfree(replaces);
10871091
}
1092+
else
1093+
{
1094+
/*
1095+
* No need to change anything. But when working on a large object, we
1096+
* have to translate back to the OID conventionally used for LOs'
1097+
* classId, or the post-alter hook (if any) will get confused.
1098+
*/
1099+
if (classId==LargeObjectMetadataRelationId)
1100+
classId=LargeObjectRelationId;
1101+
}
10881102

10891103
InvokeObjectPostAlterHook(classId,objectId,0);
10901104
}

‎src/backend/libpq/be-fsstubs.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
#include<unistd.h>
4444

4545
#include"access/xact.h"
46-
#include"catalog/pg_largeobject_metadata.h"
46+
#include"catalog/pg_largeobject.h"
4747
#include"libpq/be-fsstubs.h"
4848
#include"libpq/libpq-fs.h"
4949
#include"miscadmin.h"
@@ -323,7 +323,7 @@ be_lo_unlink(PG_FUNCTION_ARGS)
323323
* relevant FDs.
324324
*/
325325
if (!lo_compat_privileges&&
326-
!object_ownercheck(LargeObjectMetadataRelationId,lobjId,GetUserId()))
326+
!object_ownercheck(LargeObjectRelationId,lobjId,GetUserId()))
327327
ereport(ERROR,
328328
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
329329
errmsg("must be owner of large object %u",lobjId)));

‎src/backend/storage/large_object/inv_api.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -221,11 +221,10 @@ inv_create(Oid lobjId)
221221
/*
222222
* dependency on the owner of largeobject
223223
*
224-
* The reason why we use LargeObjectRelationId instead of
225-
* LargeObjectMetadataRelationId here is to provide backward compatibility
226-
* to the applications which utilize a knowledge about internal layout of
227-
* system catalogs. OID of pg_largeobject_metadata and loid of
228-
* pg_largeobject are same value, so there are no actual differences here.
224+
* Note that LO dependencies are recorded using classId
225+
* LargeObjectRelationId for backwards-compatibility reasons. Using
226+
* LargeObjectMetadataRelationId instead would simplify matters for the
227+
* backend, but it'd complicate pg_dump and possibly break other clients.
229228
*/
230229
recordDependencyOnOwner(LargeObjectRelationId,
231230
lobjId_new,GetUserId());

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
\getenv abs_builddir PG_ABS_BUILDDIR
77
-- ensure consistent test output regardless of the default bytea format
88
SET bytea_output TO escape;
9-
-- Test ALTER LARGE OBJECT OWNER, GRANT, COMMENT
9+
-- Test ALTER LARGE OBJECT OWNER
1010
CREATE ROLE regress_lo_user;
1111
SELECT lo_create(42);
1212
lo_create
@@ -15,8 +15,11 @@ SELECT lo_create(42);
1515
(1 row)
1616

1717
ALTER LARGE OBJECT 42 OWNER TO regress_lo_user;
18+
-- Test GRANT, COMMENT as non-superuser
19+
SET SESSION AUTHORIZATION regress_lo_user;
1820
GRANT SELECT ON LARGE OBJECT 42 TO public;
1921
COMMENT ON LARGE OBJECT 42 IS 'the ultimate answer';
22+
RESET SESSION AUTHORIZATION;
2023
-- Test psql's \lo_list et al (we assume no other LOs exist yet)
2124
\lo_list
2225
Large objects

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
\getenv abs_builddir PG_ABS_BUILDDIR
77
-- ensure consistent test output regardless of the default bytea format
88
SET bytea_output TO escape;
9-
-- Test ALTER LARGE OBJECT OWNER, GRANT, COMMENT
9+
-- Test ALTER LARGE OBJECT OWNER
1010
CREATE ROLE regress_lo_user;
1111
SELECT lo_create(42);
1212
lo_create
@@ -15,8 +15,11 @@ SELECT lo_create(42);
1515
(1 row)
1616

1717
ALTER LARGE OBJECT 42 OWNER TO regress_lo_user;
18+
-- Test GRANT, COMMENT as non-superuser
19+
SET SESSION AUTHORIZATION regress_lo_user;
1820
GRANT SELECT ON LARGE OBJECT 42 TO public;
1921
COMMENT ON LARGE OBJECT 42 IS 'the ultimate answer';
22+
RESET SESSION AUTHORIZATION;
2023
-- Test psql's \lo_list et al (we assume no other LOs exist yet)
2124
\lo_list
2225
Large objects

‎src/test/regress/sql/largeobject.sql

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,19 @@
99
-- ensure consistent test output regardless of the default bytea format
1010
SET bytea_output TO escape;
1111

12-
-- Test ALTER LARGE OBJECT OWNER, GRANT, COMMENT
12+
-- Test ALTER LARGE OBJECT OWNER
1313
CREATE ROLE regress_lo_user;
1414
SELECT lo_create(42);
1515
ALTER LARGE OBJECT42 OWNER TO regress_lo_user;
16+
17+
-- Test GRANT, COMMENT as non-superuser
18+
SET SESSION AUTHORIZATION regress_lo_user;
19+
1620
GRANTSELECTON LARGE OBJECT42 TO public;
1721
COMMENTON LARGE OBJECT42 IS'the ultimate answer';
1822

23+
RESET SESSION AUTHORIZATION;
24+
1925
-- Test psql's \lo_list et al (we assume no other LOs exist yet)
2026
\lo_list
2127
\lo_list+

0 commit comments

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