From c7484de9edc7fed369eae72b0f2563fcc21480c5 Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Mon, 23 Jul 2018 10:59:53 -0700 Subject: [PATCH 1/2] update to hyper 0.12.7 to fix a keep-alive bug Specifically proxied bodies would make use of an optimization in hyper that resulted in the connection not knowing (but it did know! just didn't tell itself...) that the body was finished, and so the connection was closed. 0.12.7 includes the fix in hyper. As part of this upgrade, the keep-alive tests have been adjusted to send a small body, since the empty body was not triggering this case. Signed-off-by: Sean McArthur --- Cargo.lock | 6 +++--- tests/support/server.rs | 10 ---------- tests/transparency.rs | 4 ++-- 3 files changed, 5 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3371257f97..776e448ced 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -340,7 +340,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "hyper" -version = "0.12.6" +version = "0.12.7" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "bytes 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)", @@ -482,7 +482,7 @@ dependencies = [ "h2 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)", "http 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", "httparse 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)", - "hyper 0.12.6 (registry+https://github.com/rust-lang/crates.io-index)", + "hyper 0.12.7 (registry+https://github.com/rust-lang/crates.io-index)", "indexmap 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "inotify 0.5.2-dev (git+https://github.com/inotify-rs/inotify)", "ipnet 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1585,7 +1585,7 @@ dependencies = [ "checksum hostname 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "58fab6e177434b0bb4cd344a4dabaa5bd6d7a8d792b1885aebcae7af1091d1cb" "checksum http 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "4fbced8864b04c030eebcb7d0dc3a81ba5231ac559f5116a29a8ba83ecee22cd" "checksum httparse 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "7b6288d7db100340ca12873fd4d08ad1b8f206a9457798dfb17c018a33fee540" -"checksum hyper 0.12.6 (registry+https://github.com/rust-lang/crates.io-index)" = "bd2dbf44d0eb8b32ac0cb7b0d75c31313554dd04d6f5dd1085e150ec5383d9b8" +"checksum hyper 0.12.7 (registry+https://github.com/rust-lang/crates.io-index)" = "c087746de95e20e4dabe86606c3a019964a8fde2d5f386152939063c116c5971" "checksum idna 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "014b298351066f1512874135335d62a789ffe78a9974f94b43ed5621951eaf7d" "checksum indexmap 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "7b9378f1f3923647a9aea6af4c6b5de68cc8a71415459ad25ef191191c48f5b7" "checksum inotify 0.5.2-dev (git+https://github.com/inotify-rs/inotify)" = "" diff --git a/tests/support/server.rs b/tests/support/server.rs index b4671ef17c..c10cfd50b2 100644 --- a/tests/support/server.rs +++ b/tests/support/server.rs @@ -67,16 +67,6 @@ impl Server { self } - /// Return a 200 OK response with no body when the path matches. - pub fn route_empty_ok(self, path: &str) -> Self { - self.route_fn(path, |_| { - Response::builder() - .header("content-length", "0") - .body(Default::default()) - .unwrap() - }) - } - /// Call a closure when the request matches, returning a response /// to send back. pub fn route_fn(mut self, path: &str, cb: F) -> Self diff --git a/tests/transparency.rs b/tests/transparency.rs index 698337ac45..68155ec392 100644 --- a/tests/transparency.rs +++ b/tests/transparency.rs @@ -816,7 +816,7 @@ fn http1_one_connection_per_host() { let _ = env_logger::try_init(); let srv = server::http1() - .route_empty_ok("/") + .route("/", "hello hosts") .run(); let proxy = proxy::new().inbound(srv).run(); @@ -875,7 +875,7 @@ fn http1_requests_without_host_have_unique_connections() { let _ = env_logger::try_init(); let srv = server::http1() - .route_empty_ok("/") + .route("/", "unique hosts") .run(); let proxy = proxy::new().inbound(srv).run(); From 4b7af4620d17811b7b7917c1cb643dacc62aeb30 Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Mon, 23 Jul 2018 13:12:39 -0700 Subject: [PATCH 2/2] update keep alive tests to use with and without body requests Signed-off-by: Sean McArthur --- tests/transparency.rs | 54 ++++++++++++++++--------------------------- 1 file changed, 20 insertions(+), 34 deletions(-) diff --git a/tests/transparency.rs b/tests/transparency.rs index 68155ec392..a1fb3777a3 100644 --- a/tests/transparency.rs +++ b/tests/transparency.rs @@ -816,7 +816,8 @@ fn http1_one_connection_per_host() { let _ = env_logger::try_init(); let srv = server::http1() - .route("/", "hello hosts") + .route("/body", "hello hosts") + .route("/no-body", "") .run(); let proxy = proxy::new().inbound(srv).run(); @@ -825,49 +826,34 @@ fn http1_one_connection_per_host() { let inbound = &proxy.inbound_server.as_ref() .expect("no inbound server!"); + // Run each case with and without a body. + let run_request = move |host, expected_conn_cnt| { + for path in &["/no-body", "/body"][..] { + let res = client.request(client.request_builder(path) + .version(http::Version::HTTP_11) + .header("host", host) + ); + assert_eq!(res.status(), http::StatusCode::OK); + assert_eq!(res.version(), http::Version::HTTP_11); + assert_eq!(inbound.connections(), expected_conn_cnt); + } + }; + // Make a request with the header "Host: foo.bar". After the request, the // server should have seen one connection. - let res1 = client.request(client.request_builder("/") - .version(http::Version::HTTP_11) - .header("host", "foo.bar") - ); - assert_eq!(res1.status(), http::StatusCode::OK); - assert_eq!(res1.version(), http::Version::HTTP_11); - assert_eq!(inbound.connections(), 1); + run_request("foo.bar", 1); // Another request with the same host. The proxy may reuse the connection. - let res1 = client.request(client.request_builder("/") - .version(http::Version::HTTP_11) - .header("host", "foo.bar") - ); - assert_eq!(res1.status(), http::StatusCode::OK); - assert_eq!(res1.version(), http::Version::HTTP_11); - assert_eq!(inbound.connections(), 1); + run_request("foo.bar", 1); // Make a request with a different Host header. This request must use a new // connection. - let res2 = client.request(client.request_builder("/") - .version(http::Version::HTTP_11) - .header("host", "bar.baz")); - assert_eq!(res2.status(), http::StatusCode::OK); - assert_eq!(res2.version(), http::Version::HTTP_11); - assert_eq!(inbound.connections(), 2); - - let res2 = client.request(client.request_builder("/") - .version(http::Version::HTTP_11) - .header("host", "bar.baz")); - assert_eq!(res2.status(), http::StatusCode::OK); - assert_eq!(res2.version(), http::Version::HTTP_11); - assert_eq!(inbound.connections(), 2); + run_request("bar.baz", 2); + run_request("bar.baz", 2); // Make a request with a different Host header. This request must use a new // connection. - let res3 = client.request(client.request_builder("/") - .version(http::Version::HTTP_11) - .header("host", "quuuux.com")); - assert_eq!(res3.status(), http::StatusCode::OK); - assert_eq!(res3.version(), http::Version::HTTP_11); - assert_eq!(inbound.connections(), 3); + run_request("quuuux.com", 3); } #[test]