From cdf66ab079aff16db17d2f1a2604c1b0002cf96e Mon Sep 17 00:00:00 2001 From: Eric Norris Date: Sat, 17 Aug 2024 13:29:04 -0400 Subject: [PATCH 1/4] feat: enable persistent `CurlShareHandle` objects This commit introduces a new function, curl_share_init_persistent, that creates a php_curlsh struct that can live beyond a single PHP request. Persisting a curl share handle would allow PHP userspace to cache things like DNS lookups, or even entire connections, between multiple PHP requests, thus reducing work for subsequent requests. I created a new function instead of reusing the existing curl_share_init function since it also takes an array of curl_share_setopt options to set when the persistent handle doesn't yet exist. It is noteworthy that calling curl_share_setopt on the persistent handle would affect future requests using the handle; we could consider preventing this. It is also noteworthy that changing the persistent share options would not take effect if the persistent share already existed; changing the persistent share ID would be sufficient to resolve that. --- ext/curl/curl.stub.php | 3 ++ ext/curl/curl_arginfo.h | 9 ++++- ext/curl/curl_private.h | 8 +++- ext/curl/interface.c | 2 +- ext/curl/share.c | 86 ++++++++++++++++++++++++++++++++++++++--- 5 files changed, 99 insertions(+), 9 deletions(-) diff --git a/ext/curl/curl.stub.php b/ext/curl/curl.stub.php index 8a20231da562b..211ea77a09e20 100644 --- a/ext/curl/curl.stub.php +++ b/ext/curl/curl.stub.php @@ -3745,6 +3745,9 @@ function curl_share_errno(CurlShareHandle $share_handle): int {} /** @refcount 1 */ function curl_share_init(): CurlShareHandle {} +/** @refcount 1 */ +function curl_share_init_persistent(string $persistent_id, array $shares): CurlShareHandle|false {} + function curl_share_setopt(CurlShareHandle $share_handle, int $option, mixed $value): bool {} /** @refcount 1 */ diff --git a/ext/curl/curl_arginfo.h b/ext/curl/curl_arginfo.h index 664cda7d32a97..9b6525f5028cf 100644 --- a/ext/curl/curl_arginfo.h +++ b/ext/curl/curl_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: e2800e5ecc33f092576c7afcdb98d89825809669 */ + * Stub hash: 6a1268dfa31c8f9f13a499cfecfcb15abb34fb99 */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_curl_close, 0, 1, IS_VOID, 0) ZEND_ARG_OBJ_INFO(0, handle, CurlHandle, 0) @@ -129,6 +129,11 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_curl_share_init, 0, 0, CurlShareHandle, 0) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(arginfo_curl_share_init_persistent, 0, 2, CurlShareHandle, MAY_BE_FALSE) + ZEND_ARG_TYPE_INFO(0, persistent_id, IS_STRING, 0) + ZEND_ARG_TYPE_INFO(0, shares, IS_ARRAY, 0) +ZEND_END_ARG_INFO() + ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_curl_share_setopt, 0, 3, _IS_BOOL, 0) ZEND_ARG_OBJ_INFO(0, share_handle, CurlShareHandle, 0) ZEND_ARG_TYPE_INFO(0, option, IS_LONG, 0) @@ -174,6 +179,7 @@ ZEND_FUNCTION(curl_setopt); ZEND_FUNCTION(curl_share_close); ZEND_FUNCTION(curl_share_errno); ZEND_FUNCTION(curl_share_init); +ZEND_FUNCTION(curl_share_init_persistent); ZEND_FUNCTION(curl_share_setopt); ZEND_FUNCTION(curl_share_strerror); ZEND_FUNCTION(curl_strerror); @@ -212,6 +218,7 @@ static const zend_function_entry ext_functions[] = { ZEND_FE(curl_share_close, arginfo_curl_share_close) ZEND_FE(curl_share_errno, arginfo_curl_share_errno) ZEND_FE(curl_share_init, arginfo_curl_share_init) + ZEND_FE(curl_share_init_persistent, arginfo_curl_share_init_persistent) ZEND_FE(curl_share_setopt, arginfo_curl_share_setopt) ZEND_FE(curl_share_strerror, arginfo_curl_share_strerror) ZEND_FE(curl_strerror, arginfo_curl_strerror) diff --git a/ext/curl/curl_private.h b/ext/curl/curl_private.h index 8e8e9586db587..47b67e76e9271 100644 --- a/ext/curl/curl_private.h +++ b/ext/curl/curl_private.h @@ -125,10 +125,13 @@ typedef struct { } php_curlm; typedef struct _php_curlsh { - CURLSH *share; + CURLSH *share; + zend_bool is_persistent; + struct { int no; } err; + zend_object std; } php_curlsh; @@ -152,7 +155,8 @@ static inline php_curlsh *curl_share_from_obj(zend_object *obj) { #define Z_CURL_SHARE_P(zv) curl_share_from_obj(Z_OBJ_P(zv)) void curl_multi_register_handlers(void); -void curl_share_register_handlers(void); +void curl_share_register_handlers(int); +void curl_share_free_obj(zend_object *object); void curlfile_register_class(void); zend_result curl_cast_object(zend_object *obj, zval *result, int type); diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 16b7c6618bc6e..901f60d002f55 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -409,7 +409,7 @@ PHP_MINIT_FUNCTION(curl) curl_multi_register_handlers(); curl_share_ce = register_class_CurlShareHandle(); - curl_share_register_handlers(); + curl_share_register_handlers(module_number); curlfile_register_class(); return SUCCESS; diff --git a/ext/curl/share.c b/ext/curl/share.c index 406982d203275..39ac95b57540f 100644 --- a/ext/curl/share.c +++ b/ext/curl/share.c @@ -28,6 +28,69 @@ #define SAVE_CURLSH_ERROR(__handle, __err) (__handle)->err.no = (int) __err; +static int le_pcurlsh; + +/* {{{ Initialize a persistent curl share handle */ +PHP_FUNCTION(curl_share_init_persistent) +{ + php_curlsh *sh; + + zend_string *persistent_id = NULL; + zend_string *persistent_key = NULL; + + zend_resource *persisted; + + zval *arr, *entry; + + ZEND_PARSE_PARAMETERS_START(2, 2) + Z_PARAM_STR_EX(persistent_id, 1, 0) + Z_PARAM_ARRAY(arr) + ZEND_PARSE_PARAMETERS_END(); + + persistent_key = strpprintf(0, "curl_share_init_persistent:id=%s", ZSTR_VAL(persistent_id)); + + object_init_ex(return_value, curl_share_ce); + + sh = Z_CURL_SHARE_P(return_value); + + if ((persisted = zend_hash_find_ptr(&EG(persistent_list), persistent_key)) != NULL) { + if (persisted->type == le_pcurlsh) { + sh->share = persisted->ptr; + sh->is_persistent = 1; + + goto cleanup; + } + } + + sh->share = curl_share_init(); + + ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(arr), entry) { + ZVAL_DEREF(entry); + + CURLSHcode error = curl_share_setopt(sh->share, CURLSHOPT_SHARE, zval_get_long(entry)); + + if (error != CURLSHE_OK) { + php_error_docref(NULL, E_WARNING, "could not construct persistent curl share: %s", curl_share_strerror(error)); + goto error; + } + } ZEND_HASH_FOREACH_END(); + + zend_register_persistent_resource(ZSTR_VAL(persistent_key), ZSTR_LEN(persistent_key), sh->share, le_pcurlsh); + + sh->is_persistent = 1; + + cleanup: + zend_string_release(persistent_key); + return; + + error: + zend_string_release(persistent_key); + curl_share_free_obj(Z_OBJ_P(return_value)); + + RETURN_FALSE; +} +/* }}} */ + /* {{{ Initialize a share curl handle */ PHP_FUNCTION(curl_share_init) { @@ -42,6 +105,14 @@ PHP_FUNCTION(curl_share_init) } /* }}} */ +ZEND_RSRC_DTOR_FUNC(php_pcurlsh_dtor) +{ + if (res->ptr) { + curl_share_cleanup(res->ptr); + res->ptr = NULL; + } +} + /* {{{ Close a set of cURL handles */ PHP_FUNCTION(curl_share_close) { @@ -79,7 +150,7 @@ static bool _php_curl_share_setopt(php_curlsh *sh, zend_long option, zval *zvalu PHP_FUNCTION(curl_share_setopt) { zval *z_sh, *zvalue; - zend_long options; + zend_long options; php_curlsh *sh; ZEND_PARSE_PARAMETERS_START(3,3) @@ -101,8 +172,8 @@ PHP_FUNCTION(curl_share_setopt) /* {{{ Return an integer containing the last share curl error number */ PHP_FUNCTION(curl_share_errno) { - zval *z_sh; - php_curlsh *sh; + zval *z_sh; + php_curlsh *sh; ZEND_PARSE_PARAMETERS_START(1,1) Z_PARAM_OBJECT_OF_CLASS(z_sh, curl_share_ce) @@ -154,13 +225,16 @@ void curl_share_free_obj(zend_object *object) { php_curlsh *sh = curl_share_from_obj(object); - curl_share_cleanup(sh->share); + if (!sh->is_persistent) { + curl_share_cleanup(sh->share); + } + zend_object_std_dtor(&sh->std); } static zend_object_handlers curl_share_handlers; -void curl_share_register_handlers(void) { +void curl_share_register_handlers(int module_number) { curl_share_ce->create_object = curl_share_create_object; curl_share_ce->default_object_handlers = &curl_share_handlers; @@ -170,4 +244,6 @@ void curl_share_register_handlers(void) { curl_share_handlers.get_constructor = curl_share_get_constructor; curl_share_handlers.clone_obj = NULL; curl_share_handlers.compare = zend_objects_not_comparable; + + le_pcurlsh = zend_register_list_destructors_ex(NULL, php_pcurlsh_dtor, "Curl persistent shared handle", module_number); } From 4a19a9877db0afea5b286c2c577dd781cd011e9c Mon Sep 17 00:00:00 2001 From: Eric Norris Date: Fri, 30 Aug 2024 16:11:55 -0400 Subject: [PATCH 2/4] refactor: use module globals instead of persistent resources --- ext/curl/curl_private.h | 15 ++++++-- ext/curl/interface.c | 23 ++++++++++-- ext/curl/share.c | 80 +++++++++++++++++++---------------------- 3 files changed, 70 insertions(+), 48 deletions(-) diff --git a/ext/curl/curl_private.h b/ext/curl/curl_private.h index 47b67e76e9271..8daf6e3ba447b 100644 --- a/ext/curl/curl_private.h +++ b/ext/curl/curl_private.h @@ -40,9 +40,19 @@ #define SAVE_CURL_ERROR(__handle, __err) \ do { (__handle)->err.no = (int) __err; } while (0) +ZEND_BEGIN_MODULE_GLOBALS(curl) + HashTable persistent_share_handles; +ZEND_END_MODULE_GLOBALS(curl) + +ZEND_EXTERN_MODULE_GLOBALS(curl) + +#define CURL_G(v) ZEND_MODULE_GLOBALS_ACCESSOR(curl, v) + PHP_MINIT_FUNCTION(curl); PHP_MSHUTDOWN_FUNCTION(curl); PHP_MINFO_FUNCTION(curl); +PHP_GINIT_FUNCTION(curl); +PHP_GSHUTDOWN_FUNCTION(curl); typedef struct { zend_fcall_info_cache fcc; @@ -126,7 +136,7 @@ typedef struct { typedef struct _php_curlsh { CURLSH *share; - zend_bool is_persistent; + zend_bool persistent; struct { int no; @@ -155,7 +165,8 @@ static inline php_curlsh *curl_share_from_obj(zend_object *obj) { #define Z_CURL_SHARE_P(zv) curl_share_from_obj(Z_OBJ_P(zv)) void curl_multi_register_handlers(void); -void curl_share_register_handlers(int); +void curl_share_free_persistent(zval *data); +void curl_share_register_handlers(void); void curl_share_free_obj(zend_object *object); void curlfile_register_class(void); zend_result curl_cast_object(zend_object *obj, zval *result, int type); diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 901f60d002f55..d0a96d4a48f4d 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -59,6 +59,8 @@ #include "ext/standard/url.h" #include "curl_private.h" +ZEND_DECLARE_MODULE_GLOBALS(curl) + #ifdef __GNUC__ /* don't complain about deprecated CURLOPT_* we're exposing to PHP; we need to keep using those to avoid breaking PHP API compatibility */ @@ -215,7 +217,11 @@ zend_module_entry curl_module_entry = { NULL, PHP_MINFO(curl), PHP_CURL_VERSION, - STANDARD_MODULE_PROPERTIES + PHP_MODULE_GLOBALS(curl), + PHP_GINIT(curl), + PHP_GSHUTDOWN(curl), + NULL, + STANDARD_MODULE_PROPERTIES_EX }; /* }}} */ @@ -409,13 +415,26 @@ PHP_MINIT_FUNCTION(curl) curl_multi_register_handlers(); curl_share_ce = register_class_CurlShareHandle(); - curl_share_register_handlers(module_number); + curl_share_register_handlers(); curlfile_register_class(); return SUCCESS; } /* }}} */ +/* {{{ PHP_GINIT_FUNCTION */ +PHP_GINIT_FUNCTION(curl) +{ + zend_hash_init(&curl_globals->persistent_share_handles, 0, NULL, curl_share_free_persistent, true); + GC_MAKE_PERSISTENT_LOCAL(&curl_globals->persistent_share_handles); +} +/* }}} */ + +PHP_GSHUTDOWN_FUNCTION(curl) +{ + zend_hash_destroy(&curl_globals->persistent_share_handles); +} + /* CurlHandle class */ static zend_object *curl_create_object(zend_class_entry *class_type) { diff --git a/ext/curl/share.c b/ext/curl/share.c index 39ac95b57540f..260a8248d1c33 100644 --- a/ext/curl/share.c +++ b/ext/curl/share.c @@ -28,64 +28,66 @@ #define SAVE_CURLSH_ERROR(__handle, __err) (__handle)->err.no = (int) __err; -static int le_pcurlsh; +void curl_share_free_persistent(zval *data) +{ + CURLSH *handle = Z_PTR_P(data); + + if (!handle) { + return; + } + + curl_share_cleanup(handle); +} /* {{{ Initialize a persistent curl share handle */ PHP_FUNCTION(curl_share_init_persistent) { - php_curlsh *sh; + zend_string *id; - zend_string *persistent_id = NULL; - zend_string *persistent_key = NULL; + zval *persisted; - zend_resource *persisted; + php_curlsh *sh; zval *arr, *entry; + CURLSHcode error; ZEND_PARSE_PARAMETERS_START(2, 2) - Z_PARAM_STR_EX(persistent_id, 1, 0) + Z_PARAM_STR_EX(id, 1, 0) Z_PARAM_ARRAY(arr) ZEND_PARSE_PARAMETERS_END(); - persistent_key = strpprintf(0, "curl_share_init_persistent:id=%s", ZSTR_VAL(persistent_id)); - object_init_ex(return_value, curl_share_ce); - sh = Z_CURL_SHARE_P(return_value); - if ((persisted = zend_hash_find_ptr(&EG(persistent_list), persistent_key)) != NULL) { - if (persisted->type == le_pcurlsh) { - sh->share = persisted->ptr; - sh->is_persistent = 1; + if ((persisted = zend_hash_find(&CURL_G(persistent_share_handles), id)) != NULL) { + sh->share = Z_PTR_P(persisted); + sh->persistent = 1; + } else { + sh->share = curl_share_init(); - goto cleanup; - } - } - - sh->share = curl_share_init(); + ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(arr), entry) { + ZVAL_DEREF(entry); - ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(arr), entry) { - ZVAL_DEREF(entry); + error = curl_share_setopt(sh->share, CURLSHOPT_SHARE, zval_get_long(entry)); - CURLSHcode error = curl_share_setopt(sh->share, CURLSHOPT_SHARE, zval_get_long(entry)); + if (error != CURLSHE_OK) { + php_error_docref(NULL, E_WARNING, "could not construct persistent curl share: %s", curl_share_strerror(error)); - if (error != CURLSHE_OK) { - php_error_docref(NULL, E_WARNING, "could not construct persistent curl share: %s", curl_share_strerror(error)); - goto error; - } - } ZEND_HASH_FOREACH_END(); + goto error; + } + } ZEND_HASH_FOREACH_END(); - zend_register_persistent_resource(ZSTR_VAL(persistent_key), ZSTR_LEN(persistent_key), sh->share, le_pcurlsh); + // It's important not to mark this as persistent until *after* we've successfully set all of the share options, + // as otherwise the curl_share_free_obj in the error handler won't free the CURLSH. + sh->persistent = 1; - sh->is_persistent = 1; + zend_hash_str_add_new_ptr(&CURL_G(persistent_share_handles), ZSTR_VAL(id), ZSTR_LEN(id), sh->share); + } - cleanup: - zend_string_release(persistent_key); return; error: - zend_string_release(persistent_key); - curl_share_free_obj(Z_OBJ_P(return_value)); + zval_ptr_dtor(return_value); RETURN_FALSE; } @@ -105,14 +107,6 @@ PHP_FUNCTION(curl_share_init) } /* }}} */ -ZEND_RSRC_DTOR_FUNC(php_pcurlsh_dtor) -{ - if (res->ptr) { - curl_share_cleanup(res->ptr); - res->ptr = NULL; - } -} - /* {{{ Close a set of cURL handles */ PHP_FUNCTION(curl_share_close) { @@ -225,7 +219,7 @@ void curl_share_free_obj(zend_object *object) { php_curlsh *sh = curl_share_from_obj(object); - if (!sh->is_persistent) { + if (!sh->persistent) { curl_share_cleanup(sh->share); } @@ -234,7 +228,7 @@ void curl_share_free_obj(zend_object *object) static zend_object_handlers curl_share_handlers; -void curl_share_register_handlers(int module_number) { +void curl_share_register_handlers(void) { curl_share_ce->create_object = curl_share_create_object; curl_share_ce->default_object_handlers = &curl_share_handlers; @@ -244,6 +238,4 @@ void curl_share_register_handlers(int module_number) { curl_share_handlers.get_constructor = curl_share_get_constructor; curl_share_handlers.clone_obj = NULL; curl_share_handlers.compare = zend_objects_not_comparable; - - le_pcurlsh = zend_register_list_destructors_ex(NULL, php_pcurlsh_dtor, "Curl persistent shared handle", module_number); } From e52ca6eafb2f0fd5b1f559975e099c2ad069cd3c Mon Sep 17 00:00:00 2001 From: Eric Norris Date: Wed, 20 Nov 2024 17:17:21 -0500 Subject: [PATCH 3/4] refactor: use `curl_share_init` signature per RFC - The signature of curl_share_init now takes two optional parameters, $share_options and $persistent_id. - I've removed vim markers, and made other small edits per suggestions from the pull request. - curl_share_close now closes persistent share handles. The php_curlsh struct needs to keep track of the persistent ID string in order to remove it from the module global hash table. - curl_share_init throws an exception if the share options are invalid. This should be backwards compatible as share options are optional, so it can only throw once they are specified incorrectly. --- ext/curl/curl.stub.php | 5 +- ext/curl/curl_arginfo.h | 11 +--- ext/curl/curl_private.h | 4 +- ext/curl/interface.c | 12 ++-- ext/curl/share.c | 120 +++++++++++++++++++++++++--------------- 5 files changed, 85 insertions(+), 67 deletions(-) diff --git a/ext/curl/curl.stub.php b/ext/curl/curl.stub.php index 211ea77a09e20..b0279f4ad80d5 100644 --- a/ext/curl/curl.stub.php +++ b/ext/curl/curl.stub.php @@ -3743,10 +3743,7 @@ function curl_share_close(CurlShareHandle $share_handle): void {} function curl_share_errno(CurlShareHandle $share_handle): int {} /** @refcount 1 */ -function curl_share_init(): CurlShareHandle {} - -/** @refcount 1 */ -function curl_share_init_persistent(string $persistent_id, array $shares): CurlShareHandle|false {} +function curl_share_init(array $share_options = [], ?string $persistent_id = null): CurlShareHandle {} function curl_share_setopt(CurlShareHandle $share_handle, int $option, mixed $value): bool {} diff --git a/ext/curl/curl_arginfo.h b/ext/curl/curl_arginfo.h index 9b6525f5028cf..b0afffb4ead60 100644 --- a/ext/curl/curl_arginfo.h +++ b/ext/curl/curl_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 6a1268dfa31c8f9f13a499cfecfcb15abb34fb99 */ + * Stub hash: 852d02349e947838d46b4a0599c6e64967cdf815 */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_curl_close, 0, 1, IS_VOID, 0) ZEND_ARG_OBJ_INFO(0, handle, CurlHandle, 0) @@ -127,11 +127,8 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_curl_share_errno, 0, 1, IS_LONG, ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_curl_share_init, 0, 0, CurlShareHandle, 0) -ZEND_END_ARG_INFO() - -ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(arginfo_curl_share_init_persistent, 0, 2, CurlShareHandle, MAY_BE_FALSE) - ZEND_ARG_TYPE_INFO(0, persistent_id, IS_STRING, 0) - ZEND_ARG_TYPE_INFO(0, shares, IS_ARRAY, 0) + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, share_options, IS_ARRAY, 0, "[]") + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, persistent_id, IS_STRING, 1, "null") ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_curl_share_setopt, 0, 3, _IS_BOOL, 0) @@ -179,7 +176,6 @@ ZEND_FUNCTION(curl_setopt); ZEND_FUNCTION(curl_share_close); ZEND_FUNCTION(curl_share_errno); ZEND_FUNCTION(curl_share_init); -ZEND_FUNCTION(curl_share_init_persistent); ZEND_FUNCTION(curl_share_setopt); ZEND_FUNCTION(curl_share_strerror); ZEND_FUNCTION(curl_strerror); @@ -218,7 +214,6 @@ static const zend_function_entry ext_functions[] = { ZEND_FE(curl_share_close, arginfo_curl_share_close) ZEND_FE(curl_share_errno, arginfo_curl_share_errno) ZEND_FE(curl_share_init, arginfo_curl_share_init) - ZEND_FE(curl_share_init_persistent, arginfo_curl_share_init_persistent) ZEND_FE(curl_share_setopt, arginfo_curl_share_setopt) ZEND_FE(curl_share_strerror, arginfo_curl_share_strerror) ZEND_FE(curl_strerror, arginfo_curl_strerror) diff --git a/ext/curl/curl_private.h b/ext/curl/curl_private.h index 8daf6e3ba447b..987ad971dc2fa 100644 --- a/ext/curl/curl_private.h +++ b/ext/curl/curl_private.h @@ -135,8 +135,8 @@ typedef struct { } php_curlm; typedef struct _php_curlsh { - CURLSH *share; - zend_bool persistent; + CURLSH *share; + zend_string *persistent_id; struct { int no; diff --git a/ext/curl/interface.c b/ext/curl/interface.c index d0a96d4a48f4d..b41a459b83088 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -217,10 +217,10 @@ zend_module_entry curl_module_entry = { NULL, PHP_MINFO(curl), PHP_CURL_VERSION, - PHP_MODULE_GLOBALS(curl), - PHP_GINIT(curl), + PHP_MODULE_GLOBALS(curl), + PHP_GINIT(curl), PHP_GSHUTDOWN(curl), - NULL, + NULL, STANDARD_MODULE_PROPERTIES_EX }; /* }}} */ @@ -422,13 +422,11 @@ PHP_MINIT_FUNCTION(curl) } /* }}} */ -/* {{{ PHP_GINIT_FUNCTION */ PHP_GINIT_FUNCTION(curl) { zend_hash_init(&curl_globals->persistent_share_handles, 0, NULL, curl_share_free_persistent, true); GC_MAKE_PERSISTENT_LOCAL(&curl_globals->persistent_share_handles); } -/* }}} */ PHP_GSHUTDOWN_FUNCTION(curl) { @@ -747,8 +745,8 @@ static int curl_prereqfunction(void *clientp, char *conn_primary_ip, char *conn_ // gets called. Return CURL_PREREQFUNC_OK immediately in this case to avoid // zend_call_known_fcc() with an uninitialized FCC. if (!ZEND_FCC_INITIALIZED(ch->handlers.prereq)) { - return rval; - } + return rval; + } #if PHP_CURL_DEBUG fprintf(stderr, "curl_prereqfunction() called\n"); diff --git a/ext/curl/share.c b/ext/curl/share.c index 260a8248d1c33..0a05c44dc4287 100644 --- a/ext/curl/share.c +++ b/ext/curl/share.c @@ -21,6 +21,7 @@ #endif #include "php.h" +#include "Zend/zend_exceptions.h" #include "curl_private.h" @@ -28,6 +29,9 @@ #define SAVE_CURLSH_ERROR(__handle, __err) (__handle)->err.no = (int) __err; +/** + * Free a persistent curl share handle. + */ void curl_share_free_persistent(zval *data) { CURLSH *handle = Z_PTR_P(data); @@ -39,49 +43,74 @@ void curl_share_free_persistent(zval *data) curl_share_cleanup(handle); } -/* {{{ Initialize a persistent curl share handle */ -PHP_FUNCTION(curl_share_init_persistent) +/** + * Initialize a share curl handle, optionally with share options and a persistent ID. + */ +PHP_FUNCTION(curl_share_init) { - zend_string *id; - - zval *persisted; + zval *share_opts = NULL, *entry = NULL; + zend_string *persistent_id = NULL; php_curlsh *sh; - zval *arr, *entry; CURLSHcode error; - ZEND_PARSE_PARAMETERS_START(2, 2) - Z_PARAM_STR_EX(id, 1, 0) - Z_PARAM_ARRAY(arr) + ZEND_PARSE_PARAMETERS_START(0, 2) + Z_PARAM_OPTIONAL + Z_PARAM_ARRAY(share_opts) + Z_PARAM_STR_OR_NULL(persistent_id) ZEND_PARSE_PARAMETERS_END(); object_init_ex(return_value, curl_share_ce); sh = Z_CURL_SHARE_P(return_value); - if ((persisted = zend_hash_find(&CURL_G(persistent_share_handles), id)) != NULL) { - sh->share = Z_PTR_P(persisted); - sh->persistent = 1; - } else { - sh->share = curl_share_init(); + if (persistent_id) { + zval *persisted = zend_hash_find(&CURL_G(persistent_share_handles), persistent_id); + + if (persisted) { + sh->share = Z_PTR_P(persisted); + sh->persistent_id = zend_string_copy(persistent_id); + + return; + } + } + + // The user did not provide a persistent share ID, or we could not find an existing share handle, so we'll have to + // create one. + sh->share = curl_share_init(); - ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(arr), entry) { + if (share_opts) { + ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(share_opts), entry) { ZVAL_DEREF(entry); - error = curl_share_setopt(sh->share, CURLSHOPT_SHARE, zval_get_long(entry)); + error = curl_share_setopt(sh->share, CURLSHOPT_SHARE, zval_get_long_ex(entry, true)); if (error != CURLSHE_OK) { - php_error_docref(NULL, E_WARNING, "could not construct persistent curl share: %s", curl_share_strerror(error)); + zend_throw_exception_ex( + NULL, + 0, + "Could not construct cURL share handle: %s", + curl_share_strerror(error) + ); goto error; } } ZEND_HASH_FOREACH_END(); + } + if (persistent_id) { // It's important not to mark this as persistent until *after* we've successfully set all of the share options, // as otherwise the curl_share_free_obj in the error handler won't free the CURLSH. - sh->persistent = 1; - - zend_hash_str_add_new_ptr(&CURL_G(persistent_share_handles), ZSTR_VAL(id), ZSTR_LEN(id), sh->share); + sh->persistent_id = zend_string_copy(persistent_id); + + // We use the zend_hash_str form here, since we want to ensure it allocates a new persistent string, instead of + // using the request-bound persistent_id. + zend_hash_str_add_new_ptr( + &CURL_G(persistent_share_handles), + ZSTR_VAL(persistent_id), + ZSTR_LEN(persistent_id), + sh->share + ); } return; @@ -89,34 +118,29 @@ PHP_FUNCTION(curl_share_init_persistent) error: zval_ptr_dtor(return_value); - RETURN_FALSE; + RETURN_THROWS(); } -/* }}} */ - -/* {{{ Initialize a share curl handle */ -PHP_FUNCTION(curl_share_init) -{ - php_curlsh *sh; - - ZEND_PARSE_PARAMETERS_NONE(); - object_init_ex(return_value, curl_share_ce); - sh = Z_CURL_SHARE_P(return_value); - - sh->share = curl_share_init(); -} -/* }}} */ - -/* {{{ Close a set of cURL handles */ +/** + * Close a persistent curl share handle. NOP for non-persistent share handles. + */ PHP_FUNCTION(curl_share_close) { zval *z_sh; + php_curlsh *sh; + ZEND_PARSE_PARAMETERS_START(1,1) Z_PARAM_OBJECT_OF_CLASS(z_sh, curl_share_ce) ZEND_PARSE_PARAMETERS_END(); + + sh = Z_CURL_SHARE_P(z_sh); + + if (sh->persistent_id) { + // Deleting the share from the hash table will trigger curl_share_free_persistent, so no need to call it here. + zend_hash_del(&CURL_G(persistent_share_handles), sh->persistent_id); + } } -/* }}} */ static bool _php_curl_share_setopt(php_curlsh *sh, zend_long option, zval *zvalue, zval *return_value) /* {{{ */ { @@ -138,9 +162,10 @@ static bool _php_curl_share_setopt(php_curlsh *sh, zend_long option, zval *zvalu return error == CURLSHE_OK; } -/* }}} */ -/* {{{ Set an option for a cURL transfer */ +/** + * Set an option for a cURL transfer. + */ PHP_FUNCTION(curl_share_setopt) { zval *z_sh, *zvalue; @@ -161,9 +186,10 @@ PHP_FUNCTION(curl_share_setopt) RETURN_FALSE; } } -/* }}} */ -/* {{{ Return an integer containing the last share curl error number */ +/** + * Return an integer containing the last share curl error number. + */ PHP_FUNCTION(curl_share_errno) { zval *z_sh; @@ -177,10 +203,11 @@ PHP_FUNCTION(curl_share_errno) RETURN_LONG(sh->err.no); } -/* }}} */ -/* {{{ return string describing error code */ +/** + * Return a string describing the error code. + */ PHP_FUNCTION(curl_share_strerror) { zend_long code; @@ -197,7 +224,6 @@ PHP_FUNCTION(curl_share_strerror) RETURN_NULL(); } } -/* }}} */ /* CurlShareHandle class */ @@ -219,8 +245,10 @@ void curl_share_free_obj(zend_object *object) { php_curlsh *sh = curl_share_from_obj(object); - if (!sh->persistent) { + if (!sh->persistent_id) { curl_share_cleanup(sh->share); + } else { + zend_string_release(sh->persistent_id); } zend_object_std_dtor(&sh->std); From 4de6a1d8f218f56aeb611b993e100d098e0469cb Mon Sep 17 00:00:00 2001 From: Eric Norris Date: Thu, 21 Nov 2024 19:27:59 -0500 Subject: [PATCH 4/4] test: persistent curl share handles I opted to use the existing Caddy testing infrastructure since it supports keepalives, whereas it seems the PHP development server does not. Alternatively I could write just enough of a socket listener to confirm that there is only ever a single connection coming from curl, but I believe it is safe to rely on connect_time being zero when a connection is reused. --- ext/curl/tests/curl_share_persistent.phpt | 44 +++++++++++++++++++++++ ext/curl/tests/skipif-nocaddy.inc | 1 + 2 files changed, 45 insertions(+) create mode 100644 ext/curl/tests/curl_share_persistent.phpt diff --git a/ext/curl/tests/curl_share_persistent.phpt b/ext/curl/tests/curl_share_persistent.phpt new file mode 100644 index 0000000000000..cdf41fe739d81 --- /dev/null +++ b/ext/curl/tests/curl_share_persistent.phpt @@ -0,0 +1,44 @@ +--TEST-- +Basic curl_share test +--EXTENSIONS-- +curl +--SKIPIF-- + +--FILE-- + +--EXPECT-- +string(23) "Caddy is up and running" +string(23) "Caddy is up and running" +string(22) "second connect_time: 0" diff --git a/ext/curl/tests/skipif-nocaddy.inc b/ext/curl/tests/skipif-nocaddy.inc index 21a06c12af106..ae5442ff28ede 100644 --- a/ext/curl/tests/skipif-nocaddy.inc +++ b/ext/curl/tests/skipif-nocaddy.inc @@ -2,6 +2,7 @@ $ch = curl_init("https://localhost"); curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); +curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false); $body = curl_exec($ch);