curl / Mailing Lists / curl-library / Single Mail
Buy commercial curl support from WolfSSL. We help you work out your issues, debug your libcurl applications, use the API, port to new platforms, add new features and more. With a team lead by the curl founder himself.


From: Jiří Hruška via curl-library <>
Date: Mon, 27 Nov 2023 10:38:59 +0100

A week ago, Daniel Stenberg wrote:
> Here it is =>

Sorry for not being available for a while, thanks for taking care of it.
Good point about mixing EOF from data and trailers there too.

Let me just close this with a bit more info that I still gathered earlier.
I was trying to reproduce and test this more while preparing a PR and I
remembered that duh, it happened on HTTP/2, which has a protocol-level flow
control, that's probably how it got triggered in the first place. So I should
be able to simulate it after all, passing --frontend-http2-window-size=2000
or similar to nghttpx and that should make it work.

So I wrote a nice new libtest, CURLOPT_UPLOAD, CURLOPT_READFUNCTION, validate
that it gets called just once, add the --frontend-http2-window-size=2000 to, add some 100-continue hack to make sure the client actually
sees the small window before blasting out the default 64k too quickly, etc.

Aaaaand it did not reproduce. I verified that the data was sent in DATA blocks
of 2000 bytes over the HTTP/2 connection as expected and my test had to expect
them to come in these chunks on the server side, but Curl_fillreadbuffer() was
not called for them individually. It got called once with 65536 bytes of space
(UPLOADBUFFER_DEFAULT) and then again only after all of that was used.

I investigated why, and it turned out that this behavior changed in
"HTTP/2: upload handling fixes" (65937f0d, 2023-06-20, released in 8.2.0)

Before that, the HTTP/2 connection filter cf_h2_send() would work like e.g.
fwrite() - if only some of the data got accepted, it would "eat" a smaller
number of bytes between 1 and N and the caller could shift/refill the buffer.
This commit changed it to return 0 with CURLE_AGAIN instead, keeping the whole
buffer as it is, until everything gets processed.

So, the test I wrote does what it should when cherry-picked on top of that~1,
but no longer reproduces the issue after. Which probably means that:

1. My original problem would not reproduce since cURL 8.2.0 (Git4Win 2.42) too
   (and I should just upgrade stuff everywhere more often, and I should take
   less than 1 year to investigate and report bugs, yeah, fair enough).

2. The whole code with `ssize_t offset` and partial upload buffer refilling
   in readwrite_upload() seems pointless now, because afaics, it has been
   all added only specifically for HTTP/2 and nghttp2 in this commit:
   But now the HTTP/2 path never partially uses a buffer, and so perhaps
   this whole extra complexity there along with curl_upload_refill_watermark()
   magic numbers etc. could be removed again. And as said before, no hits of
   offset != 0 ever in the whole test suite either (even when executed with
   nghttpx --frontend-http2-window-size=2000).

3. As the bug could realistically reproduce only between 7.84.0 and 8.1.2,
   there does not seem to be a strong reason to bother anymore, and also
   if no reports of CURLOPT_READFUNCTION called twice were ever received
   outside of these versions and HTTP/2, my whole point here is moot and
   probably nothing needed to be done at all...

So, in summary, sorry for the trouble, and I'll do a better job next time :-)

Kind regards,
Received on 2023-11-27