reuse existing feature provider-tls-s2n

Author: boquan-fang

.github/workflows/ci.yml

@@ -415,16 +415,16 @@ jobs:
         run: |
           find . -name 'corpus.tar.gz' -exec dirname {} ';' | xargs -L 1 bash -c 'cd "$0" && rm -rf corpus && tar xf corpus.tar.gz'
 
-      # Force s2n-tls as the default TLS provider so the whole suite runs through s2n-tls.
+      # Exercise s2n-tls on Windows. s2n-quic-tests enables `provider-tls-s2n`, which overrides
+      # `provider-tls-default` and makes s2n-tls the default TLS provider on the GNU/MinGW target for tests.
       - name: Run cargo test
         shell: msys2 {0}
         run: |
           set -eu
           cargo test --workspace \
             --exclude s2n-quic-dc \
             --exclude s2n-quic-dc-benches \
-            --exclude s2n-quic-dc-metrics \
-            --features s2n-tls-default
+            --exclude s2n-quic-dc-metrics
 
   asan:
     runs-on: ubuntu-latest

quic/s2n-quic-tests/Cargo.toml

@@ -10,11 +10,6 @@ license = "Apache-2.0"
 # this only contains internal tests and should not be published
 publish = false
 
-[features]
-# Run the integration tests using s2n-tls as the default TLS provider instead of the platform
-# default. Intended for exercising s2n-tls on Windows with the GNU/MinGW toolchain.
-s2n-tls-default = ["s2n-quic/provider-tls-default-s2n"]
-
 [dependencies]
 bach = "0.1.0"
 bytes = { version = "1", default-features = false }
@@ -37,7 +32,9 @@ zerocopy = { version = "0.8", features = ["derive"] }
 quiche = "0.29"
 
 # s2n-tls is required by ch_callback_server_local_address_test. It builds on unix and on Windows
-# with the GNU/MinGW toolchain (target_env = "gnu"), but not with MSVC.
+# with the GNU/MinGW toolchain (target_env = "gnu"), but not with MSVC. Enabling `provider-tls-s2n`
+# makes s2n-tls the default provider on these targets (it overrides `provider-tls-default`), so the
+# whole suite exercises s2n-tls.
 [target.'cfg(any(unix, all(target_os = "windows", target_env = "gnu")))'.dependencies]
 s2n-tls = "0.3.40"
 s2n-quic = { path = "../s2n-quic", features = ["provider-event-tracing", "provider-tls-s2n", "unstable-provider-io-testing", "unstable-provider-dc", "unstable-provider-packet-interceptor", "unstable-provider-random", "unstable-offload-tls", "unstable_client_hello"] }

quic/s2n-quic-tests/src/tests/buffer_limit.rs

@@ -99,9 +99,9 @@ fn buffer_limit_test() {
     // Rustls emits INTERNAL_ERROR and S2N-TLS emits UNEXPECTED_MESSAGE error
     // when the server close the connection due to large Client Hello.
     //
-    // rustls is the default TLS provider on Windows, unless the `s2n-tls-default` feature forces
-    // s2n-tls (e.g. when exercising s2n-tls on Windows with the GNU/MinGW toolchain).
-    let expected_error = if cfg!(target_os = "windows") && !cfg!(feature = "s2n-tls-default") {
+    // rustls is only the default TLS provider on Windows with the MSVC toolchain. On unix and on
+    // Windows with the GNU/MinGW toolchain (target_env = "gnu"), s2n-tls is the default.
+    let expected_error = if cfg!(all(target_os = "windows", target_env = "msvc")) {
         TlsError::INTERNAL_ERROR
     } else {
         TlsError::UNEXPECTED_MESSAGE

quic/s2n-quic-tls-default/Cargo.toml

@@ -16,10 +16,6 @@ exclude = ["corpus.tar.gz"]
 # preserves the selective compilation of the two tls crates.
 fips = ["s2n-quic-tls?/fips"]
 
-# Force s2n-tls to be the default TLS provider even on targets where rustls is the default.
-# It is primarily useful for exercising s2n-tls on Windows with GNU/MinGW toolchain.
-s2n-tls-default = ["dep:s2n-quic-tls"]
-
 # Declare `s2n-quic-tls` as an optional dependency since the `?` syntax for features requires
 # the dependency be optional.
 #

quic/s2n-quic-tls-default/src/lib.rs

@@ -1,10 +1,7 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-// By default s2n-tls is the default provider on unix and rustls elsewhere (e.g. Windows).
-// Enabling the `s2n-tls-default` feature forces s2n-tls as the default on every target where it
-// builds, which is primarily used to exercise s2n-tls on Windows.
-#[cfg(all(not(unix), not(feature = "s2n-tls-default")))]
+#[cfg(not(unix))]
 pub use s2n_quic_rustls::*;
-#[cfg(any(unix, feature = "s2n-tls-default"))]
+#[cfg(unix)]
 pub use s2n_quic_tls::*;

quic/s2n-quic/Cargo.toml

@@ -34,9 +34,6 @@ provider-event-tracing = ["s2n-quic-core/event-tracing", "tracing"]
 provider-tls-default = ["s2n-quic-tls-default"]
 provider-tls-rustls = ["s2n-quic-rustls"]
 provider-tls-s2n = ["s2n-quic-tls"]
-# Force s2n-tls to be the default TLS provider even on targets where rustls is the default.
-# It is primarily useful for exercising s2n-tls on Windows with GNU/MinGW toolchain.
-provider-tls-default-s2n = ["provider-tls-default", "s2n-quic-tls-default/s2n-tls-default"]
 
 # List of unstable features. Add new unstable features to the check in s2n-quic/src/lib.rs
 #

quic/s2n-quic/src/provider/tls.rs

@@ -23,16 +23,19 @@ pub trait Provider {
 impl_provider_utils!();
 
 cfg_if! {
-    if #[cfg(feature = "provider-tls-default")] {
+    // An explicit provider selection overrides the platform default. `provider-tls-s2n` and
+    // `provider-tls-rustls` lets a consumer force a specific TLS implementation just by enabling its feature,
+    // without having to disable default features.
+    if #[cfg(feature = "provider-tls-s2n")] {
+        pub use s2n_tls as default;
+    } else if #[cfg(feature = "provider-tls-rustls")] {
+        pub use rustls as default;
+    } else if #[cfg(feature = "provider-tls-default")] {
         #[cfg_attr(docsrs, doc(cfg(feature = "provider-tls-default")))]
         pub mod default {
             //! Provides the recommended implementation of TLS using platform detection
             pub use super::default_tls::*;
         }
-    } else if #[cfg(feature = "provider-tls-s2n")] {
-        pub use s2n_tls as default;
-    } else if #[cfg(feature = "provider-tls-rustls")] {
-        pub use rustls as default;
     } else {
         pub mod default {
             //! Provides the recommended implementation of TLS using platform detection
@@ -190,7 +193,15 @@ impl Provider for &str {
     }
 }
 
-#[cfg(feature = "provider-tls-default")]
+// `default_tls` only backs the `default` module when `provider-tls-default` is the effective
+// choice, i.e. when neither `provider-tls-s2n` nor `provider-tls-rustls` overrides it (see the
+// cfg_if above). When an override is active, the re-export below would be unused, so we compile
+// the stub instead to avoid dead code.
+#[cfg(all(
+    feature = "provider-tls-default",
+    not(feature = "provider-tls-s2n"),
+    not(feature = "provider-tls-rustls")
+))]
 mod default_tls {
     pub use s2n_quic_tls_default::*;
 
@@ -199,19 +210,9 @@ mod default_tls {
     //
     // Note: I know this looks like a mess. And it is. Hopefully in the future cargo will support
     // platform-specific default features.
-    //
-    // The default provider is s2n-tls on unix and whenever `provider-tls-default-s2n` forces it;
-    // otherwise it is rustls. We only add the impl below when the default type doesn't already
-    // have one from the explicit `rustls`/`s2n_tls` provider modules, to avoid conflicting impls.
     #[cfg(not(any(
-        all(
-            not(any(unix, feature = "provider-tls-default-s2n")),
-            feature = "s2n-quic-rustls"
-        ),
-        all(
-            any(unix, feature = "provider-tls-default-s2n"),
-            feature = "s2n-quic-tls"
-        )
+        all(not(unix), feature = "s2n-quic-rustls"),
+        all(unix, feature = "s2n-quic-tls")
     )))]
     mod default_provider {
         use super::*;
@@ -246,7 +247,11 @@ mod default_tls {
         }
     }
 }
-#[cfg(not(feature = "provider-tls-default"))]
+#[cfg(not(all(
+    feature = "provider-tls-default",
+    not(feature = "provider-tls-s2n"),
+    not(feature = "provider-tls-rustls")
+)))]
 mod default_tls {
     // TODO stub out default that fails with error when started
 }