From 023768c03fb9e87c7dc4614860c12b91ac56adc6 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sun, 14 Feb 2021 17:08:25 +0900 Subject: [PATCH] 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. --- .github/workflows/ci.yaml | 19 ++++++++++++-- Cargo.toml | 2 +- src/h1/mod.rs | 54 +++++++++++++++++++++++++-------------- src/h1/tls.rs | 37 +++++++++++++++------------ 4 files changed, 74 insertions(+), 38 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 53d1d6d..94dff6f 100644 --- a/.github/workflows/ci.yaml +++ b/.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 diff --git a/Cargo.toml b/Cargo.toml index 4cd8915..a36da97 100644 --- a/Cargo.toml +++ b/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" diff --git a/src/h1/mod.rs b/src/h1/mod.rs index 37811bc..f0a4df9 100644 --- a/src/h1/mod.rs +++ b/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>; +#[cfg(any(feature = "native-tls", feature = "rustls"))] type HttpsPool = DashMap, 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::>() + } else { + vec![] + }; + f.debug_struct("H1Client") .field( "http_pools", @@ -52,20 +74,7 @@ impl Debug for H1Client { }) .collect::>(), ) - .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::>(), - ) + .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,6 +117,7 @@ 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() @@ -113,7 +125,9 @@ impl HttpClient for H1Client { .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 diff --git a/src/h1/tls.rs b/src/h1/tls.rs index 1e7ddd6..8d7056a 100644 --- a/src/h1/tls.rs +++ b/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, Error> for TlsConnection { } } -#[cfg(feature = "native-tls")] -async fn add_tls( - host: &str, - stream: TcpStream, -) -> Result, async_native_tls::Error> { - async_native_tls::connect(host, stream).await -} - -#[cfg(feature = "rustls")] -async fn add_tls(host: &str, stream: TcpStream) -> Result, 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, 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, async_native_tls::Error> { + async_native_tls::connect(host, stream).await + } + } }