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

Commit59bd34c

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 parent0e91750 commit59bd34c

File tree

9 files changed

+54
-55
lines changed

9 files changed

+54
-55
lines changed

‎src/backend/catalog/aclchk.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4103,9 +4103,14 @@ object_ownercheck(Oid classid, Oid objectid, Oid roleid)
41034103
if (superuser_arg(roleid))
41044104
return true;
41054105

4106+
/* For large objects, the catalog to consult is pg_largeobject_metadata */
4107+
if (classid==LargeObjectRelationId)
4108+
classid=LargeObjectMetadataRelationId;
4109+
41064110
cacheid=get_object_catcache_oid(classid);
41074111
if (cacheid!=-1)
41084112
{
4113+
/* we can get the object's tuple from the syscache */
41094114
HeapTupletuple;
41104115

41114116
tuple=SearchSysCache1(cacheid,ObjectIdGetDatum(objectid));
@@ -4122,7 +4127,6 @@ object_ownercheck(Oid classid, Oid objectid, Oid roleid)
41224127
else
41234128
{
41244129
/* for catalogs without an appropriate syscache */
4125-
41264130
Relationrel;
41274131
ScanKeyDataentry[1];
41284132
SysScanDescscan;
@@ -4442,9 +4446,9 @@ recordExtObjInitPriv(Oid objoid, Oid classoid)
44424446

44434447
ReleaseSysCache(tuple);
44444448
}
4445-
/* pg_largeobject_metadata */
4446-
elseif (classoid==LargeObjectMetadataRelationId)
4449+
elseif (classoid==LargeObjectRelationId)
44474450
{
4451+
/* For large objects, we must consult pg_largeobject_metadata */
44484452
DatumaclDatum;
44494453
boolisNull;
44504454
HeapTupletuple;

‎src/backend/catalog/pg_shdepend.c

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1614,20 +1614,9 @@ shdepReassignOwned(List *roleids, Oid newrole)
16141614
caseDatabaseRelationId:
16151615
caseTSConfigRelationId:
16161616
caseTSDictionaryRelationId:
1617-
{
1618-
OidclassId=sdepForm->classid;
1619-
Relationcatalog;
1620-
1621-
if (classId==LargeObjectRelationId)
1622-
classId=LargeObjectMetadataRelationId;
1623-
1624-
catalog=table_open(classId,RowExclusiveLock);
1625-
1626-
AlterObjectOwner_internal(catalog,sdepForm->objid,
1627-
newrole);
1628-
1629-
table_close(catalog,NoLock);
1630-
}
1617+
AlterObjectOwner_internal(sdepForm->classid,
1618+
sdepForm->objid,
1619+
newrole);
16311620
break;
16321621

16331622
default:

‎src/backend/commands/alter.c

Lines changed: 22 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -918,9 +918,7 @@ ExecAlterOwnerStmt(AlterOwnerStmt *stmt)
918918
caseOBJECT_TSDICTIONARY:
919919
caseOBJECT_TSCONFIGURATION:
920920
{
921-
Relationcatalog;
922921
Relationrelation;
923-
OidclassId;
924922
ObjectAddressaddress;
925923

926924
address=get_object_address(stmt->objectType,
@@ -929,20 +927,9 @@ ExecAlterOwnerStmt(AlterOwnerStmt *stmt)
929927
AccessExclusiveLock,
930928
false);
931929
Assert(relation==NULL);
932-
classId=address.classId;
933930

934-
/*
935-
* XXX - get_object_address returns Oid of pg_largeobject
936-
* catalog for OBJECT_LARGEOBJECT because of historical
937-
* reasons. Fix up it here.
938-
*/
939-
if (classId==LargeObjectRelationId)
940-
classId=LargeObjectMetadataRelationId;
941-
942-
catalog=table_open(classId,RowExclusiveLock);
943-
944-
AlterObjectOwner_internal(catalog,address.objectId,newowner);
945-
table_close(catalog,RowExclusiveLock);
931+
AlterObjectOwner_internal(address.classId,address.objectId,
932+
newowner);
946933

947934
returnaddress;
948935
}
@@ -960,25 +947,32 @@ ExecAlterOwnerStmt(AlterOwnerStmt *stmt)
960947
* cases (won't work for tables, nor other cases where we need to do more than
961948
* change the ownership column of a single catalog entry).
962949
*
963-
*rel: catalog relationcontaining object (RowExclusiveLock'd by caller)
950+
*classId: OID of catalogcontaining object
964951
* objectId: OID of object to change the ownership of
965952
* new_ownerId: OID of new object owner
953+
*
954+
* This will work on large objects, but we have to beware of the fact that
955+
* classId isn't the OID of the catalog to modify in that case.
966956
*/
967957
void
968-
AlterObjectOwner_internal(Relationrel,OidobjectId,Oidnew_ownerId)
958+
AlterObjectOwner_internal(OidclassId,OidobjectId,Oidnew_ownerId)
969959
{
970-
OidclassId=RelationGetRelid(rel);
971-
AttrNumberAnum_oid=get_object_attnum_oid(classId);
972-
AttrNumberAnum_owner=get_object_attnum_owner(classId);
973-
AttrNumberAnum_namespace=get_object_attnum_namespace(classId);
974-
AttrNumberAnum_acl=get_object_attnum_acl(classId);
975-
AttrNumberAnum_name=get_object_attnum_name(classId);
960+
/* For large objects, the catalog to modify is pg_largeobject_metadata */
961+
OidcatalogId= (classId==LargeObjectRelationId) ?LargeObjectMetadataRelationId :classId;
962+
AttrNumberAnum_oid=get_object_attnum_oid(catalogId);
963+
AttrNumberAnum_owner=get_object_attnum_owner(catalogId);
964+
AttrNumberAnum_namespace=get_object_attnum_namespace(catalogId);
965+
AttrNumberAnum_acl=get_object_attnum_acl(catalogId);
966+
AttrNumberAnum_name=get_object_attnum_name(catalogId);
967+
Relationrel;
976968
HeapTupleoldtup;
977969
Datumdatum;
978970
boolisnull;
979971
Oidold_ownerId;
980972
OidnamespaceId=InvalidOid;
981973

974+
rel=table_open(catalogId,RowExclusiveLock);
975+
982976
oldtup=get_catalog_object_by_oid(rel,Anum_oid,objectId);
983977
if (oldtup==NULL)
984978
elog(ERROR,"cache lookup failed for object %u of catalog \"%s\"",
@@ -1026,7 +1020,8 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId)
10261020
snprintf(namebuf,sizeof(namebuf),"%u",objectId);
10271021
objname=namebuf;
10281022
}
1029-
aclcheck_error(ACLCHECK_NOT_OWNER,get_object_type(classId,objectId),
1023+
aclcheck_error(ACLCHECK_NOT_OWNER,
1024+
get_object_type(catalogId,objectId),
10301025
objname);
10311026
}
10321027
/* Must be able to become new owner */
@@ -1079,8 +1074,6 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId)
10791074
CatalogTupleUpdate(rel,&newtup->t_self,newtup);
10801075

10811076
/* Update owner dependency reference */
1082-
if (classId==LargeObjectMetadataRelationId)
1083-
classId=LargeObjectRelationId;
10841077
changeDependencyOnOwner(classId,objectId,new_ownerId);
10851078

10861079
/* Release memory */
@@ -1089,5 +1082,8 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId)
10891082
pfree(replaces);
10901083
}
10911084

1085+
/* Note the post-alter hook gets classId not catalogId */
10921086
InvokeObjectPostAlterHook(classId,objectId,0);
1087+
1088+
table_close(rel,RowExclusiveLock);
10931089
}

‎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/include/commands/alter.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
#include"catalog/dependency.h"
1818
#include"catalog/objectaddress.h"
1919
#include"nodes/parsenodes.h"
20-
#include"utils/relcache.h"
2120

2221
externObjectAddressExecRenameStmt(RenameStmt*stmt);
2322

@@ -29,7 +28,7 @@ extern OidAlterObjectNamespace_oid(Oid classId, Oid objid, Oid nspOid,
2928
ObjectAddresses*objsMoved);
3029

3130
externObjectAddressExecAlterOwnerStmt(AlterOwnerStmt*stmt);
32-
externvoidAlterObjectOwner_internal(Relationrel,OidobjectId,
31+
externvoidAlterObjectOwner_internal(OidclassId,OidobjectId,
3332
Oidnew_ownerId);
3433

3534
#endif/* ALTER_H */

‎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