Unverified Commit 10520d7f authored by Zelda Hessler's avatar Zelda Hessler Committed by GitHub
Browse files

Fix: native-tls client creation to support HTTPS again (#2360) 

* update: use `enforce_http = false` when creating native-tls hyper connector
refactor: move smithy conns module to its own file
add: sanity tests ensuring we can make HTTP and HTTPS requests with the rustls and native-tls connectors
remove: `use crate::*` imports in favor of explicit imports

* update: CHANGELOG.next.toml

* add: feature gate to conns tests
parent f7c550ee

CHANGELOG.next.toml

+12 −0
Original line number Diff line number Diff line
@@ -198,3 +198,15 @@ message = "Fix inconsistent casing in services re-export." 
references = ["smithy-rs#2349"]

meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "server" }

author = "hlbarber"



[[aws-sdk-rust]]

message = "Fix issue where clients using native-tls connector were prevented from making HTTPS requests."

references = ["aws-sdk-rust#736"]

meta = { "breaking" = false, "tada" = false, "bug" = true }

author = "Velfi"



[[smithy-rs]]

message = "Fix issue where clients using native-tls connector were prevented from making HTTPS requests."

references = ["aws-sdk-rust#736"]

meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client" }

author = "Velfi"

rust-runtime/aws-smithy-client/src/bounds.rs

+14 −10
Original line number Diff line number Diff line
@@ -7,7 +7,7 @@ 
//! required for `call` and friends.

//!

//! The short-hands will one day be true [trait aliases], but for now they are traits with blanket

//! implementations. Also, due to [compiler limitations], the bounds repeat a nubmer of associated

//! implementations. Also, due to [compiler limitations], the bounds repeat a number of associated

//! types with bounds so that those bounds [do not need to be repeated] at the call site. It's a

//! bit of a mess to define, but _should_ be invisible to callers.

//!
@@ -17,8 +17,12 @@ 


use crate::erase::DynConnector;

use crate::http_connector::HttpConnector;

use crate::*;

use aws_smithy_http::result::ConnectorError;

use aws_smithy_http::body::SdkBody;

use aws_smithy_http::operation::{self, Operation};

use aws_smithy_http::response::ParseHttpResponse;

use aws_smithy_http::result::{ConnectorError, SdkError, SdkSuccess};

use aws_smithy_http::retry::ClassifyRetry;

use tower::{Layer, Service};



/// A service that has parsed a raw Smithy response.

pub type Parsed<S, O, Retry> =
@@ -75,14 +79,14 @@ where 
    }

}



/// A Smithy middleware service that adjusts [`aws_smithy_http::operation::Request`]s.

/// A Smithy middleware service that adjusts [`aws_smithy_http::operation::Request`](operation::Request)s.

///

/// This trait has a blanket implementation for all compatible types, and should never be

/// implemented.

pub trait SmithyMiddlewareService:

    Service<

    aws_smithy_http::operation::Request,

    Response = aws_smithy_http::operation::Response,

    operation::Request,

    Response = operation::Response,

    Error = aws_smithy_http_tower::SendOperationError,

    Future = <Self as SmithyMiddlewareService>::Future,

>
@@ -96,8 +100,8 @@ pub trait SmithyMiddlewareService: 
impl<T> SmithyMiddlewareService for T

where

    T: Service<

        aws_smithy_http::operation::Request,

        Response = aws_smithy_http::operation::Response,

        operation::Request,

        Response = operation::Response,

        Error = aws_smithy_http_tower::SendOperationError,

    >,

    T::Future: Send + 'static,
@@ -143,7 +147,7 @@ pub trait SmithyRetryPolicy<O, T, E, Retry>: 
    /// Forwarding type to `E` for bound inference.

    ///

    /// See module-level docs for details.

    type E: Error;

    type E: std::error::Error;



    /// Forwarding type to `Retry` for bound inference.

    ///
@@ -155,7 +159,7 @@ impl<R, O, T, E, Retry> SmithyRetryPolicy<O, T, E, Retry> for R 
where

    R: tower::retry::Policy<Operation<O, Retry>, SdkSuccess<T>, SdkError<E>> + Clone,

    O: ParseHttpResponse<Output = Result<T, E>> + Send + Sync + Clone + 'static,

    E: Error,

    E: std::error::Error,

    Retry: ClassifyRetry<SdkSuccess<T>, SdkError<E>>,

{

    type O = O;

rust-runtime/aws-smithy-client/src/conns.rs

0 → 100644
+129 −0
Original line number Diff line number Diff line 
/*

 * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.

 * SPDX-License-Identifier: Apache-2.0

 */



//! Type aliases for standard connection types.



#[cfg(feature = "rustls")]

/// A `hyper` connector that uses the `rustls` crate for TLS. To use this in a smithy client,

/// wrap it in a [hyper_ext::Adapter](crate::hyper_ext::Adapter).

pub type Https = hyper_rustls::HttpsConnector<hyper::client::HttpConnector>;



#[cfg(feature = "native-tls")]

/// A `hyper` connector that uses the `native-tls` crate for TLS. To use this in a smithy client,

/// wrap it in a [hyper_ext::Adapter](crate::hyper_ext::Adapter).

pub type NativeTls = hyper_tls::HttpsConnector<hyper::client::HttpConnector>;



#[cfg(feature = "rustls")]

/// A smithy connector that uses the `rustls` crate for TLS.

pub type Rustls = crate::hyper_ext::Adapter<Https>;



// Creating a `with_native_roots` HTTP client takes 300ms on OS X. Cache this so that we

// don't need to repeatedly incur that cost.

#[cfg(feature = "rustls")]

lazy_static::lazy_static! {

    static ref HTTPS_NATIVE_ROOTS: Https = {

        hyper_rustls::HttpsConnectorBuilder::new()

            .with_native_roots()

            .https_or_http()

            .enable_http1()

            .enable_http2()

            .build()

    };

}



#[cfg(feature = "rustls")]

/// Return a default HTTPS connector backed by the `rustls` crate.

///

/// It requires a minimum TLS version of 1.2.

/// It allows you to connect to both `http` and `https` URLs.

pub fn https() -> Https {

    HTTPS_NATIVE_ROOTS.clone()

}



#[cfg(feature = "native-tls")]

/// Return a default HTTPS connector backed by the `hyper_tls` crate.

///

/// It requires a minimum TLS version of 1.2.

/// It allows you to connect to both `http` and `https` URLs.

pub fn native_tls() -> NativeTls {

    // `TlsConnector` actually comes for here: https://docs.rs/native-tls/latest/native_tls/

    // hyper_tls just re-exports the crate for convenience.

    let mut tls = hyper_tls::native_tls::TlsConnector::builder();

    let tls = tls

        .min_protocol_version(Some(hyper_tls::native_tls::Protocol::Tlsv12))

        .build()

        .unwrap_or_else(|e| panic!("Error while creating TLS connector: {}", e));

    let mut http = hyper::client::HttpConnector::new();

    http.enforce_http(false);

    hyper_tls::HttpsConnector::from((http, tls.into()))

}



#[cfg(all(test, any(feature = "native-tls", feature = "rustls")))]

mod tests {

    use crate::erase::DynConnector;

    use crate::hyper_ext::Adapter;

    use aws_smithy_http::body::SdkBody;

    use http::{Method, Request, Uri};

    use tower::{Service, ServiceBuilder};



    async fn send_request_and_assert_success(conn: DynConnector, uri: &Uri) {

        let mut svc = ServiceBuilder::new().service(conn);

        let req = Request::builder()

            .uri(uri)

            .method(Method::GET)

            .body(SdkBody::empty())

            .unwrap();

        let res = svc.call(req).await.unwrap();

        assert!(res.status().is_success());

    }



    #[cfg(feature = "native-tls")]

    mod native_tls_tests {

        use super::super::native_tls;

        use super::*;



        #[tokio::test]

        async fn test_native_tls_connector_can_make_http_requests() {

            let conn = Adapter::builder().build(native_tls());

            let conn = DynConnector::new(conn);

            let http_uri: Uri = "http://example.com/".parse().unwrap();



            send_request_and_assert_success(conn, &http_uri).await;

        }



        #[tokio::test]

        async fn test_native_tls_connector_can_make_https_requests() {

            let conn = Adapter::builder().build(native_tls());

            let conn = DynConnector::new(conn);

            let https_uri: Uri = "https://example.com/".parse().unwrap();



            send_request_and_assert_success(conn, &https_uri).await;

        }

    }



    #[cfg(feature = "rustls")]

    mod rustls_tests {

        use super::super::https;

        use super::*;



        #[tokio::test]

        async fn test_rustls_connector_can_make_http_requests() {

            let conn = Adapter::builder().build(https());

            let conn = DynConnector::new(conn);

            let http_uri: Uri = "http://example.com/".parse().unwrap();



            send_request_and_assert_success(conn, &http_uri).await;

        }



        #[tokio::test]

        async fn test_rustls_connector_can_make_https_requests() {

            let conn = Adapter::builder().build(https());

            let conn = DynConnector::new(conn);

            let https_uri: Uri = "https://example.com/".parse().unwrap();



            send_request_and_assert_success(conn, &https_uri).await;

        }

    }

}

rust-runtime/aws-smithy-client/src/lib.rs

+11 −67
Original line number Diff line number Diff line
@@ -24,7 +24,10 @@ 


pub mod bounds;

pub mod erase;

pub mod http_connector;

pub mod never;

pub mod retry;

pub mod timeout;



// https://github.com/rust-lang/rust/issues/72081

#[allow(rustdoc::private_doc_tests)]
@@ -36,8 +39,8 @@ pub mod dvr; 
#[cfg(feature = "test-util")]

pub mod test_connection;



pub mod http_connector;



#[cfg(feature = "client-hyper")]

pub mod conns;

#[cfg(feature = "client-hyper")]

pub mod hyper_ext;
@@ -47,78 +50,19 @@ pub mod hyper_ext; 
#[doc(hidden)]

pub mod static_tests;



pub mod never;

pub mod timeout;

pub use timeout::TimeoutLayer;



/// Type aliases for standard connection types.

#[cfg(feature = "client-hyper")]

#[allow(missing_docs)]

pub mod conns {

    #[cfg(feature = "rustls")]

    pub type Https = hyper_rustls::HttpsConnector<hyper::client::HttpConnector>;



    // Creating a `with_native_roots` HTTP client takes 300ms on OS X. Cache this so that we

    // don't need to repeatedly incur that cost.

    #[cfg(feature = "rustls")]

    lazy_static::lazy_static! {

        static ref HTTPS_NATIVE_ROOTS: Https = {

            hyper_rustls::HttpsConnectorBuilder::new()

                .with_native_roots()

                .https_or_http()

                .enable_http1()

                .enable_http2()

                .build()

        };

    }



    #[cfg(feature = "rustls")]

    /// Return a default HTTPS connector backed by the `rustls` crate.

    ///

    /// It requires a minimum TLS version of 1.2.

    /// It allows you to connect to both `http` and `https` URLs.

    pub fn https() -> Https {

        HTTPS_NATIVE_ROOTS.clone()

    }



    #[cfg(feature = "native-tls")]

    /// Return a default HTTPS connector backed by the `hyper_tls` crate.

    ///

    /// It requires a minimum TLS version of 1.2.

    /// It allows you to connect to both `http` and `https` URLs.

    pub fn native_tls() -> NativeTls {

        let mut tls = hyper_tls::native_tls::TlsConnector::builder();

        let tls = tls

            .min_protocol_version(Some(hyper_tls::native_tls::Protocol::Tlsv12))

            .build()

            .unwrap_or_else(|e| panic!("Error while creating TLS connector: {}", e));

        let http = hyper::client::HttpConnector::new();

        hyper_tls::HttpsConnector::from((http, tls.into()))

    }



    #[cfg(feature = "native-tls")]

    pub type NativeTls = hyper_tls::HttpsConnector<hyper::client::HttpConnector>;



    #[cfg(feature = "rustls")]

    pub type Rustls =

        crate::hyper_ext::Adapter<hyper_rustls::HttpsConnector<hyper::client::HttpConnector>>;

}



use aws_smithy_async::rt::sleep::AsyncSleep;

use aws_smithy_http::body::SdkBody;

use aws_smithy_http::operation::Operation;

use aws_smithy_http::response::ParseHttpResponse;

pub use aws_smithy_http::result::{SdkError, SdkSuccess};

use aws_smithy_http::retry::ClassifyRetry;

use aws_smithy_http_tower::dispatch::DispatchLayer;

use aws_smithy_http_tower::parse_response::ParseResponseLayer;

use aws_smithy_types::error::display::DisplayErrorContext;

use aws_smithy_types::retry::ProvideErrorKind;

use aws_smithy_types::timeout::OperationTimeoutConfig;

use std::error::Error;

use std::sync::Arc;

use timeout::ClientTimeoutParams;

use tower::{Layer, Service, ServiceBuilder, ServiceExt};

pub use timeout::TimeoutLayer;

use tower::{Service, ServiceBuilder, ServiceExt};

use tracing::{debug_span, field, field::display, Instrument};



/// Smithy service client.
@@ -131,7 +75,7 @@ use tracing::{debug_span, field, field::display, Instrument}; 
/// such as those used for routing (like the URL), authentication, and authorization.

///

/// The middleware takes the form of a [`tower::Layer`] that wraps the actual connection for each

/// request. The [`tower::Service`] that the middleware produces must accept requests of the type

/// request. The [`tower::Service`](Service) that the middleware produces must accept requests of the type

/// [`aws_smithy_http::operation::Request`] and return responses of the type

/// [`http::Response<SdkBody>`], most likely by modifying the provided request in place, passing it

/// to the inner service, and then ultimately returning the inner service's response.
@@ -165,9 +109,9 @@ impl<C, M> Client<C, M> 
where

    M: Default,

{

    /// Create a Smithy client from the given `connector`, a middleware default, the [standard

    /// retry policy](crate::retry::Standard), and the [`default_async_sleep`](aws_smithy_async::rt::sleep::default_async_sleep)

    /// sleep implementation.

    /// Create a Smithy client from the given `connector`, a middleware default, the

    /// [standard retry policy](retry::Standard), and the

    /// [`default_async_sleep`](aws_smithy_async::rt::sleep::default_async_sleep) sleep implementation.

    pub fn new(connector: C) -> Self {

        Builder::new()

            .connector(connector)

rust-runtime/aws-smithy-client/src/static_tests.rs

+2 −2
Original line number Diff line number Diff line
@@ -5,7 +5,7 @@ 
//! This module provides types useful for static tests.

#![allow(missing_docs, missing_debug_implementations)]



use crate::{Builder, Error, Operation, ParseHttpResponse, ProvideErrorKind};

use crate::{Builder, Operation, ParseHttpResponse, ProvideErrorKind};

use aws_smithy_http::operation;

use aws_smithy_http::retry::DefaultResponseRetryClassifier;
@@ -17,7 +17,7 @@ impl std::fmt::Display for TestOperationError { 
        unreachable!("only used for static tests")

    }

}

impl Error for TestOperationError {}

impl std::error::Error for TestOperationError {}

impl ProvideErrorKind for TestOperationError {

    fn retryable_error_kind(&self) -> Option<aws_smithy_types::retry::ErrorKind> {

        unreachable!("only used for static tests")