Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 20 additions & 8 deletions src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a host header is present do we have any obligation to validate it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find anything in the HTTP2 spec stating to do anything with a received host header besides handling it like any other.

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 {
Expand Down
14 changes: 7 additions & 7 deletions tests/h2-tests/tests/client_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -474,17 +472,19 @@ 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)
.uri("/")
.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);
Expand Down
46 changes: 40 additions & 6 deletions tests/h2-tests/tests/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(|_| ())
})
});

Expand Down Expand Up @@ -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(|_| ())
})
});

Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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(|_| ())
})
});

Expand Down Expand Up @@ -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(|_| ())
})
});

Expand Down Expand Up @@ -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");
}