From 24e4b24d33dc96f7724f00e676f4e1fcd3210437 Mon Sep 17 00:00:00 2001 From: Guillaume Outters Date: Wed, 22 May 2024 16:28:31 +0200 Subject: [PATCH 1/7] Pdo\Pgsql::setNoticeCallback() Make the newly implemented setNoticeCallback() officially exposed (#6764) --- UPGRADING | 4 ++-- ext/pdo_pgsql/pdo_pgsql.stub.php | 3 +++ ext/pdo_pgsql/pdo_pgsql_arginfo.h | 8 +++++++- ext/pdo_pgsql/pgsql_driver.c | 3 +-- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/UPGRADING b/UPGRADING index 8d127d094a06f..8924f51ea2638 100644 --- a/UPGRADING +++ b/UPGRADING @@ -544,8 +544,8 @@ PHP 8.4 UPGRADE NOTES energy consumption) of the current process and pcntl_setqos_class to set it. - PDO_PGSQL: - . Added PDO::pgsqlSetNoticeCallback to allow a callback to be triggered on - every notice sent (e.g. RAISE NOTICE). + . Added Pdo\Pgsql::setNoticeCallback() / PDO::pgsqlSetNoticeCallback() to allow + a callback to be triggered on every notice sent (e.g. RAISE NOTICE). - PGSQL: . Added pg_change_password to alter a given user's password. It handles diff --git a/ext/pdo_pgsql/pdo_pgsql.stub.php b/ext/pdo_pgsql/pdo_pgsql.stub.php index f203d3e82c3e4..e33999a14ce98 100644 --- a/ext/pdo_pgsql/pdo_pgsql.stub.php +++ b/ext/pdo_pgsql/pdo_pgsql.stub.php @@ -56,4 +56,7 @@ public function lobUnlink(string $oid): bool {} public function getNotify(int $fetchMode = \PDO::FETCH_DEFAULT, int $timeoutMilliseconds = 0): array|false {} public function getPid(): int {} + + /** @implementation-alias PDO_PGSql_Ext::pgsqlSetNoticeCallback */ + public function setNoticeCallback(?callable $callback): void {} } diff --git a/ext/pdo_pgsql/pdo_pgsql_arginfo.h b/ext/pdo_pgsql/pdo_pgsql_arginfo.h index 8b2abce3f75e2..d104d0ca82577 100644 --- a/ext/pdo_pgsql/pdo_pgsql_arginfo.h +++ b/ext/pdo_pgsql/pdo_pgsql_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 519530b2da60dbd7a70af6d7c566d93930cfbb24 */ + * Stub hash: 92d66416beab5094d0ca70565e691eacc3049d9c */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Pdo_Pgsql_escapeIdentifier, 0, 1, IS_STRING, 0) ZEND_ARG_TYPE_INFO(0, input, IS_STRING, 0) @@ -50,6 +50,10 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Pdo_Pgsql_getPid, 0, 0, IS_LONG, 0) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Pdo_Pgsql_setNoticeCallback, 0, 1, IS_VOID, 0) + ZEND_ARG_TYPE_INFO(0, callback, IS_CALLABLE, 1) +ZEND_END_ARG_INFO() + ZEND_METHOD(Pdo_Pgsql, escapeIdentifier); ZEND_METHOD(Pdo_Pgsql, copyFromArray); ZEND_METHOD(Pdo_Pgsql, copyFromFile); @@ -60,6 +64,7 @@ ZEND_METHOD(Pdo_Pgsql, lobOpen); ZEND_METHOD(Pdo_Pgsql, lobUnlink); ZEND_METHOD(Pdo_Pgsql, getNotify); ZEND_METHOD(Pdo_Pgsql, getPid); +ZEND_METHOD(PDO_PGSql_Ext, pgsqlSetNoticeCallback); static const zend_function_entry class_Pdo_Pgsql_methods[] = { ZEND_ME(Pdo_Pgsql, escapeIdentifier, arginfo_class_Pdo_Pgsql_escapeIdentifier, ZEND_ACC_PUBLIC) @@ -72,6 +77,7 @@ static const zend_function_entry class_Pdo_Pgsql_methods[] = { ZEND_ME(Pdo_Pgsql, lobUnlink, arginfo_class_Pdo_Pgsql_lobUnlink, ZEND_ACC_PUBLIC) ZEND_ME(Pdo_Pgsql, getNotify, arginfo_class_Pdo_Pgsql_getNotify, ZEND_ACC_PUBLIC) ZEND_ME(Pdo_Pgsql, getPid, arginfo_class_Pdo_Pgsql_getPid, ZEND_ACC_PUBLIC) + ZEND_RAW_FENTRY("setNoticeCallback", zim_PDO_PGSql_Ext_pgsqlSetNoticeCallback, arginfo_class_Pdo_Pgsql_setNoticeCallback, ZEND_ACC_PUBLIC, NULL, NULL) ZEND_FE_END }; diff --git a/ext/pdo_pgsql/pgsql_driver.c b/ext/pdo_pgsql/pgsql_driver.c index 41d9fa85e5193..4c9b948b05f22 100644 --- a/ext/pdo_pgsql/pgsql_driver.c +++ b/ext/pdo_pgsql/pgsql_driver.c @@ -1241,8 +1241,7 @@ PHP_METHOD(PDO_PGSql_Ext, pgsqlGetPid) } /* }}} */ -/* {{{ proto void PDO::pgsqlSetNoticeCallback(mixed callback) - Sets a callback to receive DB notices (after client_min_messages has been set) */ +/* {{{ Sets a callback to receive DB notices (after client_min_messages has been set) */ PHP_METHOD(PDO_PGSql_Ext, pgsqlSetNoticeCallback) { zend_fcall_info fci = empty_fcall_info; From 4899172d67dfb66981b5b91ce38f24e8c3bf14aa Mon Sep 17 00:00:00 2001 From: Guillaume Outters Date: Thu, 23 May 2024 07:07:26 +0200 Subject: [PATCH 2/7] Pdo\Pgsql::setNoticeCallback(): test based on PDO::pgsqlSetNoticeCallback()'s one --- ext/pdo_pgsql/tests/issue78621.inc | 15 ++++++- .../tests/pdopgsql_setNoticeCallback.phpt | 43 +++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 ext/pdo_pgsql/tests/pdopgsql_setNoticeCallback.phpt diff --git a/ext/pdo_pgsql/tests/issue78621.inc b/ext/pdo_pgsql/tests/issue78621.inc index 8a42a1b50cfd2..794e214d349ff 100644 --- a/ext/pdo_pgsql/tests/issue78621.inc +++ b/ext/pdo_pgsql/tests/issue78621.inc @@ -1,9 +1,14 @@ beginTransaction(); $db->exec("set client_min_messages to notice"); @@ -11,9 +16,15 @@ $db->exec("create temporary table t (a varchar(3))"); $db->exec("create function hey() returns trigger as \$\$ begin new.a := 'oh'; raise notice 'I tampered your data, did you know?'; return new; end; \$\$ language plpgsql"); $db->exec("create trigger hop before insert on t for each row execute procedure hey()"); $db->exec("insert into t values ('ah')"); -attach($db, 'Re'); +while (count($rounds)) { +try { +attach($db, array_shift($rounds)); +} catch (TypeError $err) { +echo "Caught TypeError: ".$err->getMessage()."\n"; +} $db->exec("delete from t"); $db->exec("insert into t values ('ah')"); +} $db->pgsqlSetNoticeCallback(null); $db->exec("delete from t"); $db->exec("insert into t values ('ah')"); diff --git a/ext/pdo_pgsql/tests/pdopgsql_setNoticeCallback.phpt b/ext/pdo_pgsql/tests/pdopgsql_setNoticeCallback.phpt new file mode 100644 index 0000000000000..e01bed7640d97 --- /dev/null +++ b/ext/pdo_pgsql/tests/pdopgsql_setNoticeCallback.phpt @@ -0,0 +1,43 @@ +--TEST-- +Pdo\Pgsql::setNoticeCallback() +--EXTENSIONS-- +pdo +pdo_pgsql +--SKIPIF-- + +--FILE-- +setNoticeCallback($callback); } + +$rounds = [ + 'disp', // Correct. + 3, // Error, so the old callback is kept, and will be used in the call that follows the caught error. + null, // No callback. Hopefully this clears everything. + 'wouldAnyoneNameAFunctionThatWay', // So this one will crash and *no output will follow*. +]; +require __DIR__ . '/issue78621.inc'; + +?> +--EXPECTF-- +NOTICE: I tampered your data, did you know? +Caught TypeError: %s: Argument #1 ($callback) %s +NOTICE: I tampered your data, did you know? +Caught TypeError: %s: Argument #1 ($callback) %s +array(1) { + [0]=> + array(1) { + ["a"]=> + string(2) "oh" + } +} +Done From 0e4c7b68fd9d5e43c7a2b96f1d36d36783ad8908 Mon Sep 17 00:00:00 2001 From: Guillaume Outters Date: Thu, 23 May 2024 22:59:06 +0200 Subject: [PATCH 3/7] Pdo\Pgsql::setNoticeCallback(): theorical memory leak https://github.com/php/php-src/commit/c265b9085ae9b20bb37e0c1a052a1716827a8004#r142275842 Redefines PDO_CONSTRUCT_CHECK to make room for an optional cleanup --- ext/pdo/php_pdo.h | 17 +++++++++++++++-- ext/pdo_pgsql/pgsql_driver.c | 10 +++++++++- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/ext/pdo/php_pdo.h b/ext/pdo/php_pdo.h index 8f37990e955d1..1618c78d62284 100644 --- a/ext/pdo/php_pdo.h +++ b/ext/pdo/php_pdo.h @@ -55,11 +55,24 @@ PHP_MINFO_FUNCTION(pdo); #define LONG_CONST(c) (zend_long) c +#define PDO_CONSTRUCT_CHECK_COND dbh->driver +#define PDO_CONSTRUCT_CHECK_FAIL() \ + { \ + zend_throw_error(NULL, "%s object is uninitialized", ZSTR_VAL(Z_OBJ(EX(This))->ce->name)); \ + } \ + #define PDO_CONSTRUCT_CHECK \ - if (!dbh->driver) { \ - zend_throw_error(NULL, "%s object is uninitialized", ZSTR_VAL(Z_OBJ(EX(This))->ce->name)); \ + if (!(PDO_CONSTRUCT_CHECK_COND)) { \ + PDO_CONSTRUCT_CHECK_FAIL(); \ RETURN_THROWS(); \ } \ +#define PDO_CONSTRUCT_CHECK_WITH_CLEANUP(cleanup) \ + if (!(PDO_CONSTRUCT_CHECK_COND)) { \ + PDO_CONSTRUCT_CHECK_FAIL(); \ + goto cleanup; \ + } \ + + #endif /* PHP_PDO_H */ diff --git a/ext/pdo_pgsql/pgsql_driver.c b/ext/pdo_pgsql/pgsql_driver.c index 4c9b948b05f22..928c1c9e3ea03 100644 --- a/ext/pdo_pgsql/pgsql_driver.c +++ b/ext/pdo_pgsql/pgsql_driver.c @@ -1251,7 +1251,7 @@ PHP_METHOD(PDO_PGSql_Ext, pgsqlSetNoticeCallback) } pdo_dbh_t *dbh = Z_PDO_DBH_P(ZEND_THIS); - PDO_CONSTRUCT_CHECK; + PDO_CONSTRUCT_CHECK_WITH_CLEANUP(cleanup); pdo_pgsql_db_handle *H = (pdo_pgsql_db_handle *)dbh->driver_data; @@ -1261,6 +1261,14 @@ PHP_METHOD(PDO_PGSql_Ext, pgsqlSetNoticeCallback) H->notice_callback = emalloc(sizeof(zend_fcall_info_cache)); zend_fcc_dup(H->notice_callback, &fcc); } + + return; + +cleanup: + if (ZEND_FCC_INITIALIZED(fcc)) { + zend_fcc_dtor(&fcc); + } + RETURN_THROWS(); } /* }}} */ From e14102dbd0e7d40ba688f71a8a20dff4854c0c64 Mon Sep 17 00:00:00 2001 From: Guillaume Outters Date: Mon, 27 May 2024 21:11:50 +0200 Subject: [PATCH 4/7] Pdo\Pgsql::setNoticeCallback() preferred over PDO::pgsqlSetNoticeCallback() see #14299 --- ext/pdo_pgsql/pdo_pgsql.c | 30 ++++++++++++++++++++++++++++ ext/pdo_pgsql/pdo_pgsql.stub.php | 1 - ext/pdo_pgsql/pdo_pgsql_arginfo.h | 4 ++-- ext/pdo_pgsql/pgsql_driver.c | 2 +- ext/pdo_pgsql/pgsql_driver.stub.php | 7 ++++++- ext/pdo_pgsql/pgsql_driver_arginfo.h | 14 +++++++++---- ext/pdo_pgsql/php_pdo_pgsql_int.h | 2 ++ 7 files changed, 51 insertions(+), 9 deletions(-) diff --git a/ext/pdo_pgsql/pdo_pgsql.c b/ext/pdo_pgsql/pdo_pgsql.c index 77544d272d408..4c18806da6ba7 100644 --- a/ext/pdo_pgsql/pdo_pgsql.c +++ b/ext/pdo_pgsql/pdo_pgsql.c @@ -145,6 +145,36 @@ PHP_METHOD(Pdo_Pgsql, getPid) pgsqlGetPid_internal(INTERNAL_FUNCTION_PARAM_PASSTHRU); } +/* Sets a callback to receive DB notices (after client_min_messages has been set */ +PHP_METHOD(Pdo_Pgsql, setNoticeCallback) +{ + zend_fcall_info fci = empty_fcall_info; + zend_fcall_info_cache fcc = empty_fcall_info_cache; + if (FAILURE == zend_parse_parameters(ZEND_NUM_ARGS(), "F!", &fci, &fcc)) { + RETURN_THROWS(); + } + + pdo_dbh_t *dbh = Z_PDO_DBH_P(ZEND_THIS); + PDO_CONSTRUCT_CHECK_WITH_CLEANUP(cleanup); + + pdo_pgsql_db_handle *H = (pdo_pgsql_db_handle *)dbh->driver_data; + + pdo_pgsql_cleanup_notice_callback(H); + + if (ZEND_FCC_INITIALIZED(fcc)) { + H->notice_callback = emalloc(sizeof(zend_fcall_info_cache)); + zend_fcc_dup(H->notice_callback, &fcc); + } + + return; + +cleanup: + if (ZEND_FCC_INITIALIZED(fcc)) { + zend_fcc_dtor(&fcc); + } + RETURN_THROWS(); +} + /* true global environment */ /* {{{ PHP_MINIT_FUNCTION */ diff --git a/ext/pdo_pgsql/pdo_pgsql.stub.php b/ext/pdo_pgsql/pdo_pgsql.stub.php index e33999a14ce98..f33ace40055bc 100644 --- a/ext/pdo_pgsql/pdo_pgsql.stub.php +++ b/ext/pdo_pgsql/pdo_pgsql.stub.php @@ -57,6 +57,5 @@ public function getNotify(int $fetchMode = \PDO::FETCH_DEFAULT, int $timeoutMill public function getPid(): int {} - /** @implementation-alias PDO_PGSql_Ext::pgsqlSetNoticeCallback */ public function setNoticeCallback(?callable $callback): void {} } diff --git a/ext/pdo_pgsql/pdo_pgsql_arginfo.h b/ext/pdo_pgsql/pdo_pgsql_arginfo.h index d104d0ca82577..c86d8d87f311d 100644 --- a/ext/pdo_pgsql/pdo_pgsql_arginfo.h +++ b/ext/pdo_pgsql/pdo_pgsql_arginfo.h @@ -64,7 +64,7 @@ ZEND_METHOD(Pdo_Pgsql, lobOpen); ZEND_METHOD(Pdo_Pgsql, lobUnlink); ZEND_METHOD(Pdo_Pgsql, getNotify); ZEND_METHOD(Pdo_Pgsql, getPid); -ZEND_METHOD(PDO_PGSql_Ext, pgsqlSetNoticeCallback); +ZEND_METHOD(Pdo_Pgsql, setNoticeCallback); static const zend_function_entry class_Pdo_Pgsql_methods[] = { ZEND_ME(Pdo_Pgsql, escapeIdentifier, arginfo_class_Pdo_Pgsql_escapeIdentifier, ZEND_ACC_PUBLIC) @@ -77,7 +77,7 @@ static const zend_function_entry class_Pdo_Pgsql_methods[] = { ZEND_ME(Pdo_Pgsql, lobUnlink, arginfo_class_Pdo_Pgsql_lobUnlink, ZEND_ACC_PUBLIC) ZEND_ME(Pdo_Pgsql, getNotify, arginfo_class_Pdo_Pgsql_getNotify, ZEND_ACC_PUBLIC) ZEND_ME(Pdo_Pgsql, getPid, arginfo_class_Pdo_Pgsql_getPid, ZEND_ACC_PUBLIC) - ZEND_RAW_FENTRY("setNoticeCallback", zim_PDO_PGSql_Ext_pgsqlSetNoticeCallback, arginfo_class_Pdo_Pgsql_setNoticeCallback, ZEND_ACC_PUBLIC, NULL, NULL) + ZEND_ME(Pdo_Pgsql, setNoticeCallback, arginfo_class_Pdo_Pgsql_setNoticeCallback, ZEND_ACC_PUBLIC) ZEND_FE_END }; diff --git a/ext/pdo_pgsql/pgsql_driver.c b/ext/pdo_pgsql/pgsql_driver.c index 928c1c9e3ea03..a8d4f7efbd672 100644 --- a/ext/pdo_pgsql/pgsql_driver.c +++ b/ext/pdo_pgsql/pgsql_driver.c @@ -131,7 +131,7 @@ static void pdo_pgsql_fetch_error_func(pdo_dbh_t *dbh, pdo_stmt_t *stmt, zval *i } /* }}} */ -static void pdo_pgsql_cleanup_notice_callback(pdo_pgsql_db_handle *H) /* {{{ */ +void pdo_pgsql_cleanup_notice_callback(pdo_pgsql_db_handle *H) /* {{{ */ { if (H->notice_callback) { zend_fcc_dtor(H->notice_callback); diff --git a/ext/pdo_pgsql/pgsql_driver.stub.php b/ext/pdo_pgsql/pgsql_driver.stub.php index 5e09ec683b29a..34d23523de129 100644 --- a/ext/pdo_pgsql/pgsql_driver.stub.php +++ b/ext/pdo_pgsql/pgsql_driver.stub.php @@ -34,6 +34,11 @@ public function pgsqlGetNotify(int $fetchMode = PDO::FETCH_DEFAULT, int $timeout /** @tentative-return-type */ public function pgsqlGetPid(): int {} - /** @tentative-return-type */ + /* Do NOT add new methods here. See https://wiki.php.net/rfc/pdo_driver_specific_subclasses + * Any new feature should be declared only on Pdo\Pgsql. + */ +#if 1 + /** @implementation-alias Pdo\Pgsql::setNoticeCallback */ public function pgsqlSetNoticeCallback(?callable $callback): void {} +#endif } diff --git a/ext/pdo_pgsql/pgsql_driver_arginfo.h b/ext/pdo_pgsql/pgsql_driver_arginfo.h index d38b0fb7577dc..b70b90e9d2a2a 100644 --- a/ext/pdo_pgsql/pgsql_driver_arginfo.h +++ b/ext/pdo_pgsql/pgsql_driver_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 14174ab18f198b9916f83986d10c93b657d8ffb9 */ + * Stub hash: d8fac4d68600af70db47e772439c0a8196b34daa */ ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_PDO_PGSql_Ext_pgsqlCopyFromArray, 0, 2, _IS_BOOL, 0) ZEND_ARG_TYPE_INFO(0, tableName, IS_STRING, 0) @@ -46,9 +46,11 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_PDO_PGSql_Ext_pgsqlGetPid, 0, 0, IS_LONG, 0) ZEND_END_ARG_INFO() -ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_PDO_PGSql_Ext_pgsqlSetNoticeCallback, 0, 1, IS_VOID, 0) +#if 1 +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_PDO_PGSql_Ext_pgsqlSetNoticeCallback, 0, 1, IS_VOID, 0) ZEND_ARG_TYPE_INFO(0, callback, IS_CALLABLE, 1) ZEND_END_ARG_INFO() +#endif ZEND_METHOD(PDO_PGSql_Ext, pgsqlCopyFromArray); ZEND_METHOD(PDO_PGSql_Ext, pgsqlCopyFromFile); @@ -59,7 +61,9 @@ ZEND_METHOD(PDO_PGSql_Ext, pgsqlLOBOpen); ZEND_METHOD(PDO_PGSql_Ext, pgsqlLOBUnlink); ZEND_METHOD(PDO_PGSql_Ext, pgsqlGetNotify); ZEND_METHOD(PDO_PGSql_Ext, pgsqlGetPid); -ZEND_METHOD(PDO_PGSql_Ext, pgsqlSetNoticeCallback); +#if 1 +ZEND_METHOD(Pdo_Pgsql, setNoticeCallback); +#endif static const zend_function_entry class_PDO_PGSql_Ext_methods[] = { ZEND_ME(PDO_PGSql_Ext, pgsqlCopyFromArray, arginfo_class_PDO_PGSql_Ext_pgsqlCopyFromArray, ZEND_ACC_PUBLIC) @@ -71,6 +75,8 @@ static const zend_function_entry class_PDO_PGSql_Ext_methods[] = { ZEND_ME(PDO_PGSql_Ext, pgsqlLOBUnlink, arginfo_class_PDO_PGSql_Ext_pgsqlLOBUnlink, ZEND_ACC_PUBLIC) ZEND_ME(PDO_PGSql_Ext, pgsqlGetNotify, arginfo_class_PDO_PGSql_Ext_pgsqlGetNotify, ZEND_ACC_PUBLIC) ZEND_ME(PDO_PGSql_Ext, pgsqlGetPid, arginfo_class_PDO_PGSql_Ext_pgsqlGetPid, ZEND_ACC_PUBLIC) - ZEND_ME(PDO_PGSql_Ext, pgsqlSetNoticeCallback, arginfo_class_PDO_PGSql_Ext_pgsqlSetNoticeCallback, ZEND_ACC_PUBLIC) +#if 1 + ZEND_RAW_FENTRY("pgsqlSetNoticeCallback", zim_Pdo_Pgsql_setNoticeCallback, arginfo_class_PDO_PGSql_Ext_pgsqlSetNoticeCallback, ZEND_ACC_PUBLIC, NULL, NULL) +#endif ZEND_FE_END }; diff --git a/ext/pdo_pgsql/php_pdo_pgsql_int.h b/ext/pdo_pgsql/php_pdo_pgsql_int.h index daa3357d79acb..5b3e194f23d64 100644 --- a/ext/pdo_pgsql/php_pdo_pgsql_int.h +++ b/ext/pdo_pgsql/php_pdo_pgsql_int.h @@ -108,6 +108,8 @@ enum pdo_pgsql_specific_constants { 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_pgsql_cleanup_notice_callback(pdo_pgsql_db_handle *H); + void pdo_libpq_version(char *buf, size_t len); void pdo_pgsql_close_lob_streams(pdo_dbh_t *dbh); From a4cc92cf4b8b8ac4d5ee98c76e94c8def4226d0f Mon Sep 17 00:00:00 2001 From: Guillaume Outters Date: Mon, 27 May 2024 21:17:31 +0200 Subject: [PATCH 5/7] remove PDO::pgsqlSetNoticeCallback() Keep only Pdo\Pgsql::setNoticeCallback() --- UPGRADING | 4 ++-- ext/pdo_pgsql/pgsql_driver.stub.php | 4 ---- ext/pdo_pgsql/pgsql_driver_arginfo.h | 14 +------------- ext/pdo_pgsql/tests/issue78621.inc | 18 +++++++++--------- ext/pdo_pgsql/tests/issue78621.phpt | 4 ++-- ext/pdo_pgsql/tests/issue78621_closure.phpt | 6 +++--- ext/pdo_pgsql/tests/issue78621_method.phpt | 6 +++--- 7 files changed, 20 insertions(+), 36 deletions(-) diff --git a/UPGRADING b/UPGRADING index 8924f51ea2638..3fac4da11e0cb 100644 --- a/UPGRADING +++ b/UPGRADING @@ -544,8 +544,8 @@ PHP 8.4 UPGRADE NOTES energy consumption) of the current process and pcntl_setqos_class to set it. - PDO_PGSQL: - . Added Pdo\Pgsql::setNoticeCallback() / PDO::pgsqlSetNoticeCallback() to allow - a callback to be triggered on every notice sent (e.g. RAISE NOTICE). + . Added Pdo\Pgsql::setNoticeCallback() to allow a callback to be triggered on + every notice sent (e.g. RAISE NOTICE). - PGSQL: . Added pg_change_password to alter a given user's password. It handles diff --git a/ext/pdo_pgsql/pgsql_driver.stub.php b/ext/pdo_pgsql/pgsql_driver.stub.php index 34d23523de129..63236f0ae084d 100644 --- a/ext/pdo_pgsql/pgsql_driver.stub.php +++ b/ext/pdo_pgsql/pgsql_driver.stub.php @@ -37,8 +37,4 @@ public function pgsqlGetPid(): int {} /* Do NOT add new methods here. See https://wiki.php.net/rfc/pdo_driver_specific_subclasses * Any new feature should be declared only on Pdo\Pgsql. */ -#if 1 - /** @implementation-alias Pdo\Pgsql::setNoticeCallback */ - public function pgsqlSetNoticeCallback(?callable $callback): void {} -#endif } diff --git a/ext/pdo_pgsql/pgsql_driver_arginfo.h b/ext/pdo_pgsql/pgsql_driver_arginfo.h index b70b90e9d2a2a..5bdea01edc259 100644 --- a/ext/pdo_pgsql/pgsql_driver_arginfo.h +++ b/ext/pdo_pgsql/pgsql_driver_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: d8fac4d68600af70db47e772439c0a8196b34daa */ + * Stub hash: dd20abc5d8580d72b25bfb3c598b1ca54a501fcc */ ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_PDO_PGSql_Ext_pgsqlCopyFromArray, 0, 2, _IS_BOOL, 0) ZEND_ARG_TYPE_INFO(0, tableName, IS_STRING, 0) @@ -46,12 +46,6 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_TENTATIVE_RETURN_TYPE_INFO_EX(arginfo_class_PDO_PGSql_Ext_pgsqlGetPid, 0, 0, IS_LONG, 0) ZEND_END_ARG_INFO() -#if 1 -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_PDO_PGSql_Ext_pgsqlSetNoticeCallback, 0, 1, IS_VOID, 0) - ZEND_ARG_TYPE_INFO(0, callback, IS_CALLABLE, 1) -ZEND_END_ARG_INFO() -#endif - ZEND_METHOD(PDO_PGSql_Ext, pgsqlCopyFromArray); ZEND_METHOD(PDO_PGSql_Ext, pgsqlCopyFromFile); ZEND_METHOD(PDO_PGSql_Ext, pgsqlCopyToArray); @@ -61,9 +55,6 @@ ZEND_METHOD(PDO_PGSql_Ext, pgsqlLOBOpen); ZEND_METHOD(PDO_PGSql_Ext, pgsqlLOBUnlink); ZEND_METHOD(PDO_PGSql_Ext, pgsqlGetNotify); ZEND_METHOD(PDO_PGSql_Ext, pgsqlGetPid); -#if 1 -ZEND_METHOD(Pdo_Pgsql, setNoticeCallback); -#endif static const zend_function_entry class_PDO_PGSql_Ext_methods[] = { ZEND_ME(PDO_PGSql_Ext, pgsqlCopyFromArray, arginfo_class_PDO_PGSql_Ext_pgsqlCopyFromArray, ZEND_ACC_PUBLIC) @@ -75,8 +66,5 @@ static const zend_function_entry class_PDO_PGSql_Ext_methods[] = { ZEND_ME(PDO_PGSql_Ext, pgsqlLOBUnlink, arginfo_class_PDO_PGSql_Ext_pgsqlLOBUnlink, ZEND_ACC_PUBLIC) ZEND_ME(PDO_PGSql_Ext, pgsqlGetNotify, arginfo_class_PDO_PGSql_Ext_pgsqlGetNotify, ZEND_ACC_PUBLIC) ZEND_ME(PDO_PGSql_Ext, pgsqlGetPid, arginfo_class_PDO_PGSql_Ext_pgsqlGetPid, ZEND_ACC_PUBLIC) -#if 1 - ZEND_RAW_FENTRY("pgsqlSetNoticeCallback", zim_Pdo_Pgsql_setNoticeCallback, arginfo_class_PDO_PGSql_Ext_pgsqlSetNoticeCallback, ZEND_ACC_PUBLIC, NULL, NULL) -#endif ZEND_FE_END }; diff --git a/ext/pdo_pgsql/tests/issue78621.inc b/ext/pdo_pgsql/tests/issue78621.inc index 794e214d349ff..7a44090bfb709 100644 --- a/ext/pdo_pgsql/tests/issue78621.inc +++ b/ext/pdo_pgsql/tests/issue78621.inc @@ -2,7 +2,7 @@ require_once dirname(__FILE__) . '/../../../ext/pdo/tests/pdo_test.inc'; require_once dirname(__FILE__) . '/config.inc'; if (!isset($db)) { -$db = PDOTest::test_factory(dirname(__FILE__) . '/common.phpt'); + $db = new Pdo\Pgsql($config['ENV']['PDOTEST_DSN']); } if (!isset($rounds) || empty($rounds)) { $rounds = [ null, 'Re' ]; @@ -17,15 +17,15 @@ $db->exec("create function hey() returns trigger as \$\$ begin new.a := 'oh'; ra $db->exec("create trigger hop before insert on t for each row execute procedure hey()"); $db->exec("insert into t values ('ah')"); while (count($rounds)) { -try { -attach($db, array_shift($rounds)); -} catch (TypeError $err) { -echo "Caught TypeError: ".$err->getMessage()."\n"; -} -$db->exec("delete from t"); -$db->exec("insert into t values ('ah')"); + try { + attach($db, array_shift($rounds)); + } catch (TypeError $err) { + echo "Caught TypeError: ".$err->getMessage()."\n"; + } + $db->exec("delete from t"); + $db->exec("insert into t values ('ah')"); } -$db->pgsqlSetNoticeCallback(null); +$db->setNoticeCallback(null); $db->exec("delete from t"); $db->exec("insert into t values ('ah')"); var_dump($db->query("select * from t")->fetchAll(PDO::FETCH_ASSOC)); diff --git a/ext/pdo_pgsql/tests/issue78621.phpt b/ext/pdo_pgsql/tests/issue78621.phpt index ee474a116cbdb..9e14b89cb9e54 100644 --- a/ext/pdo_pgsql/tests/issue78621.phpt +++ b/ext/pdo_pgsql/tests/issue78621.phpt @@ -1,5 +1,5 @@ --TEST-- -pgsqlSetNoticeCallback catches Postgres "raise notice". +Pdo\Pgsql::setNoticeCallback catches Postgres "raise notice". --SKIPIF-- pgsqlSetNoticeCallback('disp'.$prefix); + $db->setNoticeCallback('disp'.$prefix); } require dirname(__FILE__) . '/issue78621.inc'; ?> diff --git a/ext/pdo_pgsql/tests/issue78621_closure.phpt b/ext/pdo_pgsql/tests/issue78621_closure.phpt index 55165e3aebc9e..45ebd1a5fa8b0 100644 --- a/ext/pdo_pgsql/tests/issue78621_closure.phpt +++ b/ext/pdo_pgsql/tests/issue78621_closure.phpt @@ -1,5 +1,5 @@ --TEST-- -pgsqlSetNoticeCallback catches Postgres "raise notice". +Pdo\Pgsql::setNoticeCallback catches Postgres "raise notice". --SKIPIF-- pgsqlSetNoticeCallback(function($message) use($prefix) { echo $prefix.trim($message)."\n"; }); + $db->setNoticeCallback(function($message) use($prefix) { echo $prefix.trim($message)."\n"; }); // https://github.com/php/php-src/pull/4823#pullrequestreview-335623806 $eraseCallbackMemoryHere = (object)[1]; break; case 1: $closure = function($message) use($prefix) { echo $prefix.'('.get_class($this).')'.trim($message)."\n"; }; - $db->pgsqlSetNoticeCallback($closure->bindTo(new \stdClass)); + $db->setNoticeCallback($closure->bindTo(new \stdClass)); break; } } diff --git a/ext/pdo_pgsql/tests/issue78621_method.phpt b/ext/pdo_pgsql/tests/issue78621_method.phpt index 2b3622ae4ab22..9b09b9bafc896 100644 --- a/ext/pdo_pgsql/tests/issue78621_method.phpt +++ b/ext/pdo_pgsql/tests/issue78621_method.phpt @@ -1,5 +1,5 @@ --TEST-- -pgsqlSetNoticeCallback catches Postgres "raise notice". +Pdo\Pgsql::setNoticeCallback catches Postgres "raise notice". --SKIPIF-- pgsqlSetNoticeCallback([ $logger, 'disp'.$prefix ]); break; - case 1: $db->pgsqlSetNoticeCallback([ $logger, 'whatever'.$prefix ]); break; + case 0: $db->setNoticeCallback([ $logger, 'disp'.$prefix ]); break; + case 1: $db->setNoticeCallback([ $logger, 'whatever'.$prefix ]); break; } } echo "Testing with method explicitely plugged:\n"; From b96e898f2f642603aab203c38702761764676004 Mon Sep 17 00:00:00 2001 From: Guillaume Outters Date: Tue, 28 May 2024 07:24:49 +0200 Subject: [PATCH 6/7] Pdo\Pgsql::setNoticeCallback(): test with unimplemented trampoline Add a test with a runtime BadMethodCallException. Streamline issue78621_method.phpt. --- ext/pdo_pgsql/tests/issue78621.inc | 12 ++-- ext/pdo_pgsql/tests/issue78621_method.phpt | 17 +++--- ..._setNoticeCallback_trampoline_no_leak.phpt | 58 +++++++++++++++++++ 3 files changed, 74 insertions(+), 13 deletions(-) create mode 100644 ext/pdo_pgsql/tests/pdopgsql_setNoticeCallback_trampoline_no_leak.phpt diff --git a/ext/pdo_pgsql/tests/issue78621.inc b/ext/pdo_pgsql/tests/issue78621.inc index 7a44090bfb709..c89aefec9c89a 100644 --- a/ext/pdo_pgsql/tests/issue78621.inc +++ b/ext/pdo_pgsql/tests/issue78621.inc @@ -19,11 +19,15 @@ $db->exec("insert into t values ('ah')"); while (count($rounds)) { try { attach($db, array_shift($rounds)); - } catch (TypeError $err) { - echo "Caught TypeError: ".$err->getMessage()."\n"; + } catch (Throwable $err) { + echo "Caught ".get_class($err).": ".$err->getMessage()."\n"; + } + try { + $db->exec("delete from t"); + $db->exec("insert into t values ('ah')"); + } catch (Throwable $err) { + echo "Caught ".get_class($err)." ".$err->getMessage()."\n"; } - $db->exec("delete from t"); - $db->exec("insert into t values ('ah')"); } $db->setNoticeCallback(null); $db->exec("delete from t"); diff --git a/ext/pdo_pgsql/tests/issue78621_method.phpt b/ext/pdo_pgsql/tests/issue78621_method.phpt index 9b09b9bafc896..2040583bd008f 100644 --- a/ext/pdo_pgsql/tests/issue78621_method.phpt +++ b/ext/pdo_pgsql/tests/issue78621_method.phpt @@ -16,26 +16,24 @@ class Logger public function __call(string $method, array $args) { $realMethod = strtr($method, [ 'whatever' => 'disp' ]); + if (!method_exists($this, $realMethod)) { + throw new BadMethodCallException('Call to undefined method '.__CLASS__.'::'.$realMethod); + } echo "$method trampoline for $realMethod\n"; return call_user_func_array([ $this, $realMethod ], $args); } } $logger = new Logger(); -function attach($db, $prefix = '') +function attach($db, $method) { global $logger; - global $flavor; - switch($flavor) - { - case 0: $db->setNoticeCallback([ $logger, 'disp'.$prefix ]); break; - case 1: $db->setNoticeCallback([ $logger, 'whatever'.$prefix ]); break; - } + $db->setNoticeCallback([ $logger, $method ]); } echo "Testing with method explicitely plugged:\n"; -$flavor = 0; +$rounds = [ 'disp', 'dispRe' ]; require dirname(__FILE__) . '/issue78621.inc'; echo "Testing with a bit of magic:\n"; -++$flavor; +$rounds = [ 'whatever', 'whateverRe', 'unexisting' ]; require dirname(__FILE__) . '/issue78621.inc'; ?> --EXPECT-- @@ -55,6 +53,7 @@ whatever trampoline for disp NOTICE: I tampered your data, did you know? whateverRe trampoline for dispRe ReNOTICE: I tampered your data, did you know? +Caught BadMethodCallException Call to undefined method Logger::unexisting array(1) { [0]=> array(1) { diff --git a/ext/pdo_pgsql/tests/pdopgsql_setNoticeCallback_trampoline_no_leak.phpt b/ext/pdo_pgsql/tests/pdopgsql_setNoticeCallback_trampoline_no_leak.phpt new file mode 100644 index 0000000000000..68602d0453710 --- /dev/null +++ b/ext/pdo_pgsql/tests/pdopgsql_setNoticeCallback_trampoline_no_leak.phpt @@ -0,0 +1,58 @@ +--TEST-- +Pdo\Pgsql::setNoticeCallback() use F ZPP for trampoline callback and does not leak +--EXTENSIONS-- +pdo +pdo_pgsql +--SKIPIF-- + +--FILE-- +newInstanceWithoutConstructor(); + } + $db->setNoticeCallback($callback); +} + +class T { public function z($m) { echo $m."\n"; } public function __call($m, $p) { echo "bah $m\n"; } } +$t = new T; +$rounds = [ + [ $t, 'disp' ], + 3, // Error; but then as the old callback is kept, it will be used in the call that follows the caught error. + null, // No callback: clear everything. + 'wouldAnyoneNameAFunctionThisWay', // So this one will crash and *no output will follow*. + [ null, [ $t, 'disp' ] ], // Valid callback on an unvalid Pdo. + [ $t, 'disp' ], +]; +require __DIR__ . '/issue78621.inc'; + +?> +--EXPECTF-- +bah disp +Caught TypeError: %s: Argument #1 ($callback) %s +bah disp +Caught TypeError: %s: Argument #1 ($callback) %s +Caught Error: %s object is uninitialized +bah disp +array(1) { + [0]=> + array(1) { + ["a"]=> + string(2) "oh" + } +} +Done + From 7d100a403c21c34546c2a3ac4a6e0f7690f90d6e Mon Sep 17 00:00:00 2001 From: Guillaume Outters Date: Thu, 30 May 2024 23:09:09 +0200 Subject: [PATCH 7/7] NEWS, UPGRADING: refer to namespaced Pdo\Xxx instead of PdoXxx Completes https://externals.io/message/123166, aligns #14299 --- NEWS | 14 +++++++------- UPGRADING | 12 ++++++------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/NEWS b/NEWS index 5671a43813339..ff0059e0ea63d 100644 --- a/NEWS +++ b/NEWS @@ -169,32 +169,32 @@ PHP NEWS - PDO_DBLIB: . Fixed setAttribute and getAttribute. (SakiTakamachi) - . Added class PdoDbLib. (danack, kocsismate) + . Added class Pdo\DbLib. (danack, kocsismate) - PDO_FIREBIRD: . Fixed setAttribute and getAttribute. (SakiTakamachi) . Feature: Add transaction isolation level and mode settings to pdo_firebird. (SakiTakamachi) - . Added class PdoFirebird. (danack, kocsismate) + . Added class Pdo\Firebird. (danack, kocsismate) - PDO_MYSQL: . Fixed setAttribute and getAttribute. (SakiTakamachi) - . Added class PdoMysql. (danack, kocsismate) + . Added class Pdo\Mysql. (danack, kocsismate) - PDO_ODBC: - . Added class PdoOdbc. (danack, kocsismate) + . Added class Pdo\Odbc. (danack, kocsismate) - PDO_PGSQL: . Fixed GH-12423, DSN credentials being prioritized over the user/password PDO constructor arguments. (SakiTakamachi) . Fixed native float support with pdo_pgsql query results. (Yurunsoft) - . Added class PdoPgsql. (danack, kocsismate) + . Added class Pdo\Pgsql. (danack, kocsismate) . Retrieve the memory usage of the query result resource. (KentarouTakeda) - . Added PDO::pgsqlSetNoticeCallBack method to receive DB notices. + . Added Pdo\Pgsql::setNoticeCallBack method to receive DB notices. (outtersg) - PDO_SQLITE: - . Added class PdoSqlite. (danack, kocsismate) + . Added class Pdo\Sqlite. (danack, kocsismate) . Fixed bug #81227 (PDO::inTransaction reports false when in transaction). (nielsdos) diff --git a/UPGRADING b/UPGRADING index 3fac4da11e0cb..e645c53a2c395 100644 --- a/UPGRADING +++ b/UPGRADING @@ -261,22 +261,22 @@ PHP 8.4 UPGRADE NOTES or by invoking their constructor directly. - PDO_DBLIB: - . Added class PdoDbLib. + . Added class Pdo\DbLib. - PDO_FIREBIRD: - . Added class PdoFirebird. + . Added class Pdo\Firebird. - PDO_MYSQL: - . Added class PdoMysql. + . Added class Pdo\Mysql. - PDO_ODBC: - . Added class PdoOdbc. + . Added class Pdo\Odbc. - PDO_PGSQL: - . Added class PdoPgsql. + . Added class Pdo\Pgsql. - PDO_SQLITE: - . Added class PdoSqlite. + . Added class Pdo\Sqlite. - POSIX: . Added constant POSIX_SC_CHILD_MAX