address first round of PR comments

- use hardcoded allow list for $share_options - use zval_try_get_long
php · Nov 27, 2024 · ee836f3 · ee836f3
1 parent 39ff28b
commit ee836f3
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 12 deletions.
diff --git a/ext/curl/share.c b/ext/curl/share.c
@@ -160,20 +160,39 @@ PHP_FUNCTION(curl_share_init_persistent)
 	ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(share_opts), entry) {
 		ZVAL_DEREF(entry);
 
-		zend_ulong option = zval_get_long_ex(entry, true);
-
-		if (option == CURL_LOCK_DATA_COOKIE) {
-			zend_throw_exception_ex(
-				NULL,
-				0,
-				"CURL_LOCK_DATA_COOKIE is not allowed with persistent curl share handles"
-			);
+		bool failed       = false;
+		zend_ulong option = zval_try_get_long(entry, &failed);
 
+		if (failed) {
+			zend_argument_type_error(1, "must contain only longs, %s given", zend_zval_value_name(entry));
 			goto error;
 		}
 
-		// Ensure that each additional option results in a unique persistent ID.
-		persistent_id += 1 << option;
+		switch (option) {
+		// Disallowed options
+		case CURL_LOCK_DATA_COOKIE:
+			zend_argument_value_error(1, "CURL_LOCK_DATA_COOKIE is not allowed");
+			goto error;
+
+		// Allowed options
+		case CURL_LOCK_DATA_DNS:
+			persistent_id |= 1 << 0;
+			break;
+		case CURL_LOCK_DATA_SSL_SESSION:
+			persistent_id |= 1 << 1;
+			break;
+		case CURL_LOCK_DATA_CONNECT:
+			persistent_id |= 1 << 2;
+			break;
+		case CURL_LOCK_DATA_PSL:
+			persistent_id |= 1 << 3;
+			break;
+
+		// Unknown options
+		default:
+			zend_argument_value_error(1, "must contain only CURL_LOCK_DATA_* constants");
+			goto error;
+		}
 	} ZEND_HASH_FOREACH_END();
 
 	zend_array_sort(Z_ARRVAL_P(share_opts), php_array_data_compare_unstable_i, 1);

diff --git a/ext/curl/tests/curl_persistent_share_003.phpt b/ext/curl/tests/curl_persistent_share_003.phpt
@@ -9,7 +9,7 @@ $sh = curl_share_init_persistent([CURL_LOCK_DATA_DNS, CURL_LOCK_DATA_CONNECT, 30
 
 ?>
 --EXPECTF--
-Fatal error: Uncaught Exception: Could not construct persistent cURL share handle: Unknown share option in %scurl_persistent_share_003.php:3
+Fatal error: Uncaught ValueError: curl_share_init_persistent(): Argument #1 ($share_options) must contain only CURL_LOCK_DATA_* constants in %scurl_persistent_share_003.php:3
 Stack trace:
 #0 %scurl_persistent_share_003.php(3): curl_share_init_persistent(Array)
 #1 {main}

diff --git a/ext/curl/tests/curl_persistent_share_004.phpt b/ext/curl/tests/curl_persistent_share_004.phpt
@@ -9,7 +9,7 @@ $sh = curl_share_init_persistent([CURL_LOCK_DATA_COOKIE]);
 
 ?>
 --EXPECTF--
-Fatal error: Uncaught Exception: CURL_LOCK_DATA_COOKIE is not allowed with persistent curl share handles in %scurl_persistent_share_004.php:3
+Fatal error: Uncaught ValueError: curl_share_init_persistent(): Argument #1 ($share_options) CURL_LOCK_DATA_COOKIE is not allowed in %scurl_persistent_share_004.php:3
 Stack trace:
 #0 %scurl_persistent_share_004.php(3): curl_share_init_persistent(Array)
 #1 {main}