curl: exceptions in callbacks do not abort the request

[MagicalTux]

Description

A curl request may take time (sometimes an infinite amount of time) to respond. When an exception is thrown in a callback it is properly forwarded to the curl_exec, however in some cases such as server-sent events, the request may never end.

When an exception happens in a callback, the request should be aborted.

The following code:

<?php
$ch = curl_init('https://sse.fastlydemo.net/flights/stream');

curl_setopt($ch, CURLOPT_RETURNTRANSFER, false);
curl_setopt($ch, CURLOPT_HEADER, false);
curl_setopt($ch, CURLOPT_TIMEOUT, 0);
curl_setopt($ch, CURLOPT_WRITEFUNCTION, function($ch, $data) {
        echo "Noping away\n";
        throw new \Exception('nope!');
});

try {
        curl_exec($ch);
        if(curl_errno($ch)) throw new \Exception('CURL error: '.curl_error($ch));
} catch(\Throwable $e) {
        echo "Caught exception: $e\n";
}

Resulted in this output:

Noping away

(then nothing, the script freezes there)

But I expected this output instead:

Noping away
Caught exception: Exception: nope! in /tmp/test.php:9
Stack trace:
#0 [internal function]: {closure}()
#1 /tmp/test.php(14): curl_exec()
#2 {main}

PHP Version

PHP 8.2.20

Operating System

No response

[cmb69]

Well, the proper way to cause the tranfer to be aborted, is to return something else than strlen($data) from the callback (except for CURL_WRITEFUNC_PAUSE which pauses the transfer). If an exception is thrown, nothing (IS_UNDEF) is returned, and there is special handling for that case:

https://github.com/php/php-src/blob/01aeaf2e95813f0cbb44b63ca44477d0adf5a217/ext/curl/interface.c#L600-L604

Frankly, I don't know why we do not signal abort in this case. Even just removing the if would give the desired result.

[MagicalTux]

The documentation indeed says:

It must return the exact number of bytes written or the transfer will be aborted with an error.

Would the exception have the priority however or would this result in a CURL error: Failure writing output to destination, passed 12 returned 0 ?

[cmb69]

Our docs are not quite up to date; better check https://curl.se/libcurl/c/CURLOPT_WRITEFUNCTION.html for details.

With the following patch:

 ext/curl/interface.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ext/curl/interface.c b/ext/curl/interface.c
index 677eaa8703..eb1511eb8a 100644
--- a/ext/curl/interface.c
+++ b/ext/curl/interface.c
@@ -601,6 +601,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 = zval_get_long(&retval);
+           } else if (EG(exception)) {
+               length = -1;
            }

            zval_ptr_dtor(&argv[0]);

the test script outputs:

Noping away
Caught exception: Exception: nope! in C:\php-sdk\phpdev\vs17\x64\gh16513.php:9
Stack trace:
#0 [internal function]: {closure:C:\php-sdk\phpdev\vs17\x64\gh16513.php:7}(Object(CurlHandle), 'retry: 1000\n')
#1 C:\php-sdk\phpdev\vs17\x64\gh16513.php(13): curl_exec(Object(CurlHandle))
#2 {main}

[MagicalTux]

I think that is an adequate resolution of the issue, and would help a lot. Can we expect this to be fixed in the next PHP release?