Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add possibility to set the SNI host for the connections #74

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "hyper-tls"
version = "0.4.3" # don't forget html_root_url in lib.rs
version = "0.4.4" # don't forget html_root_url in lib.rs
description = "Default TLS implementation for use with hyper"
authors = ["Sean McArthur <[email protected]>"]
license = "MIT/Apache-2.0"
Expand Down
37 changes: 28 additions & 9 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type BoxError = Box<dyn std::error::Error + Send + Sync>;
#[derive(Clone)]
pub struct HttpsConnector<T> {
force_https: bool,
sni_host: Option<String>,
http: T,
tls: TlsConnector,
}
Expand Down Expand Up @@ -65,20 +66,33 @@ impl<T> HttpsConnector<T> {
pub fn https_only(&mut self, enable: bool) {
self.force_https = enable;
}


/// Set the host which should be used for Server Name Indication (SNI).
///
/// if the `sni_host` is not set the default host from URI will be used
pub fn sni_host(&mut self, host: &str) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SNI host is already sent normally, just extracted from the Uri hostname. So this would really be more an override, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another concern that can be had is that this would force all requests to always use the same SNI value. It seems simple to accidentally configure this option and then use the connector for several different domains.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SNI host is already sent normally, just extracted from the Uri hostname. So this would really be more an override, right?

Yes, this is correct. In some cases we need to override the hostname with different value.

Another concern that can be had is that this would force all requests to always use the same SNI value. It seems simple to accidentally configure this option and then use the connector for several different domains.

yeah, this is the valid concern, but I haven't found a better way how I could provide the different hostname for the TLS connection. Maybe I missed something here, but if I could have access to the underlying Request I could tell http connector to extract the domain from the Host header instead if provided (in this case it might be need to have such an option exposes to configure http connecter before building the client), which might be a bit better way to set this up.

self.sni_host = Some(String::from(host));
}

/// With connector constructor
///
///
pub fn new_with_connector(http: T) -> Self {
native_tls::TlsConnector::new()
.map(|tls| HttpsConnector::from((http, tls.into())))
.unwrap_or_else(|e| panic!("HttpsConnector::new_with_connector(<connector>) failure: {}", e))
.unwrap_or_else(|e| {
panic!(
"HttpsConnector::new_with_connector(<connector>) failure: {}",
e
)
})
}
}

impl<T> From<(T, TlsConnector)> for HttpsConnector<T> {
fn from(args: (T, TlsConnector)) -> HttpsConnector<T> {
HttpsConnector {
force_https: false,
sni_host: None,
http: args.0,
tls: args.1,
}
Expand Down Expand Up @@ -120,15 +134,21 @@ where
return err(ForceHttpsButUriNotHttps.into());
}

let host = dst.host().unwrap_or("").trim_matches(|c| c == '[' || c == ']').to_owned();
let host = match self.sni_host.as_ref() {
Some(host) => host.to_owned(),
None => dst
.host()
.unwrap_or("")
.trim_matches(|c| c == '[' || c == ']')
.to_owned(),
};

let connecting = self.http.call(dst);
let tls = self.tls.clone();
let fut = async move {
let tcp = connecting.await.map_err(Into::into)?;
let maybe = if is_https {
let tls = tls
.connect(&host, tcp)
.await?;
let tls = tls.connect(&host, tcp).await?;
MaybeHttpsStream::Https(tls)
} else {
MaybeHttpsStream::Http(tcp)
Expand All @@ -143,8 +163,7 @@ fn err<T>(e: BoxError) -> HttpsConnecting<T> {
HttpsConnecting(Box::pin(async { Err(e) }))
}

type BoxedFut<T> =
Pin<Box<dyn Future<Output = Result<MaybeHttpsStream<T>, BoxError>> + Send>>;
type BoxedFut<T> = Pin<Box<dyn Future<Output = Result<MaybeHttpsStream<T>, BoxError>> + Send>>;

/// A Future representing work to connect to a URL, and a TLS handshake.
pub struct HttpsConnecting<T>(BoxedFut<T>);
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
//! Ok(())
//! }
//! ```
#![doc(html_root_url = "https://docs.rs/hyper-tls/0.4.3")]
#![doc(html_root_url = "https://docs.rs/hyper-tls/0.4.4")]
#![cfg_attr(test, deny(warnings))]
#![deny(missing_docs)]
#![deny(missing_debug_implementations)]
Expand Down