New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
hyper: does not request connection upgrade via headers when --http2
flag is set (test 1800)
#8605
Comments
I wrote this issue following this article by seanmonstar and in my infinite wisdom (/s) think I misinterpreted what he meant by
I see that 1800 was not included in this list... and what was meant is probably that in the future there will be even less on that list. I'll try and get around to actually helping soon, but I apologize in advance if this problem was already known! |
I think Sean's list was cut off early? The full list I made is in the hyper wiki page and does include 1800 (and the file |
I presume hyper has some way to help us get the |
@seanmonstar can correct me if I'm wrong, but I'm assuming that the fact that 1800 wasn't on the list in his article indicates that they already know the root cause of this problem. I think (but am not 100% sure) that there isn't currently logic to create the That being said, if there is a way to get the if(!h2 && (data->state.httpwant == CURL_HTTP_VERSION_2_0)) {
result = Curl_hyper_header(data, headers,
"Connection: Upgrade, HTTP2-Settings");
if(result)
goto error;
result = Curl_hyper_header(data, headers, "Upgrade: h2c");
if(result)
goto error;
/* Eventual callback to some HTTP2-Settings Header Generation? */
if(result)
goto error;
} |
I didn't include it in the list because this one we do know the root cause of: hyper doesn't have http1-to-http2 upgrade support. Specifically, it's h2 internal dependency doesn't know how to start a stream with just the response side being http2. |
I didn't know that... |
Steps forward for this are likely:
|
Would this also pose a problem for tests 358 and 359 (which are on the list of unknown root causes)? I'm not sure if it's necessarily the root cause (I don't know too much about alternative services/if & how hyper deals with them), but both tests have commands that set the |
Yes. Even if those tests are strictly speaking for testing alt-svc, they do check for an eventual upgrade to h2 with |
Make tests require h2c feature present to run, and only set h2c if nghttp2 is used in the build. Hyper does not support it. Remove those tests from DISABLED Ref: #8605
I did this
I ran
runtests.pl 1800 -f
(HTTP/2 upgrade refused) when curl is built with hyper, though this error is reproducible whenever hyper is the backend,--http2
is set, and curl tries to connect with http (so, this can be seen withcurl -v --http2 http://google.com
).https attempts to negotiate in the TLS handshake (by default), and behaves correctly.
I expected the following
As per the expected test output, curl should attempt to negotiate HTTP/2 and the following headers should appear in the request:
I'm not 100% sure on the fix, but I'm guessing that
Curl_http
inc-hyper.c
should check for--http2
and by some means add the corresponding headers to the request.curl/libcurl version
operating system
Linux ndd7xv 5.16.12-200.fc35.x86_64 #1 SMP PREEMPT Wed Mar 2 19:06:17 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
The text was updated successfully, but these errors were encountered: