Fix GH-9411: PgSQL large object resource is incorrectly closed

Co-authored-by: Christoph M. Becker <cmbecker69@gmx.de>

Closes GH-9411.

NEWS

@@ -18,6 +18,10 @@ PHP                                                                        NEWS
   . Fixed bug #77780 ("Headers already sent..." when previous connection was
     aborted). (Jakub Zelenka)
 
+- PDO_PGSQL:
+  . Fixed bug GH-9411 (PgSQL large object resource is incorrectly closed).
+    (Yurunsoft)
+
 - Reflection:
   . Fixed bug GH-8932 (ReflectionFunction provides no way to get the called
     class of a Closure). (cmb, Nicolas Grekas)

ext/pdo_pgsql/pgsql_driver.c

@@ -35,6 +35,8 @@
 #include "zend_exceptions.h"
 #include "pgsql_driver_arginfo.h"
 
+static int pgsql_handle_in_transaction(pdo_dbh_t *dbh);
+
 static char * _pdo_pgsql_trim_message(const char *message, int persistent)
 {
 	register int i = strlen(message)-1;
@@ -141,10 +143,12 @@ static ssize_t pgsql_lob_read(php_stream *stream, char *buf, size_t count)
 static int pgsql_lob_close(php_stream *stream, int close_handle)
 {
 	struct pdo_pgsql_lob_self *self = (struct pdo_pgsql_lob_self*)stream->abstract;
+	pdo_pgsql_db_handle *H = (pdo_pgsql_db_handle *)(Z_PDO_DBH_P(&self->dbh))->driver_data;
 
 	if (close_handle) {
 		lo_close(self->conn, self->lfd);
 	}
+	zend_hash_index_del(H->lob_streams, php_stream_get_resource_id(stream));
 	zval_ptr_dtor(&self->dbh);
 	efree(self);
 	return 0;
@@ -195,6 +199,7 @@ php_stream *pdo_pgsql_create_lob_stream(zval *dbh, int lfd, Oid oid)
 
 	if (stm) {
 		Z_ADDREF_P(dbh);
+		zend_hash_index_add_ptr(H->lob_streams, php_stream_get_resource_id(stm), stm->res);
 		return stm;
 	}
 
@@ -203,10 +208,29 @@ php_stream *pdo_pgsql_create_lob_stream(zval *dbh, int lfd, Oid oid)
 }
 /* }}} */
 
+void pdo_pgsql_close_lob_streams(pdo_dbh_t *dbh)
+{
+	zend_resource *res;
+	pdo_pgsql_db_handle *H = (pdo_pgsql_db_handle *)dbh->driver_data;
+	if (H->lob_streams) {
+		ZEND_HASH_REVERSE_FOREACH_PTR(H->lob_streams, res) {
+			if (res->type >= 0) {
+				zend_list_close(res);
+			}
+		} ZEND_HASH_FOREACH_END();
+	}
+}
+
 static int pgsql_handle_closer(pdo_dbh_t *dbh) /* {{{ */
 {
 	pdo_pgsql_db_handle *H = (pdo_pgsql_db_handle *)dbh->driver_data;
 	if (H) {
+		if (H->lob_streams) {
+			pdo_pgsql_close_lob_streams(dbh);
+			zend_hash_destroy(H->lob_streams);
+			pefree(H->lob_streams, dbh->is_persistent);
+			H->lob_streams = NULL;
+		}
 		if (H->server) {
 			PQfinish(H->server);
 			H->server = NULL;
@@ -298,6 +322,8 @@ static zend_long pgsql_handle_doer(pdo_dbh_t *dbh, const char *sql, size_t sql_l
 	zend_long ret = 1;
 	ExecStatusType qs;
 
+	bool in_trans = pgsql_handle_in_transaction(dbh);
+
 	if (!(res = PQexec(H->server, sql))) {
 		/* fatal error */
 		pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, NULL);
@@ -316,6 +342,9 @@ static zend_long pgsql_handle_doer(pdo_dbh_t *dbh, const char *sql, size_t sql_l
 		ret = Z_L(0);
 	}
 	PQclear(res);
+	if (in_trans && !pgsql_handle_in_transaction(dbh)) {
+		pdo_pgsql_close_lob_streams(dbh);
+	}
 
 	return ret;
 }
@@ -501,9 +530,7 @@ static int pdo_pgsql_check_liveness(pdo_dbh_t *dbh)
 
 static int pgsql_handle_in_transaction(pdo_dbh_t *dbh)
 {
-	pdo_pgsql_db_handle *H;
-
-	H = (pdo_pgsql_db_handle *)dbh->driver_data;
+	pdo_pgsql_db_handle *H = (pdo_pgsql_db_handle *)dbh->driver_data;
 
 	return PQtransactionStatus(H->server) > PQTRANS_IDLE;
 }
@@ -536,7 +563,9 @@ static int pgsql_handle_commit(pdo_dbh_t *dbh)
 
 	/* When deferred constraints are used the commit could
 	   fail, and a ROLLBACK implicitly ran. See bug #67462 */
-	if (!ret) {
+	if (ret) {
+		pdo_pgsql_close_lob_streams(dbh);
+	} else {
 		dbh->in_txn = pgsql_handle_in_transaction(dbh);
 	}
 
@@ -545,7 +574,13 @@ static int pgsql_handle_commit(pdo_dbh_t *dbh)
 
 static int pgsql_handle_rollback(pdo_dbh_t *dbh)
 {
-	return pdo_pgsql_transaction_cmd("ROLLBACK", dbh);
+	int ret = pdo_pgsql_transaction_cmd("ROLLBACK", dbh);
+
+	if (ret) {
+		pdo_pgsql_close_lob_streams(dbh);
+	}
+
+	return ret;
 }
 
 /* {{{ Returns true if the copy worked fine or false if error */
@@ -1233,6 +1268,8 @@ static int pdo_pgsql_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /* {{{
 	}
 
 	H->server = PQconnectdb(conn_str);
+	H->lob_streams = (HashTable *) pemalloc(sizeof(HashTable), dbh->is_persistent);
+	zend_hash_init(H->lob_streams, 0, NULL, NULL, 1);
 
 	if (tmp_user) {
 		zend_string_release_ex(tmp_user, 0);

ext/pdo_pgsql/pgsql_statement.c

@@ -134,6 +134,8 @@ static int pgsql_stmt_execute(pdo_stmt_t *stmt)
 	pdo_pgsql_db_handle *H = S->H;
 	ExecStatusType status;
 
+	bool in_trans = stmt->dbh->methods->in_transaction(stmt->dbh);
+
 	/* ensure that we free any previous unfetched results */
 	if(S->result) {
 		PQclear(S->result);
@@ -252,6 +254,10 @@ static int pgsql_stmt_execute(pdo_stmt_t *stmt)
 		stmt->row_count = (zend_long)PQntuples(S->result);
 	}
 
+	if (in_trans && !stmt->dbh->methods->in_transaction(stmt->dbh)) {
+		pdo_pgsql_close_lob_streams(stmt->dbh);
+	}
+
 	return 1;
 }
 

ext/pdo_pgsql/php_pdo_pgsql_int.h

@@ -45,6 +45,7 @@ typedef struct {
 	zend_bool		emulate_prepares;
 	zend_bool		disable_native_prepares; /* deprecated since 5.6 */
 	zend_bool		disable_prepares;
+	HashTable       *lob_streams;
 } pdo_pgsql_db_handle;
 
 typedef struct {
@@ -109,5 +110,6 @@ php_stream *pdo_pgsql_create_lob_stream(zval *pdh, int lfd, Oid oid);
 extern const php_stream_ops pdo_pgsql_lob_stream_ops;
 
 void pdo_libpq_version(char *buf, size_t len);
+void pdo_pgsql_close_lob_streams(pdo_dbh_t *dbh);
 
 #endif /* PHP_PDO_PGSQL_INT_H */

ext/pdo_pgsql/tests/gh9411.phpt

@@ -0,0 +1,41 @@
+--TEST--
+Bug GH-9411 (PgSQL large object resource is incorrectly closed)
+--SKIPIF--
+<?php
+if (!extension_loaded('pdo') || !extension_loaded('pdo_pgsql')) die('skip not loaded');
+require __DIR__ . '/config.inc';
+require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
+PDOTest::skip();
+?>
+--FILE--
+<?php
+require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
+$db = PDOTest::test_factory(__DIR__ . '/common.phpt');
+$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
+$db->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, false);
+
+$db->beginTransaction();
+$oid = $db->pgsqlLOBCreate();
+var_dump($lob = $db->pgsqlLOBOpen($oid, 'wb'));
+fwrite($lob, 'test');
+$db->rollback();
+var_dump($lob);
+
+$db->beginTransaction();
+$oid = $db->pgsqlLOBCreate();
+var_dump($lob = $db->pgsqlLOBOpen($oid, 'wb'));
+fwrite($lob, 'test');
+$db->commit();
+var_dump($lob);
+
+$db->beginTransaction();
+var_dump($lob = $db->pgsqlLOBOpen($oid, 'wb'));
+var_dump(fgets($lob));
+?>
+--EXPECTF--
+resource(%d) of type (stream)
+resource(%d) of type (Unknown)
+resource(%d) of type (stream)
+resource(%d) of type (Unknown)
+resource(%d) of type (stream)
+string(4) "test"