refactor(s2n-tls-hyper): Add HttpsConnector builder #4976
Merged
Conversation
This file contains 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
goatgoose
force-pushed
the
connector-builder
branch
from
126cef1 to
8a4db52
Compare
goatgoose
force-pushed
the
connector-builder
branch
3 times, most recently
from
4f952bd to
282d4cd
Compare
goatgoose
force-pushed
the
connector-builder
branch
from
282d4cd to
7c2f691
Compare
lrstewart
approved these changes
Dec 12, 2024
jmayclin
approved these changes
Dec 13, 2024
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.
Description of changes:
Currently HttpsConnectors are created with
HttpsConnector::newandHttpsConnector::new_with_http. This worked because nothing needed to be configured when creating an HttpsConnector.However, we're going to need a way to bypass TLS so that the same HttpsConnector can be used for both https and http endpoints. It's probably best to disallow this by default, and provide a way to opt-in to this behavior. So, we now need a way to configure HttpsConnectors when creating them. This PR adds an HttpsConnector Builder for this purpose.
Currently there are no builder options. I'll add the TLS bypass option in a followup PR.
Call-outs:
HttpsConnector::new_with_httpAPI in favor ofHttpsConnector::builder_with_http. HavingHttpsConnector::new_with_httpin addition to this API seemed excessive, so I removed it. The crate is still unstable so it seemed fine.HttpsConnector::newwasn't broken.Builderalready existed in the namespace because of thes2n_tls::connection::Builder. I renamed this type toConnBuilder.Testing:
Mostly a refactor. Existing tests should pass. I added a new test that compiles the various ways of creating an HttpsConnector, which makes sure they can be created without any weird manual type annotations.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.