Skip to content

Three curl defects the php-src suite was already reporting - #36

Merged
EdmondDantes merged 4 commits into
true-asyncfrom
curl-callback-result-codes
Sep 10, 2026
Merged

EdmondDantes merged 4 commits into
true-asyncfrom
curl-callback-result-codes

Conversation

@EdmondDantes

Copy link
Copy Markdown

ext/curl/tests had four failures on this tree, unrelated to any async issue on the
tracker and failing the same way on a build of true-async without any of my recent
commits. Three defects behind them, one commit each.

A CURLFile part could not be rewound

curl_mime_data_cb() was given NULL where libcurl expects a seek callback, so a
part whose body had already been read could not be replayed. A redirect ended the
transfer with CURLE_ABORTED_BY_CALLBACK, and curl_curlfile_seek.phpt printed
nothing where it expects curl_testdata1.txt|application/octet-stream|6.

Upstream carries this callback. The fork lost it when the CURLFile stream was made
lazy: with nothing open at registration time there was nothing for a seek to act
on, and the argument was passed as NULL. The new one seeks the stream the read
callback opened, and answers CURL_SEEKFUNC_CANTSEEK where the part cannot be
replayed — with a read in flight, whose completion would land at the position being
left. A finished read holds bytes from the old position and is released; the flags a
finished or failed read left are cleared.

curl_errno() said nothing about a callback that threw

An exception from CURLOPT_WRITEFUNCTION or CURLOPT_HEADERFUNCTION left the
handle reporting CURLE_OK, and every other callback reported
CURLE_ABORTED_BY_CALLBACK whatever it was.
curl_writefunction_throws_abort.phpt and curl_headerfunction_throws_abort.phpt
assert CURLE_WRITE_ERROR and were red.

Upstream has no such gap: the callback returns to libcurl and libcurl decides. Here
the exception is stored and the transfer is torn down beside libcurl, so the code
has to be carried. It is now stored with the exception — CURLE_WRITE_ERROR for a
write or header callback, CURLE_SEND_FAIL_REWIND for a seek one,
CURLE_ABORTED_BY_CALLBACK for the rest — and the first callback to throw decides.

It is stored twice, on the event and on the handle, because the two readers live at
different times: the completion paths read the event while it is alive, and
curl_async_perform() reads after the suspend the exception ends, by which point
the notification may already have disposed the event. An earlier draft of this
change read it from the event there and it was a use-after-free; the two-store shape
is what the test caught.

An empty upload aborted the next transfer

CURLOPT_READFUNCTION set back to null with no CURLOPT_INFILE behind it leaves
the read dispatch with no descriptor, and it answered CURL_READFUNC_ABORT. The
second curl_exec() in curl_readfunction_throws_abort.phpt therefore failed where
it had nothing to send. Upstream returns 0 in that case, which is an empty body. An
abort stays the answer where a source was named and turned out unusable.

Checks

build suites result
libcurl 8.12.1, ZTS debug ext/curl/tests + ext/async/tests 1367 passed, 0 failed
libcurl 8.12.1, ZTS ASAN ext/curl/tests + ext/async/tests/curl 240 passed, 0 failed
libcurl 8.5.0 (what the Linux CI builds) same two curl suites 235 passed, 0 failed
libcurl 7.87.0 (the minimum config.m4 admits) same 233 passed, 0 failed

ext/async/fuzzy-tests/_generated — 724 passed, 0 failed. Before these commits
ext/curl/tests was 168 passed, 4 failed on 8.12.1 and 164 passed, 4 failed on 8.5.0.

Edmond added 4 commits September 10, 2026 13:54
A CURLFile part was registered with no seek callback, so libcurl could not replay
the body it had already read. A redirect or an authentication challenge then ended
the transfer with CURLE_ABORTED_BY_CALLBACK instead of resending the file:
ext/curl/tests/curl_curlfile_seek.phpt printed nothing where it expects the part's
name, type and length.

The callback seeks the stream the read callback opened, and answers
CURL_SEEKFUNC_CANTSEEK where the part cannot be replayed from that position: with
a read in flight, whose completion would land at the position being left. A read
that already finished holds bytes from there and is released, and the flags a
finished or failed read left behind are cleared, so the replay starts clean.

Upstream carries this callback and the fork lost it when the CURLFile stream was
made lazy: opening on first read left nothing for a seek to act on at registration
time, and the argument was passed as NULL.
A user callback that throws ends the transfer, and curl_errno() said nothing about
it: an exception from CURLOPT_WRITEFUNCTION or CURLOPT_HEADERFUNCTION left the
handle reporting CURLE_OK, and every other callback reported
CURLE_ABORTED_BY_CALLBACK whatever it was. Upstream has no such gap, because the
callback returns to libcurl and libcurl decides; here the exception is stored and
the transfer is torn down beside libcurl, so the code has to be carried.

The code the callback stands for is stored with the exception -- CURLE_WRITE_ERROR
for a write or header callback, CURLE_SEND_FAIL_REWIND for a seek one,
CURLE_ABORTED_BY_CALLBACK for the rest -- and the first callback to throw decides
it. Every completion path reports it in place of the code libcurl reached on its
own.

It is stored twice, on the event and on the handle, because the two readers live at
different times: the completion paths read the event while it is alive, and
curl_async_perform() reads after the suspend the exception ends, by which point the
notification may already have disposed the event. Reading it from the event there
was a use-after-free waiting for a test to notice.

Evidence: ext/curl/tests/curl_writefunction_throws_abort.phpt and
curl_headerfunction_throws_abort.phpt, which assert CURLE_WRITE_ERROR and were red.
CURLOPT_READFUNCTION set back to null with no CURLOPT_INFILE behind it leaves the
read dispatch with no descriptor, and it answered CURL_READFUNC_ABORT: the next
curl_exec() on that handle failed with CURLE_ABORTED_BY_CALLBACK where it had
nothing to send. Upstream reads through read_handler->fp and returns 0 when there
is none, which is an empty body.

An abort stays the answer where a source was named and turned out unusable.

Evidence: the second half of ext/curl/tests/curl_readfunction_throws_abort.phpt,
which asserts CURLE_OK there.
@EdmondDantes
EdmondDantes merged commit f582852 into true-async Sep 10, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant