Bug#1037258: curl -I (HEAD request) fails with HTTP/2 against a Debian Apache instance
Since 2023-10-02, we have a fixed version of the curl package on stable-backports, so this can serve as a workaround if you're fine with the constraints of the backports repository[0]. The current investigation status is that we know which commit caused the regression and which one fixed it, but we don't know exactly what change in the fixing commit is required (it's changing a lot of things at once). We also don't know exactly what are the conditions that trigger the issue, but we know it has to do with HTTP2 and there are a couple of endpoints we can use for debugging. Regression commit: https://github.com/curl/curl/commit/8c762f59983a3e9e2b80fdb34aa5e08f1d9a1c7d (curl-7_88_0) Fixing commit: https://github.com/curl/curl/commit/744dcf22fac6cf12a9112df106b61864982afef9 (curl-8_1_0) [0]: https://backports.debian.org/ Cheers, -- Samuel Henrique
Bug#1037258: curl -I (HEAD request) fails with HTTP/2 against a Debian Apache instance
On Sun, Jun 11, 2023 at 01:56:21AM -0400, Sergio Durigan Junior wrote: > On Saturday, June 10 2023, Samuel Henrique wrote: > > > Hello, > > > > I'm not able to reproduce the issue on Bookworm with a HTTP2 localhost > > apache server. > [...] > > Hey, > > I was able to find another URL that triggers the same issue. This one: > > > https://chinarising.puntopress.com/wp-content/uploads/2023/04/Press-TV-KSA-Iran.mp3 OK so I tried reproducing it by installing WP on localhost (actually behind LXD container). The container is Debian bookworm (with deb.sury.org repo added). apache2 site configuration (`/etc/apache2/sites-enabled/test.conf`) is: ``` ServerName test.test ServerAdmin webmas...@test.test DocumentRoot/srv/www/test.test Redirect301 / https://wp.bagas.me ServerName test.test ServerAdmin webmas...@test.test DocumentRoot/srv/www/test.test DirectoryIndex index.php index.html Protocols h2 http/1.1 ... ... AllowOverride all Require all granted SetHandler "proxy:unix:/run/php/php8.2-fpm.sock|fcgi://localhost" ``` (requires mod_proxy and mod_proxy_fcgi for apache2 and php8.2-fpm package). WP configuration is pretty standard and it's rather fresh install (see the web for WP installation procedures). Log in to WP dashboard and upload random .mp3 files. Once uploaded, click `Copy URL to clipboard` besides the uploaded file and feed it to curl. In summary, I can't reproduce this bug either with above setup. Thanks. -- An old man doll... just what I always wanted! - Clara signature.asc Description: PGP signature
Bug#1037258: curl -I (HEAD request) fails with HTTP/2 against a Debian Apache instance
On Saturday, June 10 2023, Samuel Henrique wrote:
> Hello,
>
> I'm not able to reproduce the issue on Bookworm with a HTTP2 localhost
> apache server.
[...]
Hey,
I was able to find another URL that triggers the same issue. This one:
https://chinarising.puntopress.com/wp-content/uploads/2023/04/Press-TV-KSA-Iran.mp3
(I found it while reading https://github.com/curl/curl/issues/9526)
Annoyingly, I still can't determine how to reliably reproduce the
problem by configuring a local webserver... Anyway, this new URL
allowed me to continue the investigation, and I found that the following
commit seems to have introduced the problem:
8c762f59983a3e9e2b80fdb34aa5e08f1d9a1c7d is the first bad commit
commit 8c762f59983a3e9e2b80fdb34aa5e08f1d9a1c7d
Author: Stefan Eissing
Date: Wed Feb 8 15:56:57 2023 +0100
http2: minor buffer and error path fixes
It's a much smaller commit, and if we focus only on the lib/http2.c
changes, we notice that they are directly related to the handling HTTP2
stream closures.
By comparing the two commits (the one that introduces the failure and
the other that fixes it), it's pretty hard to say what exactly could be
the fix here because the code changed so much between them.
With my mad scientist hat on, I decided to experiment with something I'd
noticed: the "bad" commit indiscriminately treats "stream->reset" and
"stream->error" the same way, while the "good" commit doesn't. I
applied the following change to the code:
diff --git a/lib/http2.c b/lib/http2.c
index d5eed385e..2dc8ee348 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -1919,7 +1919,7 @@ static ssize_t cf_h2_recv(struct Curl_cfilter *cf, struct
Curl_easy *data,
Curl_expire(data, 0, EXPIRE_RUN_NOW);
}
else if(stream->closed) {
- if(stream->reset || stream->error) {
+ if(stream->reset) {
nread = http2_handle_stream_close(cf, data, stream, err);
goto out;
}
... and it seemed to "work". Of course, I don't claim the change above
to be correct, but it does seem to confirm the suspicion that
"stream->error" shouldn't be handled as a stream closure.
Things to investigate:
- Why are we getting "stream->error" here? Maybe "--trace" can help us.
- How to reliably reproduce the problem locally? It'd be really nice to
have a testcase for this.
Ah, I forgot to mention: initially I had the suspicion that this might
be related to the openssl 3 transition, but I was able to reproduce the
problem using bullseye, so I think that proves my hypothesis wrong.
It's late here now so I'm calling it "a day", but I'll try to get back
to this investigation in the following days.
Cheers,
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
https://sergiodj.net/
Bug#1037258: curl -I (HEAD request) fails with HTTP/2 against a Debian Apache instance
Hello, I'm not able to reproduce the issue on Bookworm with a HTTP2 localhost apache server. There's something specific to the way "https://git.dgit.debian.org/dgit/info/refs; is setup which triggers the issue. The next step is to find a server which can be tested against, ideally finding the configuration that triggers this so we can test locally. Once we get something to reproduce, we can do a bisect to find the commit that introduces the issue and possibly get close to a safe fix. Thank you, -- Samuel Henrique
Bug#1037258: curl -I (HEAD request) fails with HTTP/2 against a Debian Apache instance
For the record: this bug caused `dgit clone dgit` to fail. That aspect of the problem has been worked around by forcing that host to use HTTP/1 only. ([rt.debian.org #9218]). This also means that the repro in this bug report won't work any more, I'm afraid. -- Ian JacksonThese opinions are my own. Pronouns: they/he. If I emailed you from @fyvzl.net or @evade.org.uk, that is a private address which bypasses my fierce spamfilter.
Bug#1037258: curl -I (HEAD request) fails with HTTP/2 against a Debian Apache instance
On Friday, June 09 2023, Ian Jackson wrote: > Steps to reproduce: > > curl -I https://git.dgit.debian.org/dgit/info/refs > > Expected output (obtained on, eg, buster): > > HTTP/2 200 > expires: Fri, 01 Jan 1980 00:00:00 GMT > pragma: no-cache > cache-control: no-cache, max-age=0, must-revalidate > x-content-type-options: nosniff > x-frame-options: sameorigin > referrer-policy: no-referrer > x-xss-protection: 1 > permissions-policy: interest-cohort=() > strict-transport-security: max-age=15552000 > content-length: 40562 > vary: Accept-Encoding > x-clacks-overhead: GNU Terry Pratchett > content-type: text/plain > date: Fri, 09 Jun 2023 14:11:55 GMT > server: Apache > > Actual output: > > curl: (92) HTTP/2 stream 1 was not closed cleanly: PROTOCOL_ERROR (err 1) > > This seems to have stopped working very recently. I was able to reproduce the bug building from the upstream repo, ran a bisect and found that the following commit seems to have fixed the issue: 744dcf22fac6cf12a9112df106b61864982afef9 is the first fixed commit commit 744dcf22fac6cf12a9112df106b61864982afef9 Author: Stefan Eissing Date: Thu Mar 30 12:13:49 2023 +0200 http2: flow control and buffer improvements - use bufq for send/receive of network data - usd bufq for send/receive of stream data - use HTTP/2 flow control with no-auto updates to control the amount of data we are buffering for a stream HTTP/2 stream window set to 128K after local tests, defined code constant for now - elminiating PAUSEing nghttp2 processing when receiving data since a stream can now take in all DATA nghttp2 forwards Improved scorecard and adjuste http2 stream window sizes - scorecard improved output formatting and options default - scorecard now also benchmarks small requests / second Closes #10771 There are a few problems, though: - The commit is long and seems to introduce unrelated changes. In fact, it's just an "overhaul" commit, so the fix apparently happened "by chance". - According to https://github.com/curl/curl/pull/10771#issuecomment-1491200728, the commit might have introduce other problems. -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible https://sergiodj.net/
