-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test changes are WIP
@@ -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()); |
There was a problem hiding this comment.
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.
ext/curl/interface.c
Outdated
@@ -3292,7 +3301,7 @@ 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"); | |||
zend_throw_error(NULL, "curl_close(): Attempt to close cURL handle from a callback"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is writing cURL
ok?
8d56a60
to
b6e248a
Compare
b6e248a
to
24d6483
Compare
It should be checked if there is any other warnings in ext/curl that should be promoted