Skip to content

Conversation

tiran
Copy link
Member

@tiran tiran commented Jun 18, 2020

Signed-off-by: Christian Heimes [email protected]

https://bugs.python.org/issue40968

@tiran
Copy link
Member Author

tiran commented Jun 18, 2020

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

@@ -1377,6 +1377,8 @@ def __init__(self, host, port=None, key_file=None, cert_file=None,
self.cert_file = cert_file
if context is None:
context = ssl._create_default_https_context()
# send ALPN extension to indicate HTTP/1.1 protocol
Copy link
Contributor

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.

Copy link
Member Author

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 tiran added type-feature A feature request or enhancement and removed needs backport to 3.7 needs backport to 3.9 only security fixes type-bug An unexpected behavior, bug, or error labels Nov 13, 2020
@tiran tiran marked this pull request as ready for review November 13, 2020 09:05
Copy link
Contributor

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

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

LGTM

@tiran tiran merged commit f97406b into python:master Nov 13, 2020
@bedevere-bot
Copy link

@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants