Fix h1_client feature

* When both "native-tls" and "rustls" features are disabled, disable https.
* When both "native-tls" and "rustls" features are enabled, prefer "rustls".
  This is the same behavior as http-client 6.2.0.
  Ideally, it would be nice if this behavior could be controlled by
  environment variables or some other way.

This also adds a CI task to check all feature combinations work properly.

Author: taiki-e

.github/workflows/ci.yaml

@@ -33,7 +33,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@master
-    
+
       - name: check
         run: cargo check --no-default-features
 
@@ -51,7 +51,7 @@ jobs:
         override: true
         components: clippy, rustfmt
 
-    - name: clippy 
+    - name: clippy
       run: cargo clippy --all-targets --workspace --features=docs
 
     - name: fmt
@@ -79,3 +79,18 @@ jobs:
       with:
         command: check
         args: --target wasm32-unknown-unknown --no-default-features --features "native_client,wasm_client"
+
+  check_features:
+    name: Check feature combinations
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@master
+
+    - name: Install cargo-hack
+      run: cargo install cargo-hack
+
+    - name: Check all feature combinations works properly
+      # * `--feature-powerset` - run for the feature powerset of the package
+      # * `--no-dev-deps` - build without dev-dependencies to avoid https://github.com/rust-lang/cargo/issues/4866
+      # * `--skip docs` - skip `docs` feature
+      run: cargo hack check --feature-powerset --no-dev-deps --skip docs

Cargo.toml

@@ -37,6 +37,7 @@ async-trait = "0.1.37"
 dashmap = "4.0.2"
 http-types = "2.3.0"
 log = "0.4.7"
+cfg-if = "1.0.0"
 
 # h1_client
 async-h1 = { version = "2.0.0", optional = true }
@@ -91,5 +92,4 @@ tide-rustls = { version = "0.1.4" }
 tokio = { version = "0.2.21", features = ["macros"] }
 serde = "1.0"
 serde_json = "1.0"
-cfg-if = "0.1.10"
 mockito = "0.23.3"

src/h1/mod.rs

@@ -9,34 +9,56 @@ use dashmap::DashMap;
 use deadpool::managed::Pool;
 use http_types::StatusCode;
 
-#[cfg(feature = "native-tls")]
-use async_native_tls::TlsStream;
-#[cfg(feature = "rustls")]
-use async_tls::client::TlsStream;
+cfg_if::cfg_if! {
+    if #[cfg(feature = "rustls")] {
+        use async_tls::client::TlsStream;
+    } else if #[cfg(feature = "native-tls")] {
+        use async_native_tls::TlsStream;
+    }
+}
 
 use super::{async_trait, Error, HttpClient, Request, Response};
 
 mod tcp;
+#[cfg(any(feature = "native-tls", feature = "rustls"))]
 mod tls;
 
 use tcp::{TcpConnWrapper, TcpConnection};
+#[cfg(any(feature = "native-tls", feature = "rustls"))]
 use tls::{TlsConnWrapper, TlsConnection};
 
 // This number is based on a few random benchmarks and see whatever gave decent perf vs resource use.
 const DEFAULT_MAX_CONCURRENT_CONNECTIONS: usize = 50;
 
 type HttpPool = DashMap<SocketAddr, Pool<TcpStream, std::io::Error>>;
+#[cfg(any(feature = "native-tls", feature = "rustls"))]
 type HttpsPool = DashMap<SocketAddr, Pool<TlsStream<TcpStream>, Error>>;
 
 /// Async-h1 based HTTP Client, with connecton pooling ("Keep-Alive").
 pub struct H1Client {
     http_pools: HttpPool,
+    #[cfg(any(feature = "native-tls", feature = "rustls"))]
     https_pools: HttpsPool,
     max_concurrent_connections: usize,
 }
 
 impl Debug for H1Client {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let https_pools = if cfg!(any(feature = "native-tls", feature = "rustls")) {
+            self.http_pools
+                .iter()
+                .map(|pool| {
+                    let status = pool.status();
+                    format!(
+                        "Connections: {}, Available: {}, Max: {}",
+                        status.size, status.available, status.max_size
+                    )
+                })
+                .collect::<Vec<String>>()
+        } else {
+            vec![]
+        };
+
         f.debug_struct("H1Client")
             .field(
                 "http_pools",
@@ -52,20 +74,7 @@ impl Debug for H1Client {
                     })
                     .collect::<Vec<String>>(),
             )
-            .field(
-                "https_pools",
-                &self
-                    .http_pools
-                    .iter()
-                    .map(|pool| {
-                        let status = pool.status();
-                        format!(
-                            "Connections: {}, Available: {}, Max: {}",
-                            status.size, status.available, status.max_size
-                        )
-                    })
-                    .collect::<Vec<String>>(),
-            )
+            .field("https_pools", &https_pools)
             .field(
                 "max_concurrent_connections",
                 &self.max_concurrent_connections,
@@ -85,6 +94,7 @@ impl H1Client {
     pub fn new() -> Self {
         Self {
             http_pools: DashMap::new(),
+            #[cfg(any(feature = "native-tls", feature = "rustls"))]
             https_pools: DashMap::new(),
             max_concurrent_connections: DEFAULT_MAX_CONCURRENT_CONNECTIONS,
         }
@@ -94,6 +104,7 @@ impl H1Client {
     pub fn with_max_connections(max: usize) -> Self {
         Self {
             http_pools: DashMap::new(),
+            #[cfg(any(feature = "native-tls", feature = "rustls"))]
             https_pools: DashMap::new(),
             max_concurrent_connections: max,
         }
@@ -106,14 +117,17 @@ impl HttpClient for H1Client {
         req.insert_header("Connection", "keep-alive");
 
         // Insert host
+        #[cfg(any(feature = "native-tls", feature = "rustls"))]
         let host = req
             .url()
             .host_str()
             .ok_or_else(|| Error::from_str(StatusCode::BadRequest, "missing hostname"))?
             .to_string();
 
         let scheme = req.url().scheme();
-        if scheme != "http" && scheme != "https" {
+        if scheme != "http"
+            && (scheme != "https" || cfg!(not(any(feature = "native-tls", feature = "rustls"))))
+        {
             return Err(Error::from_str(
                 StatusCode::BadRequest,
                 format!("invalid url scheme '{}'", scheme),
@@ -124,6 +138,7 @@ impl HttpClient for H1Client {
             .url()
             .socket_addrs(|| match req.url().scheme() {
                 "http" => Some(80),
+                #[cfg(any(feature = "native-tls", feature = "rustls"))]
                 "https" => Some(443),
                 _ => None,
             })?
@@ -156,6 +171,7 @@ impl HttpClient for H1Client {
                 req.set_local_addr(stream.local_addr().ok());
                 client::connect(TcpConnWrapper::new(stream), req).await
             }
+            #[cfg(any(feature = "native-tls", feature = "rustls"))]
             "https" => {
                 let pool_ref = if let Some(pool_ref) = self.https_pools.get(&addr) {
                     pool_ref

src/h1/tls.rs

@@ -8,10 +8,13 @@ use deadpool::managed::{Manager, Object, RecycleResult};
 use futures::io::{AsyncRead, AsyncWrite};
 use futures::task::{Context, Poll};
 
-#[cfg(feature = "native-tls")]
-use async_native_tls::TlsStream;
-#[cfg(feature = "rustls")]
-use async_tls::client::TlsStream;
+cfg_if::cfg_if! {
+    if #[cfg(feature = "rustls")] {
+        use async_tls::client::TlsStream;
+    } else if #[cfg(feature = "native-tls")] {
+        use async_native_tls::TlsStream;
+    }
+}
 
 use crate::Error;
 
@@ -76,16 +79,18 @@ impl Manager<TlsStream<TcpStream>, Error> for TlsConnection {
     }
 }
 
-#[cfg(feature = "native-tls")]
-async fn add_tls(
-    host: &str,
-    stream: TcpStream,
-) -> Result<TlsStream<TcpStream>, async_native_tls::Error> {
-    async_native_tls::connect(host, stream).await
-}
-
-#[cfg(feature = "rustls")]
-async fn add_tls(host: &str, stream: TcpStream) -> Result<TlsStream<TcpStream>, std::io::Error> {
-    let connector = async_tls::TlsConnector::default();
-    connector.connect(host, stream).await
+cfg_if::cfg_if! {
+    if #[cfg(feature = "rustls")] {
+        async fn add_tls(host: &str, stream: TcpStream) -> Result<TlsStream<TcpStream>, std::io::Error> {
+            let connector = async_tls::TlsConnector::default();
+            connector.connect(host, stream).await
+        }
+    } else if #[cfg(feature = "native-tls")] {
+        async fn add_tls(
+            host: &str,
+            stream: TcpStream,
+        ) -> Result<TlsStream<TcpStream>, async_native_tls::Error> {
+            async_native_tls::connect(host, stream).await
+        }
+    }
 }