From 8aa8c07cec3945340ba4424b884848782612fde9 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 31 Jan 2024 18:24:28 +0000 Subject: [PATCH 01/15] ext/curl: Convert handlers.progress to just be a FCC --- ext/curl/curl_private.h | 2 +- ext/curl/interface.c | 111 ++++++++++-------- .../tests/curl_copy_handle_basic_008.phpt | 26 ---- ext/curl/tests/curl_progress.phpt | 25 ++++ ext/curl/tests/curl_setopt_callables.phpt | 31 +++++ 5 files changed, 121 insertions(+), 74 deletions(-) delete mode 100644 ext/curl/tests/curl_copy_handle_basic_008.phpt create mode 100644 ext/curl/tests/curl_progress.phpt create mode 100644 ext/curl/tests/curl_setopt_callables.phpt diff --git a/ext/curl/curl_private.h b/ext/curl/curl_private.h index aeef5937a0de8..79f342b1e8671 100644 --- a/ext/curl/curl_private.h +++ b/ext/curl/curl_private.h @@ -72,7 +72,7 @@ typedef struct { php_curl_write *write_header; php_curl_read *read; zval std_err; - php_curl_callback *progress; + zend_fcall_info_cache progress; php_curl_callback *xferinfo; php_curl_callback *fnmatch; #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 60481947dddea..7aca38ced158f 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -488,8 +488,8 @@ static HashTable *curl_get_gc(zend_object *object, zval **table, int *n) zend_get_gc_buffer_add_zval(gc_buffer, &curl->handlers.write_header->stream); } - if (curl->handlers.progress) { - zend_get_gc_buffer_add_zval(gc_buffer, &curl->handlers.progress->func_name); + if (ZEND_FCC_INITIALIZED(curl->handlers.progress)) { + zend_get_gc_buffer_add_fcc(gc_buffer, &curl->handlers.progress); } if (curl->handlers.xferinfo) { @@ -665,7 +665,6 @@ static int curl_fnmatch(void *ctx, const char *pattern, const char *string) static size_t curl_progress(void *clientp, double dltotal, double dlnow, double ultotal, double ulnow) { php_curl *ch = (php_curl *)clientp; - php_curl_callback *t = ch->handlers.progress; size_t rval = 0; #if PHP_CURL_DEBUG @@ -673,38 +672,29 @@ static size_t curl_progress(void *clientp, double dltotal, double dlnow, double fprintf(stderr, "clientp = %x, dltotal = %f, dlnow = %f, ultotal = %f, ulnow = %f\n", clientp, dltotal, dlnow, ultotal, ulnow); #endif - zval argv[5]; + zval args[5]; zval retval; - zend_result error; - zend_fcall_info fci; GC_ADDREF(&ch->std); - ZVAL_OBJ(&argv[0], &ch->std); - ZVAL_LONG(&argv[1], (zend_long)dltotal); - ZVAL_LONG(&argv[2], (zend_long)dlnow); - ZVAL_LONG(&argv[3], (zend_long)ultotal); - ZVAL_LONG(&argv[4], (zend_long)ulnow); - - fci.size = sizeof(fci); - ZVAL_COPY_VALUE(&fci.function_name, &t->func_name); - fci.object = NULL; - fci.retval = &retval; - fci.param_count = 5; - fci.params = argv; - fci.named_params = NULL; - - ch->in_callback = 1; - error = zend_call_function(&fci, &t->fci_cache); - ch->in_callback = 0; - if (error == FAILURE) { - php_error_docref(NULL, E_WARNING, "Cannot call the CURLOPT_PROGRESSFUNCTION"); - } else if (!Z_ISUNDEF(retval)) { - _php_curl_verify_handlers(ch, /* reporterror */ true); - if (0 != zval_get_long(&retval)) { - rval = 1; - } - } - zval_ptr_dtor(&argv[0]); + ZVAL_OBJ(&args[0], &ch->std); + ZVAL_LONG(&args[1], (zend_long)dltotal); + ZVAL_LONG(&args[2], (zend_long)dlnow); + 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; + + if (!Z_ISUNDEF(retval)) { + _php_curl_verify_handlers(ch, /* reporterror */ true); + /* TODO Check callback returns an int or something castable to int */ + if (0 != zval_get_long(&retval)) { + rval = 1; + } + } + + zval_ptr_dtor(&args[0]); return rval; } /* }}} */ @@ -1117,7 +1107,7 @@ void init_curl_handle(php_curl *ch) ch->handlers.write = ecalloc(1, sizeof(php_curl_write)); ch->handlers.write_header = ecalloc(1, sizeof(php_curl_write)); ch->handlers.read = ecalloc(1, sizeof(php_curl_read)); - ch->handlers.progress = NULL; + ch->handlers.progress = empty_fcall_info_cache; ch->handlers.xferinfo = NULL; ch->handlers.fnmatch = NULL; #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ @@ -1254,6 +1244,14 @@ static void _php_copy_callback(php_curl *ch, php_curl_callback **new_callback, p } } +static void php_curl_copy_fcc_with_option(php_curl *ch, CURLoption option, zend_fcall_info_cache *target_fcc, zend_fcall_info_cache *source_fcc) +{ + if (ZEND_FCC_INITIALIZED(*source_fcc)) { + zend_fcc_dup(target_fcc, source_fcc); + curl_easy_setopt(ch->cp, option, (void *) ch); + } +} + void _php_setup_easy_copy_handlers(php_curl *ch, php_curl *source) { if (!Z_ISUNDEF(source->handlers.write->stream)) { @@ -1293,7 +1291,7 @@ void _php_setup_easy_copy_handlers(php_curl *ch, php_curl *source) curl_easy_setopt(ch->cp, CURLOPT_WRITEHEADER, (void *) ch); curl_easy_setopt(ch->cp, CURLOPT_DEBUGDATA, (void *) ch); - _php_copy_callback(ch, &ch->handlers.progress, source->handlers.progress, CURLOPT_PROGRESSDATA); + php_curl_copy_fcc_with_option(ch, CURLOPT_PROGRESSDATA, &ch->handlers.progress, &source->handlers.progress); _php_copy_callback(ch, &ch->handlers.xferinfo, source->handlers.xferinfo, CURLOPT_XFERINFODATA); _php_copy_callback(ch, &ch->handlers.fnmatch, source->handlers.fnmatch, CURLOPT_FNMATCH_DATA); #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ @@ -1598,6 +1596,25 @@ PHP_FUNCTION(curl_copy_handle) } /* }}} */ +static bool php_curl_set_callable_handler(zend_fcall_info_cache *const handler_fcc, zval *callable, bool is_array_config, const char *option_name) +{ + if (ZEND_FCC_INITIALIZED(*handler_fcc)) { + zend_fcc_dtor(handler_fcc); + handler_fcc->function_handler = NULL; + } + + char *error = NULL; + if (UNEXPECTED(!zend_is_callable_ex(callable, /* object */ NULL, /* check_flags */ 0, /* callable_name */ NULL, handler_fcc, /* error */ &error))) { + if (!EG(exception)) { + zend_argument_type_error(2 + !is_array_config, "must be a valid callback for option %s, %s", option_name, error); + } + efree(error); + return false; + } + zend_fcc_addref(handler_fcc); + return true; +} + static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue, bool is_array_config) /* {{{ */ { CURLcode error = CURLE_OK; @@ -2142,17 +2159,17 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue } break; - case CURLOPT_PROGRESSFUNCTION: + case CURLOPT_PROGRESSFUNCTION: { + /* Check value is actually a callable and set it */ + const char option_name[] = "CURLOPT_PROGRESSFUNCTION"; + bool result = php_curl_set_callable_handler(&ch->handlers.progress, zvalue, is_array_config, option_name); + if (!result) { + return FAILURE; + } curl_easy_setopt(ch->cp, CURLOPT_PROGRESSFUNCTION, curl_progress); curl_easy_setopt(ch->cp, CURLOPT_PROGRESSDATA, ch); - if (ch->handlers.progress == NULL) { - ch->handlers.progress = ecalloc(1, sizeof(php_curl_callback)); - } else if (!Z_ISUNDEF(ch->handlers.progress->func_name)) { - zval_ptr_dtor(&ch->handlers.progress->func_name); - ch->handlers.progress->fci_cache = empty_fcall_info_cache; - } - ZVAL_COPY(&ch->handlers.progress->func_name, zvalue); break; + } #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ case CURLOPT_SSH_HOSTKEYFUNCTION: @@ -2843,7 +2860,9 @@ static void curl_free_obj(zend_object *object) efree(ch->handlers.write_header); efree(ch->handlers.read); - _php_curl_free_callback(ch->handlers.progress); + if (ZEND_FCC_INITIALIZED(ch->handlers.progress)) { + zend_fcc_dtor(&ch->handlers.progress); + } _php_curl_free_callback(ch->handlers.xferinfo); _php_curl_free_callback(ch->handlers.fnmatch); #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ @@ -2911,10 +2930,8 @@ static void _php_curl_reset_handlers(php_curl *ch) ZVAL_UNDEF(&ch->handlers.std_err); } - if (ch->handlers.progress) { - zval_ptr_dtor(&ch->handlers.progress->func_name); - efree(ch->handlers.progress); - ch->handlers.progress = NULL; + if (ZEND_FCC_INITIALIZED(ch->handlers.progress)) { + zend_fcc_dtor(&ch->handlers.progress); } if (ch->handlers.xferinfo) { diff --git a/ext/curl/tests/curl_copy_handle_basic_008.phpt b/ext/curl/tests/curl_copy_handle_basic_008.phpt deleted file mode 100644 index c1ab325660cf8..0000000000000 --- a/ext/curl/tests/curl_copy_handle_basic_008.phpt +++ /dev/null @@ -1,26 +0,0 @@ ---TEST-- -Test curl_copy_handle() with CURLOPT_PROGRESSFUNCTION ---EXTENSIONS-- -curl ---FILE-- - ---EXPECT-- -Hello World! -Hello World! -Hello World! -Hello World! diff --git a/ext/curl/tests/curl_progress.phpt b/ext/curl/tests/curl_progress.phpt new file mode 100644 index 0000000000000..5a7e488812233 --- /dev/null +++ b/ext/curl/tests/curl_progress.phpt @@ -0,0 +1,25 @@ +--TEST-- +Test curl_copy_handle() with CURLOPT_PROGRESSFUNCTION +--EXTENSIONS-- +curl +--FILE-- + +--EXPECT-- +Hello World! +Hello World! +Hello World! +Hello World! diff --git a/ext/curl/tests/curl_setopt_callables.phpt b/ext/curl/tests/curl_setopt_callables.phpt new file mode 100644 index 0000000000000..ad6ca7c833b69 --- /dev/null +++ b/ext/curl/tests/curl_setopt_callables.phpt @@ -0,0 +1,31 @@ +--TEST-- +Test curl_setopt(_array)() with options that take callabes +--EXTENSIONS-- +curl +--FILE-- +getMessage(), PHP_EOL; + } + + try { + var_dump(curl_setopt_array($handle, [$option => 'undefined'])); + } catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; + } +} + +$url = "{$host}/get.inc"; +$ch = curl_init($url); +testOption($ch, CURLOPT_PROGRESSFUNCTION); + +?> +--EXPECT-- +TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for option CURLOPT_PROGRESSFUNCTION, function "undefined" not found or invalid function name +TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_PROGRESSFUNCTION, function "undefined" not found or invalid function name From 854257fd77e1394972ef6eb282a80bb20db9b2d0 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 31 Jan 2024 18:38:11 +0000 Subject: [PATCH 02/15] ext/curl: Convert handlers.sshhostkey to just be a FCC --- ext/curl/curl_private.h | 2 +- ext/curl/interface.c | 69 ++++++++++------------- ext/curl/tests/curl_setopt_callables.phpt | 3 + 3 files changed, 33 insertions(+), 41 deletions(-) diff --git a/ext/curl/curl_private.h b/ext/curl/curl_private.h index 79f342b1e8671..517f0888ba0d4 100644 --- a/ext/curl/curl_private.h +++ b/ext/curl/curl_private.h @@ -76,7 +76,7 @@ typedef struct { php_curl_callback *xferinfo; php_curl_callback *fnmatch; #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ - php_curl_callback *sshhostkey; + zend_fcall_info_cache sshhostkey; #endif } php_curl_handlers; diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 7aca38ced158f..549628ce801f4 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -501,8 +501,8 @@ static HashTable *curl_get_gc(zend_object *object, zval **table, int *n) } #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ - if (curl->handlers.sshhostkey) { - zend_get_gc_buffer_add_zval(gc_buffer, &curl->handlers.sshhostkey->func_name); + if (ZEND_FCC_INITIALIZED(curl->handlers.sshhostkey)) { + zend_get_gc_buffer_add_fcc(gc_buffer, &curl->handlers.sshhostkey); } #endif @@ -751,7 +751,6 @@ static size_t curl_xferinfo(void *clientp, curl_off_t dltotal, curl_off_t dlnow, static int curl_ssh_hostkeyfunction(void *clientp, int keytype, const char *key, size_t keylen) { php_curl *ch = (php_curl *)clientp; - php_curl_callback *t = ch->handlers.sshhostkey; int rval = CURLKHMATCH_MISMATCH; /* cancel connection in case of an exception */ #if PHP_CURL_DEBUG @@ -759,31 +758,20 @@ static int curl_ssh_hostkeyfunction(void *clientp, int keytype, const char *key, fprintf(stderr, "clientp = %x, keytype = %d, key = %s, keylen = %zu\n", clientp, keytype, key, keylen); #endif - zval argv[4]; + zval args[4]; zval retval; - zend_result error; - zend_fcall_info fci; GC_ADDREF(&ch->std); - ZVAL_OBJ(&argv[0], &ch->std); - ZVAL_LONG(&argv[1], keytype); - ZVAL_STRINGL(&argv[2], key, keylen); - ZVAL_LONG(&argv[3], keylen); + ZVAL_OBJ(&args[0], &ch->std); + ZVAL_LONG(&args[1], keytype); + ZVAL_STRINGL(&args[2], key, keylen); + ZVAL_LONG(&args[3], keylen); - fci.size = sizeof(fci); - ZVAL_COPY_VALUE(&fci.function_name, &t->func_name); - fci.object = NULL; - fci.retval = &retval; - fci.param_count = 4; - fci.params = argv; - fci.named_params = NULL; + ch->in_callback = true; + zend_call_known_fcc(&ch->handlers.sshhostkey, &retval, /* param_count */ 4, args, /* named_params */ NULL); + ch->in_callback = false; - ch->in_callback = 1; - error = zend_call_function(&fci, &t->fci_cache); - ch->in_callback = 0; - if (error == FAILURE) { - php_error_docref(NULL, E_WARNING, "Cannot call the CURLOPT_SSH_HOSTKEYFUNCTION"); - } else if (!Z_ISUNDEF(retval)) { + if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); if (Z_TYPE(retval) == IS_LONG) { zend_long retval_long = Z_LVAL(retval); @@ -796,8 +784,9 @@ static int curl_ssh_hostkeyfunction(void *clientp, int keytype, const char *key, zend_throw_error(NULL, "The CURLOPT_SSH_HOSTKEYFUNCTION callback must return either CURLKHMATCH_OK or CURLKHMATCH_MISMATCH"); } } - zval_ptr_dtor(&argv[0]); - zval_ptr_dtor(&argv[2]); + + zval_ptr_dtor(&args[0]); + zval_ptr_dtor(&args[2]); return rval; } #endif @@ -1111,7 +1100,7 @@ void init_curl_handle(php_curl *ch) ch->handlers.xferinfo = NULL; ch->handlers.fnmatch = NULL; #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ - ch->handlers.sshhostkey = NULL; + ch->handlers.sshhostkey = empty_fcall_info_cache; #endif ch->clone = emalloc(sizeof(uint32_t)); *ch->clone = 1; @@ -1295,7 +1284,7 @@ void _php_setup_easy_copy_handlers(php_curl *ch, php_curl *source) _php_copy_callback(ch, &ch->handlers.xferinfo, source->handlers.xferinfo, CURLOPT_XFERINFODATA); _php_copy_callback(ch, &ch->handlers.fnmatch, source->handlers.fnmatch, CURLOPT_FNMATCH_DATA); #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ - _php_copy_callback(ch, &ch->handlers.sshhostkey, source->handlers.sshhostkey, CURLOPT_SSH_HOSTKEYDATA); + php_curl_copy_fcc_with_option(ch, CURLOPT_SSH_HOSTKEYDATA, &ch->handlers.sshhostkey, &source->handlers.sshhostkey); #endif ZVAL_COPY(&ch->private_data, &source->private_data); @@ -2172,17 +2161,17 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue } #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ - case CURLOPT_SSH_HOSTKEYFUNCTION: + case CURLOPT_SSH_HOSTKEYFUNCTION: { + /* Check value is actually a callable and set it */ + const char option_name[] = "CURLOPT_SSH_HOSTKEYFUNCTION"; + bool result = php_curl_set_callable_handler(&ch->handlers.sshhostkey, zvalue, is_array_config, option_name); + if (!result) { + return FAILURE; + } curl_easy_setopt(ch->cp, CURLOPT_SSH_HOSTKEYFUNCTION, curl_ssh_hostkeyfunction); curl_easy_setopt(ch->cp, CURLOPT_SSH_HOSTKEYDATA, ch); - if (ch->handlers.sshhostkey == NULL) { - ch->handlers.sshhostkey = ecalloc(1, sizeof(php_curl_callback)); - } else if (!Z_ISUNDEF(ch->handlers.sshhostkey->func_name)) { - zval_ptr_dtor(&ch->handlers.sshhostkey->func_name); - ch->handlers.sshhostkey->fci_cache = empty_fcall_info_cache; - } - ZVAL_COPY(&ch->handlers.sshhostkey->func_name, zvalue); break; + } #endif case CURLOPT_READFUNCTION: @@ -2866,7 +2855,9 @@ static void curl_free_obj(zend_object *object) _php_curl_free_callback(ch->handlers.xferinfo); _php_curl_free_callback(ch->handlers.fnmatch); #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ - _php_curl_free_callback(ch->handlers.sshhostkey); + if (ZEND_FCC_INITIALIZED(ch->handlers.sshhostkey)) { + zend_fcc_dtor(&ch->handlers.sshhostkey); + } #endif zval_ptr_dtor(&ch->postfields); @@ -2947,10 +2938,8 @@ static void _php_curl_reset_handlers(php_curl *ch) } #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ - if (ch->handlers.sshhostkey) { - zval_ptr_dtor(&ch->handlers.sshhostkey->func_name); - efree(ch->handlers.sshhostkey); - ch->handlers.sshhostkey = NULL; + if (ZEND_FCC_INITIALIZED(ch->handlers.sshhostkey)) { + zend_fcc_dtor(&ch->handlers.sshhostkey); } #endif } diff --git a/ext/curl/tests/curl_setopt_callables.phpt b/ext/curl/tests/curl_setopt_callables.phpt index ad6ca7c833b69..20fc27536cc9c 100644 --- a/ext/curl/tests/curl_setopt_callables.phpt +++ b/ext/curl/tests/curl_setopt_callables.phpt @@ -24,8 +24,11 @@ function testOption(CurlHandle $handle, int $option) { $url = "{$host}/get.inc"; $ch = curl_init($url); testOption($ch, CURLOPT_PROGRESSFUNCTION); +testOption($ch, CURLOPT_SSH_HOSTKEYFUNCTION); ?> --EXPECT-- TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for option CURLOPT_PROGRESSFUNCTION, function "undefined" not found or invalid function name TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_PROGRESSFUNCTION, function "undefined" not found or invalid function name +TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for option CURLOPT_SSH_HOSTKEYFUNCTION, function "undefined" not found or invalid function name +TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_SSH_HOSTKEYFUNCTION, function "undefined" not found or invalid function name From 85d182099116eda697caaca6e5ad50fea4e43950 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 31 Jan 2024 19:05:41 +0000 Subject: [PATCH 03/15] ext/curl: Convert handlers.xferinfo to just be a FCC --- ext/curl/curl_private.h | 2 +- ext/curl/interface.c | 75 ++++++++----------- ext/curl/tests/curl_copy_handle_xferinfo.phpt | 26 ++++--- ext/curl/tests/curl_setopt_callables.phpt | 3 + 4 files changed, 52 insertions(+), 54 deletions(-) diff --git a/ext/curl/curl_private.h b/ext/curl/curl_private.h index 517f0888ba0d4..c442a9bee50cd 100644 --- a/ext/curl/curl_private.h +++ b/ext/curl/curl_private.h @@ -73,7 +73,7 @@ typedef struct { php_curl_read *read; zval std_err; zend_fcall_info_cache progress; - php_curl_callback *xferinfo; + zend_fcall_info_cache xferinfo; php_curl_callback *fnmatch; #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ zend_fcall_info_cache sshhostkey; diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 549628ce801f4..e64133facdbfe 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -492,8 +492,8 @@ static HashTable *curl_get_gc(zend_object *object, zval **table, int *n) zend_get_gc_buffer_add_fcc(gc_buffer, &curl->handlers.progress); } - if (curl->handlers.xferinfo) { - zend_get_gc_buffer_add_zval(gc_buffer, &curl->handlers.xferinfo->func_name); + if (ZEND_FCC_INITIALIZED(curl->handlers.xferinfo)) { + zend_get_gc_buffer_add_fcc(gc_buffer, &curl->handlers.xferinfo); } if (curl->handlers.fnmatch) { @@ -703,7 +703,6 @@ static size_t curl_progress(void *clientp, double dltotal, double dlnow, double static size_t curl_xferinfo(void *clientp, curl_off_t dltotal, curl_off_t dlnow, curl_off_t ultotal, curl_off_t ulnow) { php_curl *ch = (php_curl *)clientp; - php_curl_callback *t = ch->handlers.xferinfo; size_t rval = 0; #if PHP_CURL_DEBUG @@ -713,8 +712,6 @@ static size_t curl_xferinfo(void *clientp, curl_off_t dltotal, curl_off_t dlnow, zval argv[5]; zval retval; - zend_result error; - zend_fcall_info fci; GC_ADDREF(&ch->std); ZVAL_OBJ(&argv[0], &ch->std); @@ -723,25 +720,18 @@ static size_t curl_xferinfo(void *clientp, curl_off_t dltotal, curl_off_t dlnow, ZVAL_LONG(&argv[3], ultotal); ZVAL_LONG(&argv[4], ulnow); - fci.size = sizeof(fci); - ZVAL_COPY_VALUE(&fci.function_name, &t->func_name); - fci.object = NULL; - fci.retval = &retval; - fci.param_count = 5; - fci.params = argv; - fci.named_params = NULL; + ch->in_callback = true; + zend_call_known_fcc(&ch->handlers.xferinfo, &retval, /* param_count */ 5, argv, /* named_params */ NULL); + ch->in_callback = false; + + if (!Z_ISUNDEF(retval)) { + _php_curl_verify_handlers(ch, /* reporterror */ true); + /* TODO Check callback returns an int or something castable to int */ + if (0 != zval_get_long(&retval)) { + rval = 1; + } + } - ch->in_callback = 1; - error = zend_call_function(&fci, &t->fci_cache); - ch->in_callback = 0; - if (error == FAILURE) { - php_error_docref(NULL, E_WARNING, "Cannot call the CURLOPT_XFERINFOFUNCTION"); - } else if (!Z_ISUNDEF(retval)) { - _php_curl_verify_handlers(ch, /* reporterror */ true); - if (0 != zval_get_long(&retval)) { - rval = 1; - } - } zval_ptr_dtor(&argv[0]); return rval; } @@ -1097,7 +1087,7 @@ void init_curl_handle(php_curl *ch) ch->handlers.write_header = ecalloc(1, sizeof(php_curl_write)); ch->handlers.read = ecalloc(1, sizeof(php_curl_read)); ch->handlers.progress = empty_fcall_info_cache; - ch->handlers.xferinfo = NULL; + ch->handlers.xferinfo = empty_fcall_info_cache; ch->handlers.fnmatch = NULL; #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ ch->handlers.sshhostkey = empty_fcall_info_cache; @@ -1281,7 +1271,7 @@ void _php_setup_easy_copy_handlers(php_curl *ch, php_curl *source) curl_easy_setopt(ch->cp, CURLOPT_DEBUGDATA, (void *) ch); php_curl_copy_fcc_with_option(ch, CURLOPT_PROGRESSDATA, &ch->handlers.progress, &source->handlers.progress); - _php_copy_callback(ch, &ch->handlers.xferinfo, source->handlers.xferinfo, CURLOPT_XFERINFODATA); + php_curl_copy_fcc_with_option(ch, CURLOPT_XFERINFODATA, &ch->handlers.xferinfo, &source->handlers.xferinfo); _php_copy_callback(ch, &ch->handlers.fnmatch, source->handlers.fnmatch, CURLOPT_FNMATCH_DATA); #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ php_curl_copy_fcc_with_option(ch, CURLOPT_SSH_HOSTKEYDATA, &ch->handlers.sshhostkey, &source->handlers.sshhostkey); @@ -2160,6 +2150,18 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue break; } + case CURLOPT_XFERINFOFUNCTION: { + /* Check value is actually a callable and set it */ + const char option_name[] = "CURLOPT_XFERINFOFUNCTION"; + bool result = php_curl_set_callable_handler(&ch->handlers.xferinfo, zvalue, is_array_config, option_name); + if (!result) { + return FAILURE; + } + curl_easy_setopt(ch->cp, CURLOPT_XFERINFOFUNCTION, curl_xferinfo); + curl_easy_setopt(ch->cp, CURLOPT_XFERINFODATA, ch); + break; + } + #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ case CURLOPT_SSH_HOSTKEYFUNCTION: { /* Check value is actually a callable and set it */ @@ -2200,18 +2202,6 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue ch->handlers.write->method = PHP_CURL_USER; break; - case CURLOPT_XFERINFOFUNCTION: - curl_easy_setopt(ch->cp, CURLOPT_XFERINFOFUNCTION, curl_xferinfo); - curl_easy_setopt(ch->cp, CURLOPT_XFERINFODATA, ch); - if (ch->handlers.xferinfo == NULL) { - ch->handlers.xferinfo = ecalloc(1, sizeof(php_curl_callback)); - } else if (!Z_ISUNDEF(ch->handlers.xferinfo->func_name)) { - zval_ptr_dtor(&ch->handlers.xferinfo->func_name); - ch->handlers.xferinfo->fci_cache = empty_fcall_info_cache; - } - ZVAL_COPY(&ch->handlers.xferinfo->func_name, zvalue); - break; - /* Curl off_t options */ case CURLOPT_MAX_RECV_SPEED_LARGE: case CURLOPT_MAX_SEND_SPEED_LARGE: @@ -2852,7 +2842,9 @@ static void curl_free_obj(zend_object *object) if (ZEND_FCC_INITIALIZED(ch->handlers.progress)) { zend_fcc_dtor(&ch->handlers.progress); } - _php_curl_free_callback(ch->handlers.xferinfo); + if (ZEND_FCC_INITIALIZED(ch->handlers.xferinfo)) { + zend_fcc_dtor(&ch->handlers.xferinfo); + } _php_curl_free_callback(ch->handlers.fnmatch); #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ if (ZEND_FCC_INITIALIZED(ch->handlers.sshhostkey)) { @@ -2925,10 +2917,9 @@ static void _php_curl_reset_handlers(php_curl *ch) zend_fcc_dtor(&ch->handlers.progress); } - if (ch->handlers.xferinfo) { - zval_ptr_dtor(&ch->handlers.xferinfo->func_name); - efree(ch->handlers.xferinfo); - ch->handlers.xferinfo = NULL; + if (ZEND_FCC_INITIALIZED(ch->handlers.xferinfo)) { + zend_fcc_dtor(&ch->handlers.xferinfo); + ch->handlers.xferinfo.function_handler = NULL; } if (ch->handlers.fnmatch) { diff --git a/ext/curl/tests/curl_copy_handle_xferinfo.phpt b/ext/curl/tests/curl_copy_handle_xferinfo.phpt index 55050551b797e..bf9c1f43beea0 100644 --- a/ext/curl/tests/curl_copy_handle_xferinfo.phpt +++ b/ext/curl/tests/curl_copy_handle_xferinfo.phpt @@ -4,19 +4,23 @@ Test curl_copy_handle() with CURLOPT_XFERINFOFUNCTION curl --FILE-- --EXPECT-- diff --git a/ext/curl/tests/curl_setopt_callables.phpt b/ext/curl/tests/curl_setopt_callables.phpt index 20fc27536cc9c..ffb0401c98ddb 100644 --- a/ext/curl/tests/curl_setopt_callables.phpt +++ b/ext/curl/tests/curl_setopt_callables.phpt @@ -25,6 +25,7 @@ $url = "{$host}/get.inc"; $ch = curl_init($url); testOption($ch, CURLOPT_PROGRESSFUNCTION); testOption($ch, CURLOPT_SSH_HOSTKEYFUNCTION); +testOption($ch, CURLOPT_XFERINFOFUNCTION); ?> --EXPECT-- @@ -32,3 +33,5 @@ TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for opti TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_PROGRESSFUNCTION, function "undefined" not found or invalid function name TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for option CURLOPT_SSH_HOSTKEYFUNCTION, function "undefined" not found or invalid function name TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_SSH_HOSTKEYFUNCTION, function "undefined" not found or invalid function name +TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for option CURLOPT_XFERINFOFUNCTION, function "undefined" not found or invalid function name +TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_XFERINFOFUNCTION, function "undefined" not found or invalid function name From d3fb21c095a7530bba26f8d78fc86c1660ff176a Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 31 Jan 2024 19:12:09 +0000 Subject: [PATCH 04/15] ext/curl: Convert handlers.fnmatch to just be a FCC --- ext/curl/curl_private.h | 2 +- ext/curl/interface.c | 84 ++++++++--------------- ext/curl/tests/curl_setopt_callables.phpt | 3 + 3 files changed, 31 insertions(+), 58 deletions(-) diff --git a/ext/curl/curl_private.h b/ext/curl/curl_private.h index c442a9bee50cd..e3eb2ad23fb61 100644 --- a/ext/curl/curl_private.h +++ b/ext/curl/curl_private.h @@ -74,7 +74,7 @@ typedef struct { zval std_err; zend_fcall_info_cache progress; zend_fcall_info_cache xferinfo; - php_curl_callback *fnmatch; + zend_fcall_info_cache fnmatch; #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ zend_fcall_info_cache sshhostkey; #endif diff --git a/ext/curl/interface.c b/ext/curl/interface.c index e64133facdbfe..de927e33bba79 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -496,8 +496,8 @@ static HashTable *curl_get_gc(zend_object *object, zval **table, int *n) zend_get_gc_buffer_add_fcc(gc_buffer, &curl->handlers.xferinfo); } - if (curl->handlers.fnmatch) { - zend_get_gc_buffer_add_zval(gc_buffer, &curl->handlers.fnmatch->func_name); + if (ZEND_FCC_INITIALIZED(curl->handlers.fnmatch)) { + zend_get_gc_buffer_add_fcc(gc_buffer, &curl->handlers.fnmatch); } #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ @@ -625,33 +625,22 @@ static size_t curl_write(char *data, size_t size, size_t nmemb, void *ctx) static int curl_fnmatch(void *ctx, const char *pattern, const char *string) { php_curl *ch = (php_curl *) ctx; - php_curl_callback *t = ch->handlers.fnmatch; int rval = CURL_FNMATCHFUNC_FAIL; zval argv[3]; zval retval; - zend_result error; - zend_fcall_info fci; GC_ADDREF(&ch->std); ZVAL_OBJ(&argv[0], &ch->std); ZVAL_STRING(&argv[1], pattern); ZVAL_STRING(&argv[2], string); - fci.size = sizeof(fci); - ZVAL_COPY_VALUE(&fci.function_name, &t->func_name); - fci.object = NULL; - fci.retval = &retval; - fci.param_count = 3; - fci.params = argv; - fci.named_params = NULL; - - ch->in_callback = 1; - error = zend_call_function(&fci, &t->fci_cache); - ch->in_callback = 0; - if (error == FAILURE) { - php_error_docref(NULL, E_WARNING, "Cannot call the CURLOPT_FNMATCH_FUNCTION"); - } else if (!Z_ISUNDEF(retval)) { + ch->in_callback = true; + zend_call_known_fcc(&ch->handlers.fnmatch, &retval, /* param_count */ 3, argv, /* named_params */ NULL); + ch->in_callback = false; + + if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); + /* TODO Check callback returns an int or something castable to int */ rval = zval_get_long(&retval); } zval_ptr_dtor(&argv[0]); @@ -1088,7 +1077,7 @@ void init_curl_handle(php_curl *ch) ch->handlers.read = ecalloc(1, sizeof(php_curl_read)); ch->handlers.progress = empty_fcall_info_cache; ch->handlers.xferinfo = empty_fcall_info_cache; - ch->handlers.fnmatch = NULL; + ch->handlers.fnmatch = empty_fcall_info_cache; #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ ch->handlers.sshhostkey = empty_fcall_info_cache; #endif @@ -1212,17 +1201,6 @@ PHP_FUNCTION(curl_init) } /* }}} */ -static void _php_copy_callback(php_curl *ch, php_curl_callback **new_callback, php_curl_callback *source_callback, CURLoption option) -{ - if (source_callback) { - *new_callback = ecalloc(1, sizeof(php_curl_callback)); - if (!Z_ISUNDEF(source_callback->func_name)) { - ZVAL_COPY(&(*new_callback)->func_name, &source_callback->func_name); - } - curl_easy_setopt(ch->cp, option, (void *) ch); - } -} - static void php_curl_copy_fcc_with_option(php_curl *ch, CURLoption option, zend_fcall_info_cache *target_fcc, zend_fcall_info_cache *source_fcc) { if (ZEND_FCC_INITIALIZED(*source_fcc)) { @@ -1272,7 +1250,7 @@ void _php_setup_easy_copy_handlers(php_curl *ch, php_curl *source) php_curl_copy_fcc_with_option(ch, CURLOPT_PROGRESSDATA, &ch->handlers.progress, &source->handlers.progress); php_curl_copy_fcc_with_option(ch, CURLOPT_XFERINFODATA, &ch->handlers.xferinfo, &source->handlers.xferinfo); - _php_copy_callback(ch, &ch->handlers.fnmatch, source->handlers.fnmatch, CURLOPT_FNMATCH_DATA); + php_curl_copy_fcc_with_option(ch, CURLOPT_FNMATCH_DATA, &ch->handlers.fnmatch, &source->handlers.fnmatch); #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ php_curl_copy_fcc_with_option(ch, CURLOPT_SSH_HOSTKEYDATA, &ch->handlers.sshhostkey, &source->handlers.sshhostkey); #endif @@ -2162,6 +2140,18 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue break; } + case CURLOPT_FNMATCH_FUNCTION: { + /* Check value is actually a callable and set it */ + const char option_name[] = "CURLOPT_FNMATCH_FUNCTION"; + bool result = php_curl_set_callable_handler(&ch->handlers.fnmatch, zvalue, is_array_config, option_name); + if (!result) { + return FAILURE; + } + curl_easy_setopt(ch->cp, CURLOPT_FNMATCH_FUNCTION, curl_fnmatch); + curl_easy_setopt(ch->cp, CURLOPT_FNMATCH_DATA, ch); + break; + } + #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ case CURLOPT_SSH_HOSTKEYFUNCTION: { /* Check value is actually a callable and set it */ @@ -2271,18 +2261,6 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue } break; - case CURLOPT_FNMATCH_FUNCTION: - curl_easy_setopt(ch->cp, CURLOPT_FNMATCH_FUNCTION, curl_fnmatch); - curl_easy_setopt(ch->cp, CURLOPT_FNMATCH_DATA, ch); - if (ch->handlers.fnmatch == NULL) { - ch->handlers.fnmatch = ecalloc(1, sizeof(php_curl_callback)); - } else if (!Z_ISUNDEF(ch->handlers.fnmatch->func_name)) { - zval_ptr_dtor(&ch->handlers.fnmatch->func_name); - ch->handlers.fnmatch->fci_cache = empty_fcall_info_cache; - } - ZVAL_COPY(&ch->handlers.fnmatch->func_name, zvalue); - break; - /* Curl blob options */ #if LIBCURL_VERSION_NUM >= 0x074700 /* Available since 7.71.0 */ case CURLOPT_ISSUERCERT_BLOB: @@ -2771,14 +2749,6 @@ PHP_FUNCTION(curl_close) } /* }}} */ -static void _php_curl_free_callback(php_curl_callback* callback) -{ - if (callback) { - zval_ptr_dtor(&callback->func_name); - efree(callback); - } -} - static void curl_free_obj(zend_object *object) { php_curl *ch = curl_from_obj(object); @@ -2845,7 +2815,9 @@ static void curl_free_obj(zend_object *object) if (ZEND_FCC_INITIALIZED(ch->handlers.xferinfo)) { zend_fcc_dtor(&ch->handlers.xferinfo); } - _php_curl_free_callback(ch->handlers.fnmatch); + if (ZEND_FCC_INITIALIZED(ch->handlers.fnmatch)) { + zend_fcc_dtor(&ch->handlers.fnmatch); + } #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ if (ZEND_FCC_INITIALIZED(ch->handlers.sshhostkey)) { zend_fcc_dtor(&ch->handlers.sshhostkey); @@ -2922,10 +2894,8 @@ static void _php_curl_reset_handlers(php_curl *ch) ch->handlers.xferinfo.function_handler = NULL; } - if (ch->handlers.fnmatch) { - zval_ptr_dtor(&ch->handlers.fnmatch->func_name); - efree(ch->handlers.fnmatch); - ch->handlers.fnmatch = NULL; + if (ZEND_FCC_INITIALIZED(ch->handlers.fnmatch)) { + zend_fcc_dtor(&ch->handlers.fnmatch); } #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ diff --git a/ext/curl/tests/curl_setopt_callables.phpt b/ext/curl/tests/curl_setopt_callables.phpt index ffb0401c98ddb..71f3ae671018b 100644 --- a/ext/curl/tests/curl_setopt_callables.phpt +++ b/ext/curl/tests/curl_setopt_callables.phpt @@ -26,6 +26,7 @@ $ch = curl_init($url); testOption($ch, CURLOPT_PROGRESSFUNCTION); testOption($ch, CURLOPT_SSH_HOSTKEYFUNCTION); testOption($ch, CURLOPT_XFERINFOFUNCTION); +testOption($ch, CURLOPT_FNMATCH_FUNCTION); ?> --EXPECT-- @@ -35,3 +36,5 @@ TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for opti TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_SSH_HOSTKEYFUNCTION, function "undefined" not found or invalid function name TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for option CURLOPT_XFERINFOFUNCTION, function "undefined" not found or invalid function name TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_XFERINFOFUNCTION, function "undefined" not found or invalid function name +TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for option CURLOPT_FNMATCH_FUNCTION, function "undefined" not found or invalid function name +TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_FNMATCH_FUNCTION, function "undefined" not found or invalid function name From 1dfce9d7a1031422243aa1f6041ee6beb23dded9 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 31 Jan 2024 19:27:32 +0000 Subject: [PATCH 05/15] ext/curl: Convert handlers.server_push to just be a FCC TODO Tests for multi handler --- ext/curl/curl_private.h | 2 +- ext/curl/multi.c | 55 ++++++++----------- .../tests/curl_multi_setopt_callables.phpt | 17 ++++++ ...url_pushfunction_nonexistent_callback.phpt | 54 ------------------ 4 files changed, 42 insertions(+), 86 deletions(-) create mode 100644 ext/curl/tests/curl_multi_setopt_callables.phpt delete mode 100644 ext/curl/tests/curl_pushfunction_nonexistent_callback.phpt diff --git a/ext/curl/curl_private.h b/ext/curl/curl_private.h index e3eb2ad23fb61..4f2f7ecaa2075 100644 --- a/ext/curl/curl_private.h +++ b/ext/curl/curl_private.h @@ -114,7 +114,7 @@ typedef struct { #define CURLOPT_SAFE_UPLOAD -1 typedef struct { - php_curl_callback *server_push; + zend_fcall_info_cache server_push; } php_curlm_handlers; typedef struct { diff --git a/ext/curl/multi.c b/ext/curl/multi.c index a61066b5a6c7c..183324f5bc631 100644 --- a/ext/curl/multi.c +++ b/ext/curl/multi.c @@ -372,34 +372,25 @@ static int _php_server_push_callback(CURL *parent_ch, CURL *easy, size_t num_hea php_curl *parent; php_curlm *mh = (php_curlm *)userp; size_t rval = CURL_PUSH_DENY; - php_curl_callback *t = mh->handlers.server_push; zval *pz_parent_ch = NULL; zval pz_ch; zval headers; zval retval; - char *header; - zend_result error; - zend_fcall_info fci = empty_fcall_info; pz_parent_ch = _php_curl_multi_find_easy_handle(mh, parent_ch); if (pz_parent_ch == NULL) { return rval; } - if (UNEXPECTED(zend_fcall_info_init(&t->func_name, 0, &fci, &t->fci_cache, NULL, NULL) == FAILURE)) { - php_error_docref(NULL, E_WARNING, "Cannot call the CURLMOPT_PUSHFUNCTION"); - return rval; - } - parent = Z_CURL_P(pz_parent_ch); ch = init_curl_handle_into_zval(&pz_ch); ch->cp = easy; _php_setup_easy_copy_handlers(ch, parent); - size_t i; array_init(&headers); - for(i=0; ifci_cache); + zend_call_known_fcc(&mh->handlers.server_push, &retval, /* param_count */ 3, call_args, /* named_params */ NULL); zval_ptr_dtor_nogc(&headers); - if (error == FAILURE) { - php_error_docref(NULL, E_WARNING, "Cannot call the CURLMOPT_PUSHFUNCTION"); - } else if (!Z_ISUNDEF(retval)) { + if (!Z_ISUNDEF(retval)) { if (CURL_PUSH_DENY != zval_get_long(&retval)) { rval = CURL_PUSH_OK; zend_llist_add_element(&mh->easyh, &pz_ch); @@ -458,21 +443,29 @@ static bool _php_curl_multi_setopt(php_curlm *mh, zend_long option, zval *zvalue error = curl_multi_setopt(mh->multi, option, lval); break; } - case CURLMOPT_PUSHFUNCTION: - if (mh->handlers.server_push == NULL) { - mh->handlers.server_push = ecalloc(1, sizeof(php_curl_callback)); - } else if (!Z_ISUNDEF(mh->handlers.server_push->func_name)) { - zval_ptr_dtor(&mh->handlers.server_push->func_name); - mh->handlers.server_push->fci_cache = empty_fcall_info_cache; + case CURLMOPT_PUSHFUNCTION: { + /* See php_curl_set_callable_handler */ + if (ZEND_FCC_INITIALIZED(mh->handlers.server_push)) { + zend_fcc_dtor(&mh->handlers.server_push); } - ZVAL_COPY(&mh->handlers.server_push->func_name, zvalue); + char *error_str = NULL; + if (UNEXPECTED(!zend_is_callable_ex(zvalue, /* object */ NULL, /* check_flags */ 0, /* callable_name */ NULL, &mh->handlers.server_push, /* error */ &error_str))) { + if (!EG(exception)) { + zend_argument_type_error(2, "must be a valid callback for option CURLMOPT_PUSHFUNCTION, %s", error_str); + } + efree(error_str); + return false; + } + zend_fcc_addref(&mh->handlers.server_push); + error = curl_multi_setopt(mh->multi, CURLMOPT_PUSHFUNCTION, _php_server_push_callback); if (error != CURLM_OK) { return false; } error = curl_multi_setopt(mh->multi, CURLMOPT_PUSHDATA, mh); break; + } default: zend_argument_value_error(2, "is not a valid cURL multi option"); error = CURLM_UNKNOWN_OPTION; @@ -548,9 +541,9 @@ static void curl_multi_free_obj(zend_object *object) curl_multi_cleanup(mh->multi); zend_llist_clean(&mh->easyh); - if (mh->handlers.server_push) { - zval_ptr_dtor(&mh->handlers.server_push->func_name); - efree(mh->handlers.server_push); + + if (ZEND_FCC_INITIALIZED(mh->handlers.server_push)) { + zend_fcc_dtor(&mh->handlers.server_push); } zend_object_std_dtor(&mh->std); @@ -562,8 +555,8 @@ static HashTable *curl_multi_get_gc(zend_object *object, zval **table, int *n) zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create(); - if (curl_multi->handlers.server_push) { - zend_get_gc_buffer_add_zval(gc_buffer, &curl_multi->handlers.server_push->func_name); + if (ZEND_FCC_INITIALIZED(curl_multi->handlers.server_push)) { + zend_get_gc_buffer_add_fcc(gc_buffer, &curl_multi->handlers.server_push); } zend_llist_position pos; diff --git a/ext/curl/tests/curl_multi_setopt_callables.phpt b/ext/curl/tests/curl_multi_setopt_callables.phpt new file mode 100644 index 0000000000000..c0de5add49101 --- /dev/null +++ b/ext/curl/tests/curl_multi_setopt_callables.phpt @@ -0,0 +1,17 @@ +--TEST-- +Test curl_multi_setopt() with options that take callabes +--EXTENSIONS-- +curl +--FILE-- +getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +TypeError: curl_multi_setopt(): Argument #2 ($option) must be a valid callback for option CURLMOPT_PUSHFUNCTION, function "undefined" not found or invalid function name diff --git a/ext/curl/tests/curl_pushfunction_nonexistent_callback.phpt b/ext/curl/tests/curl_pushfunction_nonexistent_callback.phpt deleted file mode 100644 index fe2defa5eea08..0000000000000 --- a/ext/curl/tests/curl_pushfunction_nonexistent_callback.phpt +++ /dev/null @@ -1,54 +0,0 @@ ---TEST-- -Test CURLMOPT_PUSHFUNCTION with non-existent callback function ---CREDITS-- -Davey Shafik -Kévin Dunglas -Niels Dossche ---EXTENSIONS-- -curl ---SKIPIF-- - ---FILE-- - ---EXPECTF-- -Warning: curl_multi_exec(): Cannot call the CURLMOPT_PUSHFUNCTION in %s on line %d From cf9228fd92b6ceb48957601772cec9dc30bbcdda Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 31 Jan 2024 23:30:43 +0000 Subject: [PATCH 06/15] ext/curl: Convert php_curl_write to just use FCC without a function name zval --- ext/curl/curl_private.h | 3 +- ext/curl/interface.c | 126 ++++++++++------------ ext/curl/tests/curl_setopt_callables.phpt | 6 ++ 3 files changed, 65 insertions(+), 70 deletions(-) diff --git a/ext/curl/curl_private.h b/ext/curl/curl_private.h index 4f2f7ecaa2075..5ab2de0415b13 100644 --- a/ext/curl/curl_private.h +++ b/ext/curl/curl_private.h @@ -45,8 +45,7 @@ PHP_MSHUTDOWN_FUNCTION(curl); PHP_MINFO_FUNCTION(curl); typedef struct { - zval func_name; - zend_fcall_info_cache fci_cache; + zend_fcall_info_cache fcc; FILE *fp; smart_str buf; int method; diff --git a/ext/curl/interface.c b/ext/curl/interface.c index de927e33bba79..fb621c9fdc5ce 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -479,12 +479,16 @@ static HashTable *curl_get_gc(zend_object *object, zval **table, int *n) } if (curl->handlers.write) { - zend_get_gc_buffer_add_zval(gc_buffer, &curl->handlers.write->func_name); + if (ZEND_FCC_INITIALIZED(curl->handlers.write->fcc)) { + zend_get_gc_buffer_add_fcc(gc_buffer, &curl->handlers.write->fcc); + } zend_get_gc_buffer_add_zval(gc_buffer, &curl->handlers.write->stream); } if (curl->handlers.write_header) { - zend_get_gc_buffer_add_zval(gc_buffer, &curl->handlers.write_header->func_name); + if (ZEND_FCC_INITIALIZED(curl->handlers.write_header->fcc)) { + zend_get_gc_buffer_add_fcc(gc_buffer, &curl->handlers.write_header->fcc); + } zend_get_gc_buffer_add_zval(gc_buffer, &curl->handlers.write_header->stream); } @@ -563,7 +567,7 @@ static size_t curl_write_nothing(char *data, size_t size, size_t nmemb, void *ct static size_t curl_write(char *data, size_t size, size_t nmemb, void *ctx) { php_curl *ch = (php_curl *) ctx; - php_curl_write *t = ch->handlers.write; + php_curl_write *write_handler = ch->handlers.write; size_t length = size * nmemb; #if PHP_CURL_DEBUG @@ -571,43 +575,31 @@ static size_t curl_write(char *data, size_t size, size_t nmemb, void *ctx) fprintf(stderr, "data = %s, size = %d, nmemb = %d, ctx = %x\n", data, size, nmemb, ctx); #endif - switch (t->method) { + switch (write_handler->method) { case PHP_CURL_STDOUT: PHPWRITE(data, length); break; case PHP_CURL_FILE: - return fwrite(data, size, nmemb, t->fp); + return fwrite(data, size, nmemb, write_handler->fp); case PHP_CURL_RETURN: if (length > 0) { - smart_str_appendl(&t->buf, data, (int) length); + smart_str_appendl(&write_handler->buf, data, (int) length); } break; case PHP_CURL_USER: { zval argv[2]; zval retval; - int error; - zend_fcall_info fci; GC_ADDREF(&ch->std); ZVAL_OBJ(&argv[0], &ch->std); ZVAL_STRINGL(&argv[1], data, length); - fci.size = sizeof(fci); - fci.object = NULL; - ZVAL_COPY_VALUE(&fci.function_name, &t->func_name); - fci.retval = &retval; - fci.param_count = 2; - fci.params = argv; - fci.named_params = NULL; - - ch->in_callback = 1; - error = zend_call_function(&fci, &t->fci_cache); - ch->in_callback = 0; - if (error == FAILURE) { - php_error_docref(NULL, E_WARNING, "Could not call the CURLOPT_WRITEFUNCTION"); - length = -1; - } else if (!Z_ISUNDEF(retval)) { + ch->in_callback = true; + zend_call_known_fcc(&write_handler->fcc, &retval, /* param_count */ 2, argv, /* named_params */ NULL); + ch->in_callback = false; + if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); + /* TODO Check callback returns an int or something castable to int */ length = zval_get_long(&retval); } @@ -838,10 +830,10 @@ static size_t curl_read(char *data, size_t size, size_t nmemb, void *ctx) static size_t curl_write_header(char *data, size_t size, size_t nmemb, void *ctx) { php_curl *ch = (php_curl *) ctx; - php_curl_write *t = ch->handlers.write_header; + php_curl_write *write_handler = ch->handlers.write_header; size_t length = size * nmemb; - switch (t->method) { + switch (write_handler->method) { case PHP_CURL_STDOUT: /* Handle special case write when we're returning the entire transfer */ @@ -852,32 +844,20 @@ static size_t curl_write_header(char *data, size_t size, size_t nmemb, void *ctx } break; case PHP_CURL_FILE: - return fwrite(data, size, nmemb, t->fp); + return fwrite(data, size, nmemb, write_handler->fp); case PHP_CURL_USER: { zval argv[2]; zval retval; - zend_result error; - zend_fcall_info fci; GC_ADDREF(&ch->std); ZVAL_OBJ(&argv[0], &ch->std); ZVAL_STRINGL(&argv[1], data, length); - fci.size = sizeof(fci); - ZVAL_COPY_VALUE(&fci.function_name, &t->func_name); - fci.object = NULL; - fci.retval = &retval; - fci.param_count = 2; - fci.params = argv; - fci.named_params = NULL; - - ch->in_callback = 1; - error = zend_call_function(&fci, &t->fci_cache); - ch->in_callback = 0; - if (error == FAILURE) { - php_error_docref(NULL, E_WARNING, "Could not call the CURLOPT_HEADERFUNCTION"); - length = -1; - } else if (!Z_ISUNDEF(retval)) { + ch->in_callback = true; + zend_call_known_fcc(&write_handler->fcc, &retval, /* param_count */ 2, argv, /* named_params */ NULL); + ch->in_callback = false; + if (!Z_ISUNDEF(retval)) { + // TODO: Check for valid int type for return value _php_curl_verify_handlers(ch, /* reporterror */ true); length = zval_get_long(&retval); } @@ -1232,15 +1212,17 @@ void _php_setup_easy_copy_handlers(php_curl *ch, php_curl *source) ch->handlers.read->fp = source->handlers.read->fp; ch->handlers.read->res = source->handlers.read->res; - if (!Z_ISUNDEF(source->handlers.write->func_name)) { - ZVAL_COPY(&ch->handlers.write->func_name, &source->handlers.write->func_name); - } if (!Z_ISUNDEF(source->handlers.read->func_name)) { ZVAL_COPY(&ch->handlers.read->func_name, &source->handlers.read->func_name); } - if (!Z_ISUNDEF(source->handlers.write_header->func_name)) { - ZVAL_COPY(&ch->handlers.write_header->func_name, &source->handlers.write_header->func_name); + if (ZEND_FCC_INITIALIZED(source->handlers.write->fcc)) { + zend_fcc_dup(&source->handlers.write->fcc, &source->handlers.write->fcc); + } + if (ZEND_FCC_INITIALIZED(source->handlers.write_header->fcc)) { + zend_fcc_dup(&source->handlers.write_header->fcc, &source->handlers.write_header->fcc); } + php_curl_copy_fcc_with_option(ch, CURLOPT_XFERINFODATA, &ch->handlers.xferinfo, &source->handlers.xferinfo); + php_curl_copy_fcc_with_option(ch, CURLOPT_XFERINFODATA, &ch->handlers.xferinfo, &source->handlers.xferinfo); curl_easy_setopt(ch->cp, CURLOPT_ERRORBUFFER, ch->err.str); curl_easy_setopt(ch->cp, CURLOPT_FILE, (void *) ch); @@ -2087,15 +2069,6 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue error = curl_easy_setopt(ch->cp, option, lval); break; - case CURLOPT_HEADERFUNCTION: - if (!Z_ISUNDEF(ch->handlers.write_header->func_name)) { - zval_ptr_dtor(&ch->handlers.write_header->func_name); - ch->handlers.write_header->fci_cache = empty_fcall_info_cache; - } - ZVAL_COPY(&ch->handlers.write_header->func_name, zvalue); - ch->handlers.write_header->method = PHP_CURL_USER; - break; - case CURLOPT_POSTFIELDS: if (Z_TYPE_P(zvalue) == IS_ARRAY) { if (zend_hash_num_elements(HASH_OF(zvalue)) == 0) { @@ -2116,6 +2089,28 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue } break; + case CURLOPT_WRITEFUNCTION: { + /* Check value is actually a callable and set it */ + const char option_name[] = "CURLOPT_WRITEFUNCTION"; + bool result = php_curl_set_callable_handler(&ch->handlers.write->fcc, zvalue, is_array_config, option_name); + if (!result) { + return FAILURE; + } + ch->handlers.write->method = PHP_CURL_USER; + break; + } + + case CURLOPT_HEADERFUNCTION: { + /* Check value is actually a callable and set it */ + const char option_name[] = "CURLOPT_HEADERFUNCTION"; + bool result = php_curl_set_callable_handler(&ch->handlers.write_header->fcc, zvalue, is_array_config, option_name); + if (!result) { + return FAILURE; + } + ch->handlers.write_header->method = PHP_CURL_USER; + break; + } + case CURLOPT_PROGRESSFUNCTION: { /* Check value is actually a callable and set it */ const char option_name[] = "CURLOPT_PROGRESSFUNCTION"; @@ -2183,15 +2178,6 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue } break; - case CURLOPT_WRITEFUNCTION: - if (!Z_ISUNDEF(ch->handlers.write->func_name)) { - zval_ptr_dtor(&ch->handlers.write->func_name); - ch->handlers.write->fci_cache = empty_fcall_info_cache; - } - ZVAL_COPY(&ch->handlers.write->func_name, zvalue); - ch->handlers.write->method = PHP_CURL_USER; - break; - /* Curl off_t options */ case CURLOPT_MAX_RECV_SPEED_LARGE: case CURLOPT_MAX_SEND_SPEED_LARGE: @@ -2793,9 +2779,13 @@ static void curl_free_obj(zend_object *object) } smart_str_free(&ch->handlers.write->buf); - zval_ptr_dtor(&ch->handlers.write->func_name); zval_ptr_dtor(&ch->handlers.read->func_name); - zval_ptr_dtor(&ch->handlers.write_header->func_name); + if (ZEND_FCC_INITIALIZED(ch->handlers.write->fcc)) { + zend_fcc_dtor(&ch->handlers.write->fcc); + } + if (ZEND_FCC_INITIALIZED(ch->handlers.write_header->fcc)) { + zend_fcc_dtor(&ch->handlers.write_header->fcc); + } zval_ptr_dtor(&ch->handlers.std_err); if (ch->header.str) { zend_string_release_ex(ch->header.str, 0); diff --git a/ext/curl/tests/curl_setopt_callables.phpt b/ext/curl/tests/curl_setopt_callables.phpt index 71f3ae671018b..542685546169f 100644 --- a/ext/curl/tests/curl_setopt_callables.phpt +++ b/ext/curl/tests/curl_setopt_callables.phpt @@ -27,6 +27,8 @@ testOption($ch, CURLOPT_PROGRESSFUNCTION); testOption($ch, CURLOPT_SSH_HOSTKEYFUNCTION); testOption($ch, CURLOPT_XFERINFOFUNCTION); testOption($ch, CURLOPT_FNMATCH_FUNCTION); +testOption($ch, CURLOPT_WRITEFUNCTION); +testOption($ch, CURLOPT_HEADERFUNCTION); ?> --EXPECT-- @@ -38,3 +40,7 @@ TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for opti TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_XFERINFOFUNCTION, function "undefined" not found or invalid function name TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for option CURLOPT_FNMATCH_FUNCTION, function "undefined" not found or invalid function name TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_FNMATCH_FUNCTION, function "undefined" not found or invalid function name +TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for option CURLOPT_WRITEFUNCTION, function "undefined" not found or invalid function name +TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_WRITEFUNCTION, function "undefined" not found or invalid function name +TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for option CURLOPT_HEADERFUNCTION, function "undefined" not found or invalid function name +TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_HEADERFUNCTION, function "undefined" not found or invalid function name From c3f60a24c9beb6097d7b1b154f0d6ec8c77be8b1 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 31 Jan 2024 23:38:47 +0000 Subject: [PATCH 07/15] ext/curl: Convert php_curl_read to just use FCC without a function name zval --- ext/curl/curl_private.h | 8 +-- ext/curl/interface.c | 67 ++++++++++------------- ext/curl/tests/curl_setopt_callables.phpt | 3 + 3 files changed, 34 insertions(+), 44 deletions(-) diff --git a/ext/curl/curl_private.h b/ext/curl/curl_private.h index 5ab2de0415b13..c15396f255b17 100644 --- a/ext/curl/curl_private.h +++ b/ext/curl/curl_private.h @@ -53,19 +53,13 @@ typedef struct { } php_curl_write; typedef struct { - zval func_name; - zend_fcall_info_cache fci_cache; + zend_fcall_info_cache fcc; FILE *fp; zend_resource *res; int method; zval stream; } php_curl_read; -typedef struct { - zval func_name; - zend_fcall_info_cache fci_cache; -} php_curl_callback; - typedef struct { php_curl_write *write; php_curl_write *write_header; diff --git a/ext/curl/interface.c b/ext/curl/interface.c index fb621c9fdc5ce..e075194545c66 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -474,7 +474,9 @@ static HashTable *curl_get_gc(zend_object *object, zval **table, int *n) zend_get_gc_buffer_add_zval(gc_buffer, &curl->postfields); if (curl->handlers.read) { - zend_get_gc_buffer_add_zval(gc_buffer, &curl->handlers.read->func_name); + if (ZEND_FCC_INITIALIZED(curl->handlers.read->fcc)) { + zend_get_gc_buffer_add_fcc(gc_buffer, &curl->handlers.read->fcc); + } zend_get_gc_buffer_add_zval(gc_buffer, &curl->handlers.read->stream); } @@ -766,46 +768,33 @@ static int curl_ssh_hostkeyfunction(void *clientp, int keytype, const char *key, static size_t curl_read(char *data, size_t size, size_t nmemb, void *ctx) { php_curl *ch = (php_curl *)ctx; - php_curl_read *t = ch->handlers.read; + php_curl_read *read_handler = ch->handlers.read; int length = 0; - switch (t->method) { + switch (read_handler->method) { case PHP_CURL_DIRECT: - if (t->fp) { - length = fread(data, size, nmemb, t->fp); + if (read_handler->fp) { + length = fread(data, size, nmemb, read_handler->fp); } break; case PHP_CURL_USER: { zval argv[3]; zval retval; - zend_result error; - zend_fcall_info fci; GC_ADDREF(&ch->std); ZVAL_OBJ(&argv[0], &ch->std); - if (t->res) { - GC_ADDREF(t->res); - ZVAL_RES(&argv[1], t->res); + if (read_handler->res) { + GC_ADDREF(read_handler->res); + ZVAL_RES(&argv[1], read_handler->res); } else { ZVAL_NULL(&argv[1]); } ZVAL_LONG(&argv[2], (int)size * nmemb); - fci.size = sizeof(fci); - ZVAL_COPY_VALUE(&fci.function_name, &t->func_name); - fci.object = NULL; - fci.retval = &retval; - fci.param_count = 3; - fci.params = argv; - fci.named_params = NULL; - - ch->in_callback = 1; - error = zend_call_function(&fci, &t->fci_cache); - ch->in_callback = 0; - if (error == FAILURE) { - php_error_docref(NULL, E_WARNING, "Cannot call the CURLOPT_READFUNCTION"); - length = CURL_READFUNC_ABORT; - } else if (!Z_ISUNDEF(retval)) { + ch->in_callback = true; + zend_call_known_fcc(&read_handler->fcc, &retval, /* param_count */ 3, argv, /* named_params */ NULL); + ch->in_callback = false; + if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); if (Z_TYPE(retval) == IS_STRING) { length = MIN((int) (size * nmemb), Z_STRLEN(retval)); @@ -813,6 +802,7 @@ static size_t curl_read(char *data, size_t size, size_t nmemb, void *ctx) } else if (Z_TYPE(retval) == IS_LONG) { length = Z_LVAL_P(&retval); } + // TODO Do type error if invalid type? zval_ptr_dtor(&retval); } @@ -1212,8 +1202,8 @@ void _php_setup_easy_copy_handlers(php_curl *ch, php_curl *source) ch->handlers.read->fp = source->handlers.read->fp; ch->handlers.read->res = source->handlers.read->res; - if (!Z_ISUNDEF(source->handlers.read->func_name)) { - ZVAL_COPY(&ch->handlers.read->func_name, &source->handlers.read->func_name); + if (ZEND_FCC_INITIALIZED(source->handlers.read->fcc)) { + zend_fcc_dup(&source->handlers.read->fcc, &source->handlers.read->fcc); } if (ZEND_FCC_INITIALIZED(source->handlers.write->fcc)) { zend_fcc_dup(&source->handlers.write->fcc, &source->handlers.write->fcc); @@ -2111,6 +2101,16 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue break; } + case CURLOPT_READFUNCTION: + /* Check value is actually a callable and set it */ + const char option_name[] = "CURLOPT_READFUNCTION"; + bool result = php_curl_set_callable_handler(&ch->handlers.read->fcc, zvalue, is_array_config, option_name); + if (!result) { + return FAILURE; + } + ch->handlers.read->method = PHP_CURL_USER; + break; + case CURLOPT_PROGRESSFUNCTION: { /* Check value is actually a callable and set it */ const char option_name[] = "CURLOPT_PROGRESSFUNCTION"; @@ -2161,15 +2161,6 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue } #endif - case CURLOPT_READFUNCTION: - if (!Z_ISUNDEF(ch->handlers.read->func_name)) { - zval_ptr_dtor(&ch->handlers.read->func_name); - ch->handlers.read->fci_cache = empty_fcall_info_cache; - } - ZVAL_COPY(&ch->handlers.read->func_name, zvalue); - ch->handlers.read->method = PHP_CURL_USER; - break; - case CURLOPT_RETURNTRANSFER: if (zend_is_true(zvalue)) { ch->handlers.write->method = PHP_CURL_RETURN; @@ -2779,13 +2770,15 @@ static void curl_free_obj(zend_object *object) } smart_str_free(&ch->handlers.write->buf); - zval_ptr_dtor(&ch->handlers.read->func_name); if (ZEND_FCC_INITIALIZED(ch->handlers.write->fcc)) { zend_fcc_dtor(&ch->handlers.write->fcc); } if (ZEND_FCC_INITIALIZED(ch->handlers.write_header->fcc)) { zend_fcc_dtor(&ch->handlers.write_header->fcc); } + if (ZEND_FCC_INITIALIZED(ch->handlers.read->fcc)) { + zend_fcc_dtor(&ch->handlers.read->fcc); + } zval_ptr_dtor(&ch->handlers.std_err); if (ch->header.str) { zend_string_release_ex(ch->header.str, 0); diff --git a/ext/curl/tests/curl_setopt_callables.phpt b/ext/curl/tests/curl_setopt_callables.phpt index 542685546169f..1b39ee03963cd 100644 --- a/ext/curl/tests/curl_setopt_callables.phpt +++ b/ext/curl/tests/curl_setopt_callables.phpt @@ -29,6 +29,7 @@ testOption($ch, CURLOPT_XFERINFOFUNCTION); testOption($ch, CURLOPT_FNMATCH_FUNCTION); testOption($ch, CURLOPT_WRITEFUNCTION); testOption($ch, CURLOPT_HEADERFUNCTION); +testOption($ch, CURLOPT_READFUNCTION); ?> --EXPECT-- @@ -44,3 +45,5 @@ TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for opti TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_WRITEFUNCTION, function "undefined" not found or invalid function name TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for option CURLOPT_HEADERFUNCTION, function "undefined" not found or invalid function name TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_HEADERFUNCTION, function "undefined" not found or invalid function name +TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for option CURLOPT_READFUNCTION, function "undefined" not found or invalid function name +TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_READFUNCTION, function "undefined" not found or invalid function name From 51c8e216abb9290836f8dbc24489aa246e6b1fac Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 31 Jan 2024 23:56:13 +0000 Subject: [PATCH 08/15] ext/curl: Remove workaround for old libcurl --- ext/curl/interface.c | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/ext/curl/interface.c b/ext/curl/interface.c index e075194545c66..8cab761d061a2 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -556,15 +556,6 @@ PHP_MSHUTDOWN_FUNCTION(curl) } /* }}} */ -/* {{{ curl_write_nothing - * Used as a work around. See _php_curl_close_ex - */ -static size_t curl_write_nothing(char *data, size_t size, size_t nmemb, void *ctx) -{ - return size * nmemb; -} -/* }}} */ - /* {{{ curl_write */ static size_t curl_write(char *data, size_t size, size_t nmemb, void *ctx) { @@ -2742,20 +2733,6 @@ static void curl_free_obj(zend_object *object) _php_curl_verify_handlers(ch, /* reporterror */ false); - /* - * Libcurl is doing connection caching. When easy handle is cleaned up, - * if the handle was previously used by the curl_multi_api, the connection - * remains open un the curl multi handle is cleaned up. Some protocols are - * sending content like the FTP one, and libcurl try to use the - * WRITEFUNCTION or the HEADERFUNCTION. Since structures used in those - * callback are freed, we need to use an other callback to which avoid - * segfaults. - * - * Libcurl commit d021f2e8a00 fix this issue and should be part of 7.28.2 - */ - curl_easy_setopt(ch->cp, CURLOPT_HEADERFUNCTION, curl_write_nothing); - curl_easy_setopt(ch->cp, CURLOPT_WRITEFUNCTION, curl_write_nothing); - curl_easy_cleanup(ch->cp); /* cURL destructors should be invoked only by last curl handle */ From fd4fafd70b2d64946a53cb91628a1dd2508d60b1 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Thu, 1 Feb 2024 00:25:11 +0000 Subject: [PATCH 09/15] ext/curl: Create macros to codegen the handling of callable options --- ext/curl/interface.c | 116 +++++++++++++------------------------------ 1 file changed, 34 insertions(+), 82 deletions(-) diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 8cab761d061a2..5beafe0319c90 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -1535,12 +1535,46 @@ static bool php_curl_set_callable_handler(zend_fcall_info_cache *const handler_f return true; } + +#define HANDLE_CURL_OPTION_CALLABLE_PHP_CURL_USER(curl_ptr, constant_no_function, handler_type) \ + case 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) { \ + return FAILURE; \ + } \ + curl_ptr->handlers.handler_type->method = PHP_CURL_USER; \ + break; \ + } + +#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->handler_fcc, zvalue, is_array_config, #constant_no_function "FUNCTION"); \ + if (!result) { \ + return FAILURE; \ + } \ + curl_easy_setopt(curl_ptr->cp, constant_no_function##FUNCTION, (c_callback)); \ + curl_easy_setopt(curl_ptr->cp, constant_no_function##DATA, curl_ptr); \ + break; \ + } + static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue, bool is_array_config) /* {{{ */ { CURLcode error = CURLE_OK; zend_long lval; switch (option) { + /* Callable options */ + HANDLE_CURL_OPTION_CALLABLE_PHP_CURL_USER(ch, CURLOPT_WRITE, write); + HANDLE_CURL_OPTION_CALLABLE_PHP_CURL_USER(ch, CURLOPT_HEADER, write_header); + HANDLE_CURL_OPTION_CALLABLE_PHP_CURL_USER(ch, CURLOPT_READ, read); + + HANDLE_CURL_OPTION_CALLABLE(ch, CURLOPT_PROGRESS, handlers.progress, curl_progress); + HANDLE_CURL_OPTION_CALLABLE(ch, CURLOPT_XFERINFO, handlers.xferinfo, curl_xferinfo); + HANDLE_CURL_OPTION_CALLABLE(ch, CURLOPT_FNMATCH_, handlers.fnmatch, curl_fnmatch); +#if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ + HANDLE_CURL_OPTION_CALLABLE(ch, CURLOPT_SSH_HOSTKEY, handlers.sshhostkey, curl_ssh_hostkeyfunction); +#endif + /* Long options */ case CURLOPT_SSL_VERIFYHOST: lval = zval_get_long(zvalue); @@ -2070,88 +2104,6 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue } break; - case CURLOPT_WRITEFUNCTION: { - /* Check value is actually a callable and set it */ - const char option_name[] = "CURLOPT_WRITEFUNCTION"; - bool result = php_curl_set_callable_handler(&ch->handlers.write->fcc, zvalue, is_array_config, option_name); - if (!result) { - return FAILURE; - } - ch->handlers.write->method = PHP_CURL_USER; - break; - } - - case CURLOPT_HEADERFUNCTION: { - /* Check value is actually a callable and set it */ - const char option_name[] = "CURLOPT_HEADERFUNCTION"; - bool result = php_curl_set_callable_handler(&ch->handlers.write_header->fcc, zvalue, is_array_config, option_name); - if (!result) { - return FAILURE; - } - ch->handlers.write_header->method = PHP_CURL_USER; - break; - } - - case CURLOPT_READFUNCTION: - /* Check value is actually a callable and set it */ - const char option_name[] = "CURLOPT_READFUNCTION"; - bool result = php_curl_set_callable_handler(&ch->handlers.read->fcc, zvalue, is_array_config, option_name); - if (!result) { - return FAILURE; - } - ch->handlers.read->method = PHP_CURL_USER; - break; - - case CURLOPT_PROGRESSFUNCTION: { - /* Check value is actually a callable and set it */ - const char option_name[] = "CURLOPT_PROGRESSFUNCTION"; - bool result = php_curl_set_callable_handler(&ch->handlers.progress, zvalue, is_array_config, option_name); - if (!result) { - return FAILURE; - } - curl_easy_setopt(ch->cp, CURLOPT_PROGRESSFUNCTION, curl_progress); - curl_easy_setopt(ch->cp, CURLOPT_PROGRESSDATA, ch); - break; - } - - case CURLOPT_XFERINFOFUNCTION: { - /* Check value is actually a callable and set it */ - const char option_name[] = "CURLOPT_XFERINFOFUNCTION"; - bool result = php_curl_set_callable_handler(&ch->handlers.xferinfo, zvalue, is_array_config, option_name); - if (!result) { - return FAILURE; - } - curl_easy_setopt(ch->cp, CURLOPT_XFERINFOFUNCTION, curl_xferinfo); - curl_easy_setopt(ch->cp, CURLOPT_XFERINFODATA, ch); - break; - } - - case CURLOPT_FNMATCH_FUNCTION: { - /* Check value is actually a callable and set it */ - const char option_name[] = "CURLOPT_FNMATCH_FUNCTION"; - bool result = php_curl_set_callable_handler(&ch->handlers.fnmatch, zvalue, is_array_config, option_name); - if (!result) { - return FAILURE; - } - curl_easy_setopt(ch->cp, CURLOPT_FNMATCH_FUNCTION, curl_fnmatch); - curl_easy_setopt(ch->cp, CURLOPT_FNMATCH_DATA, ch); - break; - } - -#if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ - case CURLOPT_SSH_HOSTKEYFUNCTION: { - /* Check value is actually a callable and set it */ - const char option_name[] = "CURLOPT_SSH_HOSTKEYFUNCTION"; - bool result = php_curl_set_callable_handler(&ch->handlers.sshhostkey, zvalue, is_array_config, option_name); - if (!result) { - return FAILURE; - } - curl_easy_setopt(ch->cp, CURLOPT_SSH_HOSTKEYFUNCTION, curl_ssh_hostkeyfunction); - curl_easy_setopt(ch->cp, CURLOPT_SSH_HOSTKEYDATA, ch); - break; - } -#endif - case CURLOPT_RETURNTRANSFER: if (zend_is_true(zvalue)) { ch->handlers.write->method = PHP_CURL_RETURN; From b9731c84774e52d0b4150d456fbee026f3389bb0 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 6 Feb 2024 13:10:58 +0000 Subject: [PATCH 10/15] Fix test due to unavailable SSH_HOSTKEY option --- .../curl_setopt_SSH_HOSTKEY_callable.phpt | 36 +++++++++++++++++++ ext/curl/tests/curl_setopt_callables.phpt | 7 +--- 2 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 ext/curl/tests/curl_setopt_SSH_HOSTKEY_callable.phpt diff --git a/ext/curl/tests/curl_setopt_SSH_HOSTKEY_callable.phpt b/ext/curl/tests/curl_setopt_SSH_HOSTKEY_callable.phpt new file mode 100644 index 0000000000000..f17fae9cbaa2d --- /dev/null +++ b/ext/curl/tests/curl_setopt_SSH_HOSTKEY_callable.phpt @@ -0,0 +1,36 @@ +--TEST-- +Test curl_setopt(_array)() with CURLOPT_SSH_HOSTKEYFUNCTION option +--EXTENSIONS-- +curl +--SKIPIF-- + +--FILE-- +getMessage(), PHP_EOL; + } + + try { + var_dump(curl_setopt_array($handle, [$option => 'undefined'])); + } catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; + } +} + +$url = "https://example.com"; +$ch = curl_init($url); +testOption($ch, CURLOPT_SSH_HOSTKEYFUNCTION); + +?> +--EXPECT-- +TypeError: curl_setopt(): Argument #3 ($value) must be a valid callback for option CURLOPT_SSH_HOSTKEYFUNCTION, function "undefined" not found or invalid function name +TypeError: curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_SSH_HOSTKEYFUNCTION, function "undefined" not found or invalid function name diff --git a/ext/curl/tests/curl_setopt_callables.phpt b/ext/curl/tests/curl_setopt_callables.phpt index 1b39ee03963cd..6cde8fef6d9c7 100644 --- a/ext/curl/tests/curl_setopt_callables.phpt +++ b/ext/curl/tests/curl_setopt_callables.phpt @@ -4,8 +4,6 @@ Test curl_setopt(_array)() with options that take callabes curl --FILE-- Date: Tue, 6 Feb 2024 14:20:16 +0000 Subject: [PATCH 11/15] Add trampoline tests --- ext/curl/tests/curl_fnmatch_trampoline.phpt | 34 ++++++++++ ext/curl/tests/curl_progress_trampoline.phpt | 34 ++++++++++ .../tests/curl_pushfunction_trampoline.phpt | 68 +++++++++++++++++++ ...DFUNCTION.phpt => curl_read_callback.phpt} | 0 ext/curl/tests/curl_read_trampoline.phpt | 49 +++++++++++++ ...=> curl_ssh_hostkey_invalid_callable.phpt} | 0 .../tests/curl_ssh_hostkey_trampoline.phpt | 41 +++++++++++ ext/curl/tests/curl_write_trampoline.phpt | 36 ++++++++++ .../tests/curl_writeheader_tranpoline.phpt | 28 ++++++++ ext/curl/tests/curl_xferinfo_trampoline.phpt | 34 ++++++++++ 10 files changed, 324 insertions(+) create mode 100644 ext/curl/tests/curl_fnmatch_trampoline.phpt create mode 100644 ext/curl/tests/curl_progress_trampoline.phpt create mode 100644 ext/curl/tests/curl_pushfunction_trampoline.phpt rename ext/curl/tests/{curl_setopt_CURLOPT_READFUNCTION.phpt => curl_read_callback.phpt} (100%) create mode 100644 ext/curl/tests/curl_read_trampoline.phpt rename ext/curl/tests/{curl_setopt_SSH_HOSTKEY_callable.phpt => curl_ssh_hostkey_invalid_callable.phpt} (100%) create mode 100644 ext/curl/tests/curl_ssh_hostkey_trampoline.phpt create mode 100644 ext/curl/tests/curl_write_trampoline.phpt create mode 100644 ext/curl/tests/curl_writeheader_tranpoline.phpt create mode 100644 ext/curl/tests/curl_xferinfo_trampoline.phpt diff --git a/ext/curl/tests/curl_fnmatch_trampoline.phpt b/ext/curl/tests/curl_fnmatch_trampoline.phpt new file mode 100644 index 0000000000000..ffc286cae8846 --- /dev/null +++ b/ext/curl/tests/curl_fnmatch_trampoline.phpt @@ -0,0 +1,34 @@ +--TEST-- +Test trampoline for curl option CURLOPT_FNMATCH_FUNCTION +--EXTENSIONS-- +curl +--SKIPIF-- + +--FILE-- + +--EXPECT-- +Trampoline for trampoline +Hello World! +Hello World! diff --git a/ext/curl/tests/curl_progress_trampoline.phpt b/ext/curl/tests/curl_progress_trampoline.phpt new file mode 100644 index 0000000000000..d3d31e6aec891 --- /dev/null +++ b/ext/curl/tests/curl_progress_trampoline.phpt @@ -0,0 +1,34 @@ +--TEST-- +Test trampoline for curl option CURLOPT_PROGRESSFUNCTION +--EXTENSIONS-- +curl +--FILE-- + +--EXPECT-- +Trampoline for trampoline +Hello World! +Hello World! diff --git a/ext/curl/tests/curl_pushfunction_trampoline.phpt b/ext/curl/tests/curl_pushfunction_trampoline.phpt new file mode 100644 index 0000000000000..436a5e52e6dde --- /dev/null +++ b/ext/curl/tests/curl_pushfunction_trampoline.phpt @@ -0,0 +1,68 @@ +--TEST-- +Test trampoline for curl option CURLMOPT_PUSHFUNCTION +--EXTENSIONS-- +curl +--SKIPIF-- + +--XFAIL-- +Need CI to test caddy +--FILE-- + +--EXPECT-- +Array +( + [0] => main response + [1] => pushed response +) diff --git a/ext/curl/tests/curl_setopt_CURLOPT_READFUNCTION.phpt b/ext/curl/tests/curl_read_callback.phpt similarity index 100% rename from ext/curl/tests/curl_setopt_CURLOPT_READFUNCTION.phpt rename to ext/curl/tests/curl_read_callback.phpt diff --git a/ext/curl/tests/curl_read_trampoline.phpt b/ext/curl/tests/curl_read_trampoline.phpt new file mode 100644 index 0000000000000..f69caebf79875 --- /dev/null +++ b/ext/curl/tests/curl_read_trampoline.phpt @@ -0,0 +1,49 @@ +--TEST-- +Test trampoline for curl option CURLOPT_READFUNCTION +--EXTENSIONS-- +curl +--FILE-- + +--CLEAN-- + +--EXPECT-- +Trampoline for trampoline +string(0) "" diff --git a/ext/curl/tests/curl_setopt_SSH_HOSTKEY_callable.phpt b/ext/curl/tests/curl_ssh_hostkey_invalid_callable.phpt similarity index 100% rename from ext/curl/tests/curl_setopt_SSH_HOSTKEY_callable.phpt rename to ext/curl/tests/curl_ssh_hostkey_invalid_callable.phpt diff --git a/ext/curl/tests/curl_ssh_hostkey_trampoline.phpt b/ext/curl/tests/curl_ssh_hostkey_trampoline.phpt new file mode 100644 index 0000000000000..84ce8eb85115a --- /dev/null +++ b/ext/curl/tests/curl_ssh_hostkey_trampoline.phpt @@ -0,0 +1,41 @@ +--TEST-- +Test trampoline for curl option CURLOPT_SSH_HOSTKEYFUNCTION +--EXTENSIONS-- +curl +--SKIPIF-- += 7.84.0"); +} +exit("skip: cannot properly test CURLOPT_SSH_HOSTKEYFUNCTION"); +?> +--FILE-- + +--EXPECT-- +Trampoline for trampoline +FAKE diff --git a/ext/curl/tests/curl_write_trampoline.phpt b/ext/curl/tests/curl_write_trampoline.phpt new file mode 100644 index 0000000000000..8d604bc7bd4fc --- /dev/null +++ b/ext/curl/tests/curl_write_trampoline.phpt @@ -0,0 +1,36 @@ +--TEST-- +Test trampoline for curl option CURLOPT_WRITEFUNCTION +--EXTENSIONS-- +curl +--FILE-- + +--CLEAN-- + +--EXPECT-- +Trampoline for trampoline diff --git a/ext/curl/tests/curl_writeheader_tranpoline.phpt b/ext/curl/tests/curl_writeheader_tranpoline.phpt new file mode 100644 index 0000000000000..48c53f3996a44 --- /dev/null +++ b/ext/curl/tests/curl_writeheader_tranpoline.phpt @@ -0,0 +1,28 @@ +--TEST-- +Test trampoline for curl option CURLOPT_HEADERFUNCTION +--EXTENSIONS-- +curl +--FILE-- + +--EXPECT-- +Trampoline for trampoline diff --git a/ext/curl/tests/curl_xferinfo_trampoline.phpt b/ext/curl/tests/curl_xferinfo_trampoline.phpt new file mode 100644 index 0000000000000..f10ea890dcba8 --- /dev/null +++ b/ext/curl/tests/curl_xferinfo_trampoline.phpt @@ -0,0 +1,34 @@ +--TEST-- +Test trampoline for curl option CURLOPT_XFERINFOFUNCTION +--EXTENSIONS-- +curl +--FILE-- + +--EXPECT-- +Trampoline for trampoline +Hello World! +Hello World! From 617b1863f0a1f284f03ace4bc31fd799f2d0e464 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 16 Apr 2024 20:40:02 +0100 Subject: [PATCH 12/15] Remove XFAIL --- ext/curl/tests/curl_pushfunction_trampoline.phpt | 2 -- 1 file changed, 2 deletions(-) diff --git a/ext/curl/tests/curl_pushfunction_trampoline.phpt b/ext/curl/tests/curl_pushfunction_trampoline.phpt index 436a5e52e6dde..de6da63914045 100644 --- a/ext/curl/tests/curl_pushfunction_trampoline.phpt +++ b/ext/curl/tests/curl_pushfunction_trampoline.phpt @@ -11,8 +11,6 @@ if ($curl_version['version_number'] < 0x080100) { exit("skip: test may crash with curl < 8.1.0"); } ?> ---XFAIL-- -Need CI to test caddy --FILE-- Date: Tue, 23 Apr 2024 23:37:57 +0100 Subject: [PATCH 13/15] Address review comments --- ext/curl/interface.c | 50 +++++++++++++++++++++++++------------------- ext/curl/multi.c | 3 +-- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 5beafe0319c90..df8f07779af8e 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -592,7 +592,7 @@ static size_t curl_write(char *data, size_t size, size_t nmemb, void *ctx) ch->in_callback = false; if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); - /* TODO Check callback returns an int or something castable to int */ + /* TODO Check callback returns an int or something castable to int */ length = zval_get_long(&retval); } @@ -625,7 +625,7 @@ static int curl_fnmatch(void *ctx, const char *pattern, const char *string) if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); - /* TODO Check callback returns an int or something castable to int */ + /* TODO Check callback returns an int or something castable to int */ rval = zval_get_long(&retval); } zval_ptr_dtor(&argv[0]); @@ -661,12 +661,12 @@ static size_t curl_progress(void *clientp, double dltotal, double dlnow, double ch->in_callback = false; if (!Z_ISUNDEF(retval)) { - _php_curl_verify_handlers(ch, /* reporterror */ true); - /* TODO Check callback returns an int or something castable to int */ - if (0 != zval_get_long(&retval)) { - rval = 1; - } - } + _php_curl_verify_handlers(ch, /* reporterror */ true); + /* TODO Check callback returns an int or something castable to int */ + if (0 != zval_get_long(&retval)) { + rval = 1; + } + } zval_ptr_dtor(&args[0]); return rval; @@ -699,12 +699,12 @@ static size_t curl_xferinfo(void *clientp, curl_off_t dltotal, curl_off_t dlnow, ch->in_callback = false; if (!Z_ISUNDEF(retval)) { - _php_curl_verify_handlers(ch, /* reporterror */ true); - /* TODO Check callback returns an int or something castable to int */ - if (0 != zval_get_long(&retval)) { - rval = 1; - } - } + _php_curl_verify_handlers(ch, /* reporterror */ true); + /* TODO Check callback returns an int or something castable to int */ + if (0 != zval_get_long(&retval)) { + rval = 1; + } + } zval_ptr_dtor(&argv[0]); return rval; @@ -923,8 +923,8 @@ PHP_FUNCTION(curl_version) /* Add an array of features */ { struct feat { - const char *name; - int bitmask; + const char *name; + int bitmask; }; unsigned int i; @@ -974,7 +974,7 @@ PHP_FUNCTION(curl_version) #endif }; - for(i = 0; i < sizeof(feats) / sizeof(feats[0]); i++) { + for(i = 0; i < sizeof(feats) / sizeof(feats[0]); i++) { if (feats[i].name) { add_assoc_bool(&feature_list, feats[i].name, d->features & feats[i].bitmask ? true : false); } @@ -1202,8 +1202,6 @@ void _php_setup_easy_copy_handlers(php_curl *ch, php_curl *source) if (ZEND_FCC_INITIALIZED(source->handlers.write_header->fcc)) { zend_fcc_dup(&source->handlers.write_header->fcc, &source->handlers.write_header->fcc); } - php_curl_copy_fcc_with_option(ch, CURLOPT_XFERINFODATA, &ch->handlers.xferinfo, &source->handlers.xferinfo); - php_curl_copy_fcc_with_option(ch, CURLOPT_XFERINFODATA, &ch->handlers.xferinfo, &source->handlers.xferinfo); curl_easy_setopt(ch->cp, CURLOPT_ERRORBUFFER, ch->err.str); curl_easy_setopt(ch->cp, CURLOPT_FILE, (void *) ch); @@ -2140,7 +2138,7 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue case CURLOPT_ISSUERCERT: case CURLOPT_SSH_KNOWNHOSTS: { - zend_string *tmp_str; + zend_string *tmp_str; zend_string *str = zval_get_tmp_string(zvalue, &tmp_str); zend_result ret; @@ -2701,12 +2699,15 @@ static void curl_free_obj(zend_object *object) smart_str_free(&ch->handlers.write->buf); if (ZEND_FCC_INITIALIZED(ch->handlers.write->fcc)) { zend_fcc_dtor(&ch->handlers.write->fcc); + ch->handlers.write->fcc = empty_fcall_info_cache; } if (ZEND_FCC_INITIALIZED(ch->handlers.write_header->fcc)) { zend_fcc_dtor(&ch->handlers.write_header->fcc); + ch->handlers.write_header->fcc = empty_fcall_info_cache; } if (ZEND_FCC_INITIALIZED(ch->handlers.read->fcc)) { zend_fcc_dtor(&ch->handlers.read->fcc); + ch->handlers.read->fcc = empty_fcall_info_cache; } zval_ptr_dtor(&ch->handlers.std_err); if (ch->header.str) { @@ -2723,16 +2724,20 @@ static void curl_free_obj(zend_object *object) if (ZEND_FCC_INITIALIZED(ch->handlers.progress)) { zend_fcc_dtor(&ch->handlers.progress); + ch->handlers.progress = empty_fcall_info_cache; } if (ZEND_FCC_INITIALIZED(ch->handlers.xferinfo)) { zend_fcc_dtor(&ch->handlers.xferinfo); + ch->handlers.xferinfo = empty_fcall_info_cache; } if (ZEND_FCC_INITIALIZED(ch->handlers.fnmatch)) { zend_fcc_dtor(&ch->handlers.fnmatch); + ch->handlers.fnmatch = empty_fcall_info_cache; } #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ if (ZEND_FCC_INITIALIZED(ch->handlers.sshhostkey)) { zend_fcc_dtor(&ch->handlers.sshhostkey); + ch->handlers.sshhostkey = empty_fcall_info_cache; } #endif @@ -2799,20 +2804,23 @@ static void _php_curl_reset_handlers(php_curl *ch) if (ZEND_FCC_INITIALIZED(ch->handlers.progress)) { zend_fcc_dtor(&ch->handlers.progress); + ch->handlers.progress = empty_fcall_info_cache; } if (ZEND_FCC_INITIALIZED(ch->handlers.xferinfo)) { zend_fcc_dtor(&ch->handlers.xferinfo); - ch->handlers.xferinfo.function_handler = NULL; + ch->handlers.xferinfo = empty_fcall_info_cache; } if (ZEND_FCC_INITIALIZED(ch->handlers.fnmatch)) { zend_fcc_dtor(&ch->handlers.fnmatch); + ch->handlers.fnmatch = empty_fcall_info_cache; } #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ if (ZEND_FCC_INITIALIZED(ch->handlers.sshhostkey)) { zend_fcc_dtor(&ch->handlers.sshhostkey); + ch->handlers.sshhostkey = empty_fcall_info_cache; } #endif } diff --git a/ext/curl/multi.c b/ext/curl/multi.c index 183324f5bc631..586f48911b261 100644 --- a/ext/curl/multi.c +++ b/ext/curl/multi.c @@ -390,8 +390,7 @@ static int _php_server_push_callback(CURL *parent_ch, CURL *easy, size_t num_hea array_init(&headers); for (size_t i = 0; i < num_headers; i++) { - char *header; - header = curl_pushheader_bynum(push_headers, i); + char *header = curl_pushheader_bynum(push_headers, i); add_next_index_string(&headers, header); } From 4b7230b097bdec7725107644c0d0301f88aa5ae7 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 30 Apr 2024 22:13:42 +0100 Subject: [PATCH 14/15] Address review comments about missing empty FCC assignment --- ext/curl/multi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/curl/multi.c b/ext/curl/multi.c index 586f48911b261..9492eb1e8bfbd 100644 --- a/ext/curl/multi.c +++ b/ext/curl/multi.c @@ -446,6 +446,7 @@ static bool _php_curl_multi_setopt(php_curlm *mh, zend_long option, zval *zvalue /* See php_curl_set_callable_handler */ if (ZEND_FCC_INITIALIZED(mh->handlers.server_push)) { zend_fcc_dtor(&mh->handlers.server_push); + mh->handlers.server_push = empty_fcall_info_cache; } char *error_str = NULL; @@ -543,6 +544,7 @@ static void curl_multi_free_obj(zend_object *object) if (ZEND_FCC_INITIALIZED(mh->handlers.server_push)) { zend_fcc_dtor(&mh->handlers.server_push); + mh->handlers.server_push = empty_fcall_info_cache; } zend_object_std_dtor(&mh->std); From 431d24a6be2ff26474687ca4e69f010d56d5a0b1 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 1 May 2024 00:43:11 +0100 Subject: [PATCH 15/15] Remove unnecessary set to NULL/empty FCC --- ext/curl/interface.c | 12 ------------ ext/curl/multi.c | 2 -- 2 files changed, 14 deletions(-) diff --git a/ext/curl/interface.c b/ext/curl/interface.c index df8f07779af8e..59362adddb7bd 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -1518,7 +1518,6 @@ static bool php_curl_set_callable_handler(zend_fcall_info_cache *const handler_f { if (ZEND_FCC_INITIALIZED(*handler_fcc)) { zend_fcc_dtor(handler_fcc); - handler_fcc->function_handler = NULL; } char *error = NULL; @@ -2699,15 +2698,12 @@ static void curl_free_obj(zend_object *object) smart_str_free(&ch->handlers.write->buf); if (ZEND_FCC_INITIALIZED(ch->handlers.write->fcc)) { zend_fcc_dtor(&ch->handlers.write->fcc); - ch->handlers.write->fcc = empty_fcall_info_cache; } if (ZEND_FCC_INITIALIZED(ch->handlers.write_header->fcc)) { zend_fcc_dtor(&ch->handlers.write_header->fcc); - ch->handlers.write_header->fcc = empty_fcall_info_cache; } if (ZEND_FCC_INITIALIZED(ch->handlers.read->fcc)) { zend_fcc_dtor(&ch->handlers.read->fcc); - ch->handlers.read->fcc = empty_fcall_info_cache; } zval_ptr_dtor(&ch->handlers.std_err); if (ch->header.str) { @@ -2724,20 +2720,16 @@ static void curl_free_obj(zend_object *object) if (ZEND_FCC_INITIALIZED(ch->handlers.progress)) { zend_fcc_dtor(&ch->handlers.progress); - ch->handlers.progress = empty_fcall_info_cache; } if (ZEND_FCC_INITIALIZED(ch->handlers.xferinfo)) { zend_fcc_dtor(&ch->handlers.xferinfo); - ch->handlers.xferinfo = empty_fcall_info_cache; } if (ZEND_FCC_INITIALIZED(ch->handlers.fnmatch)) { zend_fcc_dtor(&ch->handlers.fnmatch); - ch->handlers.fnmatch = empty_fcall_info_cache; } #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ if (ZEND_FCC_INITIALIZED(ch->handlers.sshhostkey)) { zend_fcc_dtor(&ch->handlers.sshhostkey); - ch->handlers.sshhostkey = empty_fcall_info_cache; } #endif @@ -2804,23 +2796,19 @@ static void _php_curl_reset_handlers(php_curl *ch) if (ZEND_FCC_INITIALIZED(ch->handlers.progress)) { zend_fcc_dtor(&ch->handlers.progress); - ch->handlers.progress = empty_fcall_info_cache; } if (ZEND_FCC_INITIALIZED(ch->handlers.xferinfo)) { zend_fcc_dtor(&ch->handlers.xferinfo); - ch->handlers.xferinfo = empty_fcall_info_cache; } if (ZEND_FCC_INITIALIZED(ch->handlers.fnmatch)) { zend_fcc_dtor(&ch->handlers.fnmatch); - ch->handlers.fnmatch = empty_fcall_info_cache; } #if LIBCURL_VERSION_NUM >= 0x075400 /* Available since 7.84.0 */ if (ZEND_FCC_INITIALIZED(ch->handlers.sshhostkey)) { zend_fcc_dtor(&ch->handlers.sshhostkey); - ch->handlers.sshhostkey = empty_fcall_info_cache; } #endif } diff --git a/ext/curl/multi.c b/ext/curl/multi.c index 9492eb1e8bfbd..586f48911b261 100644 --- a/ext/curl/multi.c +++ b/ext/curl/multi.c @@ -446,7 +446,6 @@ static bool _php_curl_multi_setopt(php_curlm *mh, zend_long option, zval *zvalue /* See php_curl_set_callable_handler */ if (ZEND_FCC_INITIALIZED(mh->handlers.server_push)) { zend_fcc_dtor(&mh->handlers.server_push); - mh->handlers.server_push = empty_fcall_info_cache; } char *error_str = NULL; @@ -544,7 +543,6 @@ static void curl_multi_free_obj(zend_object *object) if (ZEND_FCC_INITIALIZED(mh->handlers.server_push)) { zend_fcc_dtor(&mh->handlers.server_push); - mh->handlers.server_push = empty_fcall_info_cache; } zend_object_std_dtor(&mh->std);