Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: builder with shared configuration #261

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"rust-analyzer.cargo.features": "all"
}
Comment on lines +1 to +3
Copy link
Member

Choose a reason for hiding this comment

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

Was this part of the diff intentional? I think we'd prefer to see a .vscode addition to .gitignore in a stand-alone commit and this file backed out. There isn't an established precedent for committing IDE settings to the Rustls repos AFAIK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unintentional, but it might take me a bit to get to it now I've got some life stuff occuring

Copy link
Member

Choose a reason for hiding this comment

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

No worries, best of luck.

27 changes: 17 additions & 10 deletions src/connector/builder.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::sync::Arc;

use hyper_util::client::legacy::connect::HttpConnector;
#[cfg(any(feature = "rustls-native-certs", feature = "webpki-roots"))]
use rustls::crypto::CryptoProvider;
Expand Down Expand Up @@ -35,20 +37,24 @@ impl ConnectorBuilder<WantsTlsConfig> {
Self(WantsTlsConfig(()))
}

/// Passes a rustls [`ClientConfig`] to configure the TLS connection
/// Passes a rustls [`ClientConfig`] to configure the TLS connection.
///
/// The [`alpn_protocols`](ClientConfig::alpn_protocols) field is
/// required to be empty (or the function will panic) and will be
/// rewritten to match the enabled schemes (see
/// [`enable_http1`](ConnectorBuilder::enable_http1),
/// [`enable_http2`](ConnectorBuilder::enable_http2)) before the
/// connector is built.
pub fn with_tls_config(self, config: ClientConfig) -> ConnectorBuilder<WantsSchemes> {
pub fn with_tls_config(
self,
config: impl Into<Arc<ClientConfig>>,
) -> ConnectorBuilder<WantsSchemes> {
let tls_config = config.into();
assert!(
config.alpn_protocols.is_empty(),
tls_config.alpn_protocols.is_empty(),
"ALPN protocols should not be pre-defined"
);
ConnectorBuilder(WantsSchemes { tls_config: config })
ConnectorBuilder(WantsSchemes { tls_config })
}

/// Use rustls' default crypto provider and other defaults, and the platform verifier
Expand Down Expand Up @@ -133,7 +139,7 @@ impl Default for ConnectorBuilder<WantsTlsConfig> {
/// State of a builder that needs schemes (https:// and http://) to be
/// configured next
pub struct WantsSchemes {
tls_config: ClientConfig,
tls_config: Arc<ClientConfig>,
}

impl ConnectorBuilder<WantsSchemes> {
Expand Down Expand Up @@ -166,7 +172,7 @@ impl ConnectorBuilder<WantsSchemes> {
///
/// No protocol has been enabled at this point.
pub struct WantsProtocols1 {
tls_config: ClientConfig,
tls_config: Arc<ClientConfig>,
https_only: bool,
override_server_name: Option<String>,
}
Expand All @@ -176,7 +182,7 @@ impl WantsProtocols1 {
HttpsConnector {
force_https: self.https_only,
http: conn,
tls_config: std::sync::Arc::new(self.tls_config),
tls_config: self.tls_config,
override_server_name: self.override_server_name,
}
}
Expand All @@ -203,7 +209,7 @@ impl ConnectorBuilder<WantsProtocols1> {
/// This needs to be called explicitly, no protocol is enabled by default
#[cfg(feature = "http2")]
pub fn enable_http2(mut self) -> ConnectorBuilder<WantsProtocols3> {
self.0.tls_config.alpn_protocols = vec![b"h2".to_vec()];
Arc::make_mut(&mut self.0.tls_config).alpn_protocols = vec![b"h2".to_vec()];
ConnectorBuilder(WantsProtocols3 {
inner: self.0,
enable_http1: false,
Expand All @@ -221,7 +227,7 @@ impl ConnectorBuilder<WantsProtocols1> {
#[cfg(not(feature = "http1"))]
let alpn_protocols = vec![b"h2".to_vec()];

self.0.tls_config.alpn_protocols = alpn_protocols;
Arc::make_mut(&mut self.0.tls_config).alpn_protocols = alpn_protocols;
ConnectorBuilder(WantsProtocols3 {
inner: self.0,
enable_http1: cfg!(feature = "http1"),
Expand Down Expand Up @@ -259,7 +265,8 @@ impl ConnectorBuilder<WantsProtocols2> {
/// This needs to be called explicitly, no protocol is enabled by default
#[cfg(feature = "http2")]
pub fn enable_http2(mut self) -> ConnectorBuilder<WantsProtocols3> {
self.0.inner.tls_config.alpn_protocols = vec![b"h2".to_vec(), b"http/1.1".to_vec()];
Arc::make_mut(&mut self.0.inner.tls_config).alpn_protocols =
vec![b"h2".to_vec(), b"http/1.1".to_vec()];
ConnectorBuilder(WantsProtocols3 {
inner: self.0.inner,
enable_http1: true,
Expand Down
Loading