Prevent write operations on large objects in read-only transactions

Attempting such an operation would already fail, but in various and
confusing ways.  For example, while in recovery, some elog() messages
would be reported, but these should never be user-facing.  This commit
restricts any write operations done on large objects in a read-only
context, so as the errors generated are more user-friendly.  This is per
the discussion done with Tom Lane and Robert Haas.

Some regression tests are added to check the case of all the SQL
functions working on large objects (including an update of the test's
alternate output).

Author: Yugo Nagata
Discussion: https://postgr.es/m/20220527153028.61a4608f66abcd026fd3806f@sraoss.co.jp

diff --git a/doc/src/sgml/lobj.sgml b/doc/src/sgml/lobj.sgml
index b767ba1d05155f1b6bb4f9262c310f01c4ddcac0..a27e91ecd2e16f3e0c7672b3eeb332857cf4240b 100644 (file)
--- a/doc/src/sgml/lobj.sgml
+++ b/doc/src/sgml/lobj.sgml
@@ -114,7 +114,9 @@
     All large object manipulation using these functions
     <emphasis>must</emphasis> take place within an SQL transaction block,
     since large object file descriptors are only valid for the duration of
-    a transaction.
+    a transaction. Write operations, including <function>lo_open</function>
+    with the <symbol>INV_WRITE</symbol> mode, are not allowed in a read-only
+    transaction.
    </para>
 
    <para>

diff --git a/src/backend/libpq/be-fsstubs.c b/src/backend/libpq/be-fsstubs.c
index 5804532881e1e7583ddccebc96c28b6814cc790e..3e5cada7eb53ca81c9c1d30bcc02020232fc6e4b 100644 (file)
--- a/src/backend/libpq/be-fsstubs.c
+++ b/src/backend/libpq/be-fsstubs.c
@@ -93,6 +93,9 @@ be_lo_open(PG_FUNCTION_ARGS)
    elog(DEBUG4, "lo_open(%u,%d)", lobjId, mode);
 #endif
 
+   if (mode & INV_WRITE)
+       PreventCommandIfReadOnly("lo_open(INV_WRITE)");
+
    /*
     * Allocate a large object descriptor first.  This will also create
     * 'fscxt' if this is the first LO opened in this transaction.
@@ -245,6 +248,8 @@ be_lo_creat(PG_FUNCTION_ARGS)
 {
    Oid         lobjId;
 
+   PreventCommandIfReadOnly("lo_creat()");
+
    lo_cleanup_needed = true;
    lobjId = inv_create(InvalidOid);
 
@@ -256,6 +261,8 @@ be_lo_create(PG_FUNCTION_ARGS)
 {
    Oid         lobjId = PG_GETARG_OID(0);
 
+   PreventCommandIfReadOnly("lo_create()");
+
    lo_cleanup_needed = true;
    lobjId = inv_create(lobjId);
 
@@ -306,6 +313,8 @@ be_lo_unlink(PG_FUNCTION_ARGS)
 {
    Oid         lobjId = PG_GETARG_OID(0);
 
+   PreventCommandIfReadOnly("lo_unlink()");
+
    /*
     * Must be owner of the large object.  It would be cleaner to check this
     * in inv_drop(), but we want to throw the error before not after closing
@@ -368,6 +377,8 @@ be_lowrite(PG_FUNCTION_ARGS)
    int         bytestowrite;
    int         totalwritten;
 
+   PreventCommandIfReadOnly("lowrite()");
+
    bytestowrite = VARSIZE_ANY_EXHDR(wbuf);
    totalwritten = lo_write(fd, VARDATA_ANY(wbuf), bytestowrite);
    PG_RETURN_INT32(totalwritten);
@@ -413,6 +424,8 @@ lo_import_internal(text *filename, Oid lobjOid)
    LargeObjectDesc *lobj;
    Oid         oid;
 
+   PreventCommandIfReadOnly("lo_import()");
+
    /*
     * open the file to be read in
     */
@@ -561,6 +574,8 @@ be_lo_truncate(PG_FUNCTION_ARGS)
    int32       fd = PG_GETARG_INT32(0);
    int32       len = PG_GETARG_INT32(1);
 
+   PreventCommandIfReadOnly("lo_truncate()");
+
    lo_truncate_internal(fd, len);
    PG_RETURN_INT32(0);
 }
@@ -571,6 +586,8 @@ be_lo_truncate64(PG_FUNCTION_ARGS)
    int32       fd = PG_GETARG_INT32(0);
    int64       len = PG_GETARG_INT64(1);
 
+   PreventCommandIfReadOnly("lo_truncate64()");
+
    lo_truncate_internal(fd, len);
    PG_RETURN_INT32(0);
 }
@@ -815,6 +832,8 @@ be_lo_from_bytea(PG_FUNCTION_ARGS)
    LargeObjectDesc *loDesc;
    int         written PG_USED_FOR_ASSERTS_ONLY;
 
+   PreventCommandIfReadOnly("lo_from_bytea()");
+
    lo_cleanup_needed = true;
    loOid = inv_create(loOid);
    loDesc = inv_open(loOid, INV_WRITE, CurrentMemoryContext);
@@ -837,6 +856,8 @@ be_lo_put(PG_FUNCTION_ARGS)
    LargeObjectDesc *loDesc;
    int         written PG_USED_FOR_ASSERTS_ONLY;
 
+   PreventCommandIfReadOnly("lo_put()");
+
    lo_cleanup_needed = true;
    loDesc = inv_open(loOid, INV_WRITE, CurrentMemoryContext);
 

diff --git a/src/test/regress/expected/largeobject.out b/src/test/regress/expected/largeobject.out
index 60d7b991c193fcd9d90a27aa578c9214556994ed..31fba2ff9d308fe055ca3ab70d7883e399c82889 100644 (file)
--- a/src/test/regress/expected/largeobject.out
+++ b/src/test/regress/expected/largeobject.out
@@ -506,6 +506,55 @@ SELECT lo_create(2121);
 (1 row)
 
 COMMENT ON LARGE OBJECT 2121 IS 'testing comments';
+-- Test writes on large objects in read-only transactions
+START TRANSACTION READ ONLY;
+-- INV_READ ... ok
+SELECT lo_open(2121, x'40000'::int);
+ lo_open 
+---------
+       0
+(1 row)
+
+-- INV_WRITE ... error
+SELECT lo_open(2121, x'20000'::int);
+ERROR:  cannot execute lo_open(INV_WRITE) in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_create(42);
+ERROR:  cannot execute lo_create() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_creat(42);
+ERROR:  cannot execute lo_creat() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_unlink(42);
+ERROR:  cannot execute lo_unlink() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lowrite(42, 'x');
+ERROR:  cannot execute lowrite() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_import(:'filename');
+ERROR:  cannot execute lo_import() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_truncate(42, 0);
+ERROR:  cannot execute lo_truncate() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_truncate64(42, 0);
+ERROR:  cannot execute lo_truncate64() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_from_bytea(0, 'x');
+ERROR:  cannot execute lo_from_bytea() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_put(42, 0, 'x');
+ERROR:  cannot execute lo_put() in a read-only transaction
+ROLLBACK;
 -- Clean up
 DROP TABLE lotest_stash_values;
 DROP ROLE regress_lo_user;

diff --git a/src/test/regress/expected/largeobject_1.out b/src/test/regress/expected/largeobject_1.out
index 30c8d3da800d4273fe372cf8ab79333d8bdead9b..7acd7f73e1a3aa6c6afe958a250a6d5247a8d486 100644 (file)
--- a/src/test/regress/expected/largeobject_1.out
+++ b/src/test/regress/expected/largeobject_1.out
@@ -506,6 +506,55 @@ SELECT lo_create(2121);
 (1 row)
 
 COMMENT ON LARGE OBJECT 2121 IS 'testing comments';
+-- Test writes on large objects in read-only transactions
+START TRANSACTION READ ONLY;
+-- INV_READ ... ok
+SELECT lo_open(2121, x'40000'::int);
+ lo_open 
+---------
+       0
+(1 row)
+
+-- INV_WRITE ... error
+SELECT lo_open(2121, x'20000'::int);
+ERROR:  cannot execute lo_open(INV_WRITE) in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_create(42);
+ERROR:  cannot execute lo_create() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_creat(42);
+ERROR:  cannot execute lo_creat() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_unlink(42);
+ERROR:  cannot execute lo_unlink() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lowrite(42, 'x');
+ERROR:  cannot execute lowrite() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_import(:'filename');
+ERROR:  cannot execute lo_import() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_truncate(42, 0);
+ERROR:  cannot execute lo_truncate() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_truncate64(42, 0);
+ERROR:  cannot execute lo_truncate64() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_from_bytea(0, 'x');
+ERROR:  cannot execute lo_from_bytea() in a read-only transaction
+ROLLBACK;
+START TRANSACTION READ ONLY;
+SELECT lo_put(42, 0, 'x');
+ERROR:  cannot execute lo_put() in a read-only transaction
+ROLLBACK;
 -- Clean up
 DROP TABLE lotest_stash_values;
 DROP ROLE regress_lo_user;

diff --git a/src/test/regress/sql/largeobject.sql b/src/test/regress/sql/largeobject.sql
index 2ef03cfdae0d0a349642ad02f9c8df0b7dd83059..15e0dff7a3e8da03a0f500249b2f1387b992c9e2 100644 (file)
--- a/src/test/regress/sql/largeobject.sql
+++ b/src/test/regress/sql/largeobject.sql
@@ -271,6 +271,50 @@ SELECT lo_create(2121);
 
 COMMENT ON LARGE OBJECT 2121 IS 'testing comments';
 
+-- Test writes on large objects in read-only transactions
+START TRANSACTION READ ONLY;
+-- INV_READ ... ok
+SELECT lo_open(2121, x'40000'::int);
+-- INV_WRITE ... error
+SELECT lo_open(2121, x'20000'::int);
+ROLLBACK;
+
+START TRANSACTION READ ONLY;
+SELECT lo_create(42);
+ROLLBACK;
+
+START TRANSACTION READ ONLY;
+SELECT lo_creat(42);
+ROLLBACK;
+
+START TRANSACTION READ ONLY;
+SELECT lo_unlink(42);
+ROLLBACK;
+
+START TRANSACTION READ ONLY;
+SELECT lowrite(42, 'x');
+ROLLBACK;
+
+START TRANSACTION READ ONLY;
+SELECT lo_import(:'filename');
+ROLLBACK;
+
+START TRANSACTION READ ONLY;
+SELECT lo_truncate(42, 0);
+ROLLBACK;
+
+START TRANSACTION READ ONLY;
+SELECT lo_truncate64(42, 0);
+ROLLBACK;
+
+START TRANSACTION READ ONLY;
+SELECT lo_from_bytea(0, 'x');
+ROLLBACK;
+
+START TRANSACTION READ ONLY;
+SELECT lo_put(42, 0, 'x');
+ROLLBACK;
+
 -- Clean up
 DROP TABLE lotest_stash_values;