◐ Shell
clean mode source ↗

bpo-40968: Send http/1.1 ALPN extension by tiran · Pull Request #20959 · python/cpython

@tiran

  • 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 tiran mentioned this pull request

Jun 18, 2020

sethmlarson

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).

Signed-off-by: Christian Heimes <christian@python.org>

@tiran tiran marked this pull request as ready for review

November 13, 2020 09:05

sethmlarson

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bedevere-bot

@tiran: Please replace # with GH- in the commit message next time. Thanks!

@tiran tiran deleted the bpo-40968-alpn branch

November 13, 2020 15:37

adorilson pushed a commit to adorilson/cpython that referenced this pull request

Mar 13, 2021
Signed-off-by: Christian Heimes <christian@python.org>

@pmenzel pmenzel mannequin mentioned this pull request

May 10, 2023