refactor: use curl_share_init signature per RFC

Author: ericnorris

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 {}
 

ext/curl/curl_arginfo.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;

ext/curl/curl_private.h

@@ -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");

ext/curl/interface.c

@@ -21,13 +21,17 @@
 #endif
 
 #include "php.h"
+#include "Zend/zend_exceptions.h"
 
 #include "curl_private.h"
 
 #include <curl/curl.h>
 
 #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,84 +43,104 @@ 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;
+	zend_string *persistent_id;
 
 	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;
 
  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);