diff --git a/src/server.rs b/src/server.rs index d2535d5de..7cbb6df6d 100644 --- a/src/server.rs +++ b/src/server.rs @@ -1236,20 +1236,32 @@ impl proto::Peer for Peer { // Convert the URI let mut parts = uri::Parts::default(); - if let Some(scheme) = pseudo.scheme { - let maybe_scheme = uri::Scheme::from_shared(scheme.clone().into_inner()); - parts.scheme = Some(maybe_scheme.or_else(|why| malformed!( - "malformed headers: malformed scheme ({:?}): {}", scheme, why, - ))?); - } else { - malformed!("malformed headers: missing scheme"); - } + // A request translated from HTTP/1 must not include the :authority + // header if let Some(authority) = pseudo.authority { let maybe_authority = uri::Authority::from_shared(authority.clone().into_inner()); parts.authority = Some(maybe_authority.or_else(|why| malformed!( "malformed headers: malformed authority ({:?}): {}", authority, why, ))?); + + } + + // A :scheme is always required. + if let Some(scheme) = pseudo.scheme { + let maybe_scheme = uri::Scheme::from_shared(scheme.clone().into_inner()); + let scheme = maybe_scheme.or_else(|why| malformed!( + "malformed headers: malformed scheme ({:?}): {}", scheme, why, + ))?; + + // It's not possible to build an `Uri` from a scheme and path. So, + // after validating is was a valid scheme, we just have to drop it + // if there isn't an :authority. + if parts.authority.is_some() { + parts.scheme = Some(scheme); + } + } else { + malformed!("malformed headers: missing scheme"); } if let Some(path) = pseudo.path { diff --git a/tests/h2-tests/tests/client_request.rs b/tests/h2-tests/tests/client_request.rs index f51555134..f189c9a7b 100644 --- a/tests/h2-tests/tests/client_request.rs +++ b/tests/h2-tests/tests/client_request.rs @@ -444,15 +444,13 @@ fn http_11_request_without_scheme_or_authority() { let h2 = client::handshake(io) .expect("handshake") .and_then(|(mut client, h2)| { - // we send a simple req here just to drive the connection so we can - // receive the server settings. + // HTTP_11 request with just :path is allowed let request = Request::builder() .method(Method::GET) .uri("/") .body(()) .unwrap(); - // first request is allowed let (response, _) = client.send_request(request, true).unwrap(); h2.drive(response) .map(move |(h2, _)| (client, h2)) @@ -474,8 +472,8 @@ fn http_2_request_without_scheme_or_authority() { let h2 = client::handshake(io) .expect("handshake") .and_then(|(mut client, h2)| { - // we send a simple req here just to drive the connection so we can - // receive the server settings. + // HTTP_2 with only a :path is illegal, so this request should + // be rejected as a user error. let request = Request::builder() .version(Version::HTTP_2) .method(Method::GET) @@ -483,8 +481,10 @@ fn http_2_request_without_scheme_or_authority() { .body(()) .unwrap(); - // first request is allowed - assert!(client.send_request(request, true).is_err()); + client + .send_request(request, true) + .expect_err("should be UserError"); + h2.expect("h2").map(|ret| { // Hold on to the `client` handle to avoid sending a GO_AWAY frame. drop(client); diff --git a/tests/h2-tests/tests/server.rs b/tests/h2-tests/tests/server.rs index 15359caba..5d81404db 100644 --- a/tests/h2-tests/tests/server.rs +++ b/tests/h2-tests/tests/server.rs @@ -68,7 +68,7 @@ fn server_builder_set_max_concurrent_streams() { .unwrap(); stream.send_response(rsp, true).unwrap(); - srv.into_future().unwrap() + srv.into_future().unwrap().map(|_| ()) }) }); @@ -101,7 +101,7 @@ fn serve_request() { let rsp = http::Response::builder().status(200).body(()).unwrap(); stream.send_response(rsp, true).unwrap(); - srv.into_future().unwrap() + srv.into_future().unwrap().map(|_| ()) }) }); @@ -134,7 +134,7 @@ fn recv_invalid_authority() { let srv = server::handshake(io) .expect("handshake") - .and_then(|srv| srv.into_future().unwrap()); + .and_then(|srv| srv.into_future().unwrap().map(|_| ())); srv.join(client).wait().expect("wait"); } @@ -169,7 +169,7 @@ fn recv_connection_header() { let srv = server::handshake(io) .expect("handshake") - .and_then(|srv| srv.into_future().unwrap()); + .and_then(|srv| srv.into_future().unwrap()).map(|_| ()); srv.join(client).wait().expect("wait"); } @@ -200,7 +200,7 @@ fn sends_reset_cancel_when_req_body_is_dropped() { let rsp = http::Response::builder().status(200).body(()).unwrap(); stream.send_response(rsp, true).unwrap(); - srv.into_future().unwrap() + srv.into_future().unwrap().map(|_| ()) }) }); @@ -390,7 +390,7 @@ fn sends_reset_cancel_when_res_body_is_dropped() { tx.send_data(vec![0; 10].into(), false).unwrap(); // no send_data with eos - srv.into_future().unwrap() + srv.into_future().unwrap().map(|_| ()) }) }); @@ -625,3 +625,37 @@ fn server_error_on_unclean_shutdown() { srv.wait().expect_err("should error"); } + +#[test] +fn request_without_authority() { + let _ = ::env_logger::try_init(); + let (io, client) = mock::new(); + + let client = client + .assert_server_handshake() + .unwrap() + .recv_settings() + .send_frame( + frames::headers(1) + .request("GET", "/just-a-path") + .scheme("http") + .eos() + ) + .recv_frame(frames::headers(1).response(200).eos()) + .close(); + + let srv = server::handshake(io).expect("handshake").and_then(|srv| { + srv.into_future().unwrap().and_then(|(reqstream, srv)| { + let (req, mut stream) = reqstream.unwrap(); + + assert_eq!(req.uri().path(), "/just-a-path"); + + let rsp = Response::new(()); + stream.send_response(rsp, true).unwrap(); + + srv.into_future().unwrap().map(|_| ()) + }) + }); + + srv.join(client).wait().expect("wait"); +}