diff --git a/NEWS b/NEWS index b05aaaa635d..451c0ed2ae1 100644 --- a/NEWS +++ b/NEWS @@ -23,6 +23,10 @@ PHP NEWS - PCRE: . Fixed pcre.jit on Apple Silicon. (Niklas Keller) +- 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 54bf7ede6bf..f84bfba9f84 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 bool pgsql_handle_in_transaction(pdo_dbh_t *dbh); + static char * _pdo_pgsql_trim_message(const char *message, int persistent) { size_t i = strlen(message)-1; @@ -140,10 +142,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; @@ -194,6 +198,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; } @@ -202,10 +207,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 void 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; @@ -295,6 +319,8 @@ static zend_long pgsql_handle_doer(pdo_dbh_t *dbh, const zend_string *sql) zend_long ret = 1; ExecStatusType qs; + bool in_trans = pgsql_handle_in_transaction(dbh); + if (!(res = PQexec(H->server, ZSTR_VAL(sql)))) { /* fatal error */ pdo_pgsql_error(dbh, PGRES_FATAL_ERROR, NULL); @@ -313,6 +339,9 @@ static zend_long pgsql_handle_doer(pdo_dbh_t *dbh, const zend_string *sql) ret = Z_L(0); } PQclear(res); + if (in_trans && !pgsql_handle_in_transaction(dbh)) { + pdo_pgsql_close_lob_streams(dbh); + } return ret; } @@ -503,9 +532,7 @@ static zend_result pdo_pgsql_check_liveness(pdo_dbh_t *dbh) static bool 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; } @@ -538,7 +565,9 @@ static bool 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); } @@ -547,7 +576,13 @@ static bool pgsql_handle_commit(pdo_dbh_t *dbh) static bool 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 */ @@ -1242,6 +1277,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 03ac14c32db..76157ae17fe 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 8ef8b681384..f45718f0c96 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 { bool emulate_prepares; bool disable_native_prepares; /* deprecated since 5.6 */ bool disable_prepares; + HashTable *lob_streams; } pdo_pgsql_db_handle; typedef struct { @@ -106,5 +107,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"