Downgrade error in object_aclmask_ext() to internal
authorPeter Eisentraut <[email protected]>
Wed, 15 Jan 2025 15:53:53 +0000 (16:53 +0100)
committerPeter Eisentraut <[email protected]>
Wed, 15 Jan 2025 15:58:44 +0000 (16:58 +0100)
The "does not exist" error in object_aclmask_ext() was written as
ereport(), suggesting that it is user-facing.  This is problematic:
get_object_class_descr() is meant to be for internal errors only and
does not support translation.

For the has_xxx_privilege functions, the error has not been
user-facing since commit 403ac226ddd.  The remaining users are
pg_database_size() and pg_tablespace_size().  The call stack here is
pretty deep and this dependency is not obvious.  Here we can put in an
explicit existence check with a bespoke error message early in the
function.

Then we can downgrade the error in object_aclmask_ext() to a normal
"cache lookup failed" internal error.

Reviewed-by: Alvaro Herrera <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/da2f8942-be6d-48d0-ac1c-a053370a6b1f@eisentraut.org

src/backend/catalog/aclchk.c
src/backend/utils/adt/dbsize.c

index bd006931938ff4a53119fd32cfb752101e0a1247..02a754cc30a70cbb991620036c80dd2a1a923e0e 100644 (file)
@@ -3004,10 +3004,6 @@ pg_aclmask(ObjectType objtype, Oid object_oid, AttrNumber attnum, Oid roleid,
  * Exported routines for examining a user's privileges for various objects
  *
  * See aclmask() for a description of the common API for these functions.
- *
- * Note: we give lookup failure the full ereport treatment because the
- * has_xxx_privilege() family of functions allow users to pass any random
- * OID to these functions.
  * ****************************************************************
  */
 
@@ -3074,10 +3070,8 @@ object_aclmask_ext(Oid classid, Oid objectid, Oid roleid,
                        return 0;
                }
                else
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_UNDEFINED_OBJECT),
-                                        errmsg("%s with OID %u does not exist",
-                                                       get_object_class_descr(classid), objectid)));
+                       elog(ERROR, "cache lookup failed for %s %u",
+                                get_object_class_descr(classid), objectid);
        }
 
        ownerId = DatumGetObjectId(SysCacheGetAttrNotNull(cacheid,
index e560fef69196178e5c8d17586545de59f267363c..011d8d4da5aaaee8a310694c1cbaad0d5ee2553e 100644 (file)
@@ -170,6 +170,15 @@ pg_database_size_oid(PG_FUNCTION_ARGS)
        Oid                     dbOid = PG_GETARG_OID(0);
        int64           size;
 
+       /*
+        * Not needed for correctness, but avoid non-user-facing error message
+        * later if the database doesn't exist.
+        */
+       if (!SearchSysCacheExists1(DATABASEOID, ObjectIdGetDatum(dbOid)))
+               ereport(ERROR,
+                               errcode(ERRCODE_UNDEFINED_OBJECT),
+                               errmsg("database with OID %u does not exist", dbOid));
+
        size = calculate_database_size(dbOid);
 
        if (size == 0)
@@ -274,6 +283,15 @@ pg_tablespace_size_oid(PG_FUNCTION_ARGS)
        Oid                     tblspcOid = PG_GETARG_OID(0);
        int64           size;
 
+       /*
+        * Not needed for correctness, but avoid non-user-facing error message
+        * later if the tablespace doesn't exist.
+        */
+       if (!SearchSysCacheExists1(TABLESPACEOID, ObjectIdGetDatum(tblspcOid)))
+               ereport(ERROR,
+                               errcode(ERRCODE_UNDEFINED_OBJECT),
+                               errmsg("tablespace with OID %u does not exist", tblspcOid));
+
        size = calculate_tablespace_size(tblspcOid);
 
        if (size < 0)