diff --git a/NEWS b/NEWS index 9c5f7bede786..73086c5b6a61 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,7 @@ PHP NEWS - Curl: . curl_multi_exec() now throws a TypeError when still_running is not an int. (arshidkv12) + . curl_write(): Check whether the PHP_CURL_USER callback returns an integer - FPM: . Use recommended TYPE, HELP order for OpenMetrics metadata. diff --git a/UPGRADING b/UPGRADING index d8d763986d0f..2f47c449a560 100644 --- a/UPGRADING +++ b/UPGRADING @@ -21,6 +21,8 @@ PHP 8.7 UPGRADE NOTES - Curl: . curl_multi_exec() now throws a TypeError when still_running is not an int. + . The CURLOPT_WRITEFUNCTION callback should return an integer. Returning other + types may trigger implicit conversion deprecations or conversion failures. - PDO: . PDOStatement::bindColumn() now throws a ValueError when the column name is diff --git a/ext/curl/curl_private.h b/ext/curl/curl_private.h index 25cc37b6e458..a1fb42ca9b53 100644 --- a/ext/curl/curl_private.h +++ b/ext/curl/curl_private.h @@ -150,7 +150,7 @@ void _php_curl_verify_handlers(php_curl *ch, bool reporterror); void _php_setup_easy_copy_handlers(php_curl *ch, php_curl *source); /* Consumes `zv` */ -zend_long php_curl_get_long(zval *zv); +zend_long php_curl_get_long(zval *zv, bool *failed); #define curl_from_obj(obj) ZEND_CONTAINER_OF(obj, php_curl, std) diff --git a/ext/curl/interface.c b/ext/curl/interface.c index d1f0cee3d67d..ed8fecf1cdd1 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -606,8 +606,8 @@ static size_t curl_write(char *data, size_t size, size_t nmemb, void *ctx) php_curl_call_callback(ch, &write_handler->fcc, &retval, /* argc */ 2, argv); if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); - /* TODO Check callback returns an int or something castable to int */ - length = php_curl_get_long(&retval); + bool failed; + length = php_curl_get_long(&retval, &failed); } else { length = -1; } @@ -643,8 +643,8 @@ static int curl_fnmatch(void *ctx, const char *pattern, const char *string) if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); - /* TODO Check callback returns an int or something castable to int */ - rval = php_curl_get_long(&retval); + bool failed; + rval = php_curl_get_long(&retval, &failed); } zval_ptr_dtor(&argv[0]); zval_ptr_dtor(&argv[1]); @@ -681,8 +681,9 @@ static int curl_progress(void *clientp, double dltotal, double dlnow, double ult if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); - /* TODO Check callback returns an int or something castable to int */ - if (0 == php_curl_get_long(&retval)) { + bool failed; + zend_long ret = php_curl_get_long(&retval, &failed); + if (0 == ret) { rval = 0; // ok } } @@ -720,8 +721,8 @@ static int curl_xferinfo(void *clientp, curl_off_t dltotal, curl_off_t dlnow, cu if (!Z_ISUNDEF(retval)) { _php_curl_verify_handlers(ch, /* reporterror */ true); - /* TODO Check callback returns an int or something castable to int */ - if (0 == php_curl_get_long(&retval)) { + bool failed; + if (0 == php_curl_get_long(&retval, &failed)) { rval = 0; // ok } } @@ -964,9 +965,9 @@ static size_t curl_write_header(char *data, size_t size, size_t nmemb, void *ctx php_curl_call_callback(ch, &write_handler->fcc, &retval, /* argc */ 2, argv); if (!Z_ISUNDEF(retval)) { - // TODO: Check for valid int type for return value _php_curl_verify_handlers(ch, /* reporterror */ true); - length = php_curl_get_long(&retval); + bool failed; + length = php_curl_get_long(&retval, &failed); } else { length = -1; } @@ -1334,12 +1335,12 @@ void _php_setup_easy_copy_handlers(php_curl *ch, php_curl *source) (*source->clone)++; } -zend_long php_curl_get_long(zval *zv) +zend_long php_curl_get_long(zval *zv, bool *failed) { if (EXPECTED(Z_TYPE_P(zv) == IS_LONG)) { return Z_LVAL_P(zv); } else { - zend_long ret = zval_get_long(zv); + zend_long ret = zval_try_get_long(zv, failed); zval_ptr_dtor(zv); return ret; } diff --git a/ext/curl/multi.c b/ext/curl/multi.c index c8b15ba58f1f..617508414cae 100644 --- a/ext/curl/multi.c +++ b/ext/curl/multi.c @@ -437,7 +437,9 @@ static int _php_server_push_callback(CURL *parent_ch, CURL *easy, size_t num_hea zval_ptr_dtor_nogc(&headers); if (!Z_ISUNDEF(retval)) { - if (CURL_PUSH_DENY != php_curl_get_long(&retval)) { + bool failed; + zend_long ret = php_curl_get_long(&retval, &failed); + if (!failed && CURL_PUSH_DENY != ret) { rval = CURL_PUSH_OK; zend_llist_add_element(&mh->easyh, &pz_ch); } else { diff --git a/ext/curl/tests/curl_write_callback_return.phpt b/ext/curl/tests/curl_write_callback_return.phpt new file mode 100644 index 000000000000..be87fc2c4dc8 --- /dev/null +++ b/ext/curl/tests/curl_write_callback_return.phpt @@ -0,0 +1,92 @@ +--TEST-- +curl_write(): edge cases for write callback return values +--EXTENSIONS-- +curl +--FILE-- + 'valid', + 'null' => null, + 'false' => false, + 'true' => true, + 'zero' => 0, + 'one' => 1, + 'negative one' => -1, + 'PHP_INT_MAX' => PHP_INT_MAX, + 'PHP_INT_MIN' => PHP_INT_MIN, + 'float' => 1.5, + 'negative float' => -1.5, + 'numeric string' => '25', + 'empty string' => '', + 'non-numeric string' => 'abc', + 'large numeric string' => '999999999999999999999999999999999999', + 'empty array' => [], + 'array' => [1], + 'object' => new stdClass(), +]; + +foreach ($values as $label => $value) { + echo "-- $label --\n"; + + $ch = curl_init($host); + curl_setopt($ch, CURLOPT_WRITEFUNCTION, + static function ($ch, $data) use ($value) { + return $value === 'valid' ? strlen($data) : $value; + } + ); + + try { + var_dump(curl_exec($ch)); + } catch (Throwable $e) { + echo get_class($e), "\n"; + } + +} +?> +--EXPECTF-- +-- valid length -- +bool(true) +-- null -- +bool(false) +-- false -- +bool(false) +-- true -- +bool(false) +-- zero -- +bool(false) +-- one -- +bool(false) +-- negative one -- +bool(false) +-- PHP_INT_MAX -- +bool(false) +-- PHP_INT_MIN -- +bool(false) +-- float -- + +Deprecated: Implicit conversion from float 1.5 to int loses precision in %s on line 38 +bool(false) +-- negative float -- + +Deprecated: Implicit conversion from float -1.5 to int loses precision in %s on line 38 +bool(false) +-- numeric string -- +bool(true) +-- empty string -- +bool(false) +-- non-numeric string -- +bool(false) +-- large numeric string -- + +Deprecated: Implicit conversion from float-string "999999999999999999999999999999999999" to int loses precision in %s on line 38 +bool(false) +-- empty array -- +bool(false) +-- array -- +bool(false) +-- object -- +bool(false)