http: In alt-svc negotiation only allow supported HTTP versions #17037
Conversation
This file contains hidden or 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
|
Analysis of PR #17037 at d282060f: Test 410 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Generated by Testclutch |
Without this patch, the handling of the alt-svc header added via 279a477 in curl-8.13.0 attempts to connect to alternative services via different HTTP versions, even if the target HTTP version is not supported by curl (i.e., not enabled at compile-time). If I understand the code and RFC 7838 correctly, then we should only attempt to migrate to supported protocols. Therefore, `allowed_apns` should only contain such protocols, and we need to guard its modification with `ifdefs` for supported HTTP versions. This was discovered in a downstream bug report in Alpine Linux [1] where it was reported that a Matrix client (using libcurl) was defunct after the upgrade to curl-8.13.0. Further debugging revealed that this was due to the Matrix server sending a `alt-svc: h3=":443";` HTTP header, causing curl to attempt migration to HTTP3 even though Alpine's curl version is compiled without HTTP3 support. I am not sure if this is the best place in the code to address this or if the `allowed` bitmask shouldn't contain unsupported versions in the first place. However, since there are existing `ifdefs` in this function for source (not destination) ALP selection, it may be a good fit to address this here. [1]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/17062
icing
approved these changes
Apr 15, 2025
|
Thanks! |
algitbot
pushed a commit
to alpinelinux/aports
that referenced
this pull request
Apr 16, 2025
bell-sw
pushed a commit
to bell-sw/alpaquita-aports
that referenced
this pull request
Apr 17, 2025
[ commit c9a9f0c0dc24618d718f6b9c0233da278213c12a ] See curl/curl#17037 Fixes #17062
nbaws
pushed a commit
to nbaws/curl
that referenced
this pull request
Apr 26, 2025
Without this patch, the handling of the alt-svc header added via 279a477 in curl-8.13.0 attempts to connect to alternative services via different HTTP versions, even if the target HTTP version is not supported by curl (i.e., not enabled at compile-time). If I understand the code and RFC 7838 correctly, then we should only attempt to migrate to supported protocols. Therefore, `allowed_apns` should only contain such protocols, and we need to guard its modification with `ifdefs` for supported HTTP versions. This was discovered in a downstream bug report in Alpine Linux [1] where it was reported that a Matrix client (using libcurl) was defunct after the upgrade to curl-8.13.0. Further debugging revealed that this was due to the Matrix server sending a `alt-svc: h3=":443";` HTTP header, causing curl to attempt migration to HTTP3 even though Alpine's curl version is compiled without HTTP3 support. I am not sure if this is the best place in the code to address this or if the `allowed` bitmask shouldn't contain unsupported versions in the first place. However, since there are existing `ifdefs` in this function for source (not destination) ALP selection, it may be a good fit to address this here. [1]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/17062 Closes curl#17037
nbaws
pushed a commit
to nbaws/curl
that referenced
this pull request
Apr 26, 2025
Without this patch, the handling of the alt-svc header added via 279a477 in curl-8.13.0 attempts to connect to alternative services via different HTTP versions, even if the target HTTP version is not supported by curl (i.e., not enabled at compile-time). If I understand the code and RFC 7838 correctly, then we should only attempt to migrate to supported protocols. Therefore, `allowed_apns` should only contain such protocols, and we need to guard its modification with `ifdefs` for supported HTTP versions. This was discovered in a downstream bug report in Alpine Linux [1] where it was reported that a Matrix client (using libcurl) was defunct after the upgrade to curl-8.13.0. Further debugging revealed that this was due to the Matrix server sending a `alt-svc: h3=":443";` HTTP header, causing curl to attempt migration to HTTP3 even though Alpine's curl version is compiled without HTTP3 support. I am not sure if this is the best place in the code to address this or if the `allowed` bitmask shouldn't contain unsupported versions in the first place. However, since there are existing `ifdefs` in this function for source (not destination) ALP selection, it may be a good fit to address this here. [1]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/17062 Closes curl#17037
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.
Without this patch, the handling of the alt-svc header added via 279a477 (CC: @icing) in curl-8.13.0 attempts to connect to alternative services via different HTTP versions, even if the target HTTP version is not supported by curl (i.e., not enabled at compile-time). If I understand the code and RFC 7838 correctly, then we should only attempt to migrate to supported protocols. Therefore,
allowed_apnsshould only contain such protocols, and we need to guard its modification withifdefsfor supported HTTP versions.This was discovered in a downstream bug report in Alpine Linux where it was reported that a Matrix client (using libcurl) was defunct after the upgrade to curl-8.13.0. Further debugging revealed that this was due to the Matrix server sending a
alt-svc: h3=":443";HTTP header, causing curl to attempt migration to HTTP3 even though Alpine's curl version is compiled without HTTP3 support.I am not sure if this is the best place in the code to address this or if the
allowedbitmask shouldn't contain unsupported versions in the first place. However, since there are existingifdefsin this function for source (not destination) ALP selection, it may be a good fit to address this here.Applying this patch resolves the aforementioned downstream issue.