From b0c9bebe024d6f17becf4e2313f7d637e1447918 Mon Sep 17 00:00:00 2001 From: Sjoerd Langkemper Date: Wed, 15 Jul 2026 14:32:50 +0000 Subject: [PATCH 1/3] Abort curl transfer if callback throws exception Solves bug https://github.com/php/php-src/issues/16513 Also includes https://github.com/php/php-src/pull/16790 --- ext/curl/interface.c | 22 +++++++---- .../curl_headerfunction_throws_abort.phpt | 35 +++++++++++++++++ .../curl_prereqfunction_throws_abort.phpt | 35 +++++++++++++++++ .../curl_progressfunction_throws_abort.phpt | 36 ++++++++++++++++++ .../tests/curl_readfunction_throws_abort.phpt | 38 +++++++++++++++++++ .../curl_writefunction_throws_abort.phpt | 35 +++++++++++++++++ .../curl_xferinfofunction_throws_abort.phpt | 36 ++++++++++++++++++ 7 files changed, 229 insertions(+), 8 deletions(-) create mode 100644 ext/curl/tests/curl_headerfunction_throws_abort.phpt create mode 100644 ext/curl/tests/curl_prereqfunction_throws_abort.phpt create mode 100644 ext/curl/tests/curl_progressfunction_throws_abort.phpt create mode 100644 ext/curl/tests/curl_readfunction_throws_abort.phpt create mode 100644 ext/curl/tests/curl_writefunction_throws_abort.phpt create mode 100644 ext/curl/tests/curl_xferinfofunction_throws_abort.phpt diff --git a/ext/curl/interface.c b/ext/curl/interface.c index d5d20d825652..9d0bb46e8896 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -554,6 +554,8 @@ static size_t curl_write(char *data, size_t size, size_t nmemb, void *ctx) _php_curl_verify_handlers(ch, /* reporterror */ true); /* TODO Check callback returns an int or something castable to int */ length = php_curl_get_long(&retval); + } else if (EG(exception)) { + length = -1; } zval_ptr_dtor(&argv[0]); @@ -603,7 +605,7 @@ static int curl_fnmatch(void *ctx, const char *pattern, const char *string) static int curl_progress(void *clientp, double dltotal, double dlnow, double ultotal, double ulnow) { php_curl *ch = (php_curl *)clientp; - int rval = 0; + int rval = 1; #if PHP_CURL_DEBUG fprintf(stderr, "curl_progress() called\n"); @@ -630,8 +632,8 @@ 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)) { - rval = 1; + if (0 == php_curl_get_long(&retval)) { + rval = 0; } } @@ -644,7 +646,7 @@ static int curl_progress(void *clientp, double dltotal, double dlnow, double ult static int curl_xferinfo(void *clientp, curl_off_t dltotal, curl_off_t dlnow, curl_off_t ultotal, curl_off_t ulnow) { php_curl *ch = (php_curl *)clientp; - int rval = 0; + int rval = 1; #if PHP_CURL_DEBUG fprintf(stderr, "curl_xferinfo() called\n"); @@ -671,8 +673,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)) { - rval = 1; + if (0 == php_curl_get_long(&retval)) { + rval = 0; } } @@ -685,13 +687,13 @@ static int curl_xferinfo(void *clientp, curl_off_t dltotal, curl_off_t dlnow, cu static int curl_prereqfunction(void *clientp, char *conn_primary_ip, char *conn_local_ip, int conn_primary_port, int conn_local_port) { php_curl *ch = (php_curl *)clientp; - int rval = CURL_PREREQFUNC_OK; + int rval = CURL_PREREQFUNC_ABORT; // when CURLOPT_PREREQFUNCTION is set to null, curl_prereqfunction still // gets called. Return CURL_PREREQFUNC_OK immediately in this case to avoid // zend_call_known_fcc() with an uninitialized FCC. if (!ZEND_FCC_INITIALIZED(ch->handlers.prereq)) { - return rval; + return CURL_PREREQFUNC_OK; } #if PHP_CURL_DEBUG @@ -822,6 +824,8 @@ static size_t curl_read(char *data, size_t size, size_t nmemb, void *ctx) } // TODO Do type error if invalid type? zval_ptr_dtor(&retval); + } else if (EG(exception)) { + length = CURL_READFUNC_ABORT; } zval_ptr_dtor(&argv[0]); @@ -916,6 +920,8 @@ static size_t curl_write_header(char *data, size_t size, size_t nmemb, void *ctx // TODO: Check for valid int type for return value _php_curl_verify_handlers(ch, /* reporterror */ true); length = php_curl_get_long(&retval); + } else if (EG(exception)) { + length = -1; } zval_ptr_dtor(&argv[0]); zval_ptr_dtor(&argv[1]); diff --git a/ext/curl/tests/curl_headerfunction_throws_abort.phpt b/ext/curl/tests/curl_headerfunction_throws_abort.phpt new file mode 100644 index 000000000000..648784403971 --- /dev/null +++ b/ext/curl/tests/curl_headerfunction_throws_abort.phpt @@ -0,0 +1,35 @@ +--TEST-- +CURLOPT_HEADERFUNCTION aborts transfer when callback throws +--EXTENSIONS-- +curl +--SKIPIF-- + +--FILE-- +getMessage(), "\n"; +} + +var_dump(curl_errno($ch) === CURLE_WRITE_ERROR); + +?> +--EXPECTF-- +header exception +bool(true) diff --git a/ext/curl/tests/curl_prereqfunction_throws_abort.phpt b/ext/curl/tests/curl_prereqfunction_throws_abort.phpt new file mode 100644 index 000000000000..7e8ccbf94f53 --- /dev/null +++ b/ext/curl/tests/curl_prereqfunction_throws_abort.phpt @@ -0,0 +1,35 @@ +--TEST-- +CURLOPT_PREREQFUNCTION aborts transfer when callback throws +--EXTENSIONS-- +curl +--SKIPIF-- + +--FILE-- +getMessage(), "\n"; +} + +var_dump(curl_errno($ch) === CURLE_ABORTED_BY_CALLBACK); + +?> +--EXPECTF-- +prereq exception +bool(true) diff --git a/ext/curl/tests/curl_progressfunction_throws_abort.phpt b/ext/curl/tests/curl_progressfunction_throws_abort.phpt new file mode 100644 index 000000000000..f03d7b96d90a --- /dev/null +++ b/ext/curl/tests/curl_progressfunction_throws_abort.phpt @@ -0,0 +1,36 @@ +--TEST-- +CURLOPT_PROGRESSFUNCTION aborts transfer when callback throws +--EXTENSIONS-- +curl +--SKIPIF-- + +--FILE-- +getMessage(), "\n"; +} + +var_dump(curl_errno($ch) === CURLE_ABORTED_BY_CALLBACK); + +?> +--EXPECTF-- +info exception +bool(true) diff --git a/ext/curl/tests/curl_readfunction_throws_abort.phpt b/ext/curl/tests/curl_readfunction_throws_abort.phpt new file mode 100644 index 000000000000..fdb83dbb421d --- /dev/null +++ b/ext/curl/tests/curl_readfunction_throws_abort.phpt @@ -0,0 +1,38 @@ +--TEST-- +CURLOPT_READFUNCTION aborts transfer when callback throws +--EXTENSIONS-- +curl +--SKIPIF-- + +--FILE-- + $file]); + +curl_setopt($ch, CURLOPT_READFUNCTION, + function (): int { + throw new Exception('read exception'); + } +); + +try { + curl_exec($ch); +} catch (Exception $e) { + echo $e->getMessage(), "\n"; +} + +var_dump(curl_errno($ch) === CURLE_ABORTED_BY_CALLBACK); + +?> +--EXPECTF-- +read exception +bool(true) diff --git a/ext/curl/tests/curl_writefunction_throws_abort.phpt b/ext/curl/tests/curl_writefunction_throws_abort.phpt new file mode 100644 index 000000000000..93c7a9237e95 --- /dev/null +++ b/ext/curl/tests/curl_writefunction_throws_abort.phpt @@ -0,0 +1,35 @@ +--TEST-- +CURLOPT_WRITEFUNCTION aborts transfer when callback throws +--EXTENSIONS-- +curl +--SKIPIF-- + +--FILE-- +getMessage(), "\n"; +} + +var_dump(curl_errno($ch) === CURLE_WRITE_ERROR); + +?> +--EXPECTF-- +write exception +bool(true) diff --git a/ext/curl/tests/curl_xferinfofunction_throws_abort.phpt b/ext/curl/tests/curl_xferinfofunction_throws_abort.phpt new file mode 100644 index 000000000000..2c297d7f816f --- /dev/null +++ b/ext/curl/tests/curl_xferinfofunction_throws_abort.phpt @@ -0,0 +1,36 @@ +--TEST-- +CURLOPT_XFERINFOFUNCTION aborts transfer when callback throws +--EXTENSIONS-- +curl +--SKIPIF-- + +--FILE-- +getMessage(), "\n"; +} + +var_dump(curl_errno($ch) === CURLE_ABORTED_BY_CALLBACK); + +?> +--EXPECTF-- +info exception +bool(true) From 5f7cf70b5fb8132bf649718e509cca843d88cf82 Mon Sep 17 00:00:00 2001 From: Sjoerd Langkemper Date: Mon, 27 Jul 2026 08:45:52 +0000 Subject: [PATCH 2/3] Fix CURLOPT_POST parameter --- ext/curl/tests/curl_readfunction_throws_abort.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/curl/tests/curl_readfunction_throws_abort.phpt b/ext/curl/tests/curl_readfunction_throws_abort.phpt index fdb83dbb421d..23b321b99f5d 100644 --- a/ext/curl/tests/curl_readfunction_throws_abort.phpt +++ b/ext/curl/tests/curl_readfunction_throws_abort.phpt @@ -16,7 +16,7 @@ $host = curl_cli_server_start(); $ch = curl_init("{$host}/get.inc"); $file = new CURLFile(__DIR__ . '/curl_testdata1.txt'); -curl_setopt($ch, CURLOPT_POST, ['file' => $file]); +curl_setopt($ch, CURLOPT_POST, 1); curl_setopt($ch, CURLOPT_READFUNCTION, function (): int { From 885644a317a6296c64a59d9ba9053fa37f5ed934 Mon Sep 17 00:00:00 2001 From: Sjoerd Langkemper Date: Tue, 28 Jul 2026 08:24:25 +0000 Subject: [PATCH 3/3] Handle case correctly where callback is set to null This did not raise an error before, so return OK. I didn't add a test for curl_prereqfunction because this is already tested in curl_setopt_CURLOPT_PREREQFUNCTION.phpt. I marked some callbacks being null as unexpected, where I didn't find any code that set them to null in sourcegraph. Specifically, guzzle sets header, read, write and progress to null, so maybe this is not so unexpected. --- ext/curl/interface.c | 16 ++++++++-------- .../tests/curl_headerfunction_throws_abort.phpt | 10 ++++++++++ .../curl_progressfunction_throws_abort.phpt | 10 ++++++++++ .../tests/curl_readfunction_throws_abort.phpt | 10 ++++++++++ .../tests/curl_writefunction_throws_abort.phpt | 10 ++++++++++ .../curl_xferinfofunction_throws_abort.phpt | 10 ++++++++++ 6 files changed, 58 insertions(+), 8 deletions(-) diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 9d0bb46e8896..f740a9feac10 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -605,14 +605,14 @@ static int curl_fnmatch(void *ctx, const char *pattern, const char *string) static int curl_progress(void *clientp, double dltotal, double dlnow, double ultotal, double ulnow) { php_curl *ch = (php_curl *)clientp; - int rval = 1; + int rval = 1; // error #if PHP_CURL_DEBUG fprintf(stderr, "curl_progress() called\n"); fprintf(stderr, "clientp = %p, dltotal = %f, dlnow = %f, ultotal = %f, ulnow = %f\n", clientp, dltotal, dlnow, ultotal, ulnow); #endif if (!ZEND_FCC_INITIALIZED(ch->handlers.progress)) { - return rval; + return 0; // ok } zval args[5]; @@ -633,7 +633,7 @@ static int curl_progress(void *clientp, double dltotal, double dlnow, double ult _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)) { - rval = 0; + rval = 0; // ok } } @@ -646,14 +646,14 @@ static int curl_progress(void *clientp, double dltotal, double dlnow, double ult static int curl_xferinfo(void *clientp, curl_off_t dltotal, curl_off_t dlnow, curl_off_t ultotal, curl_off_t ulnow) { php_curl *ch = (php_curl *)clientp; - int rval = 1; + int rval = 1; // error #if PHP_CURL_DEBUG fprintf(stderr, "curl_xferinfo() called\n"); fprintf(stderr, "clientp = %p, dltotal = %ld, dlnow = %ld, ultotal = %ld, ulnow = %ld\n", clientp, dltotal, dlnow, ultotal, ulnow); #endif - if (!ZEND_FCC_INITIALIZED(ch->handlers.xferinfo)) { - return rval; + if (UNEXPECTED(!ZEND_FCC_INITIALIZED(ch->handlers.xferinfo))) { + return 0; // ok } zval argv[5]; @@ -674,7 +674,7 @@ static int curl_xferinfo(void *clientp, curl_off_t dltotal, curl_off_t dlnow, cu _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)) { - rval = 0; + rval = 0; // ok } } @@ -692,7 +692,7 @@ static int curl_prereqfunction(void *clientp, char *conn_primary_ip, char *conn_ // when CURLOPT_PREREQFUNCTION is set to null, curl_prereqfunction still // gets called. Return CURL_PREREQFUNC_OK immediately in this case to avoid // zend_call_known_fcc() with an uninitialized FCC. - if (!ZEND_FCC_INITIALIZED(ch->handlers.prereq)) { + if (UNEXPECTED(!ZEND_FCC_INITIALIZED(ch->handlers.prereq))) { return CURL_PREREQFUNC_OK; } diff --git a/ext/curl/tests/curl_headerfunction_throws_abort.phpt b/ext/curl/tests/curl_headerfunction_throws_abort.phpt index 648784403971..9a69c966f144 100644 --- a/ext/curl/tests/curl_headerfunction_throws_abort.phpt +++ b/ext/curl/tests/curl_headerfunction_throws_abort.phpt @@ -15,6 +15,7 @@ include 'server.inc'; $host = curl_cli_server_start(); $ch = curl_init("{$host}/get.inc"); +echo "Test: header function throws exception\n"; curl_setopt($ch, CURLOPT_HEADERFUNCTION, function (): int { throw new Exception('header exception'); @@ -29,7 +30,16 @@ try { var_dump(curl_errno($ch) === CURLE_WRITE_ERROR); +echo "Test: header function is null\n"; +curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); +curl_setopt($ch, CURLOPT_HEADERFUNCTION, null); +curl_exec($ch); +var_dump(curl_errno($ch) === CURLE_OK); + ?> --EXPECTF-- +Test: header function throws exception header exception bool(true) +Test: header function is null +bool(true) diff --git a/ext/curl/tests/curl_progressfunction_throws_abort.phpt b/ext/curl/tests/curl_progressfunction_throws_abort.phpt index f03d7b96d90a..55e0f76cb61f 100644 --- a/ext/curl/tests/curl_progressfunction_throws_abort.phpt +++ b/ext/curl/tests/curl_progressfunction_throws_abort.phpt @@ -15,6 +15,7 @@ include 'server.inc'; $host = curl_cli_server_start(); $ch = curl_init("{$host}/get.inc"); +echo "Test: progress function throws exception\n"; curl_setopt($ch, CURLOPT_NOPROGRESS, 0); curl_setopt($ch, CURLOPT_PROGRESSFUNCTION, function (): int { @@ -30,7 +31,16 @@ try { var_dump(curl_errno($ch) === CURLE_ABORTED_BY_CALLBACK); +echo "Test: progress function is null\n"; +curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); +curl_setopt($ch, CURLOPT_PROGRESSFUNCTION, null); +curl_exec($ch); +var_dump(curl_errno($ch) === CURLE_OK); + ?> --EXPECTF-- +Test: progress function throws exception info exception bool(true) +Test: progress function is null +bool(true) diff --git a/ext/curl/tests/curl_readfunction_throws_abort.phpt b/ext/curl/tests/curl_readfunction_throws_abort.phpt index 23b321b99f5d..a030f8c4f41e 100644 --- a/ext/curl/tests/curl_readfunction_throws_abort.phpt +++ b/ext/curl/tests/curl_readfunction_throws_abort.phpt @@ -18,6 +18,7 @@ $ch = curl_init("{$host}/get.inc"); $file = new CURLFile(__DIR__ . '/curl_testdata1.txt'); curl_setopt($ch, CURLOPT_POST, 1); +echo "Test: read function throws exception\n"; curl_setopt($ch, CURLOPT_READFUNCTION, function (): int { throw new Exception('read exception'); @@ -32,7 +33,16 @@ try { var_dump(curl_errno($ch) === CURLE_ABORTED_BY_CALLBACK); +echo "Test: read function is null\n"; +curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); +curl_setopt($ch, CURLOPT_READFUNCTION, null); +curl_exec($ch); +var_dump(curl_errno($ch) === CURLE_OK); + ?> --EXPECTF-- +Test: read function throws exception read exception bool(true) +Test: read function is null +bool(true) diff --git a/ext/curl/tests/curl_writefunction_throws_abort.phpt b/ext/curl/tests/curl_writefunction_throws_abort.phpt index 93c7a9237e95..3da2fe8107b4 100644 --- a/ext/curl/tests/curl_writefunction_throws_abort.phpt +++ b/ext/curl/tests/curl_writefunction_throws_abort.phpt @@ -15,6 +15,7 @@ include 'server.inc'; $host = curl_cli_server_start(); $ch = curl_init("{$host}/get.inc"); +echo "Test: write function throws exception\n"; curl_setopt($ch, CURLOPT_WRITEFUNCTION, function (): int { throw new Exception('write exception'); @@ -29,7 +30,16 @@ try { var_dump(curl_errno($ch) === CURLE_WRITE_ERROR); +echo "Test: write function is null\n"; +curl_setopt($ch, CURLOPT_WRITEFUNCTION, null); +curl_exec($ch); +var_dump(curl_errno($ch) === CURLE_OK); + ?> --EXPECTF-- +Test: write function throws exception write exception bool(true) +Test: write function is null +Hello World! +Hello World!bool(true) diff --git a/ext/curl/tests/curl_xferinfofunction_throws_abort.phpt b/ext/curl/tests/curl_xferinfofunction_throws_abort.phpt index 2c297d7f816f..fbc28f07ee96 100644 --- a/ext/curl/tests/curl_xferinfofunction_throws_abort.phpt +++ b/ext/curl/tests/curl_xferinfofunction_throws_abort.phpt @@ -15,6 +15,7 @@ include 'server.inc'; $host = curl_cli_server_start(); $ch = curl_init("{$host}/get.inc"); +echo "Test: xfer info function throws exception\n"; curl_setopt($ch, CURLOPT_NOPROGRESS, 0); curl_setopt($ch, CURLOPT_XFERINFOFUNCTION, function (): int { @@ -30,7 +31,16 @@ try { var_dump(curl_errno($ch) === CURLE_ABORTED_BY_CALLBACK); +echo "Test: xfer info function is null\n"; +curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); +curl_setopt($ch, CURLOPT_XFERINFOFUNCTION, null); +curl_exec($ch); +var_dump(curl_errno($ch) === CURLE_OK); + ?> --EXPECTF-- +Test: xfer info function throws exception info exception bool(true) +Test: xfer info function is null +bool(true)