Skip to content

Commit

Permalink
client: simplify ClientConfigBuilder init
Browse files Browse the repository at this point in the history
We can't derive `Default` on `ClientConfigBuilder` because we want to
enable SNI by default. Instead, implement it by hand. This allows
simplifying the construction sites to use the default except for the
~1/2 fields needing to be customized.

Along the way, change where we initialize the default protocol versions.
Previously we did this in `rustls_client_config_builder_new()`, but when
we introduce ECH config it will be beneficial to know if the user has
customized protocol versions using
`rustls_client_config_builder_new_custom()` or just wants sensible
defaults. This is easier to deduce if we defer populating the default
protocol values to the time of `rustls_client_config_builder_build()`
when we find the builder's protocol versions vec to be empty, indicating
defaults are desired (explicitly customizing configuration for no
protocols makes no sense).
  • Loading branch information
cpu committed Dec 12, 2024
1 parent 0bc4d10 commit 5dfdc6e
Showing 1 changed file with 27 additions and 12 deletions.
39 changes: 27 additions & 12 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,25 @@ pub(crate) struct ClientConfigBuilder {
key_log: Option<Arc<dyn KeyLog>>,
}

impl Default for ClientConfigBuilder {
fn default() -> Self {
Self {
provider: None,
// Note: we populate default versions at build-time with the Rustls defaults.
// We leave it empty for the default builder to be able to distinguish when
// a caller has customized w/ `rustls_client_config_builder_new_custom`.
versions: Vec::new(),
verifier: None,
alpn_protocols: Vec::new(),
// Note: we don't derive Default for this struct because we want to enable SNI
// by default.
enable_sni: true,
cert_resolver: None,
key_log: None,
}
}
}

arc_castable! {
/// A client config that is done being constructed and is now read-only.
///
Expand Down Expand Up @@ -81,12 +100,7 @@ impl rustls_client_config_builder {
ffi_panic_boundary! {
let builder = ClientConfigBuilder {
provider: crypto_provider::get_default_or_install_from_crate_features(),
versions: rustls::DEFAULT_VERSIONS.to_vec(),
verifier: None,
cert_resolver: None,
alpn_protocols: vec![],
enable_sni: true,
key_log: None,
..ClientConfigBuilder::default()
};
to_boxed_mut_ptr(builder)
}
Expand Down Expand Up @@ -136,11 +150,7 @@ impl rustls_client_config_builder {
let config_builder = ClientConfigBuilder {
provider: Some(provider),
versions,
verifier: None,
cert_resolver: None,
alpn_protocols: vec![],
enable_sni: true,
key_log: None,
..ClientConfigBuilder::default()
};

set_boxed_mut_ptr(builder_out, config_builder);
Expand Down Expand Up @@ -540,8 +550,13 @@ impl rustls_client_config_builder {
None => return rustls_result::NoServerCertVerifier,
};

let versions = match builder.versions.is_empty() {
true => rustls::DEFAULT_VERSIONS,
false => builder.versions.as_slice(),
};

let config = match ClientConfig::builder_with_provider(provider)
.with_protocol_versions(&builder.versions)
.with_protocol_versions(versions)
{
Ok(c) => c,
Err(err) => return map_error(err),
Expand Down

0 comments on commit 5dfdc6e

Please sign in to comment.