From 5ccba219aa07a7226e51afde283311287a3c2795 Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Mon, 11 Nov 2024 10:34:37 -0500 Subject: [PATCH 01/13] move timeout middleware to its own module --- .../aws-smithy-http-client/src/client.rs | 6 + .../src/client/timeout.rs | 316 ++++++++++++ .../aws-smithy-http-client/src/hyper_1.rs | 463 +++--------------- .../aws-smithy-http-client/src/lib.rs | 2 + 4 files changed, 401 insertions(+), 386 deletions(-) create mode 100644 rust-runtime/aws-smithy-http-client/src/client.rs create mode 100644 rust-runtime/aws-smithy-http-client/src/client/timeout.rs diff --git a/rust-runtime/aws-smithy-http-client/src/client.rs b/rust-runtime/aws-smithy-http-client/src/client.rs new file mode 100644 index 0000000000..b685e98707 --- /dev/null +++ b/rust-runtime/aws-smithy-http-client/src/client.rs @@ -0,0 +1,6 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +pub(crate) mod timeout; diff --git a/rust-runtime/aws-smithy-http-client/src/client/timeout.rs b/rust-runtime/aws-smithy-http-client/src/client/timeout.rs new file mode 100644 index 0000000000..56c3f153d5 --- /dev/null +++ b/rust-runtime/aws-smithy-http-client/src/client/timeout.rs @@ -0,0 +1,316 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +use std::error::Error; +use std::fmt::Formatter; +use std::future::Future; +use std::pin::Pin; +use std::task::{Context, Poll}; +use std::time::Duration; + +use http_1x::Uri; +use pin_project_lite::pin_project; + +use aws_smithy_async::future::timeout::{TimedOutError, Timeout}; +use aws_smithy_async::rt::sleep::Sleep; +use aws_smithy_async::rt::sleep::{AsyncSleep, SharedAsyncSleep}; +use aws_smithy_runtime_api::box_error::BoxError; + +#[derive(Debug)] +pub(crate) struct HttpTimeoutError { + kind: &'static str, + duration: Duration, +} + +impl std::fmt::Display for HttpTimeoutError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{} timeout occurred after {:?}", + self.kind, self.duration + ) + } +} + +impl Error for HttpTimeoutError { + // We implement the `source` function as returning a `TimedOutError` because when `downcast_error` + // or `find_source` is called with an `HttpTimeoutError` (or another error wrapping an `HttpTimeoutError`) + // this method will be checked to determine if it's a timeout-related error. + fn source(&self) -> Option<&(dyn Error + 'static)> { + Some(&TimedOutError) + } +} + +/// Timeout wrapper that will timeout on the initial TCP connection +/// +/// # Stability +/// This interface is unstable. +#[derive(Clone, Debug)] +pub(crate) struct ConnectTimeout { + inner: I, + timeout: Option<(SharedAsyncSleep, Duration)>, +} + +impl ConnectTimeout { + /// Create a new `ConnectTimeout` around `inner`. + /// + /// Typically, `I` will implement [`hyper_util::client::legacy::connect::Connect`]. + pub(crate) fn new(inner: I, sleep: SharedAsyncSleep, timeout: Duration) -> Self { + Self { + inner, + timeout: Some((sleep, timeout)), + } + } + + pub(crate) fn no_timeout(inner: I) -> Self { + Self { + inner, + timeout: None, + } + } +} + +#[derive(Clone, Debug)] +pub(crate) struct HttpReadTimeout { + inner: I, + timeout: Option<(SharedAsyncSleep, Duration)>, +} + +impl HttpReadTimeout { + /// Create a new `HttpReadTimeout` around `inner`. + /// + /// Typically, `I` will implement [`tower::Service>`]. + pub(crate) fn new(inner: I, sleep: SharedAsyncSleep, timeout: Duration) -> Self { + Self { + inner, + timeout: Some((sleep, timeout)), + } + } + + pub(crate) fn no_timeout(inner: I) -> Self { + Self { + inner, + timeout: None, + } + } +} + +pin_project! { + /// Timeout future for Tower services + /// + /// Timeout future to handle timing out, mapping errors, and the possibility of not timing out + /// without incurring an additional allocation for each timeout layer. + #[project = MaybeTimeoutFutureProj] + pub enum MaybeTimeoutFuture { + Timeout { + #[pin] + timeout: Timeout, + error_type: &'static str, + duration: Duration, + }, + NoTimeout { + #[pin] + future: F + } + } +} + +impl Future for MaybeTimeoutFuture +where + F: Future>, + E: Into, +{ + type Output = Result; + + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + let (timeout_future, kind, &mut duration) = match self.project() { + MaybeTimeoutFutureProj::NoTimeout { future } => { + return future.poll(cx).map_err(|err| err.into()); + } + MaybeTimeoutFutureProj::Timeout { + timeout, + error_type, + duration, + } => (timeout, error_type, duration), + }; + match timeout_future.poll(cx) { + Poll::Ready(Ok(response)) => Poll::Ready(response.map_err(|err| err.into())), + Poll::Ready(Err(_timeout)) => { + Poll::Ready(Err(HttpTimeoutError { kind, duration }.into())) + } + Poll::Pending => Poll::Pending, + } + } +} + +impl tower::Service for ConnectTimeout +where + I: tower::Service, + I::Error: Into, +{ + type Response = I::Response; + type Error = BoxError; + type Future = MaybeTimeoutFuture; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx).map_err(|err| err.into()) + } + + fn call(&mut self, req: Uri) -> Self::Future { + match &self.timeout { + Some((sleep, duration)) => { + let sleep = sleep.sleep(*duration); + MaybeTimeoutFuture::Timeout { + timeout: Timeout::new(self.inner.call(req), sleep), + error_type: "HTTP connect", + duration: *duration, + } + } + None => MaybeTimeoutFuture::NoTimeout { + future: self.inner.call(req), + }, + } + } +} + +impl tower::Service> for HttpReadTimeout +where + I: tower::Service>, + I::Error: Send + Sync + Error + 'static, +{ + type Response = I::Response; + type Error = BoxError; + type Future = MaybeTimeoutFuture; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx).map_err(|err| err.into()) + } + + fn call(&mut self, req: http_1x::Request) -> Self::Future { + match &self.timeout { + Some((sleep, duration)) => { + let sleep = sleep.sleep(*duration); + MaybeTimeoutFuture::Timeout { + timeout: Timeout::new(self.inner.call(req), sleep), + error_type: "HTTP read", + duration: *duration, + } + } + None => MaybeTimeoutFuture::NoTimeout { + future: self.inner.call(req), + }, + } + } +} + +#[cfg(test)] +pub(crate) mod test { + use hyper::rt::ReadBufCursor; + use hyper_util::client::legacy::connect::{Connected, Connection}; + use hyper_util::rt::TokioIo; + use tokio::net::TcpStream; + + use aws_smithy_async::future::never::Never; + + use aws_smithy_runtime_api::box_error::BoxError; + use aws_smithy_runtime_api::client::result::ConnectorError; + use http::Uri; + use hyper::http; + use hyper::rt::{Read, Write}; + use std::future::Future; + use std::pin::Pin; + use std::task::{Context, Poll}; + + #[allow(unused)] + fn connect_timeout_is_correct() { + is_send_sync::>(); + } + + #[allow(unused)] + fn is_send_sync() {} + + /// A service that will never return whatever it is you want + /// + /// Returned futures will return Pending forever + #[non_exhaustive] + #[derive(Clone, Default, Debug)] + pub(crate) struct NeverConnects; + impl tower::Service for NeverConnects { + type Response = TokioIo; + type Error = ConnectorError; + type Future = Pin> + Send>>; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, _uri: Uri) -> Self::Future { + Box::pin(async move { + Never::new().await; + unreachable!() + }) + } + } + + /// A service that will connect but never send any data + #[derive(Clone, Debug, Default)] + pub(crate) struct NeverReplies; + impl tower::Service for NeverReplies { + type Response = EmptyStream; + type Error = BoxError; + type Future = std::future::Ready>; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, _req: Uri) -> Self::Future { + std::future::ready(Ok(EmptyStream)) + } + } + + /// A stream that will never return or accept any data + #[non_exhaustive] + #[derive(Debug, Default)] + pub(crate) struct EmptyStream; + impl Read for EmptyStream { + fn poll_read( + self: Pin<&mut Self>, + _cx: &mut Context<'_>, + _buf: ReadBufCursor<'_>, + ) -> Poll> { + Poll::Pending + } + } + impl Write for EmptyStream { + fn poll_write( + self: Pin<&mut Self>, + _cx: &mut Context<'_>, + _buf: &[u8], + ) -> Poll> { + Poll::Pending + } + + fn poll_flush( + self: Pin<&mut Self>, + _cx: &mut Context<'_>, + ) -> Poll> { + Poll::Pending + } + + fn poll_shutdown( + self: Pin<&mut Self>, + _cx: &mut Context<'_>, + ) -> Poll> { + Poll::Pending + } + } + + impl Connection for EmptyStream { + fn connected(&self) -> Connected { + Connected::new() + } + } +} diff --git a/rust-runtime/aws-smithy-http-client/src/hyper_1.rs b/rust-runtime/aws-smithy-http-client/src/hyper_1.rs index 3d6c8100cb..94c78af6d0 100644 --- a/rust-runtime/aws-smithy-http-client/src/hyper_1.rs +++ b/rust-runtime/aws-smithy-http-client/src/hyper_1.rs @@ -46,6 +46,8 @@ use std::task::{Context, Poll}; use std::time::Duration; use std::{fmt, vec}; +use crate::client::timeout; + /// Choice of underlying cryptography library #[derive(Debug, Eq, PartialEq, Clone, Copy)] #[non_exhaustive] @@ -313,23 +315,23 @@ impl HyperConnectorBuilder { .unwrap_or((None, None)); let connector = match connect_timeout { - Some(duration) => timeout_middleware::ConnectTimeout::new( + Some(duration) => timeout::ConnectTimeout::new( tcp_connector, sleep_impl .clone() .expect("a sleep impl must be provided in order to have a connect timeout"), duration, ), - None => timeout_middleware::ConnectTimeout::no_timeout(tcp_connector), + None => timeout::ConnectTimeout::no_timeout(tcp_connector), }; let base = client_builder.build(connector); let read_timeout = match read_timeout { - Some(duration) => timeout_middleware::HttpReadTimeout::new( + Some(duration) => timeout::HttpReadTimeout::new( base, sleep_impl.expect("a sleep impl must be provided in order to have a read timeout"), duration, ), - None => timeout_middleware::HttpReadTimeout::no_timeout(base), + None => timeout::HttpReadTimeout::no_timeout(base), }; HyperConnector { adapter: Box::new(Adapter { @@ -398,8 +400,8 @@ impl HyperConnectorBuilder { /// /// This adapter also enables TCP `CONNECT` and HTTP `READ` timeouts via [`HyperConnector::builder`]. struct Adapter { - client: timeout_middleware::HttpReadTimeout< - hyper_util::client::legacy::Client, SdkBody>, + client: timeout::HttpReadTimeout< + hyper_util::client::legacy::Client, SdkBody>, >, } @@ -442,7 +444,7 @@ where C: Clone + Send + Sync + 'static, C: tower::Service, C::Response: Connection + Read + Write + Unpin + 'static, - timeout_middleware::ConnectTimeout: Connect, + timeout::ConnectTimeout: Connect, C::Future: Unpin + Send + 'static, C::Error: Into, { @@ -500,7 +502,7 @@ fn downcast_error(err: BoxError) -> ConnectorError { /// Convert a [`hyper::Error`] into a [`ConnectorError`] fn to_connector_error(err: &hyper::Error) -> fn(BoxError) -> ConnectorError { - if err.is_timeout() || find_source::(err).is_some() { + if err.is_timeout() || find_source::(err).is_some() { return ConnectorError::timeout; } if err.is_user() { @@ -727,380 +729,6 @@ where }) } -mod timeout_middleware { - use std::error::Error; - use std::fmt::Formatter; - use std::future::Future; - use std::pin::Pin; - use std::task::{Context, Poll}; - use std::time::Duration; - - use http_1x::Uri; - use pin_project_lite::pin_project; - - use aws_smithy_async::future::timeout::{TimedOutError, Timeout}; - use aws_smithy_async::rt::sleep::Sleep; - use aws_smithy_async::rt::sleep::{AsyncSleep, SharedAsyncSleep}; - use aws_smithy_runtime_api::box_error::BoxError; - - #[derive(Debug)] - pub(crate) struct HttpTimeoutError { - kind: &'static str, - duration: Duration, - } - - impl std::fmt::Display for HttpTimeoutError { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!( - f, - "{} timeout occurred after {:?}", - self.kind, self.duration - ) - } - } - - impl Error for HttpTimeoutError { - // We implement the `source` function as returning a `TimedOutError` because when `downcast_error` - // or `find_source` is called with an `HttpTimeoutError` (or another error wrapping an `HttpTimeoutError`) - // this method will be checked to determine if it's a timeout-related error. - fn source(&self) -> Option<&(dyn Error + 'static)> { - Some(&TimedOutError) - } - } - - /// Timeout wrapper that will timeout on the initial TCP connection - /// - /// # Stability - /// This interface is unstable. - #[derive(Clone, Debug)] - pub(super) struct ConnectTimeout { - inner: I, - timeout: Option<(SharedAsyncSleep, Duration)>, - } - - impl ConnectTimeout { - /// Create a new `ConnectTimeout` around `inner`. - /// - /// Typically, `I` will implement [`hyper_util::client::legacy::connect::Connect`]. - pub(crate) fn new(inner: I, sleep: SharedAsyncSleep, timeout: Duration) -> Self { - Self { - inner, - timeout: Some((sleep, timeout)), - } - } - - pub(crate) fn no_timeout(inner: I) -> Self { - Self { - inner, - timeout: None, - } - } - } - - #[derive(Clone, Debug)] - pub(crate) struct HttpReadTimeout { - inner: I, - timeout: Option<(SharedAsyncSleep, Duration)>, - } - - impl HttpReadTimeout { - /// Create a new `HttpReadTimeout` around `inner`. - /// - /// Typically, `I` will implement [`tower::Service>`]. - pub(crate) fn new(inner: I, sleep: SharedAsyncSleep, timeout: Duration) -> Self { - Self { - inner, - timeout: Some((sleep, timeout)), - } - } - - pub(crate) fn no_timeout(inner: I) -> Self { - Self { - inner, - timeout: None, - } - } - } - - pin_project! { - /// Timeout future for Tower services - /// - /// Timeout future to handle timing out, mapping errors, and the possibility of not timing out - /// without incurring an additional allocation for each timeout layer. - #[project = MaybeTimeoutFutureProj] - pub enum MaybeTimeoutFuture { - Timeout { - #[pin] - timeout: Timeout, - error_type: &'static str, - duration: Duration, - }, - NoTimeout { - #[pin] - future: F - } - } - } - - impl Future for MaybeTimeoutFuture - where - F: Future>, - E: Into, - { - type Output = Result; - - fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - let (timeout_future, kind, &mut duration) = match self.project() { - MaybeTimeoutFutureProj::NoTimeout { future } => { - return future.poll(cx).map_err(|err| err.into()); - } - MaybeTimeoutFutureProj::Timeout { - timeout, - error_type, - duration, - } => (timeout, error_type, duration), - }; - match timeout_future.poll(cx) { - Poll::Ready(Ok(response)) => Poll::Ready(response.map_err(|err| err.into())), - Poll::Ready(Err(_timeout)) => { - Poll::Ready(Err(HttpTimeoutError { kind, duration }.into())) - } - Poll::Pending => Poll::Pending, - } - } - } - - impl tower::Service for ConnectTimeout - where - I: tower::Service, - I::Error: Into, - { - type Response = I::Response; - type Error = BoxError; - type Future = MaybeTimeoutFuture; - - fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { - self.inner.poll_ready(cx).map_err(|err| err.into()) - } - - fn call(&mut self, req: Uri) -> Self::Future { - match &self.timeout { - Some((sleep, duration)) => { - let sleep = sleep.sleep(*duration); - MaybeTimeoutFuture::Timeout { - timeout: Timeout::new(self.inner.call(req), sleep), - error_type: "HTTP connect", - duration: *duration, - } - } - None => MaybeTimeoutFuture::NoTimeout { - future: self.inner.call(req), - }, - } - } - } - - impl tower::Service> for HttpReadTimeout - where - I: tower::Service>, - I::Error: Send + Sync + Error + 'static, - { - type Response = I::Response; - type Error = BoxError; - type Future = MaybeTimeoutFuture; - - fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { - self.inner.poll_ready(cx).map_err(|err| err.into()) - } - - fn call(&mut self, req: http_1x::Request) -> Self::Future { - match &self.timeout { - Some((sleep, duration)) => { - let sleep = sleep.sleep(*duration); - MaybeTimeoutFuture::Timeout { - timeout: Timeout::new(self.inner.call(req), sleep), - error_type: "HTTP read", - duration: *duration, - } - } - None => MaybeTimeoutFuture::NoTimeout { - future: self.inner.call(req), - }, - } - } - } - - #[cfg(test)] - pub(crate) mod test { - use std::time::Duration; - - use hyper::rt::ReadBufCursor; - use hyper_util::client::legacy::connect::Connected; - use hyper_util::rt::TokioIo; - use tokio::net::TcpStream; - - use aws_smithy_async::assert_elapsed; - use aws_smithy_async::future::never::Never; - use aws_smithy_async::rt::sleep::{SharedAsyncSleep, TokioSleep}; - use aws_smithy_types::error::display::DisplayErrorContext; - - use super::super::*; - - #[allow(unused)] - fn connect_timeout_is_correct() { - is_send_sync::>(); - } - - #[allow(unused)] - fn is_send_sync() {} - - /// A service that will never return whatever it is you want - /// - /// Returned futures will return Pending forever - #[non_exhaustive] - #[derive(Clone, Default, Debug)] - pub(crate) struct NeverConnects; - impl tower::Service for NeverConnects { - type Response = TokioIo; - type Error = ConnectorError; - type Future = Pin> + Send>>; - - fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { - Poll::Ready(Ok(())) - } - - fn call(&mut self, _uri: Uri) -> Self::Future { - Box::pin(async move { - Never::new().await; - unreachable!() - }) - } - } - - /// A service that will connect but never send any data - #[derive(Clone, Debug, Default)] - struct NeverReplies; - impl tower::Service for NeverReplies { - type Response = EmptyStream; - type Error = BoxError; - type Future = std::future::Ready>; - - fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { - Poll::Ready(Ok(())) - } - - fn call(&mut self, _req: Uri) -> Self::Future { - std::future::ready(Ok(EmptyStream)) - } - } - - /// A stream that will never return or accept any data - #[non_exhaustive] - #[derive(Debug, Default)] - struct EmptyStream; - impl Read for EmptyStream { - fn poll_read( - self: Pin<&mut Self>, - _cx: &mut Context<'_>, - _buf: ReadBufCursor<'_>, - ) -> Poll> { - Poll::Pending - } - } - impl Write for EmptyStream { - fn poll_write( - self: Pin<&mut Self>, - _cx: &mut Context<'_>, - _buf: &[u8], - ) -> Poll> { - Poll::Pending - } - - fn poll_flush( - self: Pin<&mut Self>, - _cx: &mut Context<'_>, - ) -> Poll> { - Poll::Pending - } - - fn poll_shutdown( - self: Pin<&mut Self>, - _cx: &mut Context<'_>, - ) -> Poll> { - Poll::Pending - } - } - impl Connection for EmptyStream { - fn connected(&self) -> Connected { - Connected::new() - } - } - - #[tokio::test] - async fn http_connect_timeout_works() { - let tcp_connector = NeverConnects::default(); - let connector_settings = HttpConnectorSettings::builder() - .connect_timeout(Duration::from_secs(1)) - .build(); - let hyper = HyperConnector::builder() - .connector_settings(connector_settings) - .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) - .build(tcp_connector) - .adapter; - let now = tokio::time::Instant::now(); - tokio::time::pause(); - let resp = hyper - .call(HttpRequest::get("https://static-uri.com").unwrap()) - .await - .unwrap_err(); - assert!( - resp.is_timeout(), - "expected resp.is_timeout() to be true but it was false, resp == {:?}", - resp - ); - let message = DisplayErrorContext(&resp).to_string(); - let expected = - "timeout: client error (Connect): HTTP connect timeout occurred after 1s"; - assert!( - message.contains(expected), - "expected '{message}' to contain '{expected}'" - ); - assert_elapsed!(now, Duration::from_secs(1)); - } - - #[tokio::test] - async fn http_read_timeout_works() { - let tcp_connector = NeverReplies; - let connector_settings = HttpConnectorSettings::builder() - .connect_timeout(Duration::from_secs(1)) - .read_timeout(Duration::from_secs(2)) - .build(); - let hyper = HyperConnector::builder() - .connector_settings(connector_settings) - .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) - .build(tcp_connector) - .adapter; - let now = tokio::time::Instant::now(); - tokio::time::pause(); - let err = hyper - .call(HttpRequest::get("https://fake-uri.com").unwrap()) - .await - .unwrap_err(); - assert!( - err.is_timeout(), - "expected err.is_timeout() to be true but it was false, err == {err:?}", - ); - let message = format!("{}", DisplayErrorContext(&err)); - let expected = "timeout: HTTP read timeout occurred after 2s"; - assert!( - message.contains(expected), - "expected '{message}' to contain '{expected}'" - ); - assert_elapsed!(now, Duration::from_secs(2)); - } - } -} - #[cfg(test)] mod test { use std::io::{Error, ErrorKind}; @@ -1109,14 +737,15 @@ mod test { use std::sync::Arc; use std::task::{Context, Poll}; + use aws_smithy_async::assert_elapsed; + use aws_smithy_async::rt::sleep::TokioSleep; + use aws_smithy_async::time::SystemTimeSource; + use aws_smithy_runtime_api::client::runtime_components::RuntimeComponentsBuilder; use http_1x::Uri; use hyper::rt::ReadBufCursor; use hyper_util::client::legacy::connect::Connected; - use aws_smithy_async::time::SystemTimeSource; - use aws_smithy_runtime_api::client::runtime_components::RuntimeComponentsBuilder; - - use crate::hyper_1::timeout_middleware::test::NeverConnects; + use crate::client::timeout::test::NeverConnects; use super::*; @@ -1251,4 +880,66 @@ mod test { std::future::ready(Ok(self.inner.clone())) } } + + #[tokio::test] + async fn http_connect_timeout_works() { + let tcp_connector = NeverConnects::default(); + let connector_settings = HttpConnectorSettings::builder() + .connect_timeout(Duration::from_secs(1)) + .build(); + let hyper = HyperConnector::builder() + .connector_settings(connector_settings) + .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) + .build(tcp_connector) + .adapter; + let now = tokio::time::Instant::now(); + tokio::time::pause(); + let resp = hyper + .call(HttpRequest::get("https://static-uri.com").unwrap()) + .await + .unwrap_err(); + assert!( + resp.is_timeout(), + "expected resp.is_timeout() to be true but it was false, resp == {:?}", + resp + ); + let message = DisplayErrorContext(&resp).to_string(); + let expected = "timeout: client error (Connect): HTTP connect timeout occurred after 1s"; + assert!( + message.contains(expected), + "expected '{message}' to contain '{expected}'" + ); + assert_elapsed!(now, Duration::from_secs(1)); + } + + #[tokio::test] + async fn http_read_timeout_works() { + let tcp_connector = crate::client::timeout::test::NeverReplies; + let connector_settings = HttpConnectorSettings::builder() + .connect_timeout(Duration::from_secs(1)) + .read_timeout(Duration::from_secs(2)) + .build(); + let hyper = HyperConnector::builder() + .connector_settings(connector_settings) + .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) + .build(tcp_connector) + .adapter; + let now = tokio::time::Instant::now(); + tokio::time::pause(); + let err = hyper + .call(HttpRequest::get("https://fake-uri.com").unwrap()) + .await + .unwrap_err(); + assert!( + err.is_timeout(), + "expected err.is_timeout() to be true but it was false, err == {err:?}", + ); + let message = format!("{}", DisplayErrorContext(&err)); + let expected = "timeout: HTTP read timeout occurred after 2s"; + assert!( + message.contains(expected), + "expected '{message}' to contain '{expected}'" + ); + assert_elapsed!(now, Duration::from_secs(2)); + } } diff --git a/rust-runtime/aws-smithy-http-client/src/lib.rs b/rust-runtime/aws-smithy-http-client/src/lib.rs index 562b54bd4f..a3b57ec5ff 100644 --- a/rust-runtime/aws-smithy-http-client/src/lib.rs +++ b/rust-runtime/aws-smithy-http-client/src/lib.rs @@ -41,3 +41,5 @@ pub mod hyper_1; #[cfg(feature = "test-util")] pub mod test_util; + +pub(crate) mod client; From f909bc9a3fd76076f3fdcf8b9bbd0896f13bec8e Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Mon, 11 Nov 2024 11:37:36 -0500 Subject: [PATCH 02/13] move hyper1 impl into new client module --- .../examples/client-aws-lc.rs | 8 +- .../examples/client-ring.rs | 6 +- .../examples/custom-dns.rs | 4 +- .../aws-smithy-http-client/src/client.rs | 905 ++++++++++++++++- .../aws-smithy-http-client/src/client/dns.rs | 40 + .../aws-smithy-http-client/src/hyper_1.rs | 945 ------------------ .../aws-smithy-http-client/src/lib.rs | 12 +- .../src/test_util/never.rs | 2 +- .../src/test_util/wire.rs | 2 +- .../tests/smoke_test_clients.rs | 14 +- 10 files changed, 964 insertions(+), 974 deletions(-) create mode 100644 rust-runtime/aws-smithy-http-client/src/client/dns.rs delete mode 100644 rust-runtime/aws-smithy-http-client/src/hyper_1.rs diff --git a/rust-runtime/aws-smithy-http-client/examples/client-aws-lc.rs b/rust-runtime/aws-smithy-http-client/examples/client-aws-lc.rs index 50ee9edfd5..d727f03e25 100644 --- a/rust-runtime/aws-smithy-http-client/examples/client-aws-lc.rs +++ b/rust-runtime/aws-smithy-http-client/examples/client-aws-lc.rs @@ -3,18 +3,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -use aws_smithy_http_client::hyper_1::{CryptoMode, HyperClientBuilder}; +use aws_smithy_http_client::{Builder, CryptoMode}; #[tokio::main] async fn main() { // feature = crypto-aws-lc - let _client = HyperClientBuilder::new() - .crypto_mode(CryptoMode::AwsLc) - .build_https(); + let _client = Builder::new().crypto_mode(CryptoMode::AwsLc).build_https(); // feature = crypto-aws-lc-fips // A FIPS client can also be created. Note that this has a more complex build environment required. - let _client = HyperClientBuilder::new() + let _client = Builder::new() .crypto_mode(CryptoMode::AwsLcFips) .build_https(); } diff --git a/rust-runtime/aws-smithy-http-client/examples/client-ring.rs b/rust-runtime/aws-smithy-http-client/examples/client-ring.rs index 3d6df0c850..55393310e4 100644 --- a/rust-runtime/aws-smithy-http-client/examples/client-ring.rs +++ b/rust-runtime/aws-smithy-http-client/examples/client-ring.rs @@ -3,10 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -use aws_smithy_http_client::hyper_1::{CryptoMode, HyperClientBuilder}; +use aws_smithy_http_client::{Builder, CryptoMode}; fn main() { - let _client = HyperClientBuilder::new() - .crypto_mode(CryptoMode::Ring) - .build_https(); + let _client = Builder::new().crypto_mode(CryptoMode::Ring).build_https(); } diff --git a/rust-runtime/aws-smithy-http-client/examples/custom-dns.rs b/rust-runtime/aws-smithy-http-client/examples/custom-dns.rs index 6e6a08b1a0..e083f8eb8e 100644 --- a/rust-runtime/aws-smithy-http-client/examples/custom-dns.rs +++ b/rust-runtime/aws-smithy-http-client/examples/custom-dns.rs @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -use aws_smithy_http_client::hyper_1::{CryptoMode, HyperClientBuilder}; +use aws_smithy_http_client::{Builder, CryptoMode}; use aws_smithy_runtime_api::client::dns::{DnsFuture, ResolveDns}; use std::net::{IpAddr, Ipv4Addr}; @@ -17,7 +17,7 @@ impl ResolveDns for StaticResolver { } fn main() { - let _client = HyperClientBuilder::new() + let _client = Builder::new() .crypto_mode(CryptoMode::Ring) .build_with_resolver(StaticResolver); } diff --git a/rust-runtime/aws-smithy-http-client/src/client.rs b/rust-runtime/aws-smithy-http-client/src/client.rs index b685e98707..8e3bcb7d1d 100644 --- a/rust-runtime/aws-smithy-http-client/src/client.rs +++ b/rust-runtime/aws-smithy-http-client/src/client.rs @@ -3,4 +3,907 @@ * SPDX-License-Identifier: Apache-2.0 */ -pub(crate) mod timeout; +mod dns; +mod timeout; + +use aws_smithy_async::future::timeout::TimedOutError; +use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep, SharedAsyncSleep}; +use aws_smithy_runtime_api::box_error::BoxError; +use aws_smithy_runtime_api::client::connection::CaptureSmithyConnection; +use aws_smithy_runtime_api::client::connection::ConnectionMetadata; +use aws_smithy_runtime_api::client::connector_metadata::ConnectorMetadata; +use aws_smithy_runtime_api::client::dns::ResolveDns; +use aws_smithy_runtime_api::client::http::{ + HttpClient, HttpConnector, HttpConnectorFuture, HttpConnectorSettings, SharedHttpClient, + SharedHttpConnector, +}; +use aws_smithy_runtime_api::client::orchestrator::{HttpRequest, HttpResponse}; +use aws_smithy_runtime_api::client::result::ConnectorError; +use aws_smithy_runtime_api::client::runtime_components::{ + RuntimeComponents, RuntimeComponentsBuilder, +}; +use aws_smithy_runtime_api::shared::IntoShared; +use aws_smithy_types::body::SdkBody; +use aws_smithy_types::config_bag::ConfigBag; +use aws_smithy_types::error::display::DisplayErrorContext; +use aws_smithy_types::retry::ErrorKind; +use client::connect::Connection; +use h2::Reason; +use http_1x::{Extensions, Uri}; +use hyper::rt::{Read, Write}; +use hyper_util::client::legacy as client; +use hyper_util::client::legacy::connect::{ + capture_connection, CaptureConnection, Connect, HttpInfo, +}; +use hyper_util::rt::TokioExecutor; +use rustls::crypto::CryptoProvider; +use std::borrow::Cow; +use std::collections::HashMap; +use std::error::Error; +use std::fmt; +use std::sync::RwLock; +use std::time::Duration; + +/// Choice of underlying cryptography library +#[derive(Debug, Eq, PartialEq, Clone, Copy)] +#[non_exhaustive] +pub enum CryptoMode { + /// Crypto based on [ring](https://github.com/briansmith/ring) + #[cfg(feature = "crypto-ring")] + Ring, + /// Crypto based on [aws-lc](https://github.com/aws/aws-lc-rs) + #[cfg(feature = "crypto-aws-lc")] + AwsLc, + /// FIPS compliant variant of [aws-lc](https://github.com/aws/aws-lc-rs) + #[cfg(feature = "crypto-aws-lc-fips")] + AwsLcFips, +} + +impl CryptoMode { + fn provider(self) -> CryptoProvider { + match self { + #[cfg(feature = "crypto-aws-lc")] + CryptoMode::AwsLc => rustls::crypto::aws_lc_rs::default_provider(), + + #[cfg(feature = "crypto-ring")] + CryptoMode::Ring => rustls::crypto::ring::default_provider(), + + #[cfg(feature = "crypto-aws-lc-fips")] + CryptoMode::AwsLcFips => { + let provider = rustls::crypto::default_fips_provider(); + assert!( + provider.fips(), + "FIPS was requested but the provider did not support FIPS" + ); + provider + } + } + } +} + +#[allow(unused_imports)] +mod cached_connectors { + use client::connect::HttpConnector; + use hyper_util::client::legacy as client; + use hyper_util::client::legacy::connect::dns::GaiResolver; + + use crate::client::build_connector::make_tls; + use crate::client::{CryptoMode, Inner}; + + #[cfg(feature = "crypto-ring")] + pub(crate) static HTTPS_NATIVE_ROOTS_RING: once_cell::sync::Lazy< + hyper_rustls::HttpsConnector, + > = once_cell::sync::Lazy::new(|| make_tls(GaiResolver::new(), CryptoMode::Ring.provider())); + + #[cfg(feature = "crypto-aws-lc")] + pub(crate) static HTTPS_NATIVE_ROOTS_AWS_LC: once_cell::sync::Lazy< + hyper_rustls::HttpsConnector, + > = once_cell::sync::Lazy::new(|| make_tls(GaiResolver::new(), CryptoMode::AwsLc.provider())); + + #[cfg(feature = "crypto-aws-lc-fips")] + pub(crate) static HTTPS_NATIVE_ROOTS_AWS_LC_FIPS: once_cell::sync::Lazy< + hyper_rustls::HttpsConnector, + > = once_cell::sync::Lazy::new(|| { + make_tls(GaiResolver::new(), CryptoMode::AwsLcFips.provider()) + }); + + pub(super) fn cached_https(mode: Inner) -> hyper_rustls::HttpsConnector { + match mode { + #[cfg(feature = "crypto-ring")] + Inner::Standard(CryptoMode::Ring) => HTTPS_NATIVE_ROOTS_RING.clone(), + #[cfg(feature = "crypto-aws-lc")] + Inner::Standard(CryptoMode::AwsLc) => HTTPS_NATIVE_ROOTS_AWS_LC.clone(), + #[cfg(feature = "crypto-aws-lc-fips")] + Inner::Standard(CryptoMode::AwsLcFips) => HTTPS_NATIVE_ROOTS_AWS_LC_FIPS.clone(), + #[allow(unreachable_patterns)] + Inner::Standard(_) => unreachable!("unexpected mode"), + Inner::Custom(provider) => make_tls(GaiResolver::new(), provider), + } + } +} + +mod build_connector { + use crate::client::{dns::HyperUtilResolver, Inner}; + use aws_smithy_runtime_api::client::dns::ResolveDns; + use client::connect::HttpConnector; + use hyper_util::client::legacy as client; + use rustls::crypto::CryptoProvider; + use std::sync::Arc; + + fn restrict_ciphers(base: CryptoProvider) -> CryptoProvider { + let suites = &[ + rustls::CipherSuite::TLS13_AES_256_GCM_SHA384, + rustls::CipherSuite::TLS13_AES_128_GCM_SHA256, + // TLS1.2 suites + rustls::CipherSuite::TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + rustls::CipherSuite::TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + rustls::CipherSuite::TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + rustls::CipherSuite::TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + rustls::CipherSuite::TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + ]; + let supported_suites = suites + .iter() + .flat_map(|suite| { + base.cipher_suites + .iter() + .find(|s| &s.suite() == suite) + .cloned() + }) + .collect::>(); + CryptoProvider { + cipher_suites: supported_suites, + ..base + } + } + + pub(crate) fn make_tls( + resolver: R, + crypto_provider: CryptoProvider, + ) -> hyper_rustls::HttpsConnector> { + use hyper_rustls::ConfigBuilderExt; + let mut base_connector = HttpConnector::new_with_resolver(resolver); + base_connector.enforce_http(false); + hyper_rustls::HttpsConnectorBuilder::new() + .with_tls_config( + rustls::ClientConfig::builder_with_provider(Arc::new(restrict_ciphers(crypto_provider))) + .with_safe_default_protocol_versions() + .expect("Error with the TLS configuration. Please file a bug report under https://github.com/smithy-lang/smithy-rs/issues.") + .with_native_roots().expect("error with TLS configuration.") + .with_no_client_auth() + ) + .https_or_http() + .enable_http1() + .enable_http2() + .wrap_connector(base_connector) + } + + pub(super) fn https_with_resolver( + crypto_provider: Inner, + resolver: R, + ) -> hyper_rustls::HttpsConnector>> { + make_tls(HyperUtilResolver { resolver }, crypto_provider.provider()) + } +} + +/// [`HttpConnector`] used to make HTTP requests. +/// +/// This connector also implements socket connect and read timeouts. +/// +/// This shouldn't be used directly in most cases. +/// See the docs on [`Builder`] for examples of how to customize the HTTP client. +#[derive(Debug)] +pub struct Connector { + adapter: Box, +} + +impl Connector { + /// Builder for a Hyper connector. + pub fn builder() -> ConnectorBuilder { + Default::default() + } +} + +impl HttpConnector for Connector { + fn call(&self, request: HttpRequest) -> HttpConnectorFuture { + self.adapter.call(request) + } +} + +/// Builder for [`Connector`]. +#[derive(Default, Debug)] +pub struct ConnectorBuilder { + connector_settings: Option, + sleep_impl: Option, + client_builder: Option, + #[allow(unused)] + crypto: Crypto, +} + +/// Initial builder state, [`CryptoMode`] choice required +#[derive(Default)] +#[non_exhaustive] +pub struct CryptoUnset {} + +/// Crypto implementation selected +pub struct CryptoProviderSelected { + crypto_provider: Inner, +} + +#[derive(Clone)] +enum Inner { + Standard(CryptoMode), + #[allow(dead_code)] + Custom(CryptoProvider), +} + +impl Inner { + fn provider(&self) -> CryptoProvider { + match self { + Inner::Standard(mode) => mode.provider(), + Inner::Custom(provider) => provider.clone(), + } + } +} + +#[cfg(any(feature = "crypto-aws-lc", feature = "crypto-ring"))] +impl ConnectorBuilder { + /// Build a [`Connector`] that will use the given DNS resolver implementation. + pub fn build_from_resolver(self, resolver: R) -> Connector { + let connector = + build_connector::https_with_resolver(self.crypto.crypto_provider.clone(), resolver); + self.build(connector) + } +} + +impl ConnectorBuilder { + /// Create a [`Connector`] from this builder and a given connector. + pub(crate) fn build(self, tcp_connector: C) -> Connector + where + C: Send + Sync + 'static, + C: Clone, + C: tower::Service, + C::Response: Read + Write + Connection + Send + Sync + Unpin, + C: Connect, + C::Future: Unpin + Send + 'static, + C::Error: Into, + { + let client_builder = + self.client_builder + .unwrap_or(hyper_util::client::legacy::Builder::new( + TokioExecutor::new(), + )); + let sleep_impl = self.sleep_impl.or_else(default_async_sleep); + let (connect_timeout, read_timeout) = self + .connector_settings + .map(|c| (c.connect_timeout(), c.read_timeout())) + .unwrap_or((None, None)); + + let connector = match connect_timeout { + Some(duration) => timeout::ConnectTimeout::new( + tcp_connector, + sleep_impl + .clone() + .expect("a sleep impl must be provided in order to have a connect timeout"), + duration, + ), + None => timeout::ConnectTimeout::no_timeout(tcp_connector), + }; + let base = client_builder.build(connector); + let read_timeout = match read_timeout { + Some(duration) => timeout::HttpReadTimeout::new( + base, + sleep_impl.expect("a sleep impl must be provided in order to have a read timeout"), + duration, + ), + None => timeout::HttpReadTimeout::no_timeout(base), + }; + Connector { + adapter: Box::new(Adapter { + client: read_timeout, + }), + } + } + + /// Set the async sleep implementation used for timeouts + /// + /// Calling this is only necessary for testing or to use something other than + /// [`default_async_sleep`]. + pub fn sleep_impl(mut self, sleep_impl: impl AsyncSleep + 'static) -> Self { + self.sleep_impl = Some(sleep_impl.into_shared()); + self + } + + /// Set the async sleep implementation used for timeouts + /// + /// Calling this is only necessary for testing or to use something other than + /// [`default_async_sleep`]. + pub fn set_sleep_impl(&mut self, sleep_impl: Option) -> &mut Self { + self.sleep_impl = sleep_impl; + self + } + + /// Configure the HTTP settings for the `HyperAdapter` + pub fn connector_settings(mut self, connector_settings: HttpConnectorSettings) -> Self { + self.connector_settings = Some(connector_settings); + self + } + + /// Configure the HTTP settings for the `HyperAdapter` + pub fn set_connector_settings( + &mut self, + connector_settings: Option, + ) -> &mut Self { + self.connector_settings = connector_settings; + self + } + + /// Override the Hyper client [`Builder`](hyper_util::client::legacy::Builder) used to construct this client. + /// + /// This enables changing settings like forcing HTTP2 and modifying other default client behavior. + pub(crate) fn hyper_builder( + mut self, + hyper_builder: hyper_util::client::legacy::Builder, + ) -> Self { + self.set_hyper_builder(Some(hyper_builder)); + self + } + + /// Override the Hyper client [`Builder`](hyper_util::client::legacy::Builder) used to construct this client. + /// + /// This enables changing settings like forcing HTTP2 and modifying other default client behavior. + pub(crate) fn set_hyper_builder( + &mut self, + hyper_builder: Option, + ) -> &mut Self { + self.client_builder = hyper_builder; + self + } +} + +/// Adapter to use a Hyper 1.0-based Client as an `HttpConnector` +/// +/// This adapter also enables TCP `CONNECT` and HTTP `READ` timeouts via [`Connector::builder`]. +struct Adapter { + client: timeout::HttpReadTimeout< + hyper_util::client::legacy::Client, SdkBody>, + >, +} + +impl fmt::Debug for Adapter { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Adapter") + .field("client", &"** hyper client **") + .finish() + } +} + +/// Extract a smithy connection from a hyper CaptureConnection +fn extract_smithy_connection(capture_conn: &CaptureConnection) -> Option { + let capture_conn = capture_conn.clone(); + if let Some(conn) = capture_conn.clone().connection_metadata().as_ref() { + let mut extensions = Extensions::new(); + conn.get_extras(&mut extensions); + let http_info = extensions.get::(); + let mut builder = ConnectionMetadata::builder() + .proxied(conn.is_proxied()) + .poison_fn(move || match capture_conn.connection_metadata().as_ref() { + Some(conn) => conn.poison(), + None => tracing::trace!("no connection existed to poison"), + }); + + builder + .set_local_addr(http_info.map(|info| info.local_addr())) + .set_remote_addr(http_info.map(|info| info.remote_addr())); + + let smithy_connection = builder.build(); + + Some(smithy_connection) + } else { + None + } +} + +impl HttpConnector for Adapter +where + C: Clone + Send + Sync + 'static, + C: tower::Service, + C::Response: Connection + Read + Write + Unpin + 'static, + timeout::ConnectTimeout: Connect, + C::Future: Unpin + Send + 'static, + C::Error: Into, +{ + fn call(&self, request: HttpRequest) -> HttpConnectorFuture { + let mut request = match request.try_into_http1x() { + Ok(request) => request, + Err(err) => { + return HttpConnectorFuture::ready(Err(ConnectorError::user(err.into()))); + } + }; + let capture_connection = capture_connection(&mut request); + if let Some(capture_smithy_connection) = + request.extensions().get::() + { + capture_smithy_connection + .set_connection_retriever(move || extract_smithy_connection(&capture_connection)); + } + let mut client = self.client.clone(); + use tower::Service; + let fut = client.call(request); + HttpConnectorFuture::new(async move { + let response = fut + .await + .map_err(downcast_error)? + .map(SdkBody::from_body_1_x); + match HttpResponse::try_from(response) { + Ok(response) => Ok(response), + Err(err) => Err(ConnectorError::other(err.into(), None)), + } + }) + } +} + +/// Downcast errors coming out of hyper into an appropriate `ConnectorError` +fn downcast_error(err: BoxError) -> ConnectorError { + // is a `TimedOutError` (from aws_smithy_async::timeout) in the chain? if it is, this is a timeout + if find_source::(err.as_ref()).is_some() { + return ConnectorError::timeout(err); + } + // is the top of chain error actually already a `ConnectorError`? return that directly + let err = match err.downcast::() { + Ok(connector_error) => return *connector_error, + Err(box_error) => box_error, + }; + // generally, the top of chain will probably be a hyper error. Go through a set of hyper specific + // error classifications + let err = match find_source::(err.as_ref()) { + Some(hyper_error) => return to_connector_error(hyper_error)(err), + None => err, + }; + + // otherwise, we have no idea! + ConnectorError::other(err, None) +} + +/// Convert a [`hyper::Error`] into a [`ConnectorError`] +fn to_connector_error(err: &hyper::Error) -> fn(BoxError) -> ConnectorError { + if err.is_timeout() || find_source::(err).is_some() { + return ConnectorError::timeout; + } + if err.is_user() { + return ConnectorError::user; + } + if err.is_closed() || err.is_canceled() || find_source::(err).is_some() { + return ConnectorError::io; + } + // We sometimes receive this from S3: hyper::Error(IncompleteMessage) + if err.is_incomplete_message() { + return |err: BoxError| ConnectorError::other(err, Some(ErrorKind::TransientError)); + } + + if let Some(h2_err) = find_source::(err) { + if h2_err.is_go_away() + || (h2_err.is_reset() && h2_err.reason() == Some(Reason::REFUSED_STREAM)) + { + return ConnectorError::io; + } + } + + tracing::warn!(err = %DisplayErrorContext(&err), "unrecognized error from Hyper. If this error should be retried, please file an issue."); + |err: BoxError| ConnectorError::other(err, None) +} + +fn find_source<'a, E: Error + 'static>(err: &'a (dyn Error + 'static)) -> Option<&'a E> { + let mut next = Some(err); + while let Some(err) = next { + if let Some(matching_err) = err.downcast_ref::() { + return Some(matching_err); + } + next = err.source(); + } + None +} + +// TODO(https://github.com/awslabs/aws-sdk-rust/issues/1090): CacheKey must also include ptr equality to any +// runtime components that are used—sleep_impl as a base (unless we prohibit overriding sleep impl) +// If we decide to put a DnsResolver in RuntimeComponents, then we'll need to handle that as well. +#[derive(Clone, Debug, Eq, PartialEq, Hash)] +struct CacheKey { + connect_timeout: Option, + read_timeout: Option, +} + +impl From<&HttpConnectorSettings> for CacheKey { + fn from(value: &HttpConnectorSettings) -> Self { + Self { + connect_timeout: value.connect_timeout(), + read_timeout: value.read_timeout(), + } + } +} + +struct HyperClient { + connector_cache: RwLock>, + client_builder: hyper_util::client::legacy::Builder, + tcp_connector_fn: F, +} + +impl fmt::Debug for HyperClient { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("HyperClient") + .field("connector_cache", &self.connector_cache) + .field("client_builder", &self.client_builder) + .finish() + } +} + +impl HttpClient for HyperClient +where + F: Fn() -> C + Send + Sync, + C: Clone + Send + Sync + 'static, + C: tower::Service, + C::Response: Connection + Read + Write + Send + Sync + Unpin + 'static, + C::Future: Unpin + Send + 'static, + C::Error: Into, +{ + fn http_connector( + &self, + settings: &HttpConnectorSettings, + components: &RuntimeComponents, + ) -> SharedHttpConnector { + let key = CacheKey::from(settings); + let mut connector = self.connector_cache.read().unwrap().get(&key).cloned(); + if connector.is_none() { + let mut cache = self.connector_cache.write().unwrap(); + // Short-circuit if another thread already wrote a connector to the cache for this key + if !cache.contains_key(&key) { + let mut builder = Connector::builder() + .hyper_builder(self.client_builder.clone()) + .connector_settings(settings.clone()); + builder.set_sleep_impl(components.sleep_impl()); + + let start = components.time_source().map(|ts| ts.now()); + let tcp_connector = (self.tcp_connector_fn)(); + let end = components.time_source().map(|ts| ts.now()); + if let (Some(start), Some(end)) = (start, end) { + if let Ok(elapsed) = end.duration_since(start) { + tracing::debug!("new TCP connector created in {:?}", elapsed); + } + } + let connector = SharedHttpConnector::new(builder.build(tcp_connector)); + cache.insert(key.clone(), connector); + } + connector = cache.get(&key).cloned(); + } + + connector.expect("cache populated above") + } + + fn validate_base_client_config( + &self, + _: &RuntimeComponentsBuilder, + _: &ConfigBag, + ) -> Result<(), BoxError> { + // Initialize the TCP connector at this point so that native certs load + // at client initialization time instead of upon first request. We do it + // here rather than at construction so that it won't run if this is not + // the selected HTTP client for the base config (for example, if this was + // the default HTTP client, and it was overridden by a later plugin). + let _ = (self.tcp_connector_fn)(); + Ok(()) + } + + fn connector_metadata(&self) -> Option { + Some(ConnectorMetadata::new("hyper", Some(Cow::Borrowed("1.x")))) + } +} + +/// Builder for a hyper-backed [`HttpClient`] implementation. +/// +/// This builder can be used to customize the underlying TCP connector used, as well as +/// hyper client configuration. +/// +/// # Examples +/// +/// Construct a Hyper client with the RusTLS TLS implementation. +/// This can be useful when you want to share a Hyper connector between multiple +/// generated Smithy clients. +#[derive(Clone, Default, Debug)] +pub struct Builder { + client_builder: Option, + crypto_provider: Crypto, +} + +impl Builder { + /// Create a hyper client using RusTLS for TLS + /// + /// The trusted certificates will be loaded later when this becomes the selected + /// HTTP client for a Smithy client. + pub fn build_https(self) -> SharedHttpClient { + let crypto = self.crypto_provider.crypto_provider; + build_with_fn(self.client_builder, move || { + cached_connectors::cached_https(crypto.clone()) + }) + } + + /// Create a hyper client using a custom DNS resolver + pub fn build_with_resolver( + self, + resolver: impl ResolveDns + Clone + 'static, + ) -> SharedHttpClient { + build_with_fn(self.client_builder, move || { + build_connector::https_with_resolver( + self.crypto_provider.crypto_provider.clone(), + resolver.clone(), + ) + }) + } +} + +impl Builder { + /// Creates a new builder. + pub fn new() -> Self { + Self::default() + } + + /// Set the cryptography implementation to use + pub fn crypto_mode(self, provider: CryptoMode) -> Builder { + Builder { + client_builder: self.client_builder, + crypto_provider: CryptoProviderSelected { + crypto_provider: Inner::Standard(provider), + }, + } + } + + /// This interface will be broken in the future + /// + /// This exposes `CryptoProvider` from `rustls` directly and this API has no stability guarantee. + #[cfg(crypto_unstable)] + pub fn crypto_provider_unstable( + self, + provider: CryptoProvider, + ) -> Builder { + Builder { + client_builder: self.client_builder, + crypto_provider: CryptoProviderSelected { + crypto_provider: Inner::Custom(provider), + }, + } + } +} + +pub(crate) fn build_with_fn( + client_builder: Option, + tcp_connector_fn: F, +) -> SharedHttpClient +where + F: Fn() -> C + Send + Sync + 'static, + C: Clone + Send + Sync + 'static, + C: tower::Service, + C::Response: Connection + Read + Write + Send + Sync + Unpin + 'static, + C::Future: Unpin + Send + 'static, + C::Error: Into, + C: Connect, +{ + SharedHttpClient::new(HyperClient { + connector_cache: RwLock::new(HashMap::new()), + client_builder: client_builder + .unwrap_or_else(|| hyper_util::client::legacy::Builder::new(TokioExecutor::new())), + tcp_connector_fn, + }) +} + +#[cfg(test)] +mod test { + use std::io::{Error, ErrorKind}; + use std::pin::Pin; + use std::sync::atomic::{AtomicU32, Ordering}; + use std::sync::Arc; + use std::task::{Context, Poll}; + + use aws_smithy_async::assert_elapsed; + use aws_smithy_async::rt::sleep::TokioSleep; + use aws_smithy_async::time::SystemTimeSource; + use aws_smithy_runtime_api::client::runtime_components::RuntimeComponentsBuilder; + use http_1x::Uri; + use hyper::rt::ReadBufCursor; + use hyper_util::client::legacy::connect::Connected; + + use crate::client::timeout::test::NeverConnects; + + use super::*; + + #[tokio::test] + async fn connector_selection() { + // Create a client that increments a count every time it creates a new Connector + let creation_count = Arc::new(AtomicU32::new(0)); + let http_client = build_with_fn(None, { + let count = creation_count.clone(); + move || { + count.fetch_add(1, Ordering::Relaxed); + NeverConnects + } + }); + + // This configuration should result in 4 separate connectors with different timeout settings + let settings = [ + HttpConnectorSettings::builder() + .connect_timeout(Duration::from_secs(3)) + .build(), + HttpConnectorSettings::builder() + .read_timeout(Duration::from_secs(3)) + .build(), + HttpConnectorSettings::builder() + .connect_timeout(Duration::from_secs(3)) + .read_timeout(Duration::from_secs(3)) + .build(), + HttpConnectorSettings::builder() + .connect_timeout(Duration::from_secs(5)) + .read_timeout(Duration::from_secs(3)) + .build(), + ]; + + // Kick off thousands of parallel tasks that will try to create a connector + let components = RuntimeComponentsBuilder::for_tests() + .with_time_source(Some(SystemTimeSource::new())) + .build() + .unwrap(); + let mut handles = Vec::new(); + for setting in &settings { + for _ in 0..1000 { + let client = http_client.clone(); + handles.push(tokio::spawn({ + let setting = setting.clone(); + let components = components.clone(); + async move { + let _ = client.http_connector(&setting, &components); + } + })); + } + } + for handle in handles { + handle.await.unwrap(); + } + + // Verify only 4 connectors were created amidst the chaos + assert_eq!(4, creation_count.load(Ordering::Relaxed)); + } + + #[tokio::test] + async fn hyper_io_error() { + let connector = TestConnection { + inner: HangupStream, + }; + let adapter = Connector::builder().build(connector).adapter; + let err = adapter + .call(HttpRequest::get("https://socket-hangup.com").unwrap()) + .await + .expect_err("socket hangup"); + assert!(err.is_io(), "unexpected error type: {:?}", err); + } + + // ---- machinery to make a Hyper connector that responds with an IO Error + #[derive(Clone)] + struct HangupStream; + + impl Connection for HangupStream { + fn connected(&self) -> Connected { + Connected::new() + } + } + + impl Read for HangupStream { + fn poll_read( + self: Pin<&mut Self>, + _cx: &mut Context<'_>, + _buf: ReadBufCursor<'_>, + ) -> Poll> { + Poll::Ready(Err(Error::new( + ErrorKind::ConnectionReset, + "connection reset", + ))) + } + } + + impl Write for HangupStream { + fn poll_write( + self: Pin<&mut Self>, + _cx: &mut Context<'_>, + _buf: &[u8], + ) -> Poll> { + Poll::Pending + } + + fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Pending + } + + fn poll_shutdown(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Pending + } + } + + #[derive(Clone)] + struct TestConnection { + inner: T, + } + + impl tower::Service for TestConnection + where + T: Clone + Connection, + { + type Response = T; + type Error = BoxError; + type Future = std::future::Ready>; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, _req: Uri) -> Self::Future { + std::future::ready(Ok(self.inner.clone())) + } + } + + #[tokio::test] + async fn http_connect_timeout_works() { + let tcp_connector = NeverConnects::default(); + let connector_settings = HttpConnectorSettings::builder() + .connect_timeout(Duration::from_secs(1)) + .build(); + let hyper = Connector::builder() + .connector_settings(connector_settings) + .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) + .build(tcp_connector) + .adapter; + let now = tokio::time::Instant::now(); + tokio::time::pause(); + let resp = hyper + .call(HttpRequest::get("https://static-uri.com").unwrap()) + .await + .unwrap_err(); + assert!( + resp.is_timeout(), + "expected resp.is_timeout() to be true but it was false, resp == {:?}", + resp + ); + let message = DisplayErrorContext(&resp).to_string(); + let expected = "timeout: client error (Connect): HTTP connect timeout occurred after 1s"; + assert!( + message.contains(expected), + "expected '{message}' to contain '{expected}'" + ); + assert_elapsed!(now, Duration::from_secs(1)); + } + + #[tokio::test] + async fn http_read_timeout_works() { + let tcp_connector = crate::client::timeout::test::NeverReplies; + let connector_settings = HttpConnectorSettings::builder() + .connect_timeout(Duration::from_secs(1)) + .read_timeout(Duration::from_secs(2)) + .build(); + let hyper = Connector::builder() + .connector_settings(connector_settings) + .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) + .build(tcp_connector) + .adapter; + let now = tokio::time::Instant::now(); + tokio::time::pause(); + let err = hyper + .call(HttpRequest::get("https://fake-uri.com").unwrap()) + .await + .unwrap_err(); + assert!( + err.is_timeout(), + "expected err.is_timeout() to be true but it was false, err == {err:?}", + ); + let message = format!("{}", DisplayErrorContext(&err)); + let expected = "timeout: HTTP read timeout occurred after 2s"; + assert!( + message.contains(expected), + "expected '{message}' to contain '{expected}'" + ); + assert_elapsed!(now, Duration::from_secs(2)); + } +} diff --git a/rust-runtime/aws-smithy-http-client/src/client/dns.rs b/rust-runtime/aws-smithy-http-client/src/client/dns.rs new file mode 100644 index 0000000000..82c566c16b --- /dev/null +++ b/rust-runtime/aws-smithy-http-client/src/client/dns.rs @@ -0,0 +1,40 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +use aws_smithy_runtime_api::client::dns::ResolveDns; +use hyper_util::client::legacy::connect::dns::Name; +use std::error::Error; +use std::future::Future; +use std::net::SocketAddr; +use std::pin::Pin; +use std::task::{Context, Poll}; +use std::vec; + +/// A bridge that allows our `ResolveDns` trait to work with Hyper's `Resolver` interface (based on tower) +#[derive(Clone)] +pub(crate) struct HyperUtilResolver { + pub(crate) resolver: R, +} + +impl tower::Service for HyperUtilResolver { + type Response = vec::IntoIter; + type Error = Box; + type Future = Pin> + Send>>; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, req: Name) -> Self::Future { + let resolver = self.resolver.clone(); + Box::pin(async move { + let dns_entries = resolver.resolve_dns(req.as_str()).await?; + Ok(dns_entries + .into_iter() + .map(|ip_addr| SocketAddr::new(ip_addr, 0)) + .collect::>() + .into_iter()) + }) + } +} diff --git a/rust-runtime/aws-smithy-http-client/src/hyper_1.rs b/rust-runtime/aws-smithy-http-client/src/hyper_1.rs deleted file mode 100644 index 94c78af6d0..0000000000 --- a/rust-runtime/aws-smithy-http-client/src/hyper_1.rs +++ /dev/null @@ -1,945 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -use aws_smithy_async::future::timeout::TimedOutError; -use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep, SharedAsyncSleep}; -use aws_smithy_runtime_api::box_error::BoxError; -use aws_smithy_runtime_api::client::connection::CaptureSmithyConnection; -use aws_smithy_runtime_api::client::connection::ConnectionMetadata; -use aws_smithy_runtime_api::client::connector_metadata::ConnectorMetadata; -use aws_smithy_runtime_api::client::dns::ResolveDns; -use aws_smithy_runtime_api::client::http::{ - HttpClient, HttpConnector, HttpConnectorFuture, HttpConnectorSettings, SharedHttpClient, - SharedHttpConnector, -}; -use aws_smithy_runtime_api::client::orchestrator::{HttpRequest, HttpResponse}; -use aws_smithy_runtime_api::client::result::ConnectorError; -use aws_smithy_runtime_api::client::runtime_components::{ - RuntimeComponents, RuntimeComponentsBuilder, -}; -use aws_smithy_runtime_api::shared::IntoShared; -use aws_smithy_types::body::SdkBody; -use aws_smithy_types::config_bag::ConfigBag; -use aws_smithy_types::error::display::DisplayErrorContext; -use aws_smithy_types::retry::ErrorKind; -use client::connect::Connection; -use h2::Reason; -use http_1x::{Extensions, Uri}; -use hyper::rt::{Read, Write}; -use hyper_util::client::legacy as client; -use hyper_util::client::legacy::connect::dns::Name; -use hyper_util::client::legacy::connect::{ - capture_connection, CaptureConnection, Connect, HttpInfo, -}; -use hyper_util::rt::TokioExecutor; -use rustls::crypto::CryptoProvider; -use std::borrow::Cow; -use std::collections::HashMap; -use std::error::Error; -use std::future::Future; -use std::net::SocketAddr; -use std::pin::Pin; -use std::sync::RwLock; -use std::task::{Context, Poll}; -use std::time::Duration; -use std::{fmt, vec}; - -use crate::client::timeout; - -/// Choice of underlying cryptography library -#[derive(Debug, Eq, PartialEq, Clone, Copy)] -#[non_exhaustive] -pub enum CryptoMode { - /// Crypto based on [ring](https://github.com/briansmith/ring) - #[cfg(feature = "crypto-ring")] - Ring, - /// Crypto based on [aws-lc](https://github.com/aws/aws-lc-rs) - #[cfg(feature = "crypto-aws-lc")] - AwsLc, - /// FIPS compliant variant of [aws-lc](https://github.com/aws/aws-lc-rs) - #[cfg(feature = "crypto-aws-lc-fips")] - AwsLcFips, -} - -impl CryptoMode { - fn provider(self) -> CryptoProvider { - match self { - #[cfg(feature = "crypto-aws-lc")] - CryptoMode::AwsLc => rustls::crypto::aws_lc_rs::default_provider(), - - #[cfg(feature = "crypto-ring")] - CryptoMode::Ring => rustls::crypto::ring::default_provider(), - - #[cfg(feature = "crypto-aws-lc-fips")] - CryptoMode::AwsLcFips => { - let provider = rustls::crypto::default_fips_provider(); - assert!( - provider.fips(), - "FIPS was requested but the provider did not support FIPS" - ); - provider - } - } - } -} - -/// A bridge that allows our `ResolveDns` trait to work with Hyper's `Resolver` interface (based on tower) -#[derive(Clone)] -struct HyperUtilResolver { - resolver: R, -} - -impl tower::Service for HyperUtilResolver { - type Response = vec::IntoIter; - type Error = Box; - type Future = Pin> + Send>>; - - fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { - Poll::Ready(Ok(())) - } - - fn call(&mut self, req: Name) -> Self::Future { - let resolver = self.resolver.clone(); - Box::pin(async move { - let dns_entries = resolver.resolve_dns(req.as_str()).await?; - Ok(dns_entries - .into_iter() - .map(|ip_addr| SocketAddr::new(ip_addr, 0)) - .collect::>() - .into_iter()) - }) - } -} - -#[allow(unused_imports)] -mod cached_connectors { - use client::connect::HttpConnector; - use hyper_util::client::legacy as client; - use hyper_util::client::legacy::connect::dns::GaiResolver; - - use crate::hyper_1::build_connector::make_tls; - use crate::hyper_1::{CryptoMode, Inner}; - - #[cfg(feature = "crypto-ring")] - pub(crate) static HTTPS_NATIVE_ROOTS_RING: once_cell::sync::Lazy< - hyper_rustls::HttpsConnector, - > = once_cell::sync::Lazy::new(|| make_tls(GaiResolver::new(), CryptoMode::Ring.provider())); - - #[cfg(feature = "crypto-aws-lc")] - pub(crate) static HTTPS_NATIVE_ROOTS_AWS_LC: once_cell::sync::Lazy< - hyper_rustls::HttpsConnector, - > = once_cell::sync::Lazy::new(|| make_tls(GaiResolver::new(), CryptoMode::AwsLc.provider())); - - #[cfg(feature = "crypto-aws-lc-fips")] - pub(crate) static HTTPS_NATIVE_ROOTS_AWS_LC_FIPS: once_cell::sync::Lazy< - hyper_rustls::HttpsConnector, - > = once_cell::sync::Lazy::new(|| { - make_tls(GaiResolver::new(), CryptoMode::AwsLcFips.provider()) - }); - - pub(super) fn cached_https(mode: Inner) -> hyper_rustls::HttpsConnector { - match mode { - #[cfg(feature = "crypto-ring")] - Inner::Standard(CryptoMode::Ring) => HTTPS_NATIVE_ROOTS_RING.clone(), - #[cfg(feature = "crypto-aws-lc")] - Inner::Standard(CryptoMode::AwsLc) => HTTPS_NATIVE_ROOTS_AWS_LC.clone(), - #[cfg(feature = "crypto-aws-lc-fips")] - Inner::Standard(CryptoMode::AwsLcFips) => HTTPS_NATIVE_ROOTS_AWS_LC_FIPS.clone(), - #[allow(unreachable_patterns)] - Inner::Standard(_) => unreachable!("unexpected mode"), - Inner::Custom(provider) => make_tls(GaiResolver::new(), provider), - } - } -} - -mod build_connector { - use crate::hyper_1::{HyperUtilResolver, Inner}; - use aws_smithy_runtime_api::client::dns::ResolveDns; - use client::connect::HttpConnector; - use hyper_util::client::legacy as client; - use rustls::crypto::CryptoProvider; - use std::sync::Arc; - - fn restrict_ciphers(base: CryptoProvider) -> CryptoProvider { - let suites = &[ - rustls::CipherSuite::TLS13_AES_256_GCM_SHA384, - rustls::CipherSuite::TLS13_AES_128_GCM_SHA256, - // TLS1.2 suites - rustls::CipherSuite::TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, - rustls::CipherSuite::TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, - rustls::CipherSuite::TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, - rustls::CipherSuite::TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, - rustls::CipherSuite::TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, - ]; - let supported_suites = suites - .iter() - .flat_map(|suite| { - base.cipher_suites - .iter() - .find(|s| &s.suite() == suite) - .cloned() - }) - .collect::>(); - CryptoProvider { - cipher_suites: supported_suites, - ..base - } - } - - pub(crate) fn make_tls( - resolver: R, - crypto_provider: CryptoProvider, - ) -> hyper_rustls::HttpsConnector> { - use hyper_rustls::ConfigBuilderExt; - let mut base_connector = HttpConnector::new_with_resolver(resolver); - base_connector.enforce_http(false); - hyper_rustls::HttpsConnectorBuilder::new() - .with_tls_config( - rustls::ClientConfig::builder_with_provider(Arc::new(restrict_ciphers(crypto_provider))) - .with_safe_default_protocol_versions() - .expect("Error with the TLS configuration. Please file a bug report under https://github.com/smithy-lang/smithy-rs/issues.") - .with_native_roots().expect("error with TLS configuration.") - .with_no_client_auth() - ) - .https_or_http() - .enable_http1() - .enable_http2() - .wrap_connector(base_connector) - } - - pub(super) fn https_with_resolver( - crypto_provider: Inner, - resolver: R, - ) -> hyper_rustls::HttpsConnector>> { - make_tls(HyperUtilResolver { resolver }, crypto_provider.provider()) - } -} - -/// [`HttpConnector`] that uses [`hyper`] to make HTTP requests. -/// -/// This connector also implements socket connect and read timeouts. -/// -/// This shouldn't be used directly in most cases. -/// See the docs on [`HyperClientBuilder`] for examples of how -/// to customize the Hyper client. -#[derive(Debug)] -pub struct HyperConnector { - adapter: Box, -} - -impl HyperConnector { - /// Builder for a Hyper connector. - pub fn builder() -> HyperConnectorBuilder { - Default::default() - } -} - -impl HttpConnector for HyperConnector { - fn call(&self, request: HttpRequest) -> HttpConnectorFuture { - self.adapter.call(request) - } -} - -/// Builder for [`HyperConnector`]. -#[derive(Default, Debug)] -pub struct HyperConnectorBuilder { - connector_settings: Option, - sleep_impl: Option, - client_builder: Option, - #[allow(unused)] - crypto: Crypto, -} - -/// Initial builder state, [`CryptoMode`] choice required -#[derive(Default)] -#[non_exhaustive] -pub struct CryptoUnset {} - -/// Crypto implementation selected -pub struct CryptoProviderSelected { - crypto_provider: Inner, -} - -#[derive(Clone)] -enum Inner { - Standard(CryptoMode), - #[allow(dead_code)] - Custom(CryptoProvider), -} - -impl Inner { - fn provider(&self) -> CryptoProvider { - match self { - Inner::Standard(mode) => mode.provider(), - Inner::Custom(provider) => provider.clone(), - } - } -} - -#[cfg(any(feature = "crypto-aws-lc", feature = "crypto-ring"))] -impl HyperConnectorBuilder { - /// Build a [`HyperConnector`] that will use the given DNS resolver implementation. - pub fn build_from_resolver( - self, - resolver: R, - ) -> HyperConnector { - let connector = - build_connector::https_with_resolver(self.crypto.crypto_provider.clone(), resolver); - self.build(connector) - } -} - -impl HyperConnectorBuilder { - /// Create a [`HyperConnector`] from this builder and a given connector. - pub(crate) fn build(self, tcp_connector: C) -> HyperConnector - where - C: Send + Sync + 'static, - C: Clone, - C: tower::Service, - C::Response: Read + Write + Connection + Send + Sync + Unpin, - C: Connect, - C::Future: Unpin + Send + 'static, - C::Error: Into, - { - let client_builder = - self.client_builder - .unwrap_or(hyper_util::client::legacy::Builder::new( - TokioExecutor::new(), - )); - let sleep_impl = self.sleep_impl.or_else(default_async_sleep); - let (connect_timeout, read_timeout) = self - .connector_settings - .map(|c| (c.connect_timeout(), c.read_timeout())) - .unwrap_or((None, None)); - - let connector = match connect_timeout { - Some(duration) => timeout::ConnectTimeout::new( - tcp_connector, - sleep_impl - .clone() - .expect("a sleep impl must be provided in order to have a connect timeout"), - duration, - ), - None => timeout::ConnectTimeout::no_timeout(tcp_connector), - }; - let base = client_builder.build(connector); - let read_timeout = match read_timeout { - Some(duration) => timeout::HttpReadTimeout::new( - base, - sleep_impl.expect("a sleep impl must be provided in order to have a read timeout"), - duration, - ), - None => timeout::HttpReadTimeout::no_timeout(base), - }; - HyperConnector { - adapter: Box::new(Adapter { - client: read_timeout, - }), - } - } - - /// Set the async sleep implementation used for timeouts - /// - /// Calling this is only necessary for testing or to use something other than - /// [`default_async_sleep`]. - pub fn sleep_impl(mut self, sleep_impl: impl AsyncSleep + 'static) -> Self { - self.sleep_impl = Some(sleep_impl.into_shared()); - self - } - - /// Set the async sleep implementation used for timeouts - /// - /// Calling this is only necessary for testing or to use something other than - /// [`default_async_sleep`]. - pub fn set_sleep_impl(&mut self, sleep_impl: Option) -> &mut Self { - self.sleep_impl = sleep_impl; - self - } - - /// Configure the HTTP settings for the `HyperAdapter` - pub fn connector_settings(mut self, connector_settings: HttpConnectorSettings) -> Self { - self.connector_settings = Some(connector_settings); - self - } - - /// Configure the HTTP settings for the `HyperAdapter` - pub fn set_connector_settings( - &mut self, - connector_settings: Option, - ) -> &mut Self { - self.connector_settings = connector_settings; - self - } - - /// Override the Hyper client [`Builder`](hyper_util::client::legacy::Builder) used to construct this client. - /// - /// This enables changing settings like forcing HTTP2 and modifying other default client behavior. - pub(crate) fn hyper_builder( - mut self, - hyper_builder: hyper_util::client::legacy::Builder, - ) -> Self { - self.set_hyper_builder(Some(hyper_builder)); - self - } - - /// Override the Hyper client [`Builder`](hyper_util::client::legacy::Builder) used to construct this client. - /// - /// This enables changing settings like forcing HTTP2 and modifying other default client behavior. - pub(crate) fn set_hyper_builder( - &mut self, - hyper_builder: Option, - ) -> &mut Self { - self.client_builder = hyper_builder; - self - } -} - -/// Adapter to use a Hyper 1.0-based Client as an `HttpConnector` -/// -/// This adapter also enables TCP `CONNECT` and HTTP `READ` timeouts via [`HyperConnector::builder`]. -struct Adapter { - client: timeout::HttpReadTimeout< - hyper_util::client::legacy::Client, SdkBody>, - >, -} - -impl fmt::Debug for Adapter { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Adapter") - .field("client", &"** hyper client **") - .finish() - } -} - -/// Extract a smithy connection from a hyper CaptureConnection -fn extract_smithy_connection(capture_conn: &CaptureConnection) -> Option { - let capture_conn = capture_conn.clone(); - if let Some(conn) = capture_conn.clone().connection_metadata().as_ref() { - let mut extensions = Extensions::new(); - conn.get_extras(&mut extensions); - let http_info = extensions.get::(); - let mut builder = ConnectionMetadata::builder() - .proxied(conn.is_proxied()) - .poison_fn(move || match capture_conn.connection_metadata().as_ref() { - Some(conn) => conn.poison(), - None => tracing::trace!("no connection existed to poison"), - }); - - builder - .set_local_addr(http_info.map(|info| info.local_addr())) - .set_remote_addr(http_info.map(|info| info.remote_addr())); - - let smithy_connection = builder.build(); - - Some(smithy_connection) - } else { - None - } -} - -impl HttpConnector for Adapter -where - C: Clone + Send + Sync + 'static, - C: tower::Service, - C::Response: Connection + Read + Write + Unpin + 'static, - timeout::ConnectTimeout: Connect, - C::Future: Unpin + Send + 'static, - C::Error: Into, -{ - fn call(&self, request: HttpRequest) -> HttpConnectorFuture { - let mut request = match request.try_into_http1x() { - Ok(request) => request, - Err(err) => { - return HttpConnectorFuture::ready(Err(ConnectorError::user(err.into()))); - } - }; - let capture_connection = capture_connection(&mut request); - if let Some(capture_smithy_connection) = - request.extensions().get::() - { - capture_smithy_connection - .set_connection_retriever(move || extract_smithy_connection(&capture_connection)); - } - let mut client = self.client.clone(); - use tower::Service; - let fut = client.call(request); - HttpConnectorFuture::new(async move { - let response = fut - .await - .map_err(downcast_error)? - .map(SdkBody::from_body_1_x); - match HttpResponse::try_from(response) { - Ok(response) => Ok(response), - Err(err) => Err(ConnectorError::other(err.into(), None)), - } - }) - } -} - -/// Downcast errors coming out of hyper into an appropriate `ConnectorError` -fn downcast_error(err: BoxError) -> ConnectorError { - // is a `TimedOutError` (from aws_smithy_async::timeout) in the chain? if it is, this is a timeout - if find_source::(err.as_ref()).is_some() { - return ConnectorError::timeout(err); - } - // is the top of chain error actually already a `ConnectorError`? return that directly - let err = match err.downcast::() { - Ok(connector_error) => return *connector_error, - Err(box_error) => box_error, - }; - // generally, the top of chain will probably be a hyper error. Go through a set of hyper specific - // error classifications - let err = match find_source::(err.as_ref()) { - Some(hyper_error) => return to_connector_error(hyper_error)(err), - None => err, - }; - - // otherwise, we have no idea! - ConnectorError::other(err, None) -} - -/// Convert a [`hyper::Error`] into a [`ConnectorError`] -fn to_connector_error(err: &hyper::Error) -> fn(BoxError) -> ConnectorError { - if err.is_timeout() || find_source::(err).is_some() { - return ConnectorError::timeout; - } - if err.is_user() { - return ConnectorError::user; - } - if err.is_closed() || err.is_canceled() || find_source::(err).is_some() { - return ConnectorError::io; - } - // We sometimes receive this from S3: hyper::Error(IncompleteMessage) - if err.is_incomplete_message() { - return |err: BoxError| ConnectorError::other(err, Some(ErrorKind::TransientError)); - } - - if let Some(h2_err) = find_source::(err) { - if h2_err.is_go_away() - || (h2_err.is_reset() && h2_err.reason() == Some(Reason::REFUSED_STREAM)) - { - return ConnectorError::io; - } - } - - tracing::warn!(err = %DisplayErrorContext(&err), "unrecognized error from Hyper. If this error should be retried, please file an issue."); - |err: BoxError| ConnectorError::other(err, None) -} - -fn find_source<'a, E: Error + 'static>(err: &'a (dyn Error + 'static)) -> Option<&'a E> { - let mut next = Some(err); - while let Some(err) = next { - if let Some(matching_err) = err.downcast_ref::() { - return Some(matching_err); - } - next = err.source(); - } - None -} - -// TODO(https://github.com/awslabs/aws-sdk-rust/issues/1090): CacheKey must also include ptr equality to any -// runtime components that are used—sleep_impl as a base (unless we prohibit overriding sleep impl) -// If we decide to put a DnsResolver in RuntimeComponents, then we'll need to handle that as well. -#[derive(Clone, Debug, Eq, PartialEq, Hash)] -struct CacheKey { - connect_timeout: Option, - read_timeout: Option, -} - -impl From<&HttpConnectorSettings> for CacheKey { - fn from(value: &HttpConnectorSettings) -> Self { - Self { - connect_timeout: value.connect_timeout(), - read_timeout: value.read_timeout(), - } - } -} - -struct HyperClient { - connector_cache: RwLock>, - client_builder: hyper_util::client::legacy::Builder, - tcp_connector_fn: F, -} - -impl fmt::Debug for HyperClient { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("HyperClient") - .field("connector_cache", &self.connector_cache) - .field("client_builder", &self.client_builder) - .finish() - } -} - -impl HttpClient for HyperClient -where - F: Fn() -> C + Send + Sync, - C: Clone + Send + Sync + 'static, - C: tower::Service, - C::Response: Connection + Read + Write + Send + Sync + Unpin + 'static, - C::Future: Unpin + Send + 'static, - C::Error: Into, -{ - fn http_connector( - &self, - settings: &HttpConnectorSettings, - components: &RuntimeComponents, - ) -> SharedHttpConnector { - let key = CacheKey::from(settings); - let mut connector = self.connector_cache.read().unwrap().get(&key).cloned(); - if connector.is_none() { - let mut cache = self.connector_cache.write().unwrap(); - // Short-circuit if another thread already wrote a connector to the cache for this key - if !cache.contains_key(&key) { - let mut builder = HyperConnector::builder() - .hyper_builder(self.client_builder.clone()) - .connector_settings(settings.clone()); - builder.set_sleep_impl(components.sleep_impl()); - - let start = components.time_source().map(|ts| ts.now()); - let tcp_connector = (self.tcp_connector_fn)(); - let end = components.time_source().map(|ts| ts.now()); - if let (Some(start), Some(end)) = (start, end) { - if let Ok(elapsed) = end.duration_since(start) { - tracing::debug!("new TCP connector created in {:?}", elapsed); - } - } - let connector = SharedHttpConnector::new(builder.build(tcp_connector)); - cache.insert(key.clone(), connector); - } - connector = cache.get(&key).cloned(); - } - - connector.expect("cache populated above") - } - - fn validate_base_client_config( - &self, - _: &RuntimeComponentsBuilder, - _: &ConfigBag, - ) -> Result<(), BoxError> { - // Initialize the TCP connector at this point so that native certs load - // at client initialization time instead of upon first request. We do it - // here rather than at construction so that it won't run if this is not - // the selected HTTP client for the base config (for example, if this was - // the default HTTP client, and it was overridden by a later plugin). - let _ = (self.tcp_connector_fn)(); - Ok(()) - } - - fn connector_metadata(&self) -> Option { - Some(ConnectorMetadata::new("hyper", Some(Cow::Borrowed("1.x")))) - } -} - -/// Builder for a hyper-backed [`HttpClient`] implementation. -/// -/// This builder can be used to customize the underlying TCP connector used, as well as -/// hyper client configuration. -/// -/// # Examples -/// -/// Construct a Hyper client with the RusTLS TLS implementation. -/// This can be useful when you want to share a Hyper connector between multiple -/// generated Smithy clients. -#[derive(Clone, Default, Debug)] -pub struct HyperClientBuilder { - client_builder: Option, - crypto_provider: Crypto, -} - -impl HyperClientBuilder { - /// Create a hyper client using RusTLS for TLS - /// - /// The trusted certificates will be loaded later when this becomes the selected - /// HTTP client for a Smithy client. - pub fn build_https(self) -> SharedHttpClient { - let crypto = self.crypto_provider.crypto_provider; - build_with_fn(self.client_builder, move || { - cached_connectors::cached_https(crypto.clone()) - }) - } - - /// Create a hyper client using a custom DNS resolver - pub fn build_with_resolver( - self, - resolver: impl ResolveDns + Clone + 'static, - ) -> SharedHttpClient { - build_with_fn(self.client_builder, move || { - build_connector::https_with_resolver( - self.crypto_provider.crypto_provider.clone(), - resolver.clone(), - ) - }) - } -} - -impl HyperClientBuilder { - /// Creates a new builder. - pub fn new() -> Self { - Self::default() - } - - /// Set the cryptography implementation to use - pub fn crypto_mode(self, provider: CryptoMode) -> HyperClientBuilder { - HyperClientBuilder { - client_builder: self.client_builder, - crypto_provider: CryptoProviderSelected { - crypto_provider: Inner::Standard(provider), - }, - } - } - - /// This interface will be broken in the future - /// - /// This exposes `CryptoProvider` from `rustls` directly and this API has no stability guarantee. - #[cfg(crypto_unstable)] - pub fn crypto_provider_unstable( - self, - provider: CryptoProvider, - ) -> HyperClientBuilder { - HyperClientBuilder { - client_builder: self.client_builder, - crypto_provider: CryptoProviderSelected { - crypto_provider: Inner::Custom(provider), - }, - } - } -} - -pub(crate) fn build_with_fn( - client_builder: Option, - tcp_connector_fn: F, -) -> SharedHttpClient -where - F: Fn() -> C + Send + Sync + 'static, - C: Clone + Send + Sync + 'static, - C: tower::Service, - C::Response: Connection + Read + Write + Send + Sync + Unpin + 'static, - C::Future: Unpin + Send + 'static, - C::Error: Into, - C: Connect, -{ - SharedHttpClient::new(HyperClient { - connector_cache: RwLock::new(HashMap::new()), - client_builder: client_builder - .unwrap_or_else(|| hyper_util::client::legacy::Builder::new(TokioExecutor::new())), - tcp_connector_fn, - }) -} - -#[cfg(test)] -mod test { - use std::io::{Error, ErrorKind}; - use std::pin::Pin; - use std::sync::atomic::{AtomicU32, Ordering}; - use std::sync::Arc; - use std::task::{Context, Poll}; - - use aws_smithy_async::assert_elapsed; - use aws_smithy_async::rt::sleep::TokioSleep; - use aws_smithy_async::time::SystemTimeSource; - use aws_smithy_runtime_api::client::runtime_components::RuntimeComponentsBuilder; - use http_1x::Uri; - use hyper::rt::ReadBufCursor; - use hyper_util::client::legacy::connect::Connected; - - use crate::client::timeout::test::NeverConnects; - - use super::*; - - #[tokio::test] - async fn connector_selection() { - // Create a client that increments a count every time it creates a new HyperConnector - let creation_count = Arc::new(AtomicU32::new(0)); - let http_client = build_with_fn(None, { - let count = creation_count.clone(); - move || { - count.fetch_add(1, Ordering::Relaxed); - NeverConnects - } - }); - - // This configuration should result in 4 separate connectors with different timeout settings - let settings = [ - HttpConnectorSettings::builder() - .connect_timeout(Duration::from_secs(3)) - .build(), - HttpConnectorSettings::builder() - .read_timeout(Duration::from_secs(3)) - .build(), - HttpConnectorSettings::builder() - .connect_timeout(Duration::from_secs(3)) - .read_timeout(Duration::from_secs(3)) - .build(), - HttpConnectorSettings::builder() - .connect_timeout(Duration::from_secs(5)) - .read_timeout(Duration::from_secs(3)) - .build(), - ]; - - // Kick off thousands of parallel tasks that will try to create a connector - let components = RuntimeComponentsBuilder::for_tests() - .with_time_source(Some(SystemTimeSource::new())) - .build() - .unwrap(); - let mut handles = Vec::new(); - for setting in &settings { - for _ in 0..1000 { - let client = http_client.clone(); - handles.push(tokio::spawn({ - let setting = setting.clone(); - let components = components.clone(); - async move { - let _ = client.http_connector(&setting, &components); - } - })); - } - } - for handle in handles { - handle.await.unwrap(); - } - - // Verify only 4 connectors were created amidst the chaos - assert_eq!(4, creation_count.load(Ordering::Relaxed)); - } - - #[tokio::test] - async fn hyper_io_error() { - let connector = TestConnection { - inner: HangupStream, - }; - let adapter = HyperConnector::builder().build(connector).adapter; - let err = adapter - .call(HttpRequest::get("https://socket-hangup.com").unwrap()) - .await - .expect_err("socket hangup"); - assert!(err.is_io(), "unexpected error type: {:?}", err); - } - - // ---- machinery to make a Hyper connector that responds with an IO Error - #[derive(Clone)] - struct HangupStream; - - impl Connection for HangupStream { - fn connected(&self) -> Connected { - Connected::new() - } - } - - impl Read for HangupStream { - fn poll_read( - self: Pin<&mut Self>, - _cx: &mut Context<'_>, - _buf: ReadBufCursor<'_>, - ) -> Poll> { - Poll::Ready(Err(Error::new( - ErrorKind::ConnectionReset, - "connection reset", - ))) - } - } - - impl Write for HangupStream { - fn poll_write( - self: Pin<&mut Self>, - _cx: &mut Context<'_>, - _buf: &[u8], - ) -> Poll> { - Poll::Pending - } - - fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { - Poll::Pending - } - - fn poll_shutdown(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { - Poll::Pending - } - } - - #[derive(Clone)] - struct TestConnection { - inner: T, - } - - impl tower::Service for TestConnection - where - T: Clone + Connection, - { - type Response = T; - type Error = BoxError; - type Future = std::future::Ready>; - - fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { - Poll::Ready(Ok(())) - } - - fn call(&mut self, _req: Uri) -> Self::Future { - std::future::ready(Ok(self.inner.clone())) - } - } - - #[tokio::test] - async fn http_connect_timeout_works() { - let tcp_connector = NeverConnects::default(); - let connector_settings = HttpConnectorSettings::builder() - .connect_timeout(Duration::from_secs(1)) - .build(); - let hyper = HyperConnector::builder() - .connector_settings(connector_settings) - .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) - .build(tcp_connector) - .adapter; - let now = tokio::time::Instant::now(); - tokio::time::pause(); - let resp = hyper - .call(HttpRequest::get("https://static-uri.com").unwrap()) - .await - .unwrap_err(); - assert!( - resp.is_timeout(), - "expected resp.is_timeout() to be true but it was false, resp == {:?}", - resp - ); - let message = DisplayErrorContext(&resp).to_string(); - let expected = "timeout: client error (Connect): HTTP connect timeout occurred after 1s"; - assert!( - message.contains(expected), - "expected '{message}' to contain '{expected}'" - ); - assert_elapsed!(now, Duration::from_secs(1)); - } - - #[tokio::test] - async fn http_read_timeout_works() { - let tcp_connector = crate::client::timeout::test::NeverReplies; - let connector_settings = HttpConnectorSettings::builder() - .connect_timeout(Duration::from_secs(1)) - .read_timeout(Duration::from_secs(2)) - .build(); - let hyper = HyperConnector::builder() - .connector_settings(connector_settings) - .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) - .build(tcp_connector) - .adapter; - let now = tokio::time::Instant::now(); - tokio::time::pause(); - let err = hyper - .call(HttpRequest::get("https://fake-uri.com").unwrap()) - .await - .unwrap_err(); - assert!( - err.is_timeout(), - "expected err.is_timeout() to be true but it was false, err == {err:?}", - ); - let message = format!("{}", DisplayErrorContext(&err)); - let expected = "timeout: HTTP read timeout occurred after 2s"; - assert!( - message.contains(expected), - "expected '{message}' to contain '{expected}'" - ); - assert_elapsed!(now, Duration::from_secs(2)); - } -} diff --git a/rust-runtime/aws-smithy-http-client/src/lib.rs b/rust-runtime/aws-smithy-http-client/src/lib.rs index a3b57ec5ff..5e36f70445 100644 --- a/rust-runtime/aws-smithy-http-client/src/lib.rs +++ b/rust-runtime/aws-smithy-http-client/src/lib.rs @@ -26,20 +26,20 @@ #[cfg(feature = "hyper-014")] pub(crate) mod hyper_legacy; -/// Default HTTP and TLS connectors that use hyper 0.14.x and rustls. +/// Legacy HTTP and TLS connectors that use hyper 0.14.x and rustls. #[cfg(feature = "hyper-014")] #[deprecated = "hyper 0.14.x support is deprecated, please migrate to 1.x client"] pub mod hyper_014 { pub use crate::hyper_legacy::*; } -// TODO(https://github.com/smithy-lang/smithy-rs/issues/1925) - do we even want to name this/tie this to hyper? +// FIXME - remove hyper-1 from the name -/// Default HTTP and TLS connectors that use hyper 0.14.x and rustls. +/// Default HTTP and TLS connectors #[cfg(feature = "hyper-1")] -pub mod hyper_1; +pub(crate) mod client; +#[cfg(feature = "hyper-1")] +pub use client::{Builder, Connector, ConnectorBuilder, CryptoMode}; #[cfg(feature = "test-util")] pub mod test_util; - -pub(crate) mod client; diff --git a/rust-runtime/aws-smithy-http-client/src/test_util/never.rs b/rust-runtime/aws-smithy-http-client/src/test_util/never.rs index fd1633da36..a31932c104 100644 --- a/rust-runtime/aws-smithy-http-client/src/test_util/never.rs +++ b/rust-runtime/aws-smithy-http-client/src/test_util/never.rs @@ -214,7 +214,7 @@ mod test { #[cfg(feature = "hyper-1")] #[tokio::test] async fn never_tcp_connector_plugs_into_hyper_1() { - let client = crate::hyper_1::build_with_fn(None, NeverTcpConnector::new); + let client = crate::client::build_with_fn(None, NeverTcpConnector::new); let components = RuntimeComponentsBuilder::for_tests() .with_sleep_impl(Some(TokioSleep::new())) .with_time_source(Some(SystemTimeSource::new())) diff --git a/rust-runtime/aws-smithy-http-client/src/test_util/wire.rs b/rust-runtime/aws-smithy-http-client/src/test_util/wire.rs index fc0108f381..0b83824229 100644 --- a/rust-runtime/aws-smithy-http-client/src/test_util/wire.rs +++ b/rust-runtime/aws-smithy-http-client/src/test_util/wire.rs @@ -333,7 +333,7 @@ impl WireMockServer { /// **Note**: This must be used in tandem with [`Self::dns_resolver`] pub fn http_client(&self) -> SharedHttpClient { let resolver = self.dns_resolver(); - crate::hyper_1::build_with_fn(None, move || { + crate::client::build_with_fn(None, move || { hyper_util::client::legacy::connect::HttpConnector::new_with_resolver( resolver.clone().0, ) diff --git a/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs b/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs index 8b3ad7515d..8c3a1ea4df 100644 --- a/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs +++ b/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs @@ -10,7 +10,7 @@ ))] use aws_smithy_async::time::SystemTimeSource; -use aws_smithy_http_client::hyper_1::{CryptoMode, HyperClientBuilder}; +use aws_smithy_http_client::{Builder, CryptoMode}; use aws_smithy_runtime_api::client::dns::{DnsFuture, ResolveDns, ResolveDnsError}; use aws_smithy_runtime_api::client::http::{HttpClient, HttpConnector, HttpConnectorSettings}; use aws_smithy_runtime_api::client::orchestrator::HttpRequest; @@ -24,16 +24,14 @@ use tower::Service; #[cfg(feature = "crypto-ring")] #[tokio::test] async fn ring_client() { - let client = HyperClientBuilder::new() - .crypto_mode(CryptoMode::Ring) - .build_https(); + let client = Builder::new().crypto_mode(CryptoMode::Ring).build_https(); smoke_test_client(&client).await.unwrap(); } #[cfg(feature = "crypto-aws-lc-fips")] #[tokio::test] async fn aws_lc_fips_client() { - let client = HyperClientBuilder::new() + let client = Builder::new() .crypto_mode(CryptoMode::AwsLcFips) .build_https(); smoke_test_client(&client).await.unwrap(); @@ -42,9 +40,7 @@ async fn aws_lc_fips_client() { #[cfg(feature = "crypto-aws-lc")] #[tokio::test] async fn aws_lc_client() { - let client = HyperClientBuilder::new() - .crypto_mode(CryptoMode::AwsLc) - .build_https(); + let client = Builder::new().crypto_mode(CryptoMode::AwsLc).build_https(); smoke_test_client(&client).await.unwrap(); } @@ -73,7 +69,7 @@ async fn custom_dns_client() { inner: GaiResolver::new(), count: Default::default(), }; - let client = HyperClientBuilder::new() + let client = Builder::new() .crypto_mode(CryptoMode::Ring) .build_with_resolver(resolver.clone()); smoke_test_client(&client).await.unwrap(); From 58030a6e39f6d4397799ceca9515f24f3edb8a87 Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Tue, 12 Nov 2024 13:52:29 -0500 Subject: [PATCH 03/13] break out tls implementation and rename types --- .../aws-smithy-http-client/Cargo.toml | 13 +- .../examples/client-aws-lc.rs | 11 +- .../examples/client-ring.rs | 9 +- .../examples/custom-dns.rs | 7 +- .../aws-smithy-http-client/src/client.rs | 264 ++++-------------- .../aws-smithy-http-client/src/client/tls.rs | 186 ++++++++++++ .../aws-smithy-http-client/src/lib.rs | 2 +- .../tests/smoke_test_clients.rs | 38 ++- 8 files changed, 301 insertions(+), 229 deletions(-) create mode 100644 rust-runtime/aws-smithy-http-client/src/client/tls.rs diff --git a/rust-runtime/aws-smithy-http-client/Cargo.toml b/rust-runtime/aws-smithy-http-client/Cargo.toml index 718ec0e770..c0bb7f78b0 100644 --- a/rust-runtime/aws-smithy-http-client/Cargo.toml +++ b/rust-runtime/aws-smithy-http-client/Cargo.toml @@ -16,6 +16,7 @@ hyper-014 = [ "dep:hyper-0-14", ] +# FIXME - rename feature and add default feature(s) for default client/tls hyper-1 = [ "aws-smithy-runtime-api/http-1x", "aws-smithy-types/http-body-1-x", @@ -64,9 +65,9 @@ legacy-test-util = [ legacy-tls-rustls = ["dep:legacy-hyper-rustls", "dep:legacy-rustls", "hyper-014"] # FIXME - s2n-tls option, distinguish between rustls and s2n -crypto-ring = ["dep:rustls", "rustls?/ring", "dep:hyper-rustls", "hyper-1"] -crypto-aws-lc = ["dep:rustls", "rustls?/aws_lc_rs", "dep:hyper-rustls", "hyper-1"] -crypto-aws-lc-fips = ["dep:rustls", "rustls?/fips", "dep:hyper-rustls", "hyper-1"] +rustls-ring = ["dep:rustls", "rustls?/ring", "dep:hyper-rustls", "hyper-1"] +rustls-aws-lc = ["dep:rustls", "rustls?/aws_lc_rs", "dep:hyper-rustls", "hyper-1"] +rustls-aws-lc-fips = ["dep:rustls", "rustls?/fips", "dep:hyper-rustls", "hyper-1"] [dependencies] aws-smithy-async = { path = "../aws-smithy-async" } @@ -115,17 +116,17 @@ tokio = { version = "1", features = ["macros", "rt", "rt-multi-thread", "test-ut [[example]] name = "client-ring" -required-features = ["crypto-ring"] +required-features = ["rustls-ring"] doc-scrape-examples = true [[example]] name = "client-aws-lc" -required-features = ["crypto-aws-lc", "crypto-aws-lc-fips"] +required-features = ["rustls-aws-lc", "rustls-aws-lc-fips"] doc-scrape-examples = true [[example]] name = "custom-dns" -required-features = ["crypto-ring"] +required-features = ["rustls-ring"] doc-scrape-examples = true [package.metadata.smithy-rs-release-tooling] diff --git a/rust-runtime/aws-smithy-http-client/examples/client-aws-lc.rs b/rust-runtime/aws-smithy-http-client/examples/client-aws-lc.rs index d727f03e25..a013714ff4 100644 --- a/rust-runtime/aws-smithy-http-client/examples/client-aws-lc.rs +++ b/rust-runtime/aws-smithy-http-client/examples/client-aws-lc.rs @@ -3,16 +3,21 @@ * SPDX-License-Identifier: Apache-2.0 */ -use aws_smithy_http_client::{Builder, CryptoMode}; +use aws_smithy_http_client::{ + tls::{self, rustls_provider::CryptoMode}, + Builder, +}; #[tokio::main] async fn main() { // feature = crypto-aws-lc - let _client = Builder::new().crypto_mode(CryptoMode::AwsLc).build_https(); + let _client = Builder::new() + .tls_provider(tls::Provider::Rustls(CryptoMode::AwsLc)) + .build_https(); // feature = crypto-aws-lc-fips // A FIPS client can also be created. Note that this has a more complex build environment required. let _client = Builder::new() - .crypto_mode(CryptoMode::AwsLcFips) + .tls_provider(tls::Provider::Rustls(CryptoMode::AwsLcFips)) .build_https(); } diff --git a/rust-runtime/aws-smithy-http-client/examples/client-ring.rs b/rust-runtime/aws-smithy-http-client/examples/client-ring.rs index 55393310e4..790446972a 100644 --- a/rust-runtime/aws-smithy-http-client/examples/client-ring.rs +++ b/rust-runtime/aws-smithy-http-client/examples/client-ring.rs @@ -3,8 +3,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -use aws_smithy_http_client::{Builder, CryptoMode}; +use aws_smithy_http_client::{ + tls::{self, rustls_provider::CryptoMode}, + Builder, +}; fn main() { - let _client = Builder::new().crypto_mode(CryptoMode::Ring).build_https(); + let _client = Builder::new() + .tls_provider(tls::Provider::Rustls(CryptoMode::Ring)) + .build_https(); } diff --git a/rust-runtime/aws-smithy-http-client/examples/custom-dns.rs b/rust-runtime/aws-smithy-http-client/examples/custom-dns.rs index e083f8eb8e..627bf5c391 100644 --- a/rust-runtime/aws-smithy-http-client/examples/custom-dns.rs +++ b/rust-runtime/aws-smithy-http-client/examples/custom-dns.rs @@ -3,7 +3,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -use aws_smithy_http_client::{Builder, CryptoMode}; +use aws_smithy_http_client::{ + tls::{self, rustls_provider::CryptoMode}, + Builder, +}; use aws_smithy_runtime_api::client::dns::{DnsFuture, ResolveDns}; use std::net::{IpAddr, Ipv4Addr}; @@ -18,6 +21,6 @@ impl ResolveDns for StaticResolver { fn main() { let _client = Builder::new() - .crypto_mode(CryptoMode::Ring) + .tls_provider(tls::Provider::Rustls(CryptoMode::Ring)) .build_with_resolver(StaticResolver); } diff --git a/rust-runtime/aws-smithy-http-client/src/client.rs b/rust-runtime/aws-smithy-http-client/src/client.rs index 8e3bcb7d1d..82a9096e9e 100644 --- a/rust-runtime/aws-smithy-http-client/src/client.rs +++ b/rust-runtime/aws-smithy-http-client/src/client.rs @@ -5,6 +5,8 @@ mod dns; mod timeout; +/// TLS connector(s) +pub mod tls; use aws_smithy_async::future::timeout::TimedOutError; use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep, SharedAsyncSleep}; @@ -36,7 +38,6 @@ use hyper_util::client::legacy::connect::{ capture_connection, CaptureConnection, Connect, HttpInfo, }; use hyper_util::rt::TokioExecutor; -use rustls::crypto::CryptoProvider; use std::borrow::Cow; use std::collections::HashMap; use std::error::Error; @@ -44,147 +45,6 @@ use std::fmt; use std::sync::RwLock; use std::time::Duration; -/// Choice of underlying cryptography library -#[derive(Debug, Eq, PartialEq, Clone, Copy)] -#[non_exhaustive] -pub enum CryptoMode { - /// Crypto based on [ring](https://github.com/briansmith/ring) - #[cfg(feature = "crypto-ring")] - Ring, - /// Crypto based on [aws-lc](https://github.com/aws/aws-lc-rs) - #[cfg(feature = "crypto-aws-lc")] - AwsLc, - /// FIPS compliant variant of [aws-lc](https://github.com/aws/aws-lc-rs) - #[cfg(feature = "crypto-aws-lc-fips")] - AwsLcFips, -} - -impl CryptoMode { - fn provider(self) -> CryptoProvider { - match self { - #[cfg(feature = "crypto-aws-lc")] - CryptoMode::AwsLc => rustls::crypto::aws_lc_rs::default_provider(), - - #[cfg(feature = "crypto-ring")] - CryptoMode::Ring => rustls::crypto::ring::default_provider(), - - #[cfg(feature = "crypto-aws-lc-fips")] - CryptoMode::AwsLcFips => { - let provider = rustls::crypto::default_fips_provider(); - assert!( - provider.fips(), - "FIPS was requested but the provider did not support FIPS" - ); - provider - } - } - } -} - -#[allow(unused_imports)] -mod cached_connectors { - use client::connect::HttpConnector; - use hyper_util::client::legacy as client; - use hyper_util::client::legacy::connect::dns::GaiResolver; - - use crate::client::build_connector::make_tls; - use crate::client::{CryptoMode, Inner}; - - #[cfg(feature = "crypto-ring")] - pub(crate) static HTTPS_NATIVE_ROOTS_RING: once_cell::sync::Lazy< - hyper_rustls::HttpsConnector, - > = once_cell::sync::Lazy::new(|| make_tls(GaiResolver::new(), CryptoMode::Ring.provider())); - - #[cfg(feature = "crypto-aws-lc")] - pub(crate) static HTTPS_NATIVE_ROOTS_AWS_LC: once_cell::sync::Lazy< - hyper_rustls::HttpsConnector, - > = once_cell::sync::Lazy::new(|| make_tls(GaiResolver::new(), CryptoMode::AwsLc.provider())); - - #[cfg(feature = "crypto-aws-lc-fips")] - pub(crate) static HTTPS_NATIVE_ROOTS_AWS_LC_FIPS: once_cell::sync::Lazy< - hyper_rustls::HttpsConnector, - > = once_cell::sync::Lazy::new(|| { - make_tls(GaiResolver::new(), CryptoMode::AwsLcFips.provider()) - }); - - pub(super) fn cached_https(mode: Inner) -> hyper_rustls::HttpsConnector { - match mode { - #[cfg(feature = "crypto-ring")] - Inner::Standard(CryptoMode::Ring) => HTTPS_NATIVE_ROOTS_RING.clone(), - #[cfg(feature = "crypto-aws-lc")] - Inner::Standard(CryptoMode::AwsLc) => HTTPS_NATIVE_ROOTS_AWS_LC.clone(), - #[cfg(feature = "crypto-aws-lc-fips")] - Inner::Standard(CryptoMode::AwsLcFips) => HTTPS_NATIVE_ROOTS_AWS_LC_FIPS.clone(), - #[allow(unreachable_patterns)] - Inner::Standard(_) => unreachable!("unexpected mode"), - Inner::Custom(provider) => make_tls(GaiResolver::new(), provider), - } - } -} - -mod build_connector { - use crate::client::{dns::HyperUtilResolver, Inner}; - use aws_smithy_runtime_api::client::dns::ResolveDns; - use client::connect::HttpConnector; - use hyper_util::client::legacy as client; - use rustls::crypto::CryptoProvider; - use std::sync::Arc; - - fn restrict_ciphers(base: CryptoProvider) -> CryptoProvider { - let suites = &[ - rustls::CipherSuite::TLS13_AES_256_GCM_SHA384, - rustls::CipherSuite::TLS13_AES_128_GCM_SHA256, - // TLS1.2 suites - rustls::CipherSuite::TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, - rustls::CipherSuite::TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, - rustls::CipherSuite::TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, - rustls::CipherSuite::TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, - rustls::CipherSuite::TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, - ]; - let supported_suites = suites - .iter() - .flat_map(|suite| { - base.cipher_suites - .iter() - .find(|s| &s.suite() == suite) - .cloned() - }) - .collect::>(); - CryptoProvider { - cipher_suites: supported_suites, - ..base - } - } - - pub(crate) fn make_tls( - resolver: R, - crypto_provider: CryptoProvider, - ) -> hyper_rustls::HttpsConnector> { - use hyper_rustls::ConfigBuilderExt; - let mut base_connector = HttpConnector::new_with_resolver(resolver); - base_connector.enforce_http(false); - hyper_rustls::HttpsConnectorBuilder::new() - .with_tls_config( - rustls::ClientConfig::builder_with_provider(Arc::new(restrict_ciphers(crypto_provider))) - .with_safe_default_protocol_versions() - .expect("Error with the TLS configuration. Please file a bug report under https://github.com/smithy-lang/smithy-rs/issues.") - .with_native_roots().expect("error with TLS configuration.") - .with_no_client_auth() - ) - .https_or_http() - .enable_http1() - .enable_http2() - .wrap_connector(base_connector) - } - - pub(super) fn https_with_resolver( - crypto_provider: Inner, - resolver: R, - ) -> hyper_rustls::HttpsConnector>> { - make_tls(HyperUtilResolver { resolver }, crypto_provider.provider()) - } -} - /// [`HttpConnector`] used to make HTTP requests. /// /// This connector also implements socket connect and read timeouts. @@ -211,47 +71,46 @@ impl HttpConnector for Connector { /// Builder for [`Connector`]. #[derive(Default, Debug)] -pub struct ConnectorBuilder { +pub struct ConnectorBuilder { connector_settings: Option, sleep_impl: Option, client_builder: Option, #[allow(unused)] - crypto: Crypto, + tls: Tls, } -/// Initial builder state, [`CryptoMode`] choice required +/// Initial builder state, [`TlsProvider`] choice required #[derive(Default)] #[non_exhaustive] -pub struct CryptoUnset {} +pub struct TlsUnset {} /// Crypto implementation selected -pub struct CryptoProviderSelected { - crypto_provider: Inner, +pub struct TlsProviderSelected { + tls_provider: tls::Provider, } -#[derive(Clone)] -enum Inner { - Standard(CryptoMode), - #[allow(dead_code)] - Custom(CryptoProvider), -} - -impl Inner { - fn provider(&self) -> CryptoProvider { - match self { - Inner::Standard(mode) => mode.provider(), - Inner::Custom(provider) => provider.clone(), - } - } -} - -#[cfg(any(feature = "crypto-aws-lc", feature = "crypto-ring"))] -impl ConnectorBuilder { +#[cfg(any( + feature = "rustls-aws-lc", + feature = "rustls-aws-lc-fips", + feature = "rustls-ring" +))] +impl ConnectorBuilder { /// Build a [`Connector`] that will use the given DNS resolver implementation. pub fn build_from_resolver(self, resolver: R) -> Connector { - let connector = - build_connector::https_with_resolver(self.crypto.crypto_provider.clone(), resolver); - self.build(connector) + match &self.tls.tls_provider { + #[cfg(any( + feature = "rustls-aws-lc", + feature = "rustls-aws-lc-fips", + feature = "rustls-ring" + ))] + tls::Provider::Rustls(crypto_mode) => { + let connector = tls::rustls_provider::build_connector::https_with_resolver( + crypto_mode.clone(), + resolver, + ); + self.build(connector) + } + } } } @@ -608,21 +467,28 @@ where /// This can be useful when you want to share a Hyper connector between multiple /// generated Smithy clients. #[derive(Clone, Default, Debug)] -pub struct Builder { +pub struct Builder { client_builder: Option, - crypto_provider: Crypto, + tls_provider: Tls, } -impl Builder { +impl Builder { /// Create a hyper client using RusTLS for TLS /// /// The trusted certificates will be loaded later when this becomes the selected /// HTTP client for a Smithy client. pub fn build_https(self) -> SharedHttpClient { - let crypto = self.crypto_provider.crypto_provider; - build_with_fn(self.client_builder, move || { - cached_connectors::cached_https(crypto.clone()) - }) + let provider = self.tls_provider.tls_provider; + match provider { + #[cfg(any( + feature = "rustls-aws-lc", + feature = "rustls-aws-lc-fips", + feature = "rustls-ring" + ))] + tls::Provider::Rustls(crypto_mode) => build_with_fn(self.client_builder, move || { + tls::rustls_provider::cached_connectors::cached_https(crypto_mode.clone()) + }), + } } /// Create a hyper client using a custom DNS resolver @@ -630,43 +496,37 @@ impl Builder { self, resolver: impl ResolveDns + Clone + 'static, ) -> SharedHttpClient { - build_with_fn(self.client_builder, move || { - build_connector::https_with_resolver( - self.crypto_provider.crypto_provider.clone(), - resolver.clone(), - ) - }) + let provider = self.tls_provider.tls_provider; + match provider { + #[cfg(any( + feature = "rustls-aws-lc", + feature = "rustls-aws-lc-fips", + feature = "rustls-ring" + ))] + tls::Provider::Rustls(crypto_mode) => build_with_fn(self.client_builder, move || { + tls::rustls_provider::build_connector::https_with_resolver( + crypto_mode.clone(), + resolver.clone(), + ) + }), + } } } -impl Builder { +impl Builder { /// Creates a new builder. pub fn new() -> Self { Self::default() } - /// Set the cryptography implementation to use - pub fn crypto_mode(self, provider: CryptoMode) -> Builder { - Builder { - client_builder: self.client_builder, - crypto_provider: CryptoProviderSelected { - crypto_provider: Inner::Standard(provider), - }, - } - } + // TODO(hyper1) - build for unsecure HTTP? - /// This interface will be broken in the future - /// - /// This exposes `CryptoProvider` from `rustls` directly and this API has no stability guarantee. - #[cfg(crypto_unstable)] - pub fn crypto_provider_unstable( - self, - provider: CryptoProvider, - ) -> Builder { + /// Set the TLS implementation to use + pub fn tls_provider(self, provider: tls::Provider) -> Builder { Builder { client_builder: self.client_builder, - crypto_provider: CryptoProviderSelected { - crypto_provider: Inner::Custom(provider), + tls_provider: TlsProviderSelected { + tls_provider: provider, }, } } diff --git a/rust-runtime/aws-smithy-http-client/src/client/tls.rs b/rust-runtime/aws-smithy-http-client/src/client/tls.rs new file mode 100644 index 0000000000..85d9750777 --- /dev/null +++ b/rust-runtime/aws-smithy-http-client/src/client/tls.rs @@ -0,0 +1,186 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +/// Choice of underlying cryptography library +#[derive(Debug, Eq, PartialEq, Clone)] +#[non_exhaustive] +pub enum Provider { + #[cfg(any( + feature = "rustls-aws-lc", + feature = "rustls-aws-lc-fips", + feature = "rustls-ring" + ))] + /// TLS provider based on [rustls](https://github.com/rustls/rustls) + Rustls(rustls_provider::CryptoMode), + // TLS provider based on [S2N](https://github.com/aws/s2n-tls) + // S2n, + // TODO(hyper1) s2n support + + // TODO(hyper1): consider native-tls support? +} + +/// rustls based support and adapters +#[cfg(any( + feature = "rustls-aws-lc", + feature = "rustls-aws-lc-fips", + feature = "rustls-ring" +))] +pub mod rustls_provider { + use crate::client::tls::Provider; + use rustls::crypto::CryptoProvider; + + /// Choice of underlying cryptography library (this only applies to rustls) + #[derive(Debug, Eq, PartialEq, Clone)] + #[non_exhaustive] + pub enum CryptoMode { + /// Crypto based on [ring](https://github.com/briansmith/ring) + #[cfg(feature = "rustls-ring")] + Ring, + /// Crypto based on [aws-lc](https://github.com/aws/aws-lc-rs) + #[cfg(feature = "rustls-aws-lc")] + AwsLc, + /// FIPS compliant variant of [aws-lc](https://github.com/aws/aws-lc-rs) + #[cfg(feature = "rustls-aws-lc-fips")] + AwsLcFips, + } + + impl CryptoMode { + fn provider(self) -> CryptoProvider { + match self { + #[cfg(feature = "rustls-aws-lc")] + CryptoMode::AwsLc => rustls::crypto::aws_lc_rs::default_provider(), + + #[cfg(feature = "rustls-ring")] + CryptoMode::Ring => rustls::crypto::ring::default_provider(), + + #[cfg(feature = "rustls-aws-lc-fips")] + CryptoMode::AwsLcFips => { + let provider = rustls::crypto::default_fips_provider(); + assert!( + provider.fips(), + "FIPS was requested but the provider did not support FIPS" + ); + provider + } + } + } + } + + impl Provider { + /// Create a TLS provider based on [rustls](https://github.com/rustls/rustls) + /// and the given [`CryptoMode`] + pub fn rustls(mode: CryptoMode) -> Provider { + Provider::Rustls(mode) + } + } + + #[allow(unused_imports)] + pub(crate) mod cached_connectors { + use client::connect::HttpConnector; + use hyper_util::client::legacy as client; + use hyper_util::client::legacy::connect::dns::GaiResolver; + + use super::build_connector::make_tls; + use super::CryptoMode; + + #[cfg(feature = "rustls-ring")] + static HTTPS_NATIVE_ROOTS_RING: once_cell::sync::Lazy< + hyper_rustls::HttpsConnector, + > = once_cell::sync::Lazy::new(|| { + make_tls(GaiResolver::new(), CryptoMode::Ring.provider()) + }); + + #[cfg(feature = "rustls-aws-lc")] + static HTTPS_NATIVE_ROOTS_AWS_LC: once_cell::sync::Lazy< + hyper_rustls::HttpsConnector, + > = once_cell::sync::Lazy::new(|| { + make_tls(GaiResolver::new(), CryptoMode::AwsLc.provider()) + }); + + #[cfg(feature = "rustls-aws-lc-fips")] + static HTTPS_NATIVE_ROOTS_AWS_LC_FIPS: once_cell::sync::Lazy< + hyper_rustls::HttpsConnector, + > = once_cell::sync::Lazy::new(|| { + make_tls(GaiResolver::new(), CryptoMode::AwsLcFips.provider()) + }); + + pub(crate) fn cached_https( + mode: CryptoMode, + ) -> hyper_rustls::HttpsConnector { + match mode { + #[cfg(feature = "rustls-ring")] + CryptoMode::Ring => HTTPS_NATIVE_ROOTS_RING.clone(), + #[cfg(feature = "rustls-aws-lc")] + CryptoMode::AwsLc => HTTPS_NATIVE_ROOTS_AWS_LC.clone(), + #[cfg(feature = "rustls-aws-lc-fips")] + CryptoMode::AwsLcFips => HTTPS_NATIVE_ROOTS_AWS_LC_FIPS.clone(), + } + } + } + + pub(crate) mod build_connector { + use crate::client::dns::HyperUtilResolver; + use crate::client::tls::rustls_provider::CryptoMode; + use aws_smithy_runtime_api::client::dns::ResolveDns; + use client::connect::HttpConnector; + use hyper_util::client::legacy as client; + use rustls::crypto::CryptoProvider; + use std::sync::Arc; + + fn restrict_ciphers(base: CryptoProvider) -> CryptoProvider { + let suites = &[ + rustls::CipherSuite::TLS13_AES_256_GCM_SHA384, + rustls::CipherSuite::TLS13_AES_128_GCM_SHA256, + // TLS1.2 suites + rustls::CipherSuite::TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + rustls::CipherSuite::TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + rustls::CipherSuite::TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + rustls::CipherSuite::TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + rustls::CipherSuite::TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + ]; + let supported_suites = suites + .iter() + .flat_map(|suite| { + base.cipher_suites + .iter() + .find(|s| &s.suite() == suite) + .cloned() + }) + .collect::>(); + CryptoProvider { + cipher_suites: supported_suites, + ..base + } + } + + pub(super) fn make_tls( + resolver: R, + crypto_provider: CryptoProvider, + ) -> hyper_rustls::HttpsConnector> { + use hyper_rustls::ConfigBuilderExt; + let mut base_connector = HttpConnector::new_with_resolver(resolver); + base_connector.enforce_http(false); + hyper_rustls::HttpsConnectorBuilder::new() + .with_tls_config( + rustls::ClientConfig::builder_with_provider(Arc::new(restrict_ciphers(crypto_provider))) + .with_safe_default_protocol_versions() + .expect("Error with the TLS configuration. Please file a bug report under https://github.com/smithy-lang/smithy-rs/issues.") + .with_native_roots().expect("error with TLS configuration.") + .with_no_client_auth() + ) + .https_or_http() + .enable_http1() + .enable_http2() + .wrap_connector(base_connector) + } + + pub(crate) fn https_with_resolver( + crypto_mode: CryptoMode, + resolver: R, + ) -> hyper_rustls::HttpsConnector>> { + make_tls(HyperUtilResolver { resolver }, crypto_mode.provider()) + } + } +} diff --git a/rust-runtime/aws-smithy-http-client/src/lib.rs b/rust-runtime/aws-smithy-http-client/src/lib.rs index 5e36f70445..5f37dff25f 100644 --- a/rust-runtime/aws-smithy-http-client/src/lib.rs +++ b/rust-runtime/aws-smithy-http-client/src/lib.rs @@ -39,7 +39,7 @@ pub mod hyper_014 { #[cfg(feature = "hyper-1")] pub(crate) mod client; #[cfg(feature = "hyper-1")] -pub use client::{Builder, Connector, ConnectorBuilder, CryptoMode}; +pub use client::{tls, Builder, Connector, ConnectorBuilder}; #[cfg(feature = "test-util")] pub mod test_util; diff --git a/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs b/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs index 8c3a1ea4df..3ac265bed1 100644 --- a/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs +++ b/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs @@ -4,13 +4,13 @@ */ #![cfg(any( - feature = "crypto-ring", - feature = "crypto-aws-lc", - feature = "crypto-aws-lc-fips" + feature = "rustls-ring", + feature = "rustls-aws-lc", + feature = "rustls-aws-lc-fips" ))] use aws_smithy_async::time::SystemTimeSource; -use aws_smithy_http_client::{Builder, CryptoMode}; +use aws_smithy_http_client::{tls, Builder}; use aws_smithy_runtime_api::client::dns::{DnsFuture, ResolveDns, ResolveDnsError}; use aws_smithy_runtime_api::client::http::{HttpClient, HttpConnector, HttpConnectorSettings}; use aws_smithy_runtime_api::client::orchestrator::HttpRequest; @@ -21,30 +21,40 @@ use std::str::FromStr; use std::sync::Arc; use tower::Service; -#[cfg(feature = "crypto-ring")] +#[cfg(feature = "rustls-ring")] #[tokio::test] async fn ring_client() { - let client = Builder::new().crypto_mode(CryptoMode::Ring).build_https(); + let client = Builder::new() + .tls_provider(tls::Provider::Rustls( + tls::rustls_provider::CryptoMode::Ring, + )) + .build_https(); smoke_test_client(&client).await.unwrap(); } -#[cfg(feature = "crypto-aws-lc-fips")] +#[cfg(feature = "rustls-aws-lc-fips")] #[tokio::test] async fn aws_lc_fips_client() { let client = Builder::new() - .crypto_mode(CryptoMode::AwsLcFips) + .tls_provider(tls::Provider::Rustls( + tls::rustls_provider::CryptoMode::AwsLcFips, + )) .build_https(); smoke_test_client(&client).await.unwrap(); } -#[cfg(feature = "crypto-aws-lc")] +#[cfg(feature = "rustls-aws-lc")] #[tokio::test] async fn aws_lc_client() { - let client = Builder::new().crypto_mode(CryptoMode::AwsLc).build_https(); + let client = Builder::new() + .tls_provider(tls::Provider::Rustls( + tls::rustls_provider::CryptoMode::AwsLc, + )) + .build_https(); smoke_test_client(&client).await.unwrap(); } -#[cfg(feature = "crypto-ring")] +#[cfg(feature = "rustls-ring")] #[tokio::test] async fn custom_dns_client() { use std::sync::atomic::{AtomicUsize, Ordering}; @@ -70,13 +80,15 @@ async fn custom_dns_client() { count: Default::default(), }; let client = Builder::new() - .crypto_mode(CryptoMode::Ring) + .tls_provider(tls::Provider::Rustls( + tls::rustls_provider::CryptoMode::Ring, + )) .build_with_resolver(resolver.clone()); smoke_test_client(&client).await.unwrap(); assert_eq!(resolver.count.load(Ordering::Relaxed), 1); } -#[cfg(feature = "crypto-ring")] +#[cfg(feature = "rustls-ring")] async fn smoke_test_client(client: &dyn HttpClient) -> Result<(), Box> { let connector_settings = HttpConnectorSettings::builder().build(); let runtime_components = RuntimeComponentsBuilder::for_tests() From d2faf8d2acba98225faba8addcbbf0e2c08059e3 Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Wed, 13 Nov 2024 14:24:14 -0500 Subject: [PATCH 04/13] fix feature flags --- .../rustsdk/IntegrationTestDependencies.kt | 2 +- aws/sdk/integration-tests/s3/Cargo.toml | 2 +- rust-runtime/aws-smithy-http-client/Cargo.toml | 13 ++++--------- .../examples/client-aws-lc.rs | 4 ++-- .../aws-smithy-http-client/src/client/tls.rs | 1 - .../aws-smithy-http-client/src/hyper_legacy.rs | 16 ++++++++-------- .../src/test_util/dvr/record.rs | 2 +- .../src/test_util/never.rs | 3 ++- .../aws-smithy-http-client/src/test_util/wire.rs | 2 +- .../tests/smoke_test_clients.rs | 1 - rust-runtime/aws-smithy-runtime/Cargo.toml | 2 +- tools/ci-scripts/test-windows.sh | 2 +- 12 files changed, 22 insertions(+), 28 deletions(-) diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/IntegrationTestDependencies.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/IntegrationTestDependencies.kt index 05eb566e7a..6bb3e32226 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/IntegrationTestDependencies.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/IntegrationTestDependencies.kt @@ -149,7 +149,7 @@ class S3TestDependencies(private val codegenContext: ClientCodegenContext) : Lib override fun section(section: LibRsSection): Writable = writable { addDependency(awsConfig(codegenContext.runtimeConfig).toDevDependency().withFeature("behavior-version-latest")) - addDependency(smithyHttpClient(codegenContext.runtimeConfig).toDevDependency().withFeature("crypto-ring")) + addDependency(smithyHttpClient(codegenContext.runtimeConfig).toDevDependency().withFeature("rustls-ring")) addDependency(AsyncStd) addDependency(BytesUtils.toDevDependency()) addDependency(FastRand.toDevDependency()) diff --git a/aws/sdk/integration-tests/s3/Cargo.toml b/aws/sdk/integration-tests/s3/Cargo.toml index 5c35bcd7bb..74eca08315 100644 --- a/aws/sdk/integration-tests/s3/Cargo.toml +++ b/aws/sdk/integration-tests/s3/Cargo.toml @@ -26,7 +26,7 @@ aws-smithy-protocol-test = { path = "../../build/aws-sdk/sdk/aws-smithy-protocol aws-smithy-runtime = { path = "../../build/aws-sdk/sdk/aws-smithy-runtime", features = ["test-util"] } aws-smithy-runtime-api = { path = "../../build/aws-sdk/sdk/aws-smithy-runtime-api", features = ["test-util", "http-1x"] } aws-smithy-types = { path = "../../build/aws-sdk/sdk/aws-smithy-types" } -aws-smithy-http-client = { path = "../../build/aws-sdk/sdk/aws-smithy-http-client", features = ["hyper-1", "crypto-ring", "test-util", "wire-mock"] } +aws-smithy-http-client = { path = "../../build/aws-sdk/sdk/aws-smithy-http-client", features = ["hyper-1", "rustls-ring", "test-util", "wire-mock"] } aws-types = { path = "../../build/aws-sdk/sdk/aws-types" } bytes = "1" bytes-utils = "0.1.2" diff --git a/rust-runtime/aws-smithy-http-client/Cargo.toml b/rust-runtime/aws-smithy-http-client/Cargo.toml index c0bb7f78b0..706cf3dd36 100644 --- a/rust-runtime/aws-smithy-http-client/Cargo.toml +++ b/rust-runtime/aws-smithy-http-client/Cargo.toml @@ -25,10 +25,6 @@ hyper-1 = [ "hyper-util?/client-legacy", "dep:http-1x", "dep:tower", - - # FIXME - should not be needed in base http implementation, only for https and only for specific tls impl - "dep:rustls", - "dep:hyper-rustls", ] wire-mock = [ @@ -59,12 +55,12 @@ test-util = [ legacy-test-util = [ "test-util", "dep:http-02x", - "aws-smithy-runtime-api/http-02x" + "aws-smithy-runtime-api/http-02x", + "aws-smithy-types/http-body-0-4-x", ] -legacy-tls-rustls = ["dep:legacy-hyper-rustls", "dep:legacy-rustls", "hyper-014"] +legacy-rustls-ring = ["dep:legacy-hyper-rustls", "dep:legacy-rustls", "hyper-014"] -# FIXME - s2n-tls option, distinguish between rustls and s2n rustls-ring = ["dep:rustls", "rustls?/ring", "dep:hyper-rustls", "hyper-1"] rustls-aws-lc = ["dep:rustls", "rustls?/aws_lc_rs", "dep:hyper-rustls", "hyper-1"] rustls-aws-lc-fips = ["dep:rustls", "rustls?/fips", "dep:hyper-rustls", "hyper-1"] @@ -75,7 +71,6 @@ aws-smithy-runtime-api = { path = "../aws-smithy-runtime-api", features = ["clie aws-smithy-types = { path = "../aws-smithy-types" } aws-smithy-protocol-test = { path = "../aws-smithy-protocol-test", optional = true } h2 = { version = "0.4", default-features = false } -# FIXME - do we need tokio enabled by default? tokio = { version = "1.40", features = [] } once_cell = "1.20.1" pin-project-lite = "0.2.14" @@ -83,7 +78,7 @@ tracing = "0.1" # hyper 1.x stack hyper = { version = "1", features = ["client", "http1", "http2"], optional = true } -hyper-util = { version = "0.1.7", optional = true } +hyper-util = { version = "0.1.7", features = ["http1", "http2"], optional = true } http-1x = { package = "http", version = "1" , optional = true } http-body-1x = { package = "http-body", version = "1", optional = true} hyper-rustls = { version = "0.27", features = ["http2", "http1", "native-tokio", "tls12"], default-features = false, optional = true } diff --git a/rust-runtime/aws-smithy-http-client/examples/client-aws-lc.rs b/rust-runtime/aws-smithy-http-client/examples/client-aws-lc.rs index a013714ff4..462ab5e885 100644 --- a/rust-runtime/aws-smithy-http-client/examples/client-aws-lc.rs +++ b/rust-runtime/aws-smithy-http-client/examples/client-aws-lc.rs @@ -10,12 +10,12 @@ use aws_smithy_http_client::{ #[tokio::main] async fn main() { - // feature = crypto-aws-lc + // feature = rustls-aws-lc let _client = Builder::new() .tls_provider(tls::Provider::Rustls(CryptoMode::AwsLc)) .build_https(); - // feature = crypto-aws-lc-fips + // feature = rustls-aws-lc-fips // A FIPS client can also be created. Note that this has a more complex build environment required. let _client = Builder::new() .tls_provider(tls::Provider::Rustls(CryptoMode::AwsLcFips)) diff --git a/rust-runtime/aws-smithy-http-client/src/client/tls.rs b/rust-runtime/aws-smithy-http-client/src/client/tls.rs index 85d9750777..5a07937158 100644 --- a/rust-runtime/aws-smithy-http-client/src/client/tls.rs +++ b/rust-runtime/aws-smithy-http-client/src/client/tls.rs @@ -17,7 +17,6 @@ pub enum Provider { // TLS provider based on [S2N](https://github.com/aws/s2n-tls) // S2n, // TODO(hyper1) s2n support - // TODO(hyper1): consider native-tls support? } diff --git a/rust-runtime/aws-smithy-http-client/src/hyper_legacy.rs b/rust-runtime/aws-smithy-http-client/src/hyper_legacy.rs index adec6b5023..ce6f91e14d 100644 --- a/rust-runtime/aws-smithy-http-client/src/hyper_legacy.rs +++ b/rust-runtime/aws-smithy-http-client/src/hyper_legacy.rs @@ -34,7 +34,7 @@ use std::sync::RwLock; use std::time::Duration; use tokio::io::{AsyncRead, AsyncWrite}; -#[cfg(feature = "legacy-tls-rustls")] +#[cfg(feature = "legacy-rustls-ring")] mod default_connector { use aws_smithy_async::rt::sleep::SharedAsyncSleep; use aws_smithy_runtime_api::client::http::HttpConnectorSettings; @@ -100,13 +100,13 @@ pub fn default_connector( settings: &HttpConnectorSettings, sleep: Option, ) -> Option { - #[cfg(feature = "legacy-tls-rustls")] + #[cfg(feature = "legacy-rustls-ring")] { tracing::trace!(settings = ?settings, sleep = ?sleep, "creating a new default connector"); let hyper = default_connector::base(settings, sleep).build_https(); Some(SharedHttpConnector::new(hyper)) } - #[cfg(not(feature = "legacy-tls-rustls"))] + #[cfg(not(feature = "legacy-rustls-ring"))] { tracing::trace!(settings = ?settings, sleep = ?sleep, "no default connector available"); None @@ -115,12 +115,12 @@ pub fn default_connector( /// Creates a hyper-backed HTTPS client from defaults depending on what cargo features are activated. pub fn default_client() -> Option { - #[cfg(feature = "legacy-tls-rustls")] + #[cfg(feature = "legacy-rustls-ring")] { tracing::trace!("creating a new default hyper 0.14.x client"); Some(HyperClientBuilder::new().build_https()) } - #[cfg(not(feature = "legacy-tls-rustls"))] + #[cfg(not(feature = "legacy-rustls-ring"))] { tracing::trace!("no default connector available"); None @@ -204,7 +204,7 @@ impl HyperConnectorBuilder { } /// Create a [`HyperConnector`] with the default rustls HTTPS implementation. - #[cfg(feature = "legacy-tls-rustls")] + #[cfg(feature = "legacy-rustls-ring")] pub fn build_https(self) -> HyperConnector { self.build(default_connector::https()) } @@ -582,7 +582,7 @@ impl HyperClientBuilder { /// /// The trusted certificates will be loaded later when this becomes the selected /// HTTP client for a Smithy client. - #[cfg(feature = "legacy-tls-rustls")] + #[cfg(feature = "legacy-rustls-ring")] pub fn build_https(self) -> SharedHttpClient { self.build_with_fn(default_connector::https) } @@ -590,7 +590,7 @@ impl HyperClientBuilder { /// Create a [`SharedHttpClient`] from this builder and a given connector. /// #[cfg_attr( - feature = "legacy-tls-rustls", + feature = "legacy-rustls-ring", doc = "Use [`build_https`](HyperClientBuilder::build_https) if you don't want to provide a custom TCP connector." )] pub fn build(self, tcp_connector: C) -> SharedHttpClient diff --git a/rust-runtime/aws-smithy-http-client/src/test_util/dvr/record.rs b/rust-runtime/aws-smithy-http-client/src/test_util/dvr/record.rs index 946bbeed1d..51759dfeba 100644 --- a/rust-runtime/aws-smithy-http-client/src/test_util/dvr/record.rs +++ b/rust-runtime/aws-smithy-http-client/src/test_util/dvr/record.rs @@ -73,7 +73,7 @@ pub struct RecordingClient { pub(crate) inner: SharedHttpConnector, } -#[cfg(feature = "legacy-tls-rustls")] +#[cfg(feature = "legacy-rustls-ring")] impl RecordingClient { /// Construct a recording connection wrapping a default HTTPS implementation without any timeouts. pub fn https() -> Self { diff --git a/rust-runtime/aws-smithy-http-client/src/test_util/never.rs b/rust-runtime/aws-smithy-http-client/src/test_util/never.rs index a31932c104..e7e1f2cc91 100644 --- a/rust-runtime/aws-smithy-http-client/src/test_util/never.rs +++ b/rust-runtime/aws-smithy-http-client/src/test_util/never.rs @@ -177,7 +177,6 @@ mod hyper_014_support { #[cfg(test)] mod test { - use super::NeverTcpConnector; use aws_smithy_async::rt::sleep::TokioSleep; use aws_smithy_async::time::SystemTimeSource; use aws_smithy_runtime_api::client::http::{HttpClient, HttpConnector, HttpConnectorSettings}; @@ -188,6 +187,7 @@ mod test { #[cfg(feature = "hyper-014")] #[tokio::test] async fn never_tcp_connector_plugs_into_hyper_014() { + use super::NeverTcpConnector; use crate::hyper_014::HyperClientBuilder; // it should compile @@ -214,6 +214,7 @@ mod test { #[cfg(feature = "hyper-1")] #[tokio::test] async fn never_tcp_connector_plugs_into_hyper_1() { + use super::NeverTcpConnector; let client = crate::client::build_with_fn(None, NeverTcpConnector::new); let components = RuntimeComponentsBuilder::for_tests() .with_sleep_impl(Some(TokioSleep::new())) diff --git a/rust-runtime/aws-smithy-http-client/src/test_util/wire.rs b/rust-runtime/aws-smithy-http-client/src/test_util/wire.rs index 0b83824229..d945dfa102 100644 --- a/rust-runtime/aws-smithy-http-client/src/test_util/wire.rs +++ b/rust-runtime/aws-smithy-http-client/src/test_util/wire.rs @@ -407,7 +407,7 @@ impl tower::Service for InnerDnsResolver { } } -#[cfg(any(feature = "legacy-test-util", feature = "hyper-014"))] +#[cfg(all(feature = "legacy-test-util", feature = "hyper-014"))] impl hyper_0_14::service::Service for LoggingDnsResolver { type Response = Once; type Error = Infallible; diff --git a/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs b/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs index 3ac265bed1..c416ce2073 100644 --- a/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs +++ b/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs @@ -88,7 +88,6 @@ async fn custom_dns_client() { assert_eq!(resolver.count.load(Ordering::Relaxed), 1); } -#[cfg(feature = "rustls-ring")] async fn smoke_test_client(client: &dyn HttpClient) -> Result<(), Box> { let connector_settings = HttpConnectorSettings::builder().build(); let runtime_components = RuntimeComponentsBuilder::for_tests() diff --git a/rust-runtime/aws-smithy-runtime/Cargo.toml b/rust-runtime/aws-smithy-runtime/Cargo.toml index cfab3977ac..f640168517 100644 --- a/rust-runtime/aws-smithy-runtime/Cargo.toml +++ b/rust-runtime/aws-smithy-runtime/Cargo.toml @@ -14,7 +14,7 @@ repository = "https://github.com/smithy-lang/smithy-rs" client = ["aws-smithy-runtime-api/client", "aws-smithy-types/http-body-1-x"] http-auth = ["aws-smithy-runtime-api/http-auth"] connector-hyper-0-14-x = ["dep:aws-smithy-http-client", "aws-smithy-http-client?/hyper-014"] -tls-rustls = ["dep:aws-smithy-http-client", "aws-smithy-http-client?/legacy-tls-rustls", "connector-hyper-0-14-x"] +tls-rustls = ["dep:aws-smithy-http-client", "aws-smithy-http-client?/legacy-rustls-ring", "connector-hyper-0-14-x"] rt-tokio = ["tokio/rt"] # Features for testing diff --git a/tools/ci-scripts/test-windows.sh b/tools/ci-scripts/test-windows.sh index f2a4896bda..c8e5153f64 100755 --- a/tools/ci-scripts/test-windows.sh +++ b/tools/ci-scripts/test-windows.sh @@ -20,4 +20,4 @@ done # TODO(https://github.com/awslabs/aws-sdk-rust/issues/1117) We don't have a way to codegen the deps needed by the aws-config crate # (cd aws/rust-runtime/aws-config && cargo test --all-features) # aws-config is not part of the workspace so we have to test it separately echo "Testing isolated features of aws-smithy-http-client" -(cd rust-runtime && cargo test -p aws-smithy-http-client --features crypto-ring) # only ring works on windows +(cd rust-runtime && cargo test -p aws-smithy-http-client --features rustls-ring) # only ring works on windows From 76d6505ada0c3c1f72bea522feff6e2503989626 Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Fri, 15 Nov 2024 13:57:48 -0500 Subject: [PATCH 05/13] refactor connector builder and expose underlying HttpConnector settings --- .../aws-smithy-http-client/Cargo.toml | 2 +- .../aws-smithy-http-client/src/client.rs | 247 ++++++++++----- .../aws-smithy-http-client/src/client/tls.rs | 292 +++++++++--------- .../aws-smithy-http-client/src/lib.rs | 36 +++ .../src/test_util/never.rs | 2 +- .../src/test_util/wire.rs | 2 +- 6 files changed, 362 insertions(+), 219 deletions(-) diff --git a/rust-runtime/aws-smithy-http-client/Cargo.toml b/rust-runtime/aws-smithy-http-client/Cargo.toml index 706cf3dd36..bc8706b016 100644 --- a/rust-runtime/aws-smithy-http-client/Cargo.toml +++ b/rust-runtime/aws-smithy-http-client/Cargo.toml @@ -135,4 +135,4 @@ rustdoc-args = ["--cfg", "docsrs"] # End of docs.rs metadata [lints.rust] -unexpected_cfgs = { level = "warn", check-cfg = ['cfg(crypto_unstable)'] } +unexpected_cfgs = { level = "warn", check-cfg = ['cfg(aws_smithy_http_unstable)'] } diff --git a/rust-runtime/aws-smithy-http-client/src/client.rs b/rust-runtime/aws-smithy-http-client/src/client.rs index 82a9096e9e..bc7115555f 100644 --- a/rust-runtime/aws-smithy-http-client/src/client.rs +++ b/rust-runtime/aws-smithy-http-client/src/client.rs @@ -8,6 +8,7 @@ mod timeout; /// TLS connector(s) pub mod tls; +use crate::client::dns::HyperUtilResolver; use aws_smithy_async::future::timeout::TimedOutError; use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep, SharedAsyncSleep}; use aws_smithy_runtime_api::box_error::BoxError; @@ -34,14 +35,16 @@ use h2::Reason; use http_1x::{Extensions, Uri}; use hyper::rt::{Read, Write}; use hyper_util::client::legacy as client; +use hyper_util::client::legacy::connect::dns::{GaiResolver, Name}; use hyper_util::client::legacy::connect::{ - capture_connection, CaptureConnection, Connect, HttpInfo, + capture_connection, CaptureConnection, Connect, HttpConnector as HyperHttpConnector, HttpInfo, }; use hyper_util::rt::TokioExecutor; use std::borrow::Cow; use std::collections::HashMap; use std::error::Error; use std::fmt; +use std::net::SocketAddr; use std::sync::RwLock; use std::time::Duration; @@ -57,9 +60,12 @@ pub struct Connector { } impl Connector { - /// Builder for a Hyper connector. + /// Builder for an HTTP connector. pub fn builder() -> ConnectorBuilder { - Default::default() + ConnectorBuilder { + enable_tcp_nodelay: true, + ..Default::default() + } } } @@ -75,6 +81,9 @@ pub struct ConnectorBuilder { connector_settings: Option, sleep_impl: Option, client_builder: Option, + enable_tcp_nodelay: bool, + // flag to indicate that cached TLS connector is ok (i.e. there are no non-default HttpConnector settings changed) + enable_cached_tls: bool, #[allow(unused)] tls: Tls, } @@ -84,39 +93,82 @@ pub struct ConnectorBuilder { #[non_exhaustive] pub struct TlsUnset {} -/// Crypto implementation selected +/// TLS implementation selected pub struct TlsProviderSelected { tls_provider: tls::Provider, } -#[cfg(any( - feature = "rustls-aws-lc", - feature = "rustls-aws-lc-fips", - feature = "rustls-ring" -))] impl ConnectorBuilder { + /// Build a [`Connector`] that will use the default DNS resolver implementation. + pub fn build(self) -> Connector { + let http_connector = self.base_connector(); + self.build_https(http_connector) + } + /// Build a [`Connector`] that will use the given DNS resolver implementation. - pub fn build_from_resolver(self, resolver: R) -> Connector { + pub fn build_with_resolver(self, resolver: R) -> Connector { + let http_connector = self.base_connector_with_resolver(HyperUtilResolver { resolver }); + self.build_https(http_connector) + } + + fn build_https(self, http_connector: HyperHttpConnector) -> Connector + where + R: Clone + Send + Sync + 'static, + R: tower::Service, + R::Response: Iterator, + R::Future: Send, + R::Error: Into>, + { match &self.tls.tls_provider { + // TODO(hyper1) - fix cfg_rustls! to allow matching on patterns so we can re-use it and not duplicate these cfg matches everywhere #[cfg(any( feature = "rustls-aws-lc", feature = "rustls-aws-lc-fips", feature = "rustls-ring" ))] tls::Provider::Rustls(crypto_mode) => { - let connector = tls::rustls_provider::build_connector::https_with_resolver( - crypto_mode.clone(), - resolver, - ); - self.build(connector) + if self.enable_cached_tls { + let https_connector = + tls::rustls_provider::cached_connectors::cached_https(crypto_mode.clone()); + self.wrap_connector(https_connector) + } else { + let https_connector = tls::rustls_provider::build_connector::wrap_connector( + http_connector, + crypto_mode.clone(), + ); + self.wrap_connector(https_connector) + } } } } + + /// Enable cached TLS connector to be used. Used internally to indicate that no default connector + /// settings have been changed and that we can re-use and wrap an existing cached TLS connector + fn enable_cached_tls_connectors(mut self) -> Self { + self.enable_cached_tls = true; + self + } +} + +impl ConnectorBuilder { + /// Set the TLS implementation to use for this connector + pub fn tls_provider(self, provider: tls::Provider) -> ConnectorBuilder { + ConnectorBuilder { + connector_settings: self.connector_settings, + sleep_impl: self.sleep_impl, + client_builder: self.client_builder, + enable_tcp_nodelay: self.enable_tcp_nodelay, + enable_cached_tls: self.enable_cached_tls, + tls: TlsProviderSelected { + tls_provider: provider, + }, + } + } } impl ConnectorBuilder { /// Create a [`Connector`] from this builder and a given connector. - pub(crate) fn build(self, tcp_connector: C) -> Connector + pub(crate) fn wrap_connector(self, tcp_connector: C) -> Connector where C: Send + Sync + 'static, C: Clone, @@ -163,6 +215,20 @@ impl ConnectorBuilder { } } + /// Get the base TCP connector by mapping our config to the underlying `HttpConnector` from hyper + /// (which is a base TCP connector with no TLS or any wrapping) + fn base_connector(&self) -> HyperHttpConnector { + self.base_connector_with_resolver(GaiResolver::new()) + } + + /// Get the base TCP connector by mapping our config to the underlying `HttpConnector` from hyper + /// using the given resolver `R` + fn base_connector_with_resolver(&self, resolver: R) -> HyperHttpConnector { + let mut conn = HyperHttpConnector::new_with_resolver(resolver); + conn.set_nodelay(self.enable_tcp_nodelay); + conn + } + /// Set the async sleep implementation used for timeouts /// /// Calling this is only necessary for testing or to use something other than @@ -196,6 +262,18 @@ impl ConnectorBuilder { self } + /// Configure `SO_NODELAY` for all sockets to the supplied value `nodelay` + pub fn enable_tcp_nodelay(mut self, nodelay: bool) -> Self { + self.enable_tcp_nodelay = nodelay; + self + } + + /// Configure `SO_NODELAY` for all sockets to the supplied value `nodelay` + pub fn set_enable_tcp_nodelay(&mut self, nodelay: bool) -> &mut Self { + self.enable_tcp_nodelay = nodelay; + self + } + /// Override the Hyper client [`Builder`](hyper_util::client::legacy::Builder) used to construct this client. /// /// This enables changing settings like forcing HTTP2 and modifying other default client behavior. @@ -383,7 +461,7 @@ impl From<&HttpConnectorSettings> for CacheKey { struct HyperClient { connector_cache: RwLock>, client_builder: hyper_util::client::legacy::Builder, - tcp_connector_fn: F, + connector_fn: F, } impl fmt::Debug for HyperClient { @@ -395,14 +473,16 @@ impl fmt::Debug for HyperClient { } } -impl HttpClient for HyperClient +impl HttpClient for HyperClient where - F: Fn() -> C + Send + Sync, - C: Clone + Send + Sync + 'static, - C: tower::Service, - C::Response: Connection + Read + Write + Send + Sync + Unpin + 'static, - C::Future: Unpin + Send + 'static, - C::Error: Into, + F: Fn( + hyper_util::client::legacy::Builder, + Option<&HttpConnectorSettings>, + Option<&RuntimeComponents>, + ) -> Connector + + Send + + Sync + + 'static, { fn http_connector( &self, @@ -415,20 +495,19 @@ where let mut cache = self.connector_cache.write().unwrap(); // Short-circuit if another thread already wrote a connector to the cache for this key if !cache.contains_key(&key) { - let mut builder = Connector::builder() - .hyper_builder(self.client_builder.clone()) - .connector_settings(settings.clone()); - builder.set_sleep_impl(components.sleep_impl()); - let start = components.time_source().map(|ts| ts.now()); - let tcp_connector = (self.tcp_connector_fn)(); + let connector = (self.connector_fn)( + self.client_builder.clone(), + Some(settings), + Some(components), + ); let end = components.time_source().map(|ts| ts.now()); if let (Some(start), Some(end)) = (start, end) { if let Ok(elapsed) = end.duration_since(start) { - tracing::debug!("new TCP connector created in {:?}", elapsed); + tracing::debug!("new connector created in {:?}", elapsed); } } - let connector = SharedHttpConnector::new(builder.build(tcp_connector)); + let connector = SharedHttpConnector::new(connector); cache.insert(key.clone(), connector); } connector = cache.get(&key).cloned(); @@ -447,7 +526,7 @@ where // here rather than at construction so that it won't run if this is not // the selected HTTP client for the base config (for example, if this was // the default HTTP client, and it was overridden by a later plugin). - let _ = (self.tcp_connector_fn)(); + let _ = (self.connector_fn)(self.client_builder.clone(), None, None); Ok(()) } @@ -478,17 +557,17 @@ impl Builder { /// The trusted certificates will be loaded later when this becomes the selected /// HTTP client for a Smithy client. pub fn build_https(self) -> SharedHttpClient { - let provider = self.tls_provider.tls_provider; - match provider { - #[cfg(any( - feature = "rustls-aws-lc", - feature = "rustls-aws-lc-fips", - feature = "rustls-ring" - ))] - tls::Provider::Rustls(crypto_mode) => build_with_fn(self.client_builder, move || { - tls::rustls_provider::cached_connectors::cached_https(crypto_mode.clone()) - }), - } + build_with_conn_fn( + self.client_builder, + move |client_builder, settings, runtime_components| { + let builder = new_conn_builder(client_builder, settings, runtime_components) + .tls_provider(self.tls_provider.tls_provider.clone()) + // the base HTTP connector settings have not changed, and we are not using a custom resolver + // mark the connector as safe to use cached TLS connectors + .enable_cached_tls_connectors(); + builder.build() + }, + ) } /// Create a hyper client using a custom DNS resolver @@ -496,20 +575,14 @@ impl Builder { self, resolver: impl ResolveDns + Clone + 'static, ) -> SharedHttpClient { - let provider = self.tls_provider.tls_provider; - match provider { - #[cfg(any( - feature = "rustls-aws-lc", - feature = "rustls-aws-lc-fips", - feature = "rustls-ring" - ))] - tls::Provider::Rustls(crypto_mode) => build_with_fn(self.client_builder, move || { - tls::rustls_provider::build_connector::https_with_resolver( - crypto_mode.clone(), - resolver.clone(), - ) - }), - } + build_with_conn_fn( + self.client_builder, + move |client_builder, settings, runtime_components| { + let builder = new_conn_builder(client_builder, settings, runtime_components) + .tls_provider(self.tls_provider.tls_provider.clone()); + builder.build_with_resolver(resolver.clone()) + }, + ) } } @@ -532,7 +605,29 @@ impl Builder { } } -pub(crate) fn build_with_fn( +pub(crate) fn build_with_conn_fn( + client_builder: Option, + connector_fn: F, +) -> SharedHttpClient +where + F: Fn( + hyper_util::client::legacy::Builder, + Option<&HttpConnectorSettings>, + Option<&RuntimeComponents>, + ) -> Connector + + Send + + Sync + + 'static, +{ + SharedHttpClient::new(HyperClient { + connector_cache: RwLock::new(HashMap::new()), + client_builder: client_builder + .unwrap_or_else(|| hyper_util::client::legacy::Builder::new(TokioExecutor::new())), + connector_fn, + }) +} + +pub(crate) fn build_with_tcp_conn_fn( client_builder: Option, tcp_connector_fn: F, ) -> SharedHttpClient @@ -545,12 +640,26 @@ where C::Error: Into, C: Connect, { - SharedHttpClient::new(HyperClient { - connector_cache: RwLock::new(HashMap::new()), - client_builder: client_builder - .unwrap_or_else(|| hyper_util::client::legacy::Builder::new(TokioExecutor::new())), - tcp_connector_fn, - }) + build_with_conn_fn( + client_builder, + move |client_builder, settings, runtime_components| { + let builder = new_conn_builder(client_builder, settings, runtime_components); + builder.wrap_connector(tcp_connector_fn()) + }, + ) +} + +fn new_conn_builder( + client_builder: hyper_util::client::legacy::Builder, + settings: Option<&HttpConnectorSettings>, + runtime_components: Option<&RuntimeComponents>, +) -> ConnectorBuilder { + let mut builder = Connector::builder().hyper_builder(client_builder); + builder.set_connector_settings(settings.cloned()); + if let Some(components) = runtime_components { + builder.set_sleep_impl(components.sleep_impl()); + } + builder } #[cfg(test)] @@ -577,7 +686,7 @@ mod test { async fn connector_selection() { // Create a client that increments a count every time it creates a new Connector let creation_count = Arc::new(AtomicU32::new(0)); - let http_client = build_with_fn(None, { + let http_client = build_with_tcp_conn_fn(None, { let count = creation_count.clone(); move || { count.fetch_add(1, Ordering::Relaxed); @@ -634,7 +743,7 @@ mod test { let connector = TestConnection { inner: HangupStream, }; - let adapter = Connector::builder().build(connector).adapter; + let adapter = Connector::builder().wrap_connector(connector).adapter; let err = adapter .call(HttpRequest::get("https://socket-hangup.com").unwrap()) .await @@ -714,7 +823,7 @@ mod test { let hyper = Connector::builder() .connector_settings(connector_settings) .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) - .build(tcp_connector) + .wrap_connector(tcp_connector) .adapter; let now = tokio::time::Instant::now(); tokio::time::pause(); @@ -746,7 +855,7 @@ mod test { let hyper = Connector::builder() .connector_settings(connector_settings) .sleep_impl(SharedAsyncSleep::new(TokioSleep::new())) - .build(tcp_connector) + .wrap_connector(tcp_connector) .adapter; let now = tokio::time::Instant::now(); tokio::time::pause(); diff --git a/rust-runtime/aws-smithy-http-client/src/client/tls.rs b/rust-runtime/aws-smithy-http-client/src/client/tls.rs index 5a07937158..727faddea1 100644 --- a/rust-runtime/aws-smithy-http-client/src/client/tls.rs +++ b/rust-runtime/aws-smithy-http-client/src/client/tls.rs @@ -2,6 +2,7 @@ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. * SPDX-License-Identifier: Apache-2.0 */ +use crate::cfg::cfg_rustls; /// Choice of underlying cryptography library #[derive(Debug, Eq, PartialEq, Clone)] @@ -20,166 +21,163 @@ pub enum Provider { // TODO(hyper1): consider native-tls support? } -/// rustls based support and adapters -#[cfg(any( - feature = "rustls-aws-lc", - feature = "rustls-aws-lc-fips", - feature = "rustls-ring" -))] -pub mod rustls_provider { - use crate::client::tls::Provider; - use rustls::crypto::CryptoProvider; - - /// Choice of underlying cryptography library (this only applies to rustls) - #[derive(Debug, Eq, PartialEq, Clone)] - #[non_exhaustive] - pub enum CryptoMode { - /// Crypto based on [ring](https://github.com/briansmith/ring) - #[cfg(feature = "rustls-ring")] - Ring, - /// Crypto based on [aws-lc](https://github.com/aws/aws-lc-rs) - #[cfg(feature = "rustls-aws-lc")] - AwsLc, - /// FIPS compliant variant of [aws-lc](https://github.com/aws/aws-lc-rs) - #[cfg(feature = "rustls-aws-lc-fips")] - AwsLcFips, - } +cfg_rustls! { + /// rustls based support and adapters + pub mod rustls_provider { + use crate::client::tls::Provider; + use rustls::crypto::CryptoProvider; + + /// Choice of underlying cryptography library (this only applies to rustls) + #[derive(Debug, Eq, PartialEq, Clone)] + #[non_exhaustive] + pub enum CryptoMode { + /// Crypto based on [ring](https://github.com/briansmith/ring) + #[cfg(feature = "rustls-ring")] + Ring, + /// Crypto based on [aws-lc](https://github.com/aws/aws-lc-rs) + #[cfg(feature = "rustls-aws-lc")] + AwsLc, + /// FIPS compliant variant of [aws-lc](https://github.com/aws/aws-lc-rs) + #[cfg(feature = "rustls-aws-lc-fips")] + AwsLcFips, + } - impl CryptoMode { - fn provider(self) -> CryptoProvider { - match self { - #[cfg(feature = "rustls-aws-lc")] - CryptoMode::AwsLc => rustls::crypto::aws_lc_rs::default_provider(), - - #[cfg(feature = "rustls-ring")] - CryptoMode::Ring => rustls::crypto::ring::default_provider(), - - #[cfg(feature = "rustls-aws-lc-fips")] - CryptoMode::AwsLcFips => { - let provider = rustls::crypto::default_fips_provider(); - assert!( - provider.fips(), - "FIPS was requested but the provider did not support FIPS" - ); - provider + impl CryptoMode { + fn provider(self) -> CryptoProvider { + match self { + #[cfg(feature = "rustls-aws-lc")] + CryptoMode::AwsLc => rustls::crypto::aws_lc_rs::default_provider(), + + #[cfg(feature = "rustls-ring")] + CryptoMode::Ring => rustls::crypto::ring::default_provider(), + + #[cfg(feature = "rustls-aws-lc-fips")] + CryptoMode::AwsLcFips => { + let provider = rustls::crypto::default_fips_provider(); + assert!( + provider.fips(), + "FIPS was requested but the provider did not support FIPS" + ); + provider + } } } } - } - impl Provider { - /// Create a TLS provider based on [rustls](https://github.com/rustls/rustls) - /// and the given [`CryptoMode`] - pub fn rustls(mode: CryptoMode) -> Provider { - Provider::Rustls(mode) + impl Provider { + /// Create a TLS provider based on [rustls](https://github.com/rustls/rustls) + /// and the given [`CryptoMode`] + pub fn rustls(mode: CryptoMode) -> Provider { + Provider::Rustls(mode) + } } - } - #[allow(unused_imports)] - pub(crate) mod cached_connectors { - use client::connect::HttpConnector; - use hyper_util::client::legacy as client; - use hyper_util::client::legacy::connect::dns::GaiResolver; - - use super::build_connector::make_tls; - use super::CryptoMode; - - #[cfg(feature = "rustls-ring")] - static HTTPS_NATIVE_ROOTS_RING: once_cell::sync::Lazy< - hyper_rustls::HttpsConnector, - > = once_cell::sync::Lazy::new(|| { - make_tls(GaiResolver::new(), CryptoMode::Ring.provider()) - }); - - #[cfg(feature = "rustls-aws-lc")] - static HTTPS_NATIVE_ROOTS_AWS_LC: once_cell::sync::Lazy< - hyper_rustls::HttpsConnector, - > = once_cell::sync::Lazy::new(|| { - make_tls(GaiResolver::new(), CryptoMode::AwsLc.provider()) - }); - - #[cfg(feature = "rustls-aws-lc-fips")] - static HTTPS_NATIVE_ROOTS_AWS_LC_FIPS: once_cell::sync::Lazy< - hyper_rustls::HttpsConnector, - > = once_cell::sync::Lazy::new(|| { - make_tls(GaiResolver::new(), CryptoMode::AwsLcFips.provider()) - }); - - pub(crate) fn cached_https( - mode: CryptoMode, - ) -> hyper_rustls::HttpsConnector { - match mode { - #[cfg(feature = "rustls-ring")] - CryptoMode::Ring => HTTPS_NATIVE_ROOTS_RING.clone(), - #[cfg(feature = "rustls-aws-lc")] - CryptoMode::AwsLc => HTTPS_NATIVE_ROOTS_AWS_LC.clone(), - #[cfg(feature = "rustls-aws-lc-fips")] - CryptoMode::AwsLcFips => HTTPS_NATIVE_ROOTS_AWS_LC_FIPS.clone(), + #[allow(unused_imports)] + pub(crate) mod cached_connectors { + use client::connect::HttpConnector; + use hyper_util::client::legacy as client; + use hyper_util::client::legacy::connect::dns::GaiResolver; + + use super::build_connector::make_tls; + use super::CryptoMode; + + #[cfg(feature = "rustls-ring")] + static HTTPS_NATIVE_ROOTS_RING: once_cell::sync::Lazy< + hyper_rustls::HttpsConnector, + > = once_cell::sync::Lazy::new(|| { + make_tls(GaiResolver::new(), CryptoMode::Ring) + }); + + #[cfg(feature = "rustls-aws-lc")] + static HTTPS_NATIVE_ROOTS_AWS_LC: once_cell::sync::Lazy< + hyper_rustls::HttpsConnector, + > = once_cell::sync::Lazy::new(|| { + make_tls(GaiResolver::new(), CryptoMode::AwsLc) + }); + + #[cfg(feature = "rustls-aws-lc-fips")] + static HTTPS_NATIVE_ROOTS_AWS_LC_FIPS: once_cell::sync::Lazy< + hyper_rustls::HttpsConnector, + > = once_cell::sync::Lazy::new(|| { + make_tls(GaiResolver::new(), CryptoMode::AwsLcFips) + }); + + pub(crate) fn cached_https( + mode: CryptoMode, + ) -> hyper_rustls::HttpsConnector { + match mode { + #[cfg(feature = "rustls-ring")] + CryptoMode::Ring => HTTPS_NATIVE_ROOTS_RING.clone(), + #[cfg(feature = "rustls-aws-lc")] + CryptoMode::AwsLc => HTTPS_NATIVE_ROOTS_AWS_LC.clone(), + #[cfg(feature = "rustls-aws-lc-fips")] + CryptoMode::AwsLcFips => HTTPS_NATIVE_ROOTS_AWS_LC_FIPS.clone(), + } } } - } - pub(crate) mod build_connector { - use crate::client::dns::HyperUtilResolver; - use crate::client::tls::rustls_provider::CryptoMode; - use aws_smithy_runtime_api::client::dns::ResolveDns; - use client::connect::HttpConnector; - use hyper_util::client::legacy as client; - use rustls::crypto::CryptoProvider; - use std::sync::Arc; - - fn restrict_ciphers(base: CryptoProvider) -> CryptoProvider { - let suites = &[ - rustls::CipherSuite::TLS13_AES_256_GCM_SHA384, - rustls::CipherSuite::TLS13_AES_128_GCM_SHA256, - // TLS1.2 suites - rustls::CipherSuite::TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, - rustls::CipherSuite::TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, - rustls::CipherSuite::TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, - rustls::CipherSuite::TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, - rustls::CipherSuite::TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, - ]; - let supported_suites = suites - .iter() - .flat_map(|suite| { - base.cipher_suites - .iter() - .find(|s| &s.suite() == suite) - .cloned() - }) - .collect::>(); - CryptoProvider { - cipher_suites: supported_suites, - ..base + pub(crate) mod build_connector { + use crate::client::tls::rustls_provider::CryptoMode; + use hyper_util::client::legacy as client; + use client::connect::HttpConnector; + use rustls::crypto::CryptoProvider; + use std::sync::Arc; + + fn restrict_ciphers(base: CryptoProvider) -> CryptoProvider { + let suites = &[ + rustls::CipherSuite::TLS13_AES_256_GCM_SHA384, + rustls::CipherSuite::TLS13_AES_128_GCM_SHA256, + // TLS1.2 suites + rustls::CipherSuite::TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + rustls::CipherSuite::TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + rustls::CipherSuite::TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + rustls::CipherSuite::TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + rustls::CipherSuite::TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + ]; + let supported_suites = suites + .iter() + .flat_map(|suite| { + base.cipher_suites + .iter() + .find(|s| &s.suite() == suite) + .cloned() + }) + .collect::>(); + CryptoProvider { + cipher_suites: supported_suites, + ..base + } } - } - pub(super) fn make_tls( - resolver: R, - crypto_provider: CryptoProvider, - ) -> hyper_rustls::HttpsConnector> { - use hyper_rustls::ConfigBuilderExt; - let mut base_connector = HttpConnector::new_with_resolver(resolver); - base_connector.enforce_http(false); - hyper_rustls::HttpsConnectorBuilder::new() - .with_tls_config( - rustls::ClientConfig::builder_with_provider(Arc::new(restrict_ciphers(crypto_provider))) - .with_safe_default_protocol_versions() - .expect("Error with the TLS configuration. Please file a bug report under https://github.com/smithy-lang/smithy-rs/issues.") - .with_native_roots().expect("error with TLS configuration.") - .with_no_client_auth() - ) - .https_or_http() - .enable_http1() - .enable_http2() - .wrap_connector(base_connector) - } + pub(super) fn make_tls( + resolver: R, + crypto_mode: CryptoMode, + ) -> hyper_rustls::HttpsConnector> { + // use the base connector through our `Connector` type to ensure defaults are consistent + let base_connector = crate::client::Connector::builder() + .base_connector_with_resolver(resolver); + wrap_connector(base_connector, crypto_mode) + } - pub(crate) fn https_with_resolver( - crypto_mode: CryptoMode, - resolver: R, - ) -> hyper_rustls::HttpsConnector>> { - make_tls(HyperUtilResolver { resolver }, crypto_mode.provider()) + pub(crate) fn wrap_connector( + mut conn: HttpConnector, + crypto_mode: CryptoMode, + ) -> hyper_rustls::HttpsConnector> { + use hyper_rustls::ConfigBuilderExt; + conn.enforce_http(false); + hyper_rustls::HttpsConnectorBuilder::new() + .with_tls_config( + rustls::ClientConfig::builder_with_provider(Arc::new(restrict_ciphers(crypto_mode.provider()))) + .with_safe_default_protocol_versions() + .expect("Error with the TLS configuration. Please file a bug report under https://github.com/smithy-lang/smithy-rs/issues.") + .with_native_roots().expect("error with TLS configuration.") + .with_no_client_auth() + ) + .https_or_http() + .enable_http1() + .enable_http2() + .wrap_connector(conn) + } } } } diff --git a/rust-runtime/aws-smithy-http-client/src/lib.rs b/rust-runtime/aws-smithy-http-client/src/lib.rs index 5f37dff25f..b48e68624b 100644 --- a/rust-runtime/aws-smithy-http-client/src/lib.rs +++ b/rust-runtime/aws-smithy-http-client/src/lib.rs @@ -43,3 +43,39 @@ pub use client::{tls, Builder, Connector, ConnectorBuilder}; #[cfg(feature = "test-util")] pub mod test_util; + +#[allow(unused_macros, unused_imports)] +#[macro_use] +pub(crate) mod cfg { + /// Any TLS provider enabled + macro_rules! cfg_tls { + ($($item:item)*) => { + $( + #[cfg(any( + feature = "rustls-aws-lc", + feature = "rustls-aws-lc-fips", + feature = "rustls-ring" + ))] + #[cfg_attr(docsrs, doc(cfg(any(feature = "rustls-aws-lc", feature = "rustls-aws-lc-fips", feature = "rustls-ring"))))] + $item + )* + } + } + + /// Any rustls provider enabled + macro_rules! cfg_rustls { + ($($item:item)*) => { + $( + #[cfg(any( + feature = "rustls-aws-lc", + feature = "rustls-aws-lc-fips", + feature = "rustls-ring" + ))] + #[cfg_attr(docsrs, doc(cfg(any(feature = "rustls-aws-lc", feature = "rustls-aws-lc-fips", feature = "rustls-ring"))))] + $item + )* + } + } + pub(crate) use cfg_rustls; + pub(crate) use cfg_tls; +} diff --git a/rust-runtime/aws-smithy-http-client/src/test_util/never.rs b/rust-runtime/aws-smithy-http-client/src/test_util/never.rs index e7e1f2cc91..15416975f0 100644 --- a/rust-runtime/aws-smithy-http-client/src/test_util/never.rs +++ b/rust-runtime/aws-smithy-http-client/src/test_util/never.rs @@ -215,7 +215,7 @@ mod test { #[tokio::test] async fn never_tcp_connector_plugs_into_hyper_1() { use super::NeverTcpConnector; - let client = crate::client::build_with_fn(None, NeverTcpConnector::new); + let client = crate::client::build_with_tcp_conn_fn(None, NeverTcpConnector::new); let components = RuntimeComponentsBuilder::for_tests() .with_sleep_impl(Some(TokioSleep::new())) .with_time_source(Some(SystemTimeSource::new())) diff --git a/rust-runtime/aws-smithy-http-client/src/test_util/wire.rs b/rust-runtime/aws-smithy-http-client/src/test_util/wire.rs index d945dfa102..5955d210bf 100644 --- a/rust-runtime/aws-smithy-http-client/src/test_util/wire.rs +++ b/rust-runtime/aws-smithy-http-client/src/test_util/wire.rs @@ -333,7 +333,7 @@ impl WireMockServer { /// **Note**: This must be used in tandem with [`Self::dns_resolver`] pub fn http_client(&self) -> SharedHttpClient { let resolver = self.dns_resolver(); - crate::client::build_with_fn(None, move || { + crate::client::build_with_tcp_conn_fn(None, move || { hyper_util::client::legacy::connect::HttpConnector::new_with_resolver( resolver.clone().0, ) From 3fce9d8eb5a91f4cce925e1af1e10a750ee654cd Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Fri, 15 Nov 2024 15:08:26 -0500 Subject: [PATCH 06/13] add a default client --- .../aws-smithy-http-client/Cargo.toml | 20 ++++++++++++++----- .../aws-smithy-http-client/src/client.rs | 20 +++++++++++++++++++ .../aws-smithy-http-client/src/lib.rs | 5 ++--- .../tests/smoke_test_clients.rs | 7 +++++++ 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/rust-runtime/aws-smithy-http-client/Cargo.toml b/rust-runtime/aws-smithy-http-client/Cargo.toml index bc8706b016..13a698db63 100644 --- a/rust-runtime/aws-smithy-http-client/Cargo.toml +++ b/rust-runtime/aws-smithy-http-client/Cargo.toml @@ -16,7 +16,7 @@ hyper-014 = [ "dep:hyper-0-14", ] -# FIXME - rename feature and add default feature(s) for default client/tls +# FIXME(hyper1) - rename feature hyper-1 = [ "aws-smithy-runtime-api/http-1x", "aws-smithy-types/http-body-1-x", @@ -27,6 +27,11 @@ hyper-1 = [ "dep:tower", ] +default-tls = [ + "hyper-1", + "rustls-aws-lc" +] + wire-mock = [ "test-util", "hyper-1", @@ -128,11 +133,16 @@ doc-scrape-examples = true stable = true [package.metadata.docs.rs] -all-features = true +all-features = false +features = [ + "hyper-1 ", + "wire-mock", + "test-util", + "rustls-ring", + "rustls-aws-lc", +] + targets = ["x86_64-unknown-linux-gnu"] cargo-args = ["-Zunstable-options", "-Zrustdoc-scrape-examples"] rustdoc-args = ["--cfg", "docsrs"] # End of docs.rs metadata - -[lints.rust] -unexpected_cfgs = { level = "warn", check-cfg = ['cfg(aws_smithy_http_unstable)'] } diff --git a/rust-runtime/aws-smithy-http-client/src/client.rs b/rust-runtime/aws-smithy-http-client/src/client.rs index bc7115555f..8e902de0a2 100644 --- a/rust-runtime/aws-smithy-http-client/src/client.rs +++ b/rust-runtime/aws-smithy-http-client/src/client.rs @@ -48,6 +48,26 @@ use std::net::SocketAddr; use std::sync::RwLock; use std::time::Duration; +/// Creates an HTTPS client using the default TLS provider +pub fn default_client() -> Option { + #[cfg(feature = "default-tls")] + { + tracing::trace!("creating a new default hyper 1.x client using rustls"); + Some( + Builder::new() + .tls_provider(tls::Provider::Rustls( + tls::rustls_provider::CryptoMode::AwsLc, + )) + .build_https(), + ) + } + #[cfg(not(feature = "default-tls"))] + { + tracing::trace!("no default connector available"); + None + } +} + /// [`HttpConnector`] used to make HTTP requests. /// /// This connector also implements socket connect and read timeouts. diff --git a/rust-runtime/aws-smithy-http-client/src/lib.rs b/rust-runtime/aws-smithy-http-client/src/lib.rs index b48e68624b..1a3b64b916 100644 --- a/rust-runtime/aws-smithy-http-client/src/lib.rs +++ b/rust-runtime/aws-smithy-http-client/src/lib.rs @@ -11,6 +11,7 @@ //! //! # Crate Features //! +//! - `default-tls`: Enable default TLS provider (used by `default_client()` to provide a default configured HTTPS client) //! - `hyper-014`: (Deprecated) HTTP client implementation based on hyper-0.14.x. //! - `test-util`: Enables utilities for unit tests. DO NOT ENABLE IN PRODUCTION. @@ -33,13 +34,11 @@ pub mod hyper_014 { pub use crate::hyper_legacy::*; } -// FIXME - remove hyper-1 from the name - /// Default HTTP and TLS connectors #[cfg(feature = "hyper-1")] pub(crate) mod client; #[cfg(feature = "hyper-1")] -pub use client::{tls, Builder, Connector, ConnectorBuilder}; +pub use client::{default_client, tls, Builder, Connector, ConnectorBuilder}; #[cfg(feature = "test-util")] pub mod test_util; diff --git a/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs b/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs index c416ce2073..3f2bf0beed 100644 --- a/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs +++ b/rust-runtime/aws-smithy-http-client/tests/smoke_test_clients.rs @@ -54,6 +54,13 @@ async fn aws_lc_client() { smoke_test_client(&client).await.unwrap(); } +#[cfg(feature = "default-tls")] +#[tokio::test] +async fn default_tls_client() { + let client = aws_smithy_http_client::default_client().expect("default TLS client created"); + smoke_test_client(&client).await.unwrap(); +} + #[cfg(feature = "rustls-ring")] #[tokio::test] async fn custom_dns_client() { From e058b0fd77c8b317fb117ddcff1e18c576ea5660 Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Fri, 15 Nov 2024 15:47:37 -0500 Subject: [PATCH 07/13] add bind to device option --- .../aws-smithy-http-client/src/client.rs | 22 +++++++++++++++++++ .../ci-build/sdk-lints/src/lint_cargo_toml.rs | 5 ++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/rust-runtime/aws-smithy-http-client/src/client.rs b/rust-runtime/aws-smithy-http-client/src/client.rs index 8e902de0a2..5bedf03df4 100644 --- a/rust-runtime/aws-smithy-http-client/src/client.rs +++ b/rust-runtime/aws-smithy-http-client/src/client.rs @@ -102,6 +102,7 @@ pub struct ConnectorBuilder { sleep_impl: Option, client_builder: Option, enable_tcp_nodelay: bool, + interface: Option, // flag to indicate that cached TLS connector is ok (i.e. there are no non-default HttpConnector settings changed) enable_cached_tls: bool, #[allow(unused)] @@ -178,6 +179,7 @@ impl ConnectorBuilder { sleep_impl: self.sleep_impl, client_builder: self.client_builder, enable_tcp_nodelay: self.enable_tcp_nodelay, + interface: self.interface, enable_cached_tls: self.enable_cached_tls, tls: TlsProviderSelected { tls_provider: provider, @@ -246,6 +248,10 @@ impl ConnectorBuilder { fn base_connector_with_resolver(&self, resolver: R) -> HyperHttpConnector { let mut conn = HyperHttpConnector::new_with_resolver(resolver); conn.set_nodelay(self.enable_tcp_nodelay); + #[cfg(any(target_os = "android", target_os = "fuchsia", target_os = "linux"))] + if let Some(interface) = self.interface { + conn.set_interface(self.interface); + } conn } @@ -294,6 +300,22 @@ impl ConnectorBuilder { self } + /// Sets the value for the `SO_BINDTODEVICE` option on this socket. + /// + /// If a socket is bound to an interface, only packets received from that particular + /// interface are processed by the socket. Note that this only works for some socket + /// types (e.g. `AF_INET` sockets). + /// + /// On Linux it can be used to specify a [VRF], but the binary needs to either have + /// `CAP_NET_RAW` capability set or be run as root. + /// + /// This function is only availble on Android, Fuchsia, and Linux. + /// [VRF]: https://www.kernel.org/doc/Documentation/networking/vrf.txt + #[cfg(any(target_os = "android", target_os = "fuchsia", target_os = "linux"))] + pub fn set_interface>(&mut self, interface: S) -> Self { + self + } + /// Override the Hyper client [`Builder`](hyper_util::client::legacy::Builder) used to construct this client. /// /// This enables changing settings like forcing HTTP2 and modifying other default client behavior. diff --git a/tools/ci-build/sdk-lints/src/lint_cargo_toml.rs b/tools/ci-build/sdk-lints/src/lint_cargo_toml.rs index 2ea376b420..1fc2abfd30 100644 --- a/tools/ci-build/sdk-lints/src/lint_cargo_toml.rs +++ b/tools/ci-build/sdk-lints/src/lint_cargo_toml.rs @@ -168,7 +168,10 @@ impl Lint for DocsRs { } fn files_to_check(&self) -> Result> { - Ok(all_cargo_tomls()?.collect()) + Ok(all_cargo_tomls()? + // aws-lc-fips cannot build on docs.rs, grant an exception for this crate which does not follow the same cargo.toml w.r.t docs.rs + .filter(|path| !path.to_string_lossy().contains("aws-smithy-http-client")) + .collect()) } } From 1d2448e016fa3d5cf98f85f0c92612899a2c8c1f Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Fri, 15 Nov 2024 16:02:19 -0500 Subject: [PATCH 08/13] fix integration test --- aws/sdk/integration-tests/s3/tests/hyper-10.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/aws/sdk/integration-tests/s3/tests/hyper-10.rs b/aws/sdk/integration-tests/s3/tests/hyper-10.rs index 3120ea2b5e..c8edc41f8f 100644 --- a/aws/sdk/integration-tests/s3/tests/hyper-10.rs +++ b/aws/sdk/integration-tests/s3/tests/hyper-10.rs @@ -3,15 +3,18 @@ * SPDX-License-Identifier: Apache-2.0 */ -use aws_smithy_http_client::hyper_1::CryptoMode; +use aws_smithy_http_client::tls; use aws_smithy_runtime_api::client::behavior_version::BehaviorVersion; #[tokio::test] #[ignore] async fn hyper_10_end_to_end() { - let http_client = aws_smithy_http_client::hyper_1::HyperClientBuilder::default() - .crypto_mode(CryptoMode::Ring) + let http_client = aws_smithy_http_client::Builder::new() + .tls_provider(tls::Provider::Rustls( + tls::rustls_provider::CryptoMode::Ring, + )) .build_https(); + let conf = aws_config::defaults(BehaviorVersion::latest()) .http_client(http_client) .load() From 896af7e9f2ba320b6a5919c8f81451c8ea9027cf Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Mon, 18 Nov 2024 10:28:38 -0500 Subject: [PATCH 09/13] fix build --- rust-runtime/aws-smithy-http-client/src/client.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/rust-runtime/aws-smithy-http-client/src/client.rs b/rust-runtime/aws-smithy-http-client/src/client.rs index 5bedf03df4..d263c4f0c9 100644 --- a/rust-runtime/aws-smithy-http-client/src/client.rs +++ b/rust-runtime/aws-smithy-http-client/src/client.rs @@ -160,6 +160,7 @@ impl ConnectorBuilder { self.wrap_connector(https_connector) } } + _ => unreachable!("unknown TLS provider"), } } @@ -249,8 +250,8 @@ impl ConnectorBuilder { let mut conn = HyperHttpConnector::new_with_resolver(resolver); conn.set_nodelay(self.enable_tcp_nodelay); #[cfg(any(target_os = "android", target_os = "fuchsia", target_os = "linux"))] - if let Some(interface) = self.interface { - conn.set_interface(self.interface); + if let Some(interface) = &self.interface { + conn.set_interface(interface); } conn } @@ -312,7 +313,8 @@ impl ConnectorBuilder { /// This function is only availble on Android, Fuchsia, and Linux. /// [VRF]: https://www.kernel.org/doc/Documentation/networking/vrf.txt #[cfg(any(target_os = "android", target_os = "fuchsia", target_os = "linux"))] - pub fn set_interface>(&mut self, interface: S) -> Self { + pub fn set_interface>(&mut self, interface: S) -> &mut Self { + self.interface = Some(interface.into()); self } From 18dbf4fc476b028177ebfd6b1b2a233d13ee74b8 Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Mon, 18 Nov 2024 13:05:56 -0500 Subject: [PATCH 10/13] fix ci adhoc tests --- .../aws-smithy-http-client/src/client.rs | 220 +++++++++--------- .../src/test_util/dvr/replay.rs | 2 +- 2 files changed, 115 insertions(+), 107 deletions(-) diff --git a/rust-runtime/aws-smithy-http-client/src/client.rs b/rust-runtime/aws-smithy-http-client/src/client.rs index d263c4f0c9..529db25c19 100644 --- a/rust-runtime/aws-smithy-http-client/src/client.rs +++ b/rust-runtime/aws-smithy-http-client/src/client.rs @@ -8,14 +8,13 @@ mod timeout; /// TLS connector(s) pub mod tls; -use crate::client::dns::HyperUtilResolver; +use crate::cfg::cfg_tls; use aws_smithy_async::future::timeout::TimedOutError; use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep, SharedAsyncSleep}; use aws_smithy_runtime_api::box_error::BoxError; use aws_smithy_runtime_api::client::connection::CaptureSmithyConnection; use aws_smithy_runtime_api::client::connection::ConnectionMetadata; use aws_smithy_runtime_api::client::connector_metadata::ConnectorMetadata; -use aws_smithy_runtime_api::client::dns::ResolveDns; use aws_smithy_runtime_api::client::http::{ HttpClient, HttpConnector, HttpConnectorFuture, HttpConnectorSettings, SharedHttpClient, SharedHttpConnector, @@ -35,16 +34,14 @@ use h2::Reason; use http_1x::{Extensions, Uri}; use hyper::rt::{Read, Write}; use hyper_util::client::legacy as client; -use hyper_util::client::legacy::connect::dns::{GaiResolver, Name}; use hyper_util::client::legacy::connect::{ - capture_connection, CaptureConnection, Connect, HttpConnector as HyperHttpConnector, HttpInfo, + capture_connection, CaptureConnection, Connect, HttpInfo, }; use hyper_util::rt::TokioExecutor; use std::borrow::Cow; use std::collections::HashMap; use std::error::Error; use std::fmt; -use std::net::SocketAddr; use std::sync::RwLock; use std::time::Duration; @@ -116,62 +113,10 @@ pub struct TlsUnset {} /// TLS implementation selected pub struct TlsProviderSelected { + #[allow(unused)] tls_provider: tls::Provider, } -impl ConnectorBuilder { - /// Build a [`Connector`] that will use the default DNS resolver implementation. - pub fn build(self) -> Connector { - let http_connector = self.base_connector(); - self.build_https(http_connector) - } - - /// Build a [`Connector`] that will use the given DNS resolver implementation. - pub fn build_with_resolver(self, resolver: R) -> Connector { - let http_connector = self.base_connector_with_resolver(HyperUtilResolver { resolver }); - self.build_https(http_connector) - } - - fn build_https(self, http_connector: HyperHttpConnector) -> Connector - where - R: Clone + Send + Sync + 'static, - R: tower::Service, - R::Response: Iterator, - R::Future: Send, - R::Error: Into>, - { - match &self.tls.tls_provider { - // TODO(hyper1) - fix cfg_rustls! to allow matching on patterns so we can re-use it and not duplicate these cfg matches everywhere - #[cfg(any( - feature = "rustls-aws-lc", - feature = "rustls-aws-lc-fips", - feature = "rustls-ring" - ))] - tls::Provider::Rustls(crypto_mode) => { - if self.enable_cached_tls { - let https_connector = - tls::rustls_provider::cached_connectors::cached_https(crypto_mode.clone()); - self.wrap_connector(https_connector) - } else { - let https_connector = tls::rustls_provider::build_connector::wrap_connector( - http_connector, - crypto_mode.clone(), - ); - self.wrap_connector(https_connector) - } - } - _ => unreachable!("unknown TLS provider"), - } - } - - /// Enable cached TLS connector to be used. Used internally to indicate that no default connector - /// settings have been changed and that we can re-use and wrap an existing cached TLS connector - fn enable_cached_tls_connectors(mut self) -> Self { - self.enable_cached_tls = true; - self - } -} - impl ConnectorBuilder { /// Set the TLS implementation to use for this connector pub fn tls_provider(self, provider: tls::Provider) -> ConnectorBuilder { @@ -238,24 +183,6 @@ impl ConnectorBuilder { } } - /// Get the base TCP connector by mapping our config to the underlying `HttpConnector` from hyper - /// (which is a base TCP connector with no TLS or any wrapping) - fn base_connector(&self) -> HyperHttpConnector { - self.base_connector_with_resolver(GaiResolver::new()) - } - - /// Get the base TCP connector by mapping our config to the underlying `HttpConnector` from hyper - /// using the given resolver `R` - fn base_connector_with_resolver(&self, resolver: R) -> HyperHttpConnector { - let mut conn = HyperHttpConnector::new_with_resolver(resolver); - conn.set_nodelay(self.enable_tcp_nodelay); - #[cfg(any(target_os = "android", target_os = "fuchsia", target_os = "linux"))] - if let Some(interface) = &self.interface { - conn.set_interface(interface); - } - conn - } - /// Set the async sleep implementation used for timeouts /// /// Calling this is only necessary for testing or to use something other than @@ -592,41 +519,122 @@ where #[derive(Clone, Default, Debug)] pub struct Builder { client_builder: Option, + #[allow(unused)] tls_provider: Tls, } -impl Builder { - /// Create a hyper client using RusTLS for TLS - /// - /// The trusted certificates will be loaded later when this becomes the selected - /// HTTP client for a Smithy client. - pub fn build_https(self) -> SharedHttpClient { - build_with_conn_fn( - self.client_builder, - move |client_builder, settings, runtime_components| { - let builder = new_conn_builder(client_builder, settings, runtime_components) - .tls_provider(self.tls_provider.tls_provider.clone()) - // the base HTTP connector settings have not changed, and we are not using a custom resolver - // mark the connector as safe to use cached TLS connectors - .enable_cached_tls_connectors(); - builder.build() - }, - ) +cfg_tls! { + use crate::client::dns::HyperUtilResolver; + use aws_smithy_runtime_api::client::dns::ResolveDns; + use hyper_util::client::legacy::connect::dns::{GaiResolver, Name}; + use hyper_util::client::legacy::connect::HttpConnector as HyperHttpConnector; + use std::net::SocketAddr; + + impl ConnectorBuilder { + /// Get the base TCP connector by mapping our config to the underlying `HttpConnector` from hyper + /// (which is a base TCP connector with no TLS or any wrapping) + fn base_connector(&self) -> HyperHttpConnector { + self.base_connector_with_resolver(GaiResolver::new()) + } + + /// Get the base TCP connector by mapping our config to the underlying `HttpConnector` from hyper + /// using the given resolver `R` + fn base_connector_with_resolver(&self, resolver: R) -> HyperHttpConnector { + let mut conn = HyperHttpConnector::new_with_resolver(resolver); + conn.set_nodelay(self.enable_tcp_nodelay); + #[cfg(any(target_os = "android", target_os = "fuchsia", target_os = "linux"))] + if let Some(interface) = &self.interface { + conn.set_interface(interface); + } + conn + } } - /// Create a hyper client using a custom DNS resolver - pub fn build_with_resolver( - self, - resolver: impl ResolveDns + Clone + 'static, - ) -> SharedHttpClient { - build_with_conn_fn( - self.client_builder, - move |client_builder, settings, runtime_components| { - let builder = new_conn_builder(client_builder, settings, runtime_components) - .tls_provider(self.tls_provider.tls_provider.clone()); - builder.build_with_resolver(resolver.clone()) - }, - ) + impl ConnectorBuilder { + /// Build a [`Connector`] that will use the default DNS resolver implementation. + pub fn build(self) -> Connector { + let http_connector = self.base_connector(); + self.build_https(http_connector) + } + + /// Build a [`Connector`] that will use the given DNS resolver implementation. + pub fn build_with_resolver(self, resolver: R) -> Connector { + let http_connector = self.base_connector_with_resolver(HyperUtilResolver { resolver }); + self.build_https(http_connector) + } + + fn build_https(self, http_connector: HyperHttpConnector) -> Connector + where + R: Clone + Send + Sync + 'static, + R: tower::Service, + R::Response: Iterator, + R::Future: Send, + R::Error: Into>, + { + match &self.tls.tls_provider { + // TODO(hyper1) - fix cfg_rustls! to allow matching on patterns so we can re-use it and not duplicate these cfg matches everywhere + #[cfg(any( + feature = "rustls-aws-lc", + feature = "rustls-aws-lc-fips", + feature = "rustls-ring" + ))] + tls::Provider::Rustls(crypto_mode) => { + if self.enable_cached_tls { + let https_connector = + tls::rustls_provider::cached_connectors::cached_https(crypto_mode.clone()); + self.wrap_connector(https_connector) + } else { + let https_connector = tls::rustls_provider::build_connector::wrap_connector( + http_connector, + crypto_mode.clone(), + ); + self.wrap_connector(https_connector) + } + } + } + } + + /// Enable cached TLS connector to be used. Used internally to indicate that no default connector + /// settings have been changed and that we can re-use and wrap an existing cached TLS connector + fn enable_cached_tls_connectors(mut self) -> Self { + self.enable_cached_tls = true; + self + } + } + + impl Builder { + /// Create a hyper client using RusTLS for TLS + /// + /// The trusted certificates will be loaded later when this becomes the selected + /// HTTP client for a Smithy client. + pub fn build_https(self) -> SharedHttpClient { + build_with_conn_fn( + self.client_builder, + move |client_builder, settings, runtime_components| { + let builder = new_conn_builder(client_builder, settings, runtime_components) + .tls_provider(self.tls_provider.tls_provider.clone()) + // the base HTTP connector settings have not changed, and we are not using a custom resolver + // mark the connector as safe to use cached TLS connectors + .enable_cached_tls_connectors(); + builder.build() + }, + ) + } + + /// Create a hyper client using a custom DNS resolver + pub fn build_with_resolver( + self, + resolver: impl ResolveDns + Clone + 'static, + ) -> SharedHttpClient { + build_with_conn_fn( + self.client_builder, + move |client_builder, settings, runtime_components| { + let builder = new_conn_builder(client_builder, settings, runtime_components) + .tls_provider(self.tls_provider.tls_provider.clone()); + builder.build_with_resolver(resolver.clone()) + }, + ) + } } } diff --git a/rust-runtime/aws-smithy-http-client/src/test_util/dvr/replay.rs b/rust-runtime/aws-smithy-http-client/src/test_util/dvr/replay.rs index cca272a81c..0bf866a33d 100644 --- a/rust-runtime/aws-smithy-http-client/src/test_util/dvr/replay.rs +++ b/rust-runtime/aws-smithy-http-client/src/test_util/dvr/replay.rs @@ -168,7 +168,7 @@ impl ReplayingClient { .await; body_comparer(expected.body().as_ref(), actual.body().as_ref())?; let actual: HttpRequest = actual.map(SdkBody::from).try_into()?; - aws_smithy_protocol_test::assert_uris_match(&expected.uri().to_string(), actual.uri()); + aws_smithy_protocol_test::assert_uris_match(expected.uri().to_string(), actual.uri()); let expected_headers = expected .headers() .keys() From 1f1b0deaeafd1626198e0e099bd2cb0a6e6bd823 Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Mon, 18 Nov 2024 14:08:58 -0500 Subject: [PATCH 11/13] fix docs --- rust-runtime/aws-smithy-http-client/src/client.rs | 2 +- rust-runtime/aws-smithy-http-client/src/lib.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/rust-runtime/aws-smithy-http-client/src/client.rs b/rust-runtime/aws-smithy-http-client/src/client.rs index 529db25c19..ee50c778fd 100644 --- a/rust-runtime/aws-smithy-http-client/src/client.rs +++ b/rust-runtime/aws-smithy-http-client/src/client.rs @@ -106,7 +106,7 @@ pub struct ConnectorBuilder { tls: Tls, } -/// Initial builder state, [`TlsProvider`] choice required +/// Initial builder state, `TlsProvider` choice required #[derive(Default)] #[non_exhaustive] pub struct TlsUnset {} diff --git a/rust-runtime/aws-smithy-http-client/src/lib.rs b/rust-runtime/aws-smithy-http-client/src/lib.rs index 1a3b64b916..e573e2f550 100644 --- a/rust-runtime/aws-smithy-http-client/src/lib.rs +++ b/rust-runtime/aws-smithy-http-client/src/lib.rs @@ -21,6 +21,7 @@ unreachable_pub, rust_2018_idioms )] +#![cfg_attr(docsrs, feature(doc_cfg))] // ideally hyper_014 would just be exposed as is but due to // https://github.com/rust-lang/rust/issues/47238 we get clippy warnings we can't suppress From 467a0ff8b06c70d602f97915fcae35c1e1138733 Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Mon, 18 Nov 2024 15:09:30 -0500 Subject: [PATCH 12/13] fix markdown --- rust-runtime/aws-smithy-http-client/src/client.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust-runtime/aws-smithy-http-client/src/client.rs b/rust-runtime/aws-smithy-http-client/src/client.rs index ee50c778fd..99bcc18d00 100644 --- a/rust-runtime/aws-smithy-http-client/src/client.rs +++ b/rust-runtime/aws-smithy-http-client/src/client.rs @@ -238,6 +238,7 @@ impl ConnectorBuilder { /// `CAP_NET_RAW` capability set or be run as root. /// /// This function is only availble on Android, Fuchsia, and Linux. + /// /// [VRF]: https://www.kernel.org/doc/Documentation/networking/vrf.txt #[cfg(any(target_os = "android", target_os = "fuchsia", target_os = "linux"))] pub fn set_interface>(&mut self, interface: S) -> &mut Self { From 831747e9b00c37902df528f8cb9ff3034541e215 Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Mon, 18 Nov 2024 17:00:50 -0500 Subject: [PATCH 13/13] add http only builder and work around some more CI issues with specific feature flags --- .../aws-smithy-http-client/src/client.rs | 74 +++++++++++-------- 1 file changed, 44 insertions(+), 30 deletions(-) diff --git a/rust-runtime/aws-smithy-http-client/src/client.rs b/rust-runtime/aws-smithy-http-client/src/client.rs index 99bcc18d00..881a3d4c1f 100644 --- a/rust-runtime/aws-smithy-http-client/src/client.rs +++ b/rust-runtime/aws-smithy-http-client/src/client.rs @@ -3,7 +3,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -mod dns; mod timeout; /// TLS connector(s) pub mod tls; @@ -34,8 +33,9 @@ use h2::Reason; use http_1x::{Extensions, Uri}; use hyper::rt::{Read, Write}; use hyper_util::client::legacy as client; +use hyper_util::client::legacy::connect::dns::GaiResolver; use hyper_util::client::legacy::connect::{ - capture_connection, CaptureConnection, Connect, HttpInfo, + capture_connection, CaptureConnection, Connect, HttpConnector as HyperHttpConnector, HttpInfo, }; use hyper_util::rt::TokioExecutor; use std::borrow::Cow; @@ -132,6 +132,13 @@ impl ConnectorBuilder { }, } } + + /// Build an HTTP connector sans TLS + #[doc(hidden)] + pub fn build_http(self) -> Connector { + let base = self.base_connector(); + self.wrap_connector(base) + } } impl ConnectorBuilder { @@ -183,6 +190,24 @@ impl ConnectorBuilder { } } + /// Get the base TCP connector by mapping our config to the underlying `HttpConnector` from hyper + /// (which is a base TCP connector with no TLS or any wrapping) + fn base_connector(&self) -> HyperHttpConnector { + self.base_connector_with_resolver(GaiResolver::new()) + } + + /// Get the base TCP connector by mapping our config to the underlying `HttpConnector` from hyper + /// using the given resolver `R` + fn base_connector_with_resolver(&self, resolver: R) -> HyperHttpConnector { + let mut conn = HyperHttpConnector::new_with_resolver(resolver); + conn.set_nodelay(self.enable_tcp_nodelay); + #[cfg(any(target_os = "android", target_os = "fuchsia", target_os = "linux"))] + if let Some(interface) = &self.interface { + conn.set_interface(interface); + } + conn + } + /// Set the async sleep implementation used for timeouts /// /// Calling this is only necessary for testing or to use something other than @@ -237,7 +262,7 @@ impl ConnectorBuilder { /// On Linux it can be used to specify a [VRF], but the binary needs to either have /// `CAP_NET_RAW` capability set or be run as root. /// - /// This function is only availble on Android, Fuchsia, and Linux. + /// This function is only available on Android, Fuchsia, and Linux. /// /// [VRF]: https://www.kernel.org/doc/Documentation/networking/vrf.txt #[cfg(any(target_os = "android", target_os = "fuchsia", target_os = "linux"))] @@ -525,31 +550,8 @@ pub struct Builder { } cfg_tls! { - use crate::client::dns::HyperUtilResolver; + mod dns; use aws_smithy_runtime_api::client::dns::ResolveDns; - use hyper_util::client::legacy::connect::dns::{GaiResolver, Name}; - use hyper_util::client::legacy::connect::HttpConnector as HyperHttpConnector; - use std::net::SocketAddr; - - impl ConnectorBuilder { - /// Get the base TCP connector by mapping our config to the underlying `HttpConnector` from hyper - /// (which is a base TCP connector with no TLS or any wrapping) - fn base_connector(&self) -> HyperHttpConnector { - self.base_connector_with_resolver(GaiResolver::new()) - } - - /// Get the base TCP connector by mapping our config to the underlying `HttpConnector` from hyper - /// using the given resolver `R` - fn base_connector_with_resolver(&self, resolver: R) -> HyperHttpConnector { - let mut conn = HyperHttpConnector::new_with_resolver(resolver); - conn.set_nodelay(self.enable_tcp_nodelay); - #[cfg(any(target_os = "android", target_os = "fuchsia", target_os = "linux"))] - if let Some(interface) = &self.interface { - conn.set_interface(interface); - } - conn - } - } impl ConnectorBuilder { /// Build a [`Connector`] that will use the default DNS resolver implementation. @@ -560,6 +562,7 @@ cfg_tls! { /// Build a [`Connector`] that will use the given DNS resolver implementation. pub fn build_with_resolver(self, resolver: R) -> Connector { + use crate::client::dns::HyperUtilResolver; let http_connector = self.base_connector_with_resolver(HyperUtilResolver { resolver }); self.build_https(http_connector) } @@ -567,8 +570,8 @@ cfg_tls! { fn build_https(self, http_connector: HyperHttpConnector) -> Connector where R: Clone + Send + Sync + 'static, - R: tower::Service, - R::Response: Iterator, + R: tower::Service, + R::Response: Iterator, R::Future: Send, R::Error: Into>, { @@ -645,7 +648,17 @@ impl Builder { Self::default() } - // TODO(hyper1) - build for unsecure HTTP? + /// Build a new HTTP client without TLS enabled + #[doc(hidden)] + pub fn build_http(self) -> SharedHttpClient { + build_with_conn_fn( + self.client_builder, + move |client_builder, settings, runtime_components| { + let builder = new_conn_builder(client_builder, settings, runtime_components); + builder.build_http() + }, + ) + } /// Set the TLS implementation to use pub fn tls_provider(self, provider: tls::Provider) -> Builder { @@ -680,6 +693,7 @@ where }) } +#[allow(dead_code)] pub(crate) fn build_with_tcp_conn_fn( client_builder: Option, tcp_connector_fn: F,