Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ PHP NEWS
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
?? ??? ????, PHP 8.6.0beta1

- Curl:
. Improved cURL option validation errors to include the option name.
(Sjoerd Langkemper)

- GMP:
. Added optional $definitely_prime output parameter to gmp_prevprime().
(Weilin Du)
Expand Down
75 changes: 35 additions & 40 deletions ext/curl/interface.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,39 @@ ZEND_DECLARE_MODULE_GLOBALS(curl)
# define php_curl_ret(__ret) RETVAL_FALSE; return;
#endif

// php_curl_option_get_name(CURLOPT_HTTPHEADER) -> "HTTPHEADER"
static const char * php_curl_option_get_name(zend_long option) {

#if LIBCURL_VERSION_NUM >= 0x074900
const struct curl_easyoption * opt = curl_easy_option_by_id(option);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there’s a concrete case for the second fallback you mentioned. libcurl can be built with --disable-get-easy-options, where curl_easy_option_by_id() is still exported but always returns NULL. These errors would then say CURLOPT_UNKNOWN_OPTION, even though PHP’s constant table has the name.

Should the Zend scan run whenever the libcurl lookup returns NULL?

#if LIBCURL_VERSION_NUM >= 0x074900
	const struct curl_easyoption *opt = curl_easy_option_by_id(option);
	if (EXPECTED(opt != NULL)) {
		return opt->name;
	}
#endif

/* existing Zend constant scan */

if (EXPECTED(opt != NULL)) {
return opt->name;
}
#endif

const char prefix[] = "CURLOPT_";
const size_t prefix_len = sizeof(prefix) - 1;
zend_string *key;
zend_constant *constant;

ZEND_HASH_FOREACH_STR_KEY_PTR(EG(zend_constants), key, constant) {
if (!key
|| Z_TYPE(constant->value) != IS_LONG
|| strncmp(ZSTR_VAL(key), prefix, prefix_len) != 0) {
continue;
}

if (Z_LVAL(constant->value) == option) {
return ZSTR_VAL(key) + prefix_len;
}
} ZEND_HASH_FOREACH_END();
return "UNKNOWN_OPTION";
}

static zend_result php_curl_option_str(php_curl *ch, zend_long option, const char *str, const size_t len)
{
if (zend_char_has_nul_byte(str, len)) {
zend_value_error("%s(): cURL option must not contain any null bytes", get_active_function_name());
zend_value_error("%s(): cURL option CURLOPT_%s must not contain any null bytes", get_active_function_name(), php_curl_option_get_name(option));
return FAILURE;
}

Expand Down Expand Up @@ -2011,7 +2040,7 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue
ch->handlers.write->method = PHP_CURL_FILE;
ZVAL_COPY(&ch->handlers.write->stream, zvalue);
} else {
zend_value_error("%s(): The provided file handle must be writable", get_active_function_name());
zend_value_error("%s(): The file handle provided for CURLOPT_FILE must be writable", get_active_function_name());
return FAILURE;
}
break;
Expand All @@ -2029,7 +2058,7 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue
ch->handlers.write_header->method = PHP_CURL_FILE;
ZVAL_COPY(&ch->handlers.write_header->stream, zvalue);
} else {
zend_value_error("%s(): The provided file handle must be writable", get_active_function_name());
zend_value_error("%s(): The file handle provided for CURLOPT_WRITEHEADER must be writable", get_active_function_name());
return FAILURE;
}
break;
Expand Down Expand Up @@ -2058,7 +2087,7 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue
zval_ptr_dtor(&ch->handlers.std_err);
ZVAL_COPY(&ch->handlers.std_err, zvalue);
} else {
zend_value_error("%s(): The provided file handle must be writable", get_active_function_name());
zend_value_error("%s(): The file handle provided for CURLOPT_STDERR must be writable", get_active_function_name());
return FAILURE;
}
ZEND_FALLTHROUGH;
Expand All @@ -2085,43 +2114,9 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue
HashTable *ph;
zend_string *val, *tmp_val;
struct curl_slist *slist = NULL;
const char *name = NULL;

switch (option) {
case CURLOPT_HTTPHEADER:
name = "CURLOPT_HTTPHEADER";
break;
case CURLOPT_QUOTE:
name = "CURLOPT_QUOTE";
break;
case CURLOPT_HTTP200ALIASES:
name = "CURLOPT_HTTP200ALIASES";
break;
case CURLOPT_POSTQUOTE:
name = "CURLOPT_POSTQUOTE";
break;
case CURLOPT_PREQUOTE:
name = "CURLOPT_PREQUOTE";
break;
case CURLOPT_TELNETOPTIONS:
name = "CURLOPT_TELNETOPTIONS";
break;
case CURLOPT_MAIL_RCPT:
name = "CURLOPT_MAIL_RCPT";
break;
case CURLOPT_RESOLVE:
name = "CURLOPT_RESOLVE";
break;
case CURLOPT_PROXYHEADER:
name = "CURLOPT_PROXYHEADER";
break;
case CURLOPT_CONNECT_TO:
name = "CURLOPT_CONNECT_TO";
break;
}

if (Z_TYPE_P(zvalue) != IS_ARRAY) {
zend_type_error("%s(): The %s option must have an array value", get_active_function_name(), name);
zend_type_error("%s(): The CURLOPT_%s option must have an array value", get_active_function_name(), php_curl_option_get_name(option));
return FAILURE;
}

Expand All @@ -2133,7 +2128,7 @@ static zend_result _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue
if (zend_str_has_nul_byte(val)) {
curl_slist_free_all(slist);
zend_tmp_string_release(tmp_val);
zend_value_error("%s(): cURL option %s must not contain any null bytes", get_active_function_name(), name);
zend_value_error("%s(): cURL option CURLOPT_%s must not contain any null bytes", get_active_function_name(), php_curl_option_get_name(option));
return FAILURE;
}

Expand Down
19 changes: 14 additions & 5 deletions ext/curl/tests/bug48207.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,26 @@ $tempfile = tempnam(sys_get_temp_dir(), 'CURL_FILE_HANDLE');
$fp = fopen($tempfile, "r"); // Opening 'fubar' with the incorrect readonly flag

$ch = curl_init($url);
try {
curl_setopt($ch, CURLOPT_FILE, $fp);
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";

foreach ([
CURLOPT_FILE,
CURLOPT_WRITEHEADER,
CURLOPT_STDERR,
] as $option) {
try {
curl_setopt($ch, $option, $fp);
} catch (ValueError $exception) {
echo $exception->getMessage(), "\n";
}
}

curl_exec($ch);
is_file($tempfile) and @unlink($tempfile);
isset($tempname) and is_file($tempname) and @unlink($tempname);
?>
--EXPECT--
curl_setopt(): The provided file handle must be writable
curl_setopt(): The file handle provided for CURLOPT_FILE must be writable

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We change the CURLOPT_WRITEHEADER and CURLOPT_STDERR messages separately too, but this test only reaches CURLOPT_FILE. Should it loop over all three using the same read-only stream, so each option name is pinned?

foreach ([
    CURLOPT_FILE,
    CURLOPT_WRITEHEADER,
    CURLOPT_STDERR,
] as $option) {
    try {
        curl_setopt($ch, $option, $fp);
    } catch (ValueError $exception) {
        echo $exception->getMessage(), "\n";
    }
}

curl_setopt(): The file handle provided for CURLOPT_WRITEHEADER must be writable
curl_setopt(): The file handle provided for CURLOPT_STDERR must be writable
Hello World!
Hello World!
2 changes: 1 addition & 1 deletion ext/curl/tests/bug68089.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ try {
?>
Done
--EXPECT--
curl_setopt(): cURL option must not contain any null bytes
curl_setopt(): cURL option CURLOPT_URL must not contain any null bytes
Done
Loading