From 3a6080b14abecc29c9aed77be6d60d34a12b368c Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Wed, 27 Feb 2019 16:55:39 -0800 Subject: [PATCH] fix(client): coerce HTTP_2 requests to HTTP_11 Closes #1770 --- src/client/mod.rs | 16 +++++--- src/proto/h1/role.rs | 8 +++- tests/client.rs | 89 ++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 99 insertions(+), 14 deletions(-) diff --git a/src/client/mod.rs b/src/client/mod.rs index 0ee65bb224..dfb2818a19 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -236,13 +236,14 @@ where C: Connect + Sync + 'static, match req.version() { Version::HTTP_11 => (), Version::HTTP_10 => if is_http_connect { - debug!("CONNECT is not allowed for HTTP/1.0"); + warn!("CONNECT is not allowed for HTTP/1.0"); return ResponseFuture::new(Box::new(future::err(::Error::new_user_unsupported_request_method()))); }, - other => if self.config.ver != Ver::Http2 { - error!("Request has unsupported version \"{:?}\"", other); - return ResponseFuture::new(Box::new(future::err(::Error::new_user_unsupported_version()))); - } + other_h2 @ Version::HTTP_2 => if self.config.ver != Ver::Http2 { + return ResponseFuture::error_version(other_h2); + }, + // completely unsupported HTTP version (like HTTP/0.9)! + other => return ResponseFuture::error_version(other), }; let domain = match extract_domain(req.uri_mut(), is_http_connect) { @@ -591,6 +592,11 @@ impl ResponseFuture { inner: fut, } } + + fn error_version(ver: Version) -> Self { + warn!("Request has unsupported version \"{:?}\"", ver); + ResponseFuture::new(Box::new(future::err(::Error::new_user_unsupported_version()))) + } } impl fmt::Debug for ResponseFuture { diff --git a/src/proto/h1/role.rs b/src/proto/h1/role.rs index de1299dcfd..26322507c6 100644 --- a/src/proto/h1/role.rs +++ b/src/proto/h1/role.rs @@ -271,7 +271,7 @@ impl Http1Transaction for Server { warn!("response with HTTP2 version coerced to HTTP/1.1"); extend(dst, b"HTTP/1.1 "); }, - _ => unreachable!(), + other => panic!("unexpected response version: {:?}", other), } extend(dst, msg.head.subject.as_str().as_bytes()); @@ -667,7 +667,11 @@ impl Http1Transaction for Client { match msg.head.version { Version::HTTP_10 => extend(dst, b"HTTP/1.0"), Version::HTTP_11 => extend(dst, b"HTTP/1.1"), - _ => unreachable!(), + Version::HTTP_2 => { + warn!("request with HTTP2 version coerced to HTTP/1.1"); + extend(dst, b"HTTP/1.1"); + }, + other => panic!("unexpected request version: {:?}", other), } extend(dst, b"\r\n"); diff --git a/tests/client.rs b/tests/client.rs index 543c563a23..0d6d7ac96e 100644 --- a/tests/client.rs +++ b/tests/client.rs @@ -39,13 +39,6 @@ macro_rules! test { request: {$( $c_req_prop:ident: $c_req_val: tt, )*}, - /* - method: $client_method:ident, - url: $client_url:expr, - headers: { $($request_header_name:expr => $request_header_val:expr,)* }, - body: $request_body:expr, - }, - */ response: status: $client_status:ident, @@ -299,6 +292,10 @@ macro_rules! __client_req_prop { $req_builder.method(Method::$method); }); + ($req_builder:ident, $body:ident, $addr:ident, version: $version:ident) => ({ + $req_builder.version(hyper::Version::$version); + }); + ($req_builder:ident, $body:ident, $addr:ident, url: $url:expr) => ({ $req_builder.uri(format!($url, addr=$addr)); }); @@ -724,6 +721,38 @@ test! { body: None, } +test! { + name: client_h1_rejects_http2, + + server: + expected: "won't get here {addr}", + reply: "won't reply", + + client: + request: { + method: GET, + url: "http://{addr}/", + version: HTTP_2, + }, + error: |err| err.to_string() == "request has unsupported HTTP version", +} + +test! { + name: client_always_rejects_http09, + + server: + expected: "won't get here {addr}", + reply: "won't reply", + + client: + request: { + method: GET, + url: "http://{addr}/", + version: HTTP_09, + }, + error: |err| err.to_string() == "request has unsupported HTTP version", +} + mod dispatch_impl { use super::*; use std::io::{self, Read, Write}; @@ -1804,6 +1833,52 @@ mod conn { rt.block_on(res.join(rx).map(|r| r.0)).unwrap(); } + #[test] + fn http1_conn_coerces_http2_request() { + let server = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = server.local_addr().unwrap(); + let mut rt = Runtime::new().unwrap(); + + let (tx1, rx1) = oneshot::channel(); + + thread::spawn(move || { + let mut sock = server.accept().unwrap().0; + sock.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); + sock.set_write_timeout(Some(Duration::from_secs(5))).unwrap(); + let mut buf = [0; 4096]; + let n = sock.read(&mut buf).expect("read 1"); + + // Not HTTP/2, nor panicked + let expected = "GET /a HTTP/1.1\r\n\r\n"; + assert_eq!(s(&buf[..n]), expected); + + sock.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n").unwrap(); + let _ = tx1.send(()); + }); + + let tcp = rt.block_on(tcp_connect(&addr)).unwrap(); + + let (mut client, conn) = rt.block_on(conn::handshake(tcp)).unwrap(); + + rt.spawn(conn.map(|_| ()).map_err(|e| panic!("conn error: {}", e))); + + let req = Request::builder() + .uri("/a") + .version(hyper::Version::HTTP_2) + .body(Default::default()) + .unwrap(); + + let res = client.send_request(req).and_then(move |res| { + assert_eq!(res.status(), hyper::StatusCode::OK); + res.into_body().concat2() + }); + let rx = rx1.expect("thread panicked"); + + let timeout = Delay::new(Duration::from_millis(200)); + let rx = rx.and_then(move |_| timeout.expect("timeout")); + rt.block_on(res.join(rx).map(|r| r.0)).unwrap(); + } + #[test] fn pipeline() { let server = TcpListener::bind("127.0.0.1:0").unwrap();