From fb036264cc4ccaa9dd380b40f8b548a3353a7c36 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Sat, 18 Dec 2021 16:44:30 +0000 Subject: [PATCH] only build `SslConnectorBuilder` once (#2503) --- actix-http-test/src/lib.rs | 2 +- actix-test/src/lib.rs | 2 +- awc/CHANGES.md | 4 +++ awc/Cargo.toml | 2 +- awc/src/client/connector.rs | 69 ++++++++++++++++++++++++++---------- awc/tests/test_connector.rs | 2 +- awc/tests/test_ssl_client.rs | 2 +- tests/test_httpserver.rs | 2 +- 8 files changed, 61 insertions(+), 24 deletions(-) diff --git a/actix-http-test/src/lib.rs b/actix-http-test/src/lib.rs index e7e479ab2..03239ece6 100644 --- a/actix-http-test/src/lib.rs +++ b/actix-http-test/src/lib.rs @@ -107,7 +107,7 @@ pub async fn test_server_with_addr>( Connector::new() .conn_lifetime(Duration::from_secs(0)) .timeout(Duration::from_millis(30000)) - .ssl(builder.build()) + .openssl(builder.build()) }; #[cfg(not(feature = "openssl"))] diff --git a/actix-test/src/lib.rs b/actix-test/src/lib.rs index 3808ba69a..f86120f2f 100644 --- a/actix-test/src/lib.rs +++ b/actix-test/src/lib.rs @@ -340,7 +340,7 @@ where Connector::new() .conn_lifetime(Duration::from_secs(0)) .timeout(Duration::from_millis(30000)) - .ssl(builder.build()) + .openssl(builder.build()) } #[cfg(not(feature = "openssl"))] { diff --git a/awc/CHANGES.md b/awc/CHANGES.md index ef0f9faaa..7b822930c 100644 --- a/awc/CHANGES.md +++ b/awc/CHANGES.md @@ -1,6 +1,10 @@ # Changes ## Unreleased - 2021-xx-xx +* Rename `Connector::{ssl => openssl}`. [#2503] +* Improve `Client` instantiation efficiency when using `openssl` by only building connectors once. [#2503] + +[#2503]: https://github.com/actix/actix-web/pull/2503 ## 3.0.0-beta.14 - 2021-12-17 diff --git a/awc/Cargo.toml b/awc/Cargo.toml index 4cab20d05..f9a541c7e 100644 --- a/awc/Cargo.toml +++ b/awc/Cargo.toml @@ -62,7 +62,7 @@ actix-codec = "0.4.1" actix-service = "2.0.0" actix-http = "3.0.0-beta.16" actix-rt = { version = "2.1", default-features = false } -actix-tls = { version = "3.0.0-rc.1", features = ["connect", "uri"] } +actix-tls = { version = "3.0.0-rc.2", features = ["connect", "uri"] } actix-utils = "3.0.0" ahash = "0.7" diff --git a/awc/src/client/connector.rs b/awc/src/client/connector.rs index 40b3c4d32..423f656a8 100644 --- a/awc/src/client/connector.rs +++ b/awc/src/client/connector.rs @@ -22,11 +22,13 @@ use futures_core::{future::LocalBoxFuture, ready}; use http::Uri; use pin_project_lite::pin_project; -use super::config::ConnectorConfig; -use super::connection::{Connection, ConnectionIo}; -use super::error::ConnectError; -use super::pool::ConnectionPool; -use super::Connect; +use super::{ + config::ConnectorConfig, + connection::{Connection, ConnectionIo}, + error::ConnectError, + pool::ConnectionPool, + Connect, +}; enum OurTlsConnector { #[allow(dead_code)] // only dead when no TLS feature is enabled @@ -35,6 +37,12 @@ enum OurTlsConnector { #[cfg(feature = "openssl")] Openssl(actix_tls::connect::openssl::reexports::SslConnector), + /// Provided because building the OpenSSL context on newer versions can be very slow. + /// This prevents unnecessary calls to `.build()` while constructing the client connector. + #[cfg(feature = "openssl")] + #[allow(dead_code)] // false positive; used in build_ssl + OpensslBuilder(actix_tls::connect::openssl::reexports::SslConnectorBuilder), + #[cfg(feature = "rustls")] Rustls(std::sync::Arc), } @@ -57,7 +65,7 @@ pub struct Connector { config: ConnectorConfig, #[allow(dead_code)] // only dead when no TLS feature is enabled - ssl: OurTlsConnector, + tls: OurTlsConnector, } impl Connector<()> { @@ -72,7 +80,7 @@ impl Connector<()> { Connector { connector: TcpConnector::new(resolver::resolver()).service(), config: ConnectorConfig::default(), - ssl: Self::build_ssl(vec![b"h2".to_vec(), b"http/1.1".to_vec()]), + tls: Self::build_ssl(vec![b"h2".to_vec(), b"http/1.1".to_vec()]), } } @@ -116,7 +124,7 @@ impl Connector<()> { log::error!("Can not set ALPN protocol: {:?}", err); } - OurTlsConnector::Openssl(ssl.build()) + OurTlsConnector::OpensslBuilder(ssl) } } @@ -134,7 +142,7 @@ impl Connector { Connector { connector, config: self.config, - ssl: self.ssl, + tls: self.tls, } } } @@ -167,23 +175,35 @@ where self } + /// Use custom OpenSSL `SslConnector` instance. #[cfg(feature = "openssl")] - /// Use custom `SslConnector` instance. + pub fn openssl( + mut self, + connector: actix_tls::connect::openssl::reexports::SslConnector, + ) -> Self { + self.tls = OurTlsConnector::Openssl(connector); + self + } + + /// See docs for [`Connector::openssl`]. + #[doc(hidden)] + #[cfg(feature = "openssl")] + #[deprecated(since = "3.0.0", note = "Renamed to `Connector::openssl`.")] pub fn ssl( mut self, connector: actix_tls::connect::openssl::reexports::SslConnector, ) -> Self { - self.ssl = OurTlsConnector::Openssl(connector); + self.tls = OurTlsConnector::Openssl(connector); self } + /// Use custom Rustls `ClientConfig` instance. #[cfg(feature = "rustls")] - /// Use custom `ClientConfig` instance. pub fn rustls( mut self, connector: std::sync::Arc, ) -> Self { - self.ssl = OurTlsConnector::Rustls(connector); + self.tls = OurTlsConnector::Rustls(connector); self } @@ -198,7 +218,7 @@ where unimplemented!("actix-http client only supports versions http/1.1 & http/2") } }; - self.ssl = Connector::build_ssl(versions); + self.tls = Connector::build_ssl(versions); self } @@ -270,8 +290,8 @@ where } /// Finish configuration process and create connector service. - /// The Connector builder always concludes by calling `finish()` last in - /// its combinator chain. + /// + /// The `Connector` builder always concludes by calling `finish()` last in its combinator chain. pub fn finish(self) -> ConnectorService { let local_address = self.config.local_address; let timeout = self.config.timeout; @@ -284,7 +304,15 @@ where service: tcp_service_inner.clone(), }; - let tls_service = match self.ssl { + let tls = match self.tls { + #[cfg(feature = "openssl")] + OurTlsConnector::OpensslBuilder(builder) => { + OurTlsConnector::Openssl(builder.build()) + } + tls => tls, + }; + + let tls_service = match tls { OurTlsConnector::None => { #[cfg(not(feature = "dangerous-h2c"))] { @@ -374,6 +402,11 @@ where Some(actix_service::boxed::rc_service(tls_service)) } + #[cfg(feature = "openssl")] + OurTlsConnector::OpensslBuilder(_) => { + unreachable!("OpenSSL builder is built before this match."); + } + #[cfg(feature = "rustls")] OurTlsConnector::Rustls(tls) => { const H2: &[u8] = b"h2"; @@ -853,7 +886,7 @@ mod tests { let connector = Connector { connector: TcpConnector::new(resolver::resolver()).service(), config: ConnectorConfig::default(), - ssl: OurTlsConnector::None, + tls: OurTlsConnector::None, }; let client = Client::builder().connector(connector).finish(); diff --git a/awc/tests/test_connector.rs b/awc/tests/test_connector.rs index 588c51463..0f0b81414 100644 --- a/awc/tests/test_connector.rs +++ b/awc/tests/test_connector.rs @@ -58,7 +58,7 @@ async fn test_connection_window_size() { .map_err(|e| log::error!("Can not set alpn protocol: {:?}", e)); let client = awc::Client::builder() - .connector(awc::Connector::new().ssl(builder.build())) + .connector(awc::Connector::new().openssl(builder.build())) .initial_window_size(100) .initial_connection_window_size(100) .finish(); diff --git a/awc/tests/test_ssl_client.rs b/awc/tests/test_ssl_client.rs index 811efd4bc..40c9ab8f0 100644 --- a/awc/tests/test_ssl_client.rs +++ b/awc/tests/test_ssl_client.rs @@ -72,7 +72,7 @@ async fn test_connection_reuse_h2() { .map_err(|e| log::error!("Can not set alpn protocol: {:?}", e)); let client = awc::Client::builder() - .connector(awc::Connector::new().ssl(builder.build())) + .connector(awc::Connector::new().openssl(builder.build())) .finish(); // req 1 diff --git a/tests/test_httpserver.rs b/tests/test_httpserver.rs index 887b51d41..464a650a2 100644 --- a/tests/test_httpserver.rs +++ b/tests/test_httpserver.rs @@ -121,7 +121,7 @@ async fn test_start_ssl() { let client = awc::Client::builder() .connector( awc::Connector::new() - .ssl(builder.build()) + .openssl(builder.build()) .timeout(Duration::from_millis(100)), ) .finish();