Skip to content

Commit

Permalink
fix(client): coerce HTTP_2 requests to HTTP_11
Browse files Browse the repository at this point in the history
Closes #1770
  • Loading branch information
seanmonstar committed Feb 28, 2019
1 parent 2b0a5ea commit 3a6080b
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 14 deletions.
16 changes: 11 additions & 5 deletions src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 6 additions & 2 deletions src/proto/h1/role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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");

Expand Down
89 changes: 82 additions & 7 deletions tests/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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));
});
Expand Down Expand Up @@ -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};
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 3a6080b

Please sign in to comment.