From 0587208eb858d459358789ef5361912ea8dea186 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 15 Nov 2023 11:57:00 -0800 Subject: [PATCH 1/6] Re-order impls for HttpsConnector --- src/connector.rs | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/connector.rs b/src/connector.rs index 712c32bb..2951107c 100644 --- a/src/connector.rs +++ b/src/connector.rs @@ -33,28 +33,6 @@ impl HttpsConnector { } } -impl fmt::Debug for HttpsConnector { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("HttpsConnector") - .field("force_https", &self.force_https) - .finish() - } -} - -impl From<(H, C)> for HttpsConnector -where - C: Into>, -{ - fn from((http, cfg): (H, C)) -> Self { - Self { - force_https: false, - http, - tls_config: cfg.into(), - override_server_name: None, - } - } -} - impl Service for HttpsConnector where T: Service, @@ -139,3 +117,25 @@ where } } } + +impl From<(H, C)> for HttpsConnector +where + C: Into>, +{ + fn from((http, cfg): (H, C)) -> Self { + Self { + force_https: false, + http, + tls_config: cfg.into(), + override_server_name: None, + } + } +} + +impl fmt::Debug for HttpsConnector { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("HttpsConnector") + .field("force_https", &self.force_https) + .finish() + } +} From 8a5e67e725a6ee11f5661e54742e01154500f471 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 15 Nov 2023 11:58:44 -0800 Subject: [PATCH 2/6] Clean up imports --- src/acceptor.rs | 6 ++---- src/acceptor/builder.rs | 1 + src/connector.rs | 4 +++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/acceptor.rs b/src/acceptor.rs index 037b64ed..02e72d00 100644 --- a/src/acceptor.rs +++ b/src/acceptor.rs @@ -5,10 +5,8 @@ use std::pin::Pin; use std::sync::Arc; use futures_util::ready; -use hyper::server::{ - accept::Accept, - conn::{AddrIncoming, AddrStream}, -}; +use hyper::server::accept::Accept; +use hyper::server::conn::{AddrIncoming, AddrStream}; use rustls::{ServerConfig, ServerConnection}; use tokio::io::{AsyncRead, AsyncWrite, ReadBuf}; diff --git a/src/acceptor/builder.rs b/src/acceptor/builder.rs index a4b22317..ec14f17d 100644 --- a/src/acceptor/builder.rs +++ b/src/acceptor/builder.rs @@ -5,6 +5,7 @@ use pki_types::{CertificateDer, PrivateKeyDer}; use rustls::ServerConfig; use super::TlsAcceptor; + /// Builder for [`TlsAcceptor`] pub struct AcceptorBuilder(State); diff --git a/src/connector.rs b/src/connector.rs index 2951107c..7e0921f7 100644 --- a/src/connector.rs +++ b/src/connector.rs @@ -4,7 +4,9 @@ use std::sync::Arc; use std::task::{Context, Poll}; use std::{fmt, io}; -use hyper::{client::connect::Connection, service::Service, Uri}; +use hyper::client::connect::Connection; +use hyper::service::Service; +use hyper::Uri; use pki_types::ServerName; use tokio::io::{AsyncRead, AsyncWrite}; use tokio_rustls::TlsConnector; From 38c405e508060bfccaaf85adf8be60bd1ee15823 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 15 Nov 2023 12:05:16 -0800 Subject: [PATCH 3/6] Invert Connector scheme matching to reduce rightward drift --- src/connector.rs | 109 ++++++++++++++++++++++++----------------------- 1 file changed, 56 insertions(+), 53 deletions(-) diff --git a/src/connector.rs b/src/connector.rs index 7e0921f7..44f97631 100644 --- a/src/connector.rs +++ b/src/connector.rs @@ -60,61 +60,64 @@ where fn call(&mut self, dst: Uri) -> Self::Future { // dst.scheme() would need to derive Eq to be matchable; // use an if cascade instead - if let Some(sch) = dst.scheme() { - if sch == &http::uri::Scheme::HTTP && !self.force_https { - let connecting_future = self.http.call(dst); - - let f = async move { - let tcp = connecting_future - .await - .map_err(Into::into)?; - - Ok(MaybeHttpsStream::Http(tcp)) - }; - Box::pin(f) - } else if sch == &http::uri::Scheme::HTTPS { - let cfg = self.tls_config.clone(); - let mut hostname = match self.override_server_name.as_deref() { - Some(h) => h, - None => dst.host().unwrap_or_default(), - }; - - // Remove square brackets around IPv6 address. - if let Some(trimmed) = hostname - .strip_prefix('[') - .and_then(|h| h.strip_suffix(']')) - { - hostname = trimmed; - } - - let hostname = match ServerName::try_from(hostname) { - Ok(dnsname) => dnsname.to_owned(), - Err(_) => { - let err = io::Error::new(io::ErrorKind::Other, "invalid dnsname"); - return Box::pin(async move { Err(Box::new(err).into()) }); - } - }; - let connecting_future = self.http.call(dst); - - let f = async move { - let tcp = connecting_future - .await - .map_err(Into::into)?; - let connector = TlsConnector::from(cfg); - let tls = connector - .connect(hostname, tcp) - .await - .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; - Ok(MaybeHttpsStream::Https(tls)) - }; - Box::pin(f) - } else { - let err = - io::Error::new(io::ErrorKind::Other, format!("Unsupported scheme {}", sch)); - Box::pin(async move { Err(err.into()) }) + let scheme = match dst.scheme() { + Some(scheme) => scheme, + None => { + return Box::pin(async move { + Err(io::Error::new(io::ErrorKind::Other, "missing scheme").into()) + }) + } + }; + + if scheme == &http::uri::Scheme::HTTP && !self.force_https { + let connecting_future = self.http.call(dst); + + let f = async move { + let tcp = connecting_future + .await + .map_err(Into::into)?; + + Ok(MaybeHttpsStream::Http(tcp)) + }; + Box::pin(f) + } else if scheme == &http::uri::Scheme::HTTPS { + let cfg = self.tls_config.clone(); + let mut hostname = match self.override_server_name.as_deref() { + Some(h) => h, + None => dst.host().unwrap_or_default(), + }; + + // Remove square brackets around IPv6 address. + if let Some(trimmed) = hostname + .strip_prefix('[') + .and_then(|h| h.strip_suffix(']')) + { + hostname = trimmed; } + + let hostname = match ServerName::try_from(hostname) { + Ok(dns_name) => dns_name.to_owned(), + Err(_) => { + let err = io::Error::new(io::ErrorKind::Other, "invalid dnsname"); + return Box::pin(async move { Err(Box::new(err).into()) }); + } + }; + let connecting_future = self.http.call(dst); + + let f = async move { + let tcp = connecting_future + .await + .map_err(Into::into)?; + let connector = TlsConnector::from(cfg); + let tls = connector + .connect(hostname, tcp) + .await + .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; + Ok(MaybeHttpsStream::Https(tls)) + }; + Box::pin(f) } else { - let err = io::Error::new(io::ErrorKind::Other, "Missing scheme"); + let err = io::Error::new(io::ErrorKind::Other, format!("Unsupported scheme {scheme}")); Box::pin(async move { Err(err.into()) }) } } From 1ca96960549ca0d62a13eeaade222a7becff7ed2 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 15 Nov 2023 12:09:33 -0800 Subject: [PATCH 4/6] Inline expressions in Connector HTTP scheme handling --- src/connector.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/connector.rs b/src/connector.rs index 44f97631..1b8f6e89 100644 --- a/src/connector.rs +++ b/src/connector.rs @@ -70,16 +70,8 @@ where }; if scheme == &http::uri::Scheme::HTTP && !self.force_https { - let connecting_future = self.http.call(dst); - - let f = async move { - let tcp = connecting_future - .await - .map_err(Into::into)?; - - Ok(MaybeHttpsStream::Http(tcp)) - }; - Box::pin(f) + let future = self.http.call(dst); + Box::pin(async move { Ok(MaybeHttpsStream::Http(future.await.map_err(Into::into)?)) }) } else if scheme == &http::uri::Scheme::HTTPS { let cfg = self.tls_config.clone(); let mut hostname = match self.override_server_name.as_deref() { From b7ae3498acbf14bf48e7852fe9d2e5401905a4c8 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 15 Nov 2023 12:20:27 -0800 Subject: [PATCH 5/6] Reduce rightward drift by handling simple schemes --- src/connector.rs | 90 +++++++++++++++++++++++++----------------------- 1 file changed, 47 insertions(+), 43 deletions(-) diff --git a/src/connector.rs b/src/connector.rs index 1b8f6e89..03f4daba 100644 --- a/src/connector.rs +++ b/src/connector.rs @@ -60,8 +60,20 @@ where fn call(&mut self, dst: Uri) -> Self::Future { // dst.scheme() would need to derive Eq to be matchable; // use an if cascade instead - let scheme = match dst.scheme() { - Some(scheme) => scheme, + match dst.scheme() { + Some(scheme) if scheme == &http::uri::Scheme::HTTP => { + let future = self.http.call(dst); + return Box::pin(async move { + Ok(MaybeHttpsStream::Http(future.await.map_err(Into::into)?)) + }); + } + Some(scheme) if scheme != &http::uri::Scheme::HTTPS => { + let message = format!("unsupported scheme {scheme}"); + return Box::pin(async move { + Err(io::Error::new(io::ErrorKind::Other, message).into()) + }); + } + Some(_) => {} None => { return Box::pin(async move { Err(io::Error::new(io::ErrorKind::Other, "missing scheme").into()) @@ -69,49 +81,41 @@ where } }; - if scheme == &http::uri::Scheme::HTTP && !self.force_https { - let future = self.http.call(dst); - Box::pin(async move { Ok(MaybeHttpsStream::Http(future.await.map_err(Into::into)?)) }) - } else if scheme == &http::uri::Scheme::HTTPS { - let cfg = self.tls_config.clone(); - let mut hostname = match self.override_server_name.as_deref() { - Some(h) => h, - None => dst.host().unwrap_or_default(), - }; - - // Remove square brackets around IPv6 address. - if let Some(trimmed) = hostname - .strip_prefix('[') - .and_then(|h| h.strip_suffix(']')) - { - hostname = trimmed; - } + let cfg = self.tls_config.clone(); + let mut hostname = match self.override_server_name.as_deref() { + Some(h) => h, + None => dst.host().unwrap_or_default(), + }; - let hostname = match ServerName::try_from(hostname) { - Ok(dns_name) => dns_name.to_owned(), - Err(_) => { - let err = io::Error::new(io::ErrorKind::Other, "invalid dnsname"); - return Box::pin(async move { Err(Box::new(err).into()) }); - } - }; - let connecting_future = self.http.call(dst); - - let f = async move { - let tcp = connecting_future - .await - .map_err(Into::into)?; - let connector = TlsConnector::from(cfg); - let tls = connector - .connect(hostname, tcp) - .await - .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; - Ok(MaybeHttpsStream::Https(tls)) - }; - Box::pin(f) - } else { - let err = io::Error::new(io::ErrorKind::Other, format!("Unsupported scheme {scheme}")); - Box::pin(async move { Err(err.into()) }) + // Remove square brackets around IPv6 address. + if let Some(trimmed) = hostname + .strip_prefix('[') + .and_then(|h| h.strip_suffix(']')) + { + hostname = trimmed; } + + let hostname = match ServerName::try_from(hostname) { + Ok(dns_name) => dns_name.to_owned(), + Err(_) => { + let err = io::Error::new(io::ErrorKind::Other, "invalid dnsname"); + return Box::pin(async move { Err(Box::new(err).into()) }); + } + }; + let connecting_future = self.http.call(dst); + + let f = async move { + let tcp = connecting_future + .await + .map_err(Into::into)?; + let connector = TlsConnector::from(cfg); + let tls = connector + .connect(hostname, tcp) + .await + .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; + Ok(MaybeHttpsStream::Https(tls)) + }; + Box::pin(f) } } From c59fcd8f217e96bf1451a998a6b73c868c9f3ec8 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 15 Nov 2023 13:19:50 -0800 Subject: [PATCH 6/6] Inline some single-use bindings --- src/connector.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/connector.rs b/src/connector.rs index 03f4daba..6ce366ca 100644 --- a/src/connector.rs +++ b/src/connector.rs @@ -102,20 +102,19 @@ where return Box::pin(async move { Err(Box::new(err).into()) }); } }; - let connecting_future = self.http.call(dst); - let f = async move { + let connecting_future = self.http.call(dst); + Box::pin(async move { let tcp = connecting_future .await .map_err(Into::into)?; - let connector = TlsConnector::from(cfg); - let tls = connector - .connect(hostname, tcp) - .await - .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; - Ok(MaybeHttpsStream::Https(tls)) - }; - Box::pin(f) + Ok(MaybeHttpsStream::Https( + TlsConnector::from(cfg) + .connect(hostname, tcp) + .await + .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?, + )) + }) } }