bpo-40968: Send http/1.1 ALPN extension by tiran · Pull Request #20959 · python/cpython
- It's a low-risk change. cURL has been setting the ALPN indicator for years.
- The change fixes a problem with servers that require an ALPN extension to indicate HTTP version.
- Need to figure out how to test the change.
tiran
mentioned this pull request
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are this comment and the functionality out of sync here since http_vsn_str can be HTTP/1.0, etc? http/1.1, http/1.0, and http/0.9 are all valid ALPN protocol IDs but the comment and changelogs only mention 1.1. This might be fine anyways but something I noticed.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm now checking _http_vsn and only send the ALPN extension when the version is 11 (HTTP/1.1).
tiran
marked this pull request as ready for review
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@tiran: Please replace # with GH- in the commit message next time. Thanks!
tiran
deleted the
bpo-40968-alpn
branch
adorilson pushed a commit to adorilson/cpython that referenced this pull request
pmenzel
mannequin
mentioned this pull request