]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Fix bugs in manipulation of large objects.
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 15 Dec 2023 18:55:05 +0000 (13:55 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 15 Dec 2023 18:55:05 +0000 (13:55 -0500)
In v16 and up (since commit afbfc0298), large object ownership
checking has been broken because object_ownercheck() didn't take care
of the discrepancy between our object-address representation of large
objects (classId == LargeObjectRelationId) and the catalog where their
ownership 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_internal
would pass the wrong classId to the PostAlterHook in the no-op code
path where the large object already has the desired owner.  Also,
recordExtObjInitPriv checked for the wrong classId; that bug is only
latent because the stanza is dead code anyway, but as long as we're
carrying 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 by
changing AlterObjectOwner_internal's API to let the translation happen
inside that function, rather than requiring callers to know about it.

A more bulletproof fix, perhaps, would be to start using
LargeObjectMetadataRelationId as the dependency and object-address
classId for blobs.  However that has substantial risk of breaking
third-party code; even within our own code, it'd create hassles
for pg_dump which would have to cope with a version-dependent
representation.  For now, keep the status quo.

Discussion: https://postgr.es/m/2650449.1702497209@sss.pgh.pa.us

src/backend/catalog/aclchk.c
src/backend/commands/alter.c

index 30c601c23d2c5ffbbde221c82a8b9496d95f2d01..3b7c6436e6c8c7152d530b179c0fe3c861634e34 100644 (file)
@@ -5719,9 +5719,9 @@ recordExtObjInitPriv(Oid objoid, Oid classoid)
 
                ReleaseSysCache(tuple);
        }
-       /* pg_largeobject_metadata */
-       else if (classoid == LargeObjectMetadataRelationId)
+       else if (classoid == LargeObjectRelationId)
        {
+               /* For large objects, we must consult pg_largeobject_metadata */
                Datum           aclDatum;
                bool            isNull;
                HeapTuple       tuple;
index c8f471be38023cb4c748bfa973edfce27f920f4f..4429fbe8b0080b2e5e623f380036c3410ecbd421 100644 (file)
@@ -1035,9 +1035,14 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId)
                /* Perform actual update */
                CatalogTupleUpdate(rel, &newtup->t_self, newtup);
 
-               /* Update owner dependency reference */
+               /*
+                * Update owner dependency reference.  When working on a large object,
+                * we have to translate back to the OID conventionally used for LOs'
+                * classId.
+                */
                if (classId == LargeObjectMetadataRelationId)
                        classId = LargeObjectRelationId;
+
                changeDependencyOnOwner(classId, objectId, new_ownerId);
 
                /* Release memory */
@@ -1045,6 +1050,16 @@ AlterObjectOwner_internal(Relation rel, Oid objectId, Oid new_ownerId)
                pfree(nulls);
                pfree(replaces);
        }
+       else
+       {
+               /*
+                * No need to change anything.  But when working on a large object, we
+                * have to translate back to the OID conventionally used for LOs'
+                * classId, or the post-alter hook (if any) will get confused.
+                */
+               if (classId == LargeObjectMetadataRelationId)
+                       classId = LargeObjectRelationId;
+       }
 
        InvokeObjectPostAlterHook(classId, objectId, 0);
 }