From cd9fd522074bfe530c30c878e49e6ac1bd881f1f Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Fri, 13 Jan 2017 15:27:35 -0800 Subject: [PATCH] refactor(header): Host header internals made private This allows us to improve the performance. For now, a Cow is used internally, so clients can set the host to a static value and no longer need copies. Later, we can change it to also possibly have a MemSlice. BREAKING CHANGE: The fields of the `Host` header are no longer available. Use the getter methods instead. --- src/client/mod.rs | 5 +- src/header/common/host.rs | 91 ++++++++++++++++--------------------- src/header/common/origin.rs | 5 +- src/header/mod.rs | 10 ++-- 4 files changed, 45 insertions(+), 66 deletions(-) diff --git a/src/client/mod.rs b/src/client/mod.rs index 80fa6006d4..a48633aeaf 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -123,10 +123,7 @@ impl Service for Client { let (mut head, body) = request::split(req); let mut headers = Headers::new(); - headers.set(Host { - hostname: url.host_str().unwrap().to_owned(), - port: url.port().or(None), - }); + headers.set(Host::new(url.host_str().unwrap().to_owned(), url.port())); headers.extend(head.headers.iter()); head.subject.1 = RequestUri::AbsolutePath { path: url.path().to_owned(), diff --git a/src/header/common/host.rs b/src/header/common/host.rs index b22dd795bb..3f95d63f2f 100644 --- a/src/header/common/host.rs +++ b/src/header/common/host.rs @@ -1,27 +1,22 @@ -use header::{Header, Raw}; +use std::borrow::Cow; use std::fmt; use std::str::FromStr; + +use header::{Header, Raw}; use header::parsing::from_one_raw_str; -use url::idna::domain_to_unicode; /// The `Host` header. /// /// HTTP/1.1 requires that all requests include a `Host` header, and so hyper /// client requests add one automatically. /// -/// Currently is just a String, but it should probably become a better type, -/// like `url::Host` or something. -/// /// # Examples /// ``` /// use hyper::header::{Headers, Host}; /// /// let mut headers = Headers::new(); /// headers.set( -/// Host{ -/// hostname: "hyper.rs".to_owned(), -/// port: None, -/// } +/// Host::new("hyper.rs", None) /// ); /// ``` /// ``` @@ -29,18 +24,36 @@ use url::idna::domain_to_unicode; /// /// let mut headers = Headers::new(); /// headers.set( -/// Host{ -/// hostname: "hyper.rs".to_owned(), -/// port: Some(8080), -/// } +/// Host::new("hyper.rs", 8080) /// ); /// ``` #[derive(Clone, PartialEq, Debug)] pub struct Host { - /// The hostname, such a example.domain. - pub hostname: String, - /// An optional port number. - pub port: Option + hostname: Cow<'static, str>, + port: Option +} + +impl Host { + /// Create a `Host` header, providing the hostname and optional port. + pub fn new(hostname: H, port: P) -> Host + where H: Into>, + P: Into> + { + Host { + hostname: hostname.into(), + port: port.into(), + } + } + + /// Get the hostname, such as example.domain. + pub fn hostname(&self) -> &str { + self.hostname.as_ref() + } + + /// Get the optional port number. + pub fn port(&self) -> Option { + self.port + } } impl Header for Host { @@ -75,27 +88,14 @@ impl FromStr for Host { let port = idx.and_then( |idx| s[idx + 1..].parse().ok() ); - let hostname_encoded = match port { + let hostname = match port { None => s, Some(_) => &s[..idx.unwrap()] }; - let hostname = if hostname_encoded.starts_with("[") { - if !hostname_encoded.ends_with("]") { - return Err(::Error::Header) - } - hostname_encoded.to_owned() - } else { - let (hostname, res) = domain_to_unicode(hostname_encoded); - if res.is_err() { - return Err(::Error::Header) - } - hostname - }; - Ok(Host { - hostname: hostname, - port: port + hostname: hostname.to_owned().into(), + port: port, }) } } @@ -109,35 +109,20 @@ mod tests { #[test] fn test_host() { let host = Header::parse_header(&vec![b"foo.com".to_vec()].into()); - assert_eq!(host.ok(), Some(Host { - hostname: "foo.com".to_owned(), - port: None - })); + assert_eq!(host.ok(), Some(Host::new("foo.com", None))); let host = Header::parse_header(&vec![b"foo.com:8080".to_vec()].into()); - assert_eq!(host.ok(), Some(Host { - hostname: "foo.com".to_owned(), - port: Some(8080) - })); + assert_eq!(host.ok(), Some(Host::new("foo.com", 8080))); let host = Header::parse_header(&vec![b"foo.com".to_vec()].into()); - assert_eq!(host.ok(), Some(Host { - hostname: "foo.com".to_owned(), - port: None - })); + assert_eq!(host.ok(), Some(Host::new("foo.com", None))); let host = Header::parse_header(&vec![b"[::1]:8080".to_vec()].into()); - assert_eq!(host.ok(), Some(Host { - hostname: "[::1]".to_owned(), - port: Some(8080) - })); + assert_eq!(host.ok(), Some(Host::new("[::1]", 8080))); let host = Header::parse_header(&vec![b"[::1]".to_vec()].into()); - assert_eq!(host.ok(), Some(Host { - hostname: "[::1]".to_owned(), - port: None - })); + assert_eq!(host.ok(), Some(Host::new("[::1]", None))); } } diff --git a/src/header/common/origin.rs b/src/header/common/origin.rs index 0310d1de60..496455a610 100644 --- a/src/header/common/origin.rs +++ b/src/header/common/origin.rs @@ -43,10 +43,7 @@ impl Origin { pub fn new, H: Into>(scheme: S, hostname: H, port: Option) -> Origin{ Origin { scheme: scheme.into(), - host: Host { - hostname: hostname.into(), - port: port - } + host: Host::new(hostname.into(), port), } } } diff --git a/src/header/mod.rs b/src/header/mod.rs index 7c18af7ca9..ef69537e5d 100644 --- a/src/header/mod.rs +++ b/src/header/mod.rs @@ -683,10 +683,10 @@ mod tests { } #[test] - fn test_headers_show() { + fn test_headers_to_string() { let mut headers = Headers::new(); headers.set(ContentLength(15)); - headers.set(Host { hostname: "foo.bar".to_owned(), port: None }); + headers.set(Host::new("foo.bar", None)); let s = headers.to_string(); assert!(s.contains("Host: foo.bar\r\n")); @@ -694,7 +694,7 @@ mod tests { } #[test] - fn test_headers_show_raw() { + fn test_headers_to_string_raw() { let headers = Headers::from_raw(&raw!(b"Content-Length: 10")).unwrap(); let s = headers.to_string(); assert_eq!(s, "Content-Length: 10\r\n"); @@ -766,7 +766,7 @@ mod tests { assert_eq!(headers1, headers2); headers1.set(ContentLength(11)); - headers2.set(Host {hostname: "foo.bar".to_owned(), port: None}); + headers2.set(Host::new("foo.bar", None)); assert!(headers1 != headers2); headers1 = Headers::new(); @@ -782,7 +782,7 @@ mod tests { headers1 = Headers::new(); headers2 = Headers::new(); - headers1.set(Host { hostname: "foo.bar".to_owned(), port: None }); + headers1.set(Host::new("foo.bar", None)); headers1.set(ContentLength(11)); headers2.set(ContentLength(11)); assert!(headers1 != headers2);