From 6ac3f7c84dcf2091decad5cd05cc608d248d86e5 Mon Sep 17 00:00:00 2001 From: Yurun Date: Mon, 5 Sep 2022 14:53:33 +0200 Subject: [PATCH] Fix GH-9411: PgSQL large object resource is incorrectly closed Co-authored-by: Christoph M. Becker Closes GH-9411. --- NEWS | 4 +++ ext/pdo_pgsql/pgsql_driver.c | 47 +++++++++++++++++++++++++++---- ext/pdo_pgsql/pgsql_statement.c | 6 ++++ ext/pdo_pgsql/php_pdo_pgsql_int.h | 2 ++ ext/pdo_pgsql/tests/gh9411.phpt | 41 +++++++++++++++++++++++++++ 5 files changed, 95 insertions(+), 5 deletions(-) create mode 100644 ext/pdo_pgsql/tests/gh9411.phpt diff --git a/NEWS b/NEWS index 805dec249ad..3e85f1483ec 100644 --- a/NEWS +++ b/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) diff --git a/ext/pdo_pgsql/pgsql_driver.c b/ext/pdo_pgsql/pgsql_driver.c index b6449095ca0..c90ef468907 100644 --- a/ext/pdo_pgsql/pgsql_driver.c +++ b/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); diff --git a/ext/pdo_pgsql/pgsql_statement.c b/ext/pdo_pgsql/pgsql_statement.c index 21f4c83807e..4c32ac80e8d 100644 --- a/ext/pdo_pgsql/pgsql_statement.c +++ b/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 @@ stmt_retry: 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; } diff --git a/ext/pdo_pgsql/php_pdo_pgsql_int.h b/ext/pdo_pgsql/php_pdo_pgsql_int.h index bc31c9cdeee..eea1ad647ee 100644 --- a/ext/pdo_pgsql/php_pdo_pgsql_int.h +++ b/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 */ diff --git a/ext/pdo_pgsql/tests/gh9411.phpt b/ext/pdo_pgsql/tests/gh9411.phpt new file mode 100644 index 00000000000..c8a11e89df6 --- /dev/null +++ b/ext/pdo_pgsql/tests/gh9411.phpt @@ -0,0 +1,41 @@ +--TEST-- +Bug GH-9411 (PgSQL large object resource is incorrectly closed) +--SKIPIF-- + +--FILE-- +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"