http version negotiation #16100
Closed
http version negotiation #16100
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bagder
approved these changes
Jan 28, 2025
icing
force-pushed
the
http-negotiation
branch
from
d4b86ce to
ab6bec8
Compare
icing
added a commit
to icing/curl
that referenced
this pull request
Jan 28, 2025
based on curl#16100 Add a 'wanted' major HTTP version bitmask next to the 'allowed' bitmask in HTTP version negotiation. This will try connections as specified in 'wanted', but enabled Alt-Svc and HTTPS-RR to redirect to other major HTTP versions, if those are 'allowed'. Changes libcurl internal default to `CURL_HTTP_VERSION_NONE` and removes the code in curl that sets `CURL_HTTP_VERSION_2TLS` if the command line does not say anything else.
icing
force-pushed
the
http-negotiation
branch
from
ab6bec8 to
656c0da
Compare
icing
added a commit
to icing/curl
that referenced
this pull request
Jan 29, 2025
based on curl#16100 Add a 'wanted' major HTTP version bitmask next to the 'allowed' bitmask in HTTP version negotiation. This will try connections as specified in 'wanted', but enabled Alt-Svc and HTTPS-RR to redirect to other major HTTP versions, if those are 'allowed'. Changes libcurl internal default to `CURL_HTTP_VERSION_NONE` and removes the code in curl that sets `CURL_HTTP_VERSION_2TLS` if the command line does not say anything else.
|
Analysis of PR #16100 at 656c0da2: Test http/test_09_push.py::TestPush::test_09_02_h2_push failed, which has NOT been flaky recently, so there could be a real issue in this PR. Test 1192 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Test 1198 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Test 1199 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Test 1916 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Test 2202 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Test 2205 failed, which has NOT been flaky recently, so there could be a real issue in this PR. There are more failures, but that's enough from Gha. Generated by Testclutch |
Translate the `data->set.httpwant` which is one of the consts from the public API (CURL_HTTP_VERSION_*) into a major version mask plus additional flags for internal handling. `Curl_http_neg_init()` does the translation and flags setting in http.c, using new internal consts CURL_HTTP_V1x, CURL_HTTP_V2x and CURL_HTTP_V3x for the major versions. The flags are - only_10: when the application explicity asked fro HTTP/1.0 - h2_upgrade: when the application asks for upgrading 1.1 to 2. - h2_prior_knowledge: when directly talking h2 without ALPN - accept_09: when a HTTP/0.9 response is acceptable. The Alt-Svc and HTTPS RR redirections from one ALPN to another obey the allowed major versions. If a transfer has only h3 enabled, Alt-Svc redirection to h2 is ignored. This is the current implementation. It can be debated if Alt-Svc should be able to override the allowed major versions. Added test_12_06 to verify the current restriction.
icing
force-pushed
the
http-negotiation
branch
from
656c0da to
60c2d1f
Compare
icing
added a commit
to icing/curl
that referenced
this pull request
Feb 20, 2025
based on curl#16100 Add a 'wanted' major HTTP version bitmask next to the 'allowed' bitmask in HTTP version negotiation. This will try connections as specified in 'wanted', but enabled Alt-Svc and HTTPS-RR to redirect to other major HTTP versions, if those are 'allowed'. Changes libcurl internal default to `CURL_HTTP_VERSION_NONE` and removes the code in curl that sets `CURL_HTTP_VERSION_2TLS` if the command line does not say anything else.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Translate the
data->set.httpwantwhich is one of the consts from the public API (CURL_HTTP_VERSION_*) into a major version mask plus additional flags for internal handling.Curl_http_neg_init()does the translation and flags setting in http.c, using new internal consts CURL_HTTP_V1x, CURL_HTTP_V2x and CURL_HTTP_V3x for the major versions. The flags areThe Alt-Svc and HTTPS RR redirections from one ALPN to another obey the allowed major versions. If a transfer has only h3 enabled, Alt-Svc redirection to h2 is ignored.
This is the current implementation. It can be debated if Alt-Svc should be able to override the allowed major versions. Added test_12_06 to verify the current restriction.