From 9e7117f20f506c4b1022561e4ddb3d2cb6b42d8d Mon Sep 17 00:00:00 2001 From: Weilin Du Date: Thu, 24 Sep 2026 10:19:32 +0800 Subject: [PATCH 1/2] ext/curl: Preserve callback lifetime without rejecting callback changes (#23863) Keep callback receivers and closures alive until their calls return, allowing callbacks to replace or clear themselves without freeing an active receiver. This preserves existing usage such as Symfony HttpClient's write callback. Use the helper for all callback entry points and retain the in_callback guard while releasing the temporary references, as destructors may re-enter curl. Add regression tests for replacement, clearing, trampolines, exceptions and destructor re-entry. Co-authored-by: Nicolas Grekas --- ext/curl/interface.c | 76 ++++++++++--------- ext/curl/tests/curl_callback_lifetime.phpt | 65 ++++++++++++++++ .../curl_callback_lifetime_destructor.phpt | 52 +++++++++++++ .../curl_setopt_callback_reentrancy.phpt | 29 +++---- 4 files changed, 172 insertions(+), 50 deletions(-) create mode 100644 ext/curl/tests/curl_callback_lifetime.phpt create mode 100644 ext/curl/tests/curl_callback_lifetime_destructor.phpt diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 336a89a214c4..8f82bd7f285d 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -563,6 +563,34 @@ PHP_MSHUTDOWN_FUNCTION(curl) } /* }}} */ +static void php_curl_call_callback( + php_curl *ch, zend_fcall_info_cache *fcc, zval *retval, uint32_t argc, zval *argv) +{ + /* The callback may replace or clear its own FCC. Keep its objects alive until + * the call returns, without taking ownership of the FCC's trampoline. */ + zend_object *object = fcc->object; + zend_object *closure = fcc->closure; + if (object) { + GC_ADDREF(object); + } + if (closure) { + GC_ADDREF(closure); + } + + bool was_in_callback = ch->in_callback; + ch->in_callback = true; + zend_call_known_fcc(fcc, retval, argc, argv, NULL); + + /* Destructors may also call back into curl, so keep the callback guard set. */ + if (object) { + OBJ_RELEASE(object); + } + if (closure) { + OBJ_RELEASE(closure); + } + ch->in_callback = was_in_callback; +} + /* {{{ curl_write */ static size_t curl_write(char *data, size_t size, size_t nmemb, void *ctx) { @@ -594,9 +622,7 @@ static size_t curl_write(char *data, size_t size, size_t nmemb, void *ctx) ZVAL_OBJ(&argv[0], &ch->std); ZVAL_STRINGL(&argv[1], data, length); - ch->in_callback = true; - zend_call_known_fcc(&write_handler->fcc, &retval, /* param_count */ 2, argv, /* named_params */ NULL); - ch->in_callback = false; + php_curl_call_callback(ch, &write_handler->fcc, &retval, /* argc */ 2, argv); if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); /* TODO Check callback returns an int or something castable to int */ @@ -630,9 +656,7 @@ static int curl_fnmatch(void *ctx, const char *pattern, const char *string) ZVAL_STRING(&argv[1], pattern); ZVAL_STRING(&argv[2], string); - ch->in_callback = true; - zend_call_known_fcc(&ch->handlers.fnmatch, &retval, /* param_count */ 3, argv, /* named_params */ NULL); - ch->in_callback = false; + php_curl_call_callback(ch, &ch->handlers.fnmatch, &retval, /* argc */ 3, argv); if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); @@ -670,9 +694,7 @@ static int curl_progress(void *clientp, double dltotal, double dlnow, double ult ZVAL_LONG(&args[3], (zend_long)ultotal); ZVAL_LONG(&args[4], (zend_long)ulnow); - ch->in_callback = true; - zend_call_known_fcc(&ch->handlers.progress, &retval, /* param_count */ 5, args, /* named_params */ NULL); - ch->in_callback = false; + php_curl_call_callback(ch, &ch->handlers.progress, &retval, /* argc */ 5, args); if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); @@ -711,9 +733,7 @@ static int curl_xferinfo(void *clientp, curl_off_t dltotal, curl_off_t dlnow, cu ZVAL_LONG(&argv[3], ultotal); ZVAL_LONG(&argv[4], ulnow); - ch->in_callback = true; - zend_call_known_fcc(&ch->handlers.xferinfo, &retval, /* param_count */ 5, argv, /* named_params */ NULL); - ch->in_callback = false; + php_curl_call_callback(ch, &ch->handlers.xferinfo, &retval, /* argc */ 5, argv); if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); @@ -756,9 +776,7 @@ static int curl_prereqfunction(void *clientp, char *conn_primary_ip, char *conn_ ZVAL_LONG(&args[3], conn_primary_port); ZVAL_LONG(&args[4], conn_local_port); - ch->in_callback = true; - zend_call_known_fcc(&ch->handlers.prereq, &retval, /* param_count */ 5, args, /* named_params */ NULL); - ch->in_callback = false; + php_curl_call_callback(ch, &ch->handlers.prereq, &retval, /* argc */ 5, args); if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); @@ -803,9 +821,7 @@ static int curl_ssh_hostkeyfunction(void *clientp, int keytype, const char *key, ZVAL_STRINGL(&args[2], key, keylen); ZVAL_LONG(&args[3], keylen); - ch->in_callback = true; - zend_call_known_fcc(&ch->handlers.sshhostkey, &retval, /* param_count */ 4, args, /* named_params */ NULL); - ch->in_callback = false; + php_curl_call_callback(ch, &ch->handlers.sshhostkey, &retval, /* argc */ 4, args); if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); @@ -855,9 +871,7 @@ static size_t curl_read(char *data, size_t size, size_t nmemb, void *ctx) } ZVAL_LONG(&argv[2], (int)size * nmemb); - ch->in_callback = true; - zend_call_known_fcc(&read_handler->fcc, &retval, /* param_count */ 3, argv, /* named_params */ NULL); - ch->in_callback = false; + php_curl_call_callback(ch, &read_handler->fcc, &retval, /* argc */ 3, argv); if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); if (Z_TYPE(retval) == IS_STRING) { @@ -907,9 +921,7 @@ static size_t curl_write_header(char *data, size_t size, size_t nmemb, void *ctx ZVAL_OBJ(&argv[0], &ch->std); ZVAL_STRINGL(&argv[1], data, length); - ch->in_callback = true; - zend_call_known_fcc(&write_handler->fcc, &retval, /* param_count */ 2, argv, /* named_params */ NULL); - ch->in_callback = false; + php_curl_call_callback(ch, &write_handler->fcc, &retval, /* argc */ 2, argv); if (!Z_ISUNDEF(retval)) { // TODO: Check for valid int type for return value _php_curl_verify_handlers(ch, /* reporterror */ true); @@ -963,9 +975,7 @@ static int curl_debug(CURL *handle, curl_infotype type, char *data, size_t size, ZVAL_LONG(&args[1], type); ZVAL_STRINGL(&args[2], data, size); - ch->in_callback = true; - zend_call_known_fcc(&ch->handlers.debug, NULL, /* param_count */ 3, args, /* named_params */ NULL); - ch->in_callback = false; + php_curl_call_callback(ch, &ch->handlers.debug, NULL, /* argc */ 3, args); zval_ptr_dtor(&args[0]); zval_ptr_dtor(&args[2]); @@ -1635,14 +1645,8 @@ PHP_FUNCTION(curl_copy_handle) } /* }}} */ -static bool php_curl_set_callable_handler(php_curl *ch, zend_fcall_info_cache *const handler_fcc, zval *callable, bool is_array_config, const char *option_name) +static bool php_curl_set_callable_handler(zend_fcall_info_cache *const handler_fcc, zval *callable, bool is_array_config, const char *option_name) { - /* Replacing a callback would free the fcc that is still executing on the stack. */ - if (ch->in_callback) { - zend_throw_error(NULL, "%s(): Attempt to set the %s option from a callback", get_active_function_name(), option_name); - return false; - } - if (ZEND_FCC_INITIALIZED(*handler_fcc)) { zend_fcc_dtor(handler_fcc); } @@ -1666,7 +1670,7 @@ static bool php_curl_set_callable_handler(php_curl *ch, zend_fcall_info_cache *c #define HANDLE_CURL_OPTION_CALLABLE_PHP_CURL_USER(curl_ptr, constant_no_function, handler_type, default_method) \ case constant_no_function##FUNCTION: { \ - bool result = php_curl_set_callable_handler(curl_ptr, &curl_ptr->handlers.handler_type->fcc, zvalue, is_array_config, #constant_no_function "FUNCTION"); \ + bool result = php_curl_set_callable_handler(&curl_ptr->handlers.handler_type->fcc, zvalue, is_array_config, #constant_no_function "FUNCTION"); \ if (!result) { \ curl_ptr->handlers.handler_type->method = default_method; \ return FAILURE; \ @@ -1681,7 +1685,7 @@ static bool php_curl_set_callable_handler(php_curl *ch, zend_fcall_info_cache *c #define HANDLE_CURL_OPTION_CALLABLE(curl_ptr, constant_no_function, handler_fcc, c_callback) \ case constant_no_function##FUNCTION: { \ - bool result = php_curl_set_callable_handler(curl_ptr, &curl_ptr->handler_fcc, zvalue, is_array_config, #constant_no_function "FUNCTION"); \ + bool result = php_curl_set_callable_handler(&curl_ptr->handler_fcc, zvalue, is_array_config, #constant_no_function "FUNCTION"); \ if (!result) { \ return FAILURE; \ } \ diff --git a/ext/curl/tests/curl_callback_lifetime.phpt b/ext/curl/tests/curl_callback_lifetime.phpt new file mode 100644 index 000000000000..fec26b9371d9 --- /dev/null +++ b/ext/curl/tests/curl_callback_lifetime.phpt @@ -0,0 +1,65 @@ +--TEST-- +GH-23814 (Curl callbacks keep their receiver alive when replacing or clearing themselves) +--EXTENSIONS-- +curl +--SKIPIF-- + +--FILE-- +action === 'clear') { + curl_setopt_array($handle, [CURLOPT_WRITEFUNCTION => null]); + } else { + curl_setopt($handle, CURLOPT_WRITEFUNCTION, static fn($handle, $data) => strlen($data)); + } + gc_collect_cycles(); + echo $this->value, "\n"; + if ($this->action === 'throw') { + throw new Exception('Callback exception'); + } + return strlen($data); + } + + public function __call(string $name, array $args): int { + return $this->write(...$args); + } + + public function __destruct() { + echo "Destroyed\n"; + } +} + +foreach (['replace' => 'write', 'clear' => 'missing', 'throw' => 'write'] as $action => $method) { + echo "$method / $action\n"; + $handle = curl_init('file://' . __FILE__); + curl_setopt($handle, CURLOPT_WRITEFUNCTION, [new Callback($action, $handle), $method]); + try { + var_dump(curl_exec($handle)); + } catch (Exception $e) { + echo $e->getMessage(), "\n"; + } + unset($handle); +} +?> +--EXPECT-- +write / replace +Still alive +Destroyed +bool(true) +missing / clear +Still alive +Destroyed +bool(true) +write / throw +Still alive +Destroyed +Callback exception diff --git a/ext/curl/tests/curl_callback_lifetime_destructor.phpt b/ext/curl/tests/curl_callback_lifetime_destructor.phpt new file mode 100644 index 000000000000..6611cb90e003 --- /dev/null +++ b/ext/curl/tests/curl_callback_lifetime_destructor.phpt @@ -0,0 +1,52 @@ +--TEST-- +GH-23814 (Curl callback receivers are destroyed with the callback guard still set) +--EXTENSIONS-- +curl +--SKIPIF-- + +--FILE-- +handle); + } catch (Error $e) { + echo $e->getMessage(), "\n"; + } + } + curl_setopt($this->handle, CURLOPT_WRITEFUNCTION, + static function (CurlHandle $handle, string $data): int { + echo "Callback installed by destructor\n"; + return strlen($data); + }); + } +} + +$handle = curl_init('file://' . __FILE__); +curl_setopt($handle, CURLOPT_WRITEFUNCTION, [new Callback($handle), 'write']); +var_dump(curl_exec($handle)); +var_dump(curl_exec($handle)); +curl_reset($handle); +echo "Reset outside callback succeeded\n"; +?> +--EXPECT-- +Callback returning +curl_reset(): Attempt to reset cURL handle from a callback +curl_close(): Attempt to close cURL handle from a callback +bool(true) +Callback installed by destructor +bool(true) +Reset outside callback succeeded diff --git a/ext/curl/tests/curl_setopt_callback_reentrancy.phpt b/ext/curl/tests/curl_setopt_callback_reentrancy.phpt index 662c42ac6ed6..3176e753171a 100644 --- a/ext/curl/tests/curl_setopt_callback_reentrancy.phpt +++ b/ext/curl/tests/curl_setopt_callback_reentrancy.phpt @@ -1,5 +1,5 @@ --TEST-- -GH-23814 (Setting a callback option from within a curl callback is rejected) +GH-23814 / GH-23860 (A curl callback can replace itself) --EXTENSIONS-- curl --SKIPIF-- @@ -13,26 +13,27 @@ if (!in_array('file', curl_version()['protocols'], true)) { $handle = curl_init('file://' . __FILE__); $callback = static function (CurlHandle $handle, string $data): int { - try { - curl_setopt($handle, CURLOPT_WRITEFUNCTION, static fn($handle, $data) => strlen($data)); - } catch (Error $error) { - echo $error->getMessage(), "\n"; - } - - try { - curl_setopt_array($handle, [CURLOPT_WRITEFUNCTION => null]); - } catch (Error $error) { - echo $error->getMessage(), "\n"; - } + echo "Original callback\n"; + var_dump(curl_setopt($handle, CURLOPT_WRITEFUNCTION, null)); + var_dump(curl_setopt_array($handle, [CURLOPT_WRITEFUNCTION => + static function (CurlHandle $handle, string $data): int { + echo "Replacement callback\n"; + return strlen($data); + }, + ])); return strlen($data); }; curl_setopt($handle, CURLOPT_WRITEFUNCTION, $callback); var_dump(curl_exec($handle)); +var_dump(curl_exec($handle)); var_dump(curl_setopt($handle, CURLOPT_WRITEFUNCTION, null)); ?> --EXPECT-- -curl_setopt(): Attempt to set the CURLOPT_WRITEFUNCTION option from a callback -curl_setopt_array(): Attempt to set the CURLOPT_WRITEFUNCTION option from a callback +Original callback +bool(true) +bool(true) +bool(true) +Replacement callback bool(true) bool(true) From 43c601467082d0cc7b3c5df9a9eaec71cd521cf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=AD=A6=E7=94=B0=20=E6=86=B2=E5=A4=AA=E9=83=8E?= Date: Wed, 23 Sep 2026 02:56:08 +0000 Subject: [PATCH 2/2] ext/pgsql: Removed `pg_service()`, unavailable with any released libpq `PQservice()` never shipped in a released libpq; it was dropped during the PostgreSQL 18 beta. Close GH-23857 --- NEWS | 1 + UPGRADING | 1 - ext/pgsql/config.m4 | 3 --- ext/pgsql/pgsql.c | 14 -------------- ext/pgsql/pgsql.stub.php | 3 --- ext/pgsql/pgsql_arginfo.h | 14 +------------- ext/pgsql/tests/pg_service.phpt | 19 ------------------- 7 files changed, 2 insertions(+), 53 deletions(-) delete mode 100644 ext/pgsql/tests/pg_service.phpt diff --git a/NEWS b/NEWS index a23aba2642b3..75cc315653f1 100644 --- a/NEWS +++ b/NEWS @@ -68,6 +68,7 @@ PHP NEWS - PGSQL: . Fixed pg_lo_write() rejecting data containing null bytes. (Ilia Alshanetsky) + . Removed pg_service(), unavailable with any released libpq. (KentarouTakeda) - Readline: . Fixed a heap over-read in the interactive shell prompt when cli.prompt is diff --git a/UPGRADING b/UPGRADING index 50754b279439..1b8ca8b0028a 100644 --- a/UPGRADING +++ b/UPGRADING @@ -782,7 +782,6 @@ PHP 8.5 UPGRADE NOTES . pg_close_stmt offers an alternative way to close a prepared statement from the DEALLOCATE sql command in that we can reuse its name afterwards. - . pg_service returns the ongoing service name of the connection. - Reflection: . ReflectionConstant::getFileName() was introduced. diff --git a/ext/pgsql/config.m4 b/ext/pgsql/config.m4 index 1409f879b52c..48fbbae34ace 100644 --- a/ext/pgsql/config.m4 +++ b/ext/pgsql/config.m4 @@ -31,9 +31,6 @@ if test "$PHP_PGSQL" != "no"; then PHP_CHECK_LIBRARY([pq], [PQclosePrepared], [AC_DEFINE([HAVE_PG_CLOSE_STMT], [1], [PostgreSQL 17 or later])],, [$PGSQL_LIBS]) - PHP_CHECK_LIBRARY([pq], [PQservice], - [AC_DEFINE([HAVE_PG_SERVICE], [1], [PostgreSQL 18 or later])],, - [$PGSQL_LIBS]) old_CFLAGS=$CFLAGS CFLAGS="$CFLAGS $PGSQL_CFLAGS" diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index 16e8589219b8..fdf194969d2a 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -922,7 +922,6 @@ PHP_FUNCTION(pg_close) #define PHP_PG_HOST 6 #define PHP_PG_VERSION 7 #define PHP_PG_JIT 8 -#define PHP_PG_SERVICE 9 /* php_pgsql_get_link_info */ static void php_pgsql_get_link_info(INTERNAL_FUNCTION_PARAMETERS, int entry_type) @@ -1007,12 +1006,6 @@ static void php_pgsql_get_link_info(INTERNAL_FUNCTION_PARAMETERS, int entry_type PQclear(res); return; } -#if defined(HAVE_PG_SERVICE) - case PHP_PG_SERVICE: { - result = PQservice(pgsql); - break; - } -#endif EMPTY_SWITCH_DEFAULT_CASE() } if (result) { @@ -1069,13 +1062,6 @@ PHP_FUNCTION(pg_jit) php_pgsql_get_link_info(INTERNAL_FUNCTION_PARAM_PASSTHRU,PHP_PG_JIT); } -#if defined(HAVE_PG_SERVICE) -PHP_FUNCTION(pg_service) -{ - php_pgsql_get_link_info(INTERNAL_FUNCTION_PARAM_PASSTHRU,PHP_PG_SERVICE); -} -#endif - /* Returns the value of a server parameter */ PHP_FUNCTION(pg_parameter_status) { diff --git a/ext/pgsql/pgsql.stub.php b/ext/pgsql/pgsql.stub.php index e9f200e48d0a..9bd78eb3ad4f 100644 --- a/ext/pgsql/pgsql.stub.php +++ b/ext/pgsql/pgsql.stub.php @@ -508,9 +508,6 @@ function pg_version(?PgSql\Connection $connection = null): array {} */ function pg_jit(?PgSql\Connection $connection = null): array {} -#ifdef HAVE_PG_SERVICE - function pg_service(?PgSql\Connection $connection = null): string {} -#endif /** * @param PgSql\Connection|string $connection * @refcount 1 diff --git a/ext/pgsql/pgsql_arginfo.h b/ext/pgsql/pgsql_arginfo.h index 6ef2e943b893..c9b0c85880da 100644 --- a/ext/pgsql/pgsql_arginfo.h +++ b/ext/pgsql/pgsql_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 778fedc7b65675b54f3c1b42205920478bf49fbf */ + * Stub hash: 368fabe2efd0e7d490ca80ecab0d92c4d5381e5e */ ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(arginfo_pg_connect, 0, 1, PgSql\\Connection, MAY_BE_FALSE) ZEND_ARG_TYPE_INFO(0, connection_string, IS_STRING, 0) @@ -38,12 +38,6 @@ ZEND_END_ARG_INFO() #define arginfo_pg_jit arginfo_pg_version -#if defined(HAVE_PG_SERVICE) -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pg_service, 0, 0, IS_STRING, 0) - ZEND_ARG_OBJ_INFO_WITH_DEFAULT_VALUE(0, connection, PgSql\\Connection, 1, "null") -ZEND_END_ARG_INFO() -#endif - ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_pg_parameter_status, 0, 1, MAY_BE_STRING|MAY_BE_FALSE) ZEND_ARG_INFO(0, connection) ZEND_ARG_TYPE_INFO(0, name, IS_STRING, 0) @@ -520,9 +514,6 @@ ZEND_FUNCTION(pg_tty); ZEND_FUNCTION(pg_host); ZEND_FUNCTION(pg_version); ZEND_FUNCTION(pg_jit); -#if defined(HAVE_PG_SERVICE) -ZEND_FUNCTION(pg_service); -#endif ZEND_FUNCTION(pg_parameter_status); ZEND_FUNCTION(pg_ping); ZEND_FUNCTION(pg_query); @@ -632,9 +623,6 @@ static const zend_function_entry ext_functions[] = { ZEND_FE(pg_host, arginfo_pg_host) ZEND_FE(pg_version, arginfo_pg_version) ZEND_FE(pg_jit, arginfo_pg_jit) -#if defined(HAVE_PG_SERVICE) - ZEND_FE(pg_service, arginfo_pg_service) -#endif ZEND_FE(pg_parameter_status, arginfo_pg_parameter_status) ZEND_FE(pg_ping, arginfo_pg_ping) ZEND_FE(pg_query, arginfo_pg_query) diff --git a/ext/pgsql/tests/pg_service.phpt b/ext/pgsql/tests/pg_service.phpt deleted file mode 100644 index 0ce1be7285eb..000000000000 --- a/ext/pgsql/tests/pg_service.phpt +++ /dev/null @@ -1,19 +0,0 @@ ---TEST-- -PostgreSQL connection service field support ---EXTENSIONS-- -pgsql ---SKIPIF-- - ---FILE-- - ---EXPECTF-- -string(%d) "%A"