Skip to content

Promote warnings to exceptions in ext/curl #5963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Promote warnings to exceptions in ext/curl #5963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
46 changes: 25 additions & 21 deletions ext/curl/interface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ static int php_curl_option_str(php_curl *ch, zend_long option, const char *str,
CURLcode error = CURLE_OK;

if (strlen(str) != len) {
php_error_docref(NULL, E_WARNING, "Curl option contains invalid characters (\\0)");
zend_type_error("%s(): cURL option cannot contain any null-bytes", get_active_function_name());
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe it would be worth to introduce a new function that adds the current function/method name to the beginning. It might be useful if we start adding OO interfaces.

return FAILURE;
}

Expand Down Expand Up @@ -2204,7 +2204,7 @@ PHP_FUNCTION(curl_copy_handle)
}
/* }}} */

static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue) /* {{{ */
static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue, bool is_array_config) /* {{{ */
{
CURLcode error = CURLE_OK;
zend_long lval;
Expand Down Expand Up @@ -2381,7 +2381,7 @@ static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue) /* {{{
break;
case CURLOPT_SAFE_UPLOAD:
if (!zend_is_true(zvalue)) {
php_error_docref(NULL, E_WARNING, "Disabling safe uploads is no longer supported");
zend_value_error("%s(): Disabling safe uploads is no longer supported", get_active_function_name());
return FAILURE;
}
break;
Expand Down Expand Up @@ -2557,7 +2557,7 @@ static int _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 {
php_error_docref(NULL, E_WARNING, "The provided file handle is not writable");
zend_value_error("%s(): The provided file handle must be writable", get_active_function_name());
return FAILURE;
}
break;
Expand All @@ -2575,7 +2575,7 @@ static int _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 {
php_error_docref(NULL, E_WARNING, "The provided file handle is not writable");
zend_value_error("%s(): The provided file handle must be writable", get_active_function_name());
return FAILURE;
}
break;
Expand Down Expand Up @@ -2604,7 +2604,7 @@ static int _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 {
php_error_docref(NULL, E_WARNING, "The provided file handle is not writable");
zend_value_error("%s(): The provided file handle must be writable", get_active_function_name());
return FAILURE;
}
/* break omitted intentionally */
Expand Down Expand Up @@ -2674,7 +2674,8 @@ static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue) /* {{{
break;
#endif
}
php_error_docref(NULL, E_WARNING, "You must pass an array with the %s argument", name);

zend_type_error("%s(): The %s option must have an array value", get_active_function_name(), name);
return FAILURE;
}

Expand Down Expand Up @@ -2850,6 +2851,14 @@ static int _php_curl_setopt(php_curl *ch, zend_long option, zval *zvalue) /* {{{
ch->handlers->fnmatch->method = PHP_CURL_USER;
break;

default:
if (is_array_config) {
zend_argument_value_error(2, "must contain only valid cURL options");
} else {
zend_argument_value_error(2, "is not a valid cURL option");
}
error = CURLE_UNKNOWN_OPTION;
break;
}

SAVE_CURL_ERROR(ch, error);
Expand All @@ -2876,12 +2885,7 @@ PHP_FUNCTION(curl_setopt)

ch = Z_CURL_P(zid);

if (options <= 0 && options != CURLOPT_SAFE_UPLOAD) {
php_error_docref(NULL, E_WARNING, "Invalid curl configuration option");
RETURN_FALSE;
}

if (_php_curl_setopt(ch, options, zvalue) == SUCCESS) {
if (_php_curl_setopt(ch, options, zvalue, 0) == SUCCESS) {
RETURN_TRUE;
} else {
RETURN_FALSE;
Expand All @@ -2906,12 +2910,12 @@ PHP_FUNCTION(curl_setopt_array)

ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(arr), option, string_key, entry) {
if (string_key) {
php_error_docref(NULL, E_WARNING,
"Array keys must be CURLOPT constants or equivalent integer values");
RETURN_FALSE;
zend_argument_value_error(2, "contains an invalid cURL option");
RETURN_THROWS();
}

ZVAL_DEREF(entry);
if (_php_curl_setopt(ch, (zend_long) option, entry) == FAILURE) {
if (_php_curl_setopt(ch, (zend_long) option, entry, 1) == FAILURE) {
RETURN_FALSE;
}
} ZEND_HASH_FOREACH_END();
Expand Down Expand Up @@ -3292,8 +3296,8 @@ PHP_FUNCTION(curl_close)
ch = Z_CURL_P(zid);

if (ch->in_callback) {
php_error_docref(NULL, E_WARNING, "Attempt to close cURL handle from a callback");
return;
zend_throw_error(NULL, "%s(): Attempt to close cURL handle from a callback", get_active_function_name());
RETURN_THROWS();
}
}
/* }}} */
Expand Down Expand Up @@ -3449,8 +3453,8 @@ PHP_FUNCTION(curl_reset)
ch = Z_CURL_P(zid);

if (ch->in_callback) {
php_error_docref(NULL, E_WARNING, "Attempt to reset cURL handle from a callback");
return;
zend_throw_error(NULL, "%s(): Attempt to reset cURL handle from a callback", get_active_function_name());
RETURN_THROWS();
}

curl_easy_reset(ch->cp);
Expand Down
2 changes: 1 addition & 1 deletion ext/curl/multi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ static int _php_curl_multi_setopt(php_curlm *mh, zend_long option, zval *zvalue,
break;
#endif
default:
php_error_docref(NULL, E_WARNING, "Invalid curl multi configuration option");
zend_argument_value_error(2, "is not a valid cURL multi option");
error = CURLM_UNKNOWN_OPTION;
break;
}
Expand Down
2 changes: 1 addition & 1 deletion ext/curl/share.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ static int _php_curl_share_setopt(php_curlsh *sh, zend_long option, zval *zvalue
break;

default:
php_error_docref(NULL, E_WARNING, "Invalid curl share configuration option");
zend_argument_value_error(2, "is not a valid cURL share option");
error = CURLSHE_BAD_OPTION;
break;
}
Expand Down
13 changes: 9 additions & 4 deletions ext/curl/tests/bug48207.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,21 @@ if(!empty($host)) {


$tempfile = tempnam(sys_get_temp_dir(), 'CURL_FILE_HANDLE');
$fp = fopen($tempfile, "r"); // Opening 'fubar' with the incorrect readonly flag

$ch = curl_init($url);
$fp = fopen($tempfile, "r"); // Opening 'fubar' with the incorrect readonly flag
curl_setopt($ch, CURLOPT_FILE, $fp);
try {
curl_setopt($ch, CURLOPT_FILE, $fp);
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}

curl_exec($ch);
curl_close($ch);
is_file($tempfile) and @unlink($tempfile);
isset($tempname) and is_file($tempname) and @unlink($tempname);
?>
--EXPECTF--
Warning: curl_setopt(): The provided file handle is not writable in %s on line %d
--EXPECT--
curl_setopt(): The provided file handle must be writable
Hello World!
Hello World!
13 changes: 9 additions & 4 deletions ext/curl/tests/bug68089.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,15 @@ include 'skipif.inc';
<?php
$url = "file:///etc/passwd\0http://google.com";
$ch = curl_init();
var_dump(curl_setopt($ch, CURLOPT_URL, $url));

try {
curl_setopt($ch, CURLOPT_URL, $url);
} catch (TypeError $exception) {
echo $exception->getMessage() . "\n";
}

?>
Done
--EXPECTF--
Warning: curl_setopt(): Curl option contains invalid characters (\0) in %s%ebug68089.php on line 4
bool(false)
--EXPECT--
curl_setopt(): cURL option cannot contain any null-bytes
Done
10 changes: 7 additions & 3 deletions ext/curl/tests/curl_file_upload.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,12 @@ var_dump($file->getPostFilename());
curl_setopt($ch, CURLOPT_POSTFIELDS, array("file" => $file));
var_dump(curl_exec($ch));

curl_setopt($ch, CURLOPT_SAFE_UPLOAD, 0);
try {
curl_setopt($ch, CURLOPT_SAFE_UPLOAD, 0);
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}

$params = array('file' => '@' . __DIR__ . '/curl_testdata1.txt');
curl_setopt($ch, CURLOPT_POSTFIELDS, $params);
var_dump(curl_exec($ch));
Expand All @@ -69,8 +74,7 @@ string(%d) "%s/curl_testdata1.txt"
string(%d) "curl_testdata1.txt|text/plain|6"
string(%d) "foo.txt"
string(%d) "foo.txt|application/octet-stream|6"

Warning: curl_setopt(): Disabling safe uploads is no longer supported in %s on line %d
curl_setopt(): Disabling safe uploads is no longer supported
string(0) ""
string(0) ""
string(%d) "array(1) {
Expand Down
8 changes: 7 additions & 1 deletion ext/curl/tests/curl_multi_errno_strerror_001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,19 @@ $errno = curl_multi_errno($mh);
echo $errno . PHP_EOL;
echo curl_multi_strerror($errno) . PHP_EOL;

@curl_multi_setopt($mh, -1, -1);
try {
curl_multi_setopt($mh, -1, -1);
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}

$errno = curl_multi_errno($mh);
echo $errno . PHP_EOL;
echo curl_multi_strerror($errno) . PHP_EOL;
?>
--EXPECT--
0
No error
curl_multi_setopt(): Argument #2 ($option) is not a valid cURL multi option
6
Unknown option
13 changes: 8 additions & 5 deletions ext/curl/tests/curl_multi_setopt_basic001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@ if (!extension_loaded("curl")) {

$mh = curl_multi_init();
var_dump(curl_multi_setopt($mh, CURLMOPT_PIPELINING, 0));
var_dump(curl_multi_setopt($mh, -1, 0));

try {
curl_multi_setopt($mh, -1, 0);
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}

?>
--EXPECTF--
--EXPECT--
bool(true)

Warning: curl_multi_setopt(): Invalid curl multi configuration option in %s on line %d
bool(false)
curl_multi_setopt(): Argument #2 ($option) is not a valid cURL multi option
11 changes: 7 additions & 4 deletions ext/curl/tests/curl_setopt_basic003.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ echo "*** curl_setopt() call with CURLOPT_HTTPHEADER\n";
$url = "{$host}/";
$ch = curl_init();

curl_setopt($ch, CURLOPT_HTTPHEADER, 1);
try {
curl_setopt($ch, CURLOPT_HTTPHEADER, 1);
} catch (TypeError $exception) {
echo $exception->getMessage() . "\n";
}

$curl_content = curl_exec($ch);
curl_close($ch);
Expand All @@ -36,9 +40,8 @@ curl_close($ch);

var_dump( $curl_content );
?>
--EXPECTF--
--EXPECT--
*** curl_setopt() call with CURLOPT_HTTPHEADER

Warning: curl_setopt(): You must pass an array with the CURLOPT_HTTPHEADER argument in %s on line %d
curl_setopt(): The CURLOPT_HTTPHEADER option must have an array value
bool(false)
bool(true)
19 changes: 15 additions & 4 deletions ext/curl/tests/curl_setopt_error.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,21 @@ try {
echo $e->getMessage(), "\n";
}

curl_setopt($ch, -10, 0);
try {
curl_setopt($ch, -10, 0);
} catch (ValueError $e) {
echo $e->getMessage(), "\n";
}

try {
curl_setopt($ch, 1000, 0);
} catch (ValueError $e) {
echo $e->getMessage(), "\n";
}

?>
--EXPECTF--
--EXPECT--
*** curl_setopt() call with incorrect parameters
curl_setopt(): Argument #2 ($option) must be of type int, string given

Warning: curl_setopt(): Invalid curl configuration option in %scurl_setopt_error.php on line %d
curl_setopt(): Argument #2 ($option) is not a valid cURL option
curl_setopt(): Argument #2 ($option) is not a valid cURL option
8 changes: 7 additions & 1 deletion ext/curl/tests/curl_share_errno_strerror_001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,19 @@ $errno = curl_share_errno($sh);
echo $errno . PHP_EOL;
echo curl_share_strerror($errno) . PHP_EOL;

@curl_share_setopt($sh, -1, -1);
try {
curl_share_setopt($sh, -1, -1);
} catch (ValueError $e) {
echo $e->getMessage(), "\n";
}

$errno = curl_share_errno($sh);
echo $errno . PHP_EOL;
echo curl_share_strerror($errno) . PHP_EOL;
?>
--EXPECT--
0
No error
curl_share_setopt(): Argument #2 ($option) is not a valid cURL share option
1
Unknown share option
13 changes: 8 additions & 5 deletions ext/curl/tests/curl_share_setopt_basic001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ if (!extension_loaded("curl")) {
$sh = curl_share_init();
var_dump(curl_share_setopt($sh, CURLSHOPT_SHARE, CURL_LOCK_DATA_COOKIE));
var_dump(curl_share_setopt($sh, CURLSHOPT_UNSHARE, CURL_LOCK_DATA_DNS));
var_dump(curl_share_setopt($sh, -1, 0));

try {
curl_share_setopt($sh, -1, 0);
} catch (ValueError $e) {
echo $e->getMessage(), "\n";
}

?>
--EXPECTF--
--EXPECT--
bool(true)
bool(true)

Warning: curl_share_setopt(): Invalid curl share configuration option in %s on line %d
bool(false)
curl_share_setopt(): Argument #2 ($option) is not a valid cURL share option