diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 73088b0c7c94..d1f0cee3d67d 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -542,6 +542,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) { @@ -575,9 +603,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 */ @@ -613,9 +639,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); @@ -653,9 +677,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); @@ -694,9 +716,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); @@ -739,9 +759,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); @@ -786,9 +804,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); @@ -840,9 +856,7 @@ static size_t curl_read(char *data, size_t size, size_t nmemb, void *ctx) } ZVAL_LONG(&argv[2], (zend_long) 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) { @@ -948,9 +962,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); @@ -1006,9 +1018,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]); @@ -1611,14 +1621,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); } @@ -1642,7 +1646,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; \ @@ -1657,7 +1661,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..3b941bf689ec --- /dev/null +++ b/ext/curl/tests/curl_callback_lifetime_destructor.phpt @@ -0,0 +1,53 @@ +--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) diff --git a/ext/pgsql/config.m4 b/ext/pgsql/config.m4 index 63e996fe06da..6acf6ab597fe 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 de50c6929c5c..38ac6cd09ffc 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -908,7 +908,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) @@ -993,12 +992,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 default: ZEND_UNREACHABLE(); } if (result) { @@ -1055,13 +1048,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 8c5a7f4c6d98..c04ed10c693f 100644 --- a/ext/pgsql/pgsql.stub.php +++ b/ext/pgsql/pgsql.stub.php @@ -490,9 +490,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 ccaab3ddc862..d63b98bfe1b9 100644 --- a/ext/pgsql/pgsql_arginfo.h +++ b/ext/pgsql/pgsql_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit pgsql.stub.php instead. - * Stub hash: fa7cd778f4e791b15ffc8f1786384332449bda5a */ + * Stub hash: 479126d506a84c7196796f4980d1e9e471ca71ae */ #include "zend_attributes.h" #include "zend_constants.h" @@ -41,12 +41,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) @@ -523,9 +517,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); @@ -635,9 +626,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"