Skip to content

Default ssl contexts per http versions. - #3340

Closed
T-256 wants to merge 1 commit into
encode:version-1.0from
T-256:default-context
Closed

Default ssl contexts per http versions.#3340
T-256 wants to merge 1 commit into
encode:version-1.0from
T-256:default-context

Conversation

@T-256

@T-256T-256 commented Oct 9, 2024

Copy link
Copy Markdown
Contributor

Summary

Ref: #3022 (review)

Instead of creating ssl context per client, we now re-use available default contexts for different http versions configurations.

TODO

  • add logging
  • add tests
  • make it thread safe

Comment threadhttpx/_config.py Outdated
Comment on lines +133 to +135
def from_defaults(
cls, http1: bool = True, http2: bool = False
) -> "SSLContext":

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

should it be thread safe?

Comment threadhttpx/_config.py
Comment on lines +142 to +145
alpn_idents = []
if http1:
alpn_idents.append("http/1.1")
if http2:
alpn_idents.append("h2")
context.set_alpn_protocols(alpn_idents)

@T-256T-256Oct 9, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

previously, it was only using http2, was there any reason?

ifssl.HAS_ALPN:
alpn_idents= ["http/1.1", "h2"] ifhttp2else ["http/1.1"]
context.set_alpn_protocols(alpn_idents)

@lovelydinosaur

Copy link
Copy Markdown
Contributor

Heya. Gonna suggest we have a design discussion before considering this an issue..

@T-256

T-256 commented Oct 9, 2024

Copy link
Copy Markdown
ContributorAuthor

Done, #3341

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@T-256@lovelydinosaur