diff --git a/Cargo.lock b/Cargo.lock index 618f2be32f..14022c7137 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -111,7 +111,7 @@ dependencies = [ "h2 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "http 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)", "httparse 1.2.4 (registry+https://github.com/rust-lang/crates.io-index)", - "hyper 0.11.21 (registry+https://github.com/rust-lang/crates.io-index)", + "hyper 0.11.22 (registry+https://github.com/rust-lang/crates.io-index)", "indexmap 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "ipnet 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.36 (registry+https://github.com/rust-lang/crates.io-index)", @@ -287,7 +287,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "hyper" -version = "0.11.21" +version = "0.11.22" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "base64 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -961,7 +961,7 @@ dependencies = [ "checksum heck 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ea04fa3ead4e05e51a7c806fc07271fdbde4e246a6c6d1efd52e72230b771b82" "checksum http 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "75df369fd52c60635208a4d3e694777c099569b3dcf4844df8f652dc004644ab" "checksum httparse 1.2.4 (registry+https://github.com/rust-lang/crates.io-index)" = "c2f407128745b78abc95c0ffbe4e5d37427fdc0d45470710cfef8c44522a2e37" -"checksum hyper 0.11.21 (registry+https://github.com/rust-lang/crates.io-index)" = "a3a77dea5dccbf32ba4e9ddd7d80a5a3bb3b9f1f3835e18daf5dbea6bee0efbf" +"checksum hyper 0.11.22 (registry+https://github.com/rust-lang/crates.io-index)" = "d595f999e90624f64d2c4bc74c72adb0f3e0f773dc5692ca91338363b3568fa0" "checksum indexmap 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "7164c96d6e18ccc3ce43f3dedac996c21a220670a106c275b96ad92110401362" "checksum iovec 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "dbe6e417e7d0975db6512b90796e8ce223145ac4e33c377e4a42882a0e88bb08" "checksum ipnet 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "51268c3a27ad46afd1cca0bbf423a5be2e9fd3e6a7534736c195f0f834b763ef" diff --git a/proxy/Cargo.toml b/proxy/Cargo.toml index 153861eacf..c1eaf3c3e8 100644 --- a/proxy/Cargo.toml +++ b/proxy/Cargo.toml @@ -22,7 +22,7 @@ futures = "0.1" h2 = "0.1" http = "0.1" httparse = "1.2" -hyper = { version = "0.11.21", default-features = false, features = ["compat"] } +hyper = { version = "0.11.22", default-features = false, features = ["compat"] } ipnet = "1.0" log = "0.4.1" indexmap = "0.4.1" diff --git a/proxy/src/bind.rs b/proxy/src/bind.rs index 33edf4038f..f942a6ba59 100644 --- a/proxy/src/bind.rs +++ b/proxy/src/bind.rs @@ -6,8 +6,8 @@ use std::net::SocketAddr; use std::sync::Arc; use std::sync::atomic::AtomicUsize; -use futures::{future, Future, Poll}; -use futures::future::{Either, Map}; +use futures::{Future, Poll}; +use futures::future::Map; use http::{self, uri}; use tokio_core::reactor::Handle; use tower; @@ -69,11 +69,11 @@ pub enum Host { /// the authority given in the `Host:` header, or, failing that, from the /// request's original destination according to `SO_ORIGINAL_DST`. #[derive(Copy, Clone, Debug)] -pub struct ReconstructUri { +pub struct NormalizeUri { inner: S } -pub type Service = Reconnect>>; +pub type Service = Reconnect>>; pub type NewHttp = sensor::NewHttp, B, HttpBody>; @@ -204,7 +204,7 @@ where // Rewrite the HTTP/1 URI, if the authorities in the Host header // and request URI are not in agreement, or are not present. - let proxy = ReconstructUri::new(sensors); + let proxy = NormalizeUri::new(sensors); // Automatically perform reconnects if the connection fails. // @@ -241,16 +241,16 @@ where } -// ===== impl ReconstructUri ===== +// ===== impl NormalizeUri ===== -impl ReconstructUri { +impl NormalizeUri { fn new (inner: S) -> Self { Self { inner } } } -impl tower::NewService for ReconstructUri +impl tower::NewService for NormalizeUri where S: tower::NewService< Request=http::Request, @@ -260,24 +260,24 @@ where Request=http::Request, Response=HttpResponse, >, - ReconstructUri: tower::Service, + NormalizeUri: tower::Service, B: tower_h2::Body, { type Request = ::Request; type Response = ::Response; type Error = ::Error; - type Service = ReconstructUri; + type Service = NormalizeUri; type InitError = S::InitError; type Future = Map< S::Future, - fn(S::Service) -> ReconstructUri + fn(S::Service) -> NormalizeUri >; fn new_service(&self) -> Self::Future { - self.inner.new_service().map(ReconstructUri::new) + self.inner.new_service().map(NormalizeUri::new) } } -impl tower::Service for ReconstructUri +impl tower::Service for NormalizeUri where S: tower::Service< Request=http::Request, @@ -288,29 +288,17 @@ where type Request = S::Request; type Response = HttpResponse; type Error = S::Error; - type Future = Either< - S::Future, - future::FutureResult, - >; + type Future = S::Future; fn poll_ready(&mut self) -> Poll<(), S::Error> { self.inner.poll_ready() } fn call(&mut self, mut request: S::Request) -> Self::Future { - if request.version() == http::Version::HTTP_2 { - // skip `reconstruct_uri` entirely if the request is HTTP/2. - return Either::A(self.inner.call(request)); - } - - if let Err(_) = h1::reconstruct_uri(&mut request) { - let res = http::Response::builder() - .status(http::StatusCode::BAD_REQUEST) - .body(Default::default()) - .unwrap(); - return Either::B(future::ok(res)); + if request.version() != http::Version::HTTP_2 { + h1::normalize_our_view_of_uri(&mut request); } - Either::A(self.inner.call(request)) + self.inner.call(request) } } diff --git a/proxy/src/transparency/client.rs b/proxy/src/transparency/client.rs index cf80d717c8..fa0836f41f 100644 --- a/proxy/src/transparency/client.rs +++ b/proxy/src/transparency/client.rs @@ -9,6 +9,7 @@ use tower_h2; use bind; use super::glue::{BodyStream, HttpBody, HyperConnect}; +use super::h1::UriIsAbsoluteForm; /// A `NewService` that can speak either HTTP/1 or HTTP/2. pub struct Client @@ -83,6 +84,9 @@ where let h1 = hyper::Client::configure() .connector(HyperConnect::new(connect)) .body() + // hyper should never try to automatically set the Host + // header, instead always just passing whatever we received. + .set_host(false) .build(&executor); Client { inner: ClientInner::Http1(h1), @@ -177,6 +181,12 @@ where match self.inner { ClientServiceInner::Http1(ref h1) => { + // This sentinel may be set in h1::normalize_our_view_of_uri + // when the original request-target was in absolute-form. In + // that case, for hyper 0.11.x, we need to call `req.set_proxy`. + let was_absolute_form = req.extensions() + .get::() + .is_some(); // As of hyper 0.11.x, a set body implies a body must be sent. // If there is no content-length header, hyper adds a // transfer-encoding: chunked header, since it has no other way @@ -206,6 +216,7 @@ where if should_take_body { req.body_mut().take(); } + req.set_proxy(was_absolute_form); ClientServiceFuture::Http1(h1.request(req)) }, ClientServiceInner::Http2(ref mut h2) => { diff --git a/proxy/src/transparency/h1.rs b/proxy/src/transparency/h1.rs index 5d4cc50e4d..f0b869d8fa 100644 --- a/proxy/src/transparency/h1.rs +++ b/proxy/src/transparency/h1.rs @@ -4,31 +4,29 @@ use std::sync::Arc; use bytes::BytesMut; use http; -use http::header::{HeaderValue, HOST}; +use http::header::HOST; use http::uri::{Authority, Parts, Scheme, Uri}; use ctx::transport::{Server as ServerCtx}; -pub fn reconstruct_uri(req: &mut http::Request) -> Result<(), ()> { - // RFC7230#section-5.4 - // If an absolute-form uri is received, it must replace - // the host header - if let Some(auth) = req.uri().authority_part().cloned() { - if let Some(host) = req.headers().get(HOST) { - if auth.as_str().as_bytes() == host.as_bytes() { - // host and absolute-form agree, nothing more to do - return Ok(()); - } - } - let host = HeaderValue::from_shared(auth.into_bytes()) - .expect("a valid authority is valid header value"); - req.headers_mut().insert(HOST, host); - return Ok(()); +/// Sentinel extension to signal that we should tell hyper to serialize +/// the `Uri` in absolute-form. +pub struct UriIsAbsoluteForm; + +/// Tries to make sure the `Uri` of the request is in a form needed by +/// hyper's Client. +/// +/// Also sets the `UriIsAbsoluteForm` extension if received `Uri` was +/// already in absolute-form. +pub fn normalize_our_view_of_uri(req: &mut http::Request) { + if req.uri().authority_part().is_some() { + req.extensions_mut().insert(UriIsAbsoluteForm); + return; } // try to parse the Host header if let Some(auth) = authority_from_host(&req) { set_authority(req.uri_mut(), auth); - return Ok(()); + return; } // last resort is to use the so_original_dst @@ -43,11 +41,7 @@ pub fn reconstruct_uri(req: &mut http::Request) -> Result<(), ()> { let auth = Authority::from_shared(bytes) .expect("socket address is valid authority"); set_authority(req.uri_mut(), auth); - - return Ok(()); } - - Err(()) } /// Returns an Authority from a request's Host header. diff --git a/proxy/tests/transparency.rs b/proxy/tests/transparency.rs index 1c0c45f4fb..689eac316b 100644 --- a/proxy/tests/transparency.rs +++ b/proxy/tests/transparency.rs @@ -117,6 +117,8 @@ fn http10_without_host() { let srv = server::http1() .route_fn("/", move |req| { assert_eq!(req.version(), http::Version::HTTP_10); + assert!(!req.headers().contains_key("host")); + assert_eq!(req.uri().to_string(), "/"); Response::builder() .version(http::Version::HTTP_10) .body("".into()) @@ -124,34 +126,37 @@ fn http10_without_host() { }) .run(); let ctrl = controller::new() - .destination(&srv.addr.to_string(), srv.addr) .run(); let proxy = proxy::new() .controller(ctrl) - .outbound(srv) + .inbound(srv) .run(); - let client = client::http1(proxy.outbound, "foo.bar"); - let res = client.request(client.request_builder("/") - .version(http::Version::HTTP_10) - .header("host", "")); + let client = client::tcp(proxy.inbound); - assert_eq!(res.status(), http::StatusCode::OK); - assert_eq!(res.version(), http::Version::HTTP_10); + let tcp_client = client.connect(); + + tcp_client.write("\ + GET / HTTP/1.0\r\n\ + \r\n\ + "); + + let expected = "HTTP/1.0 200 OK\r\n"; + assert_eq!(s(&tcp_client.read()[..expected.len()]), expected); } #[test] fn http11_absolute_uri_differs_from_host() { let _ = env_logger::try_init(); - let host = "transparency.test.svc.cluster.local"; + // We shouldn't touch the URI or the Host, just pass directly as we got. + let auth = "transparency.test.svc.cluster.local"; + let host = "foo.bar"; let srv = server::http1() .route_fn("/", move |req| { - assert_eq!(req.version(), http::Version::HTTP_11); - assert_eq!(req.headers().get("host").unwrap(), host); - Response::builder() - .body("".into()) - .unwrap() + assert_eq!(req.headers()["host"], host); + assert_eq!(req.uri().to_string(), format!("http://{}/", auth)); + Response::new("".into()) }) .run(); let ctrl = controller::new().run(); @@ -159,11 +164,11 @@ fn http11_absolute_uri_differs_from_host() { .controller(ctrl) .inbound(srv) .run(); - let client = client::http1_absolute_uris(proxy.inbound, host); + let client = client::http1_absolute_uris(proxy.inbound, auth); let res = client.request(client.request_builder("/") .version(http::Version::HTTP_11) - .header("host", "foo.bar")); + .header("host", host)); assert_eq!(res.status(), http::StatusCode::OK); assert_eq!(res.version(), http::Version::HTTP_11);