Skip to content

Commit

Permalink
Normalize HTTP request path. (hyperium#228)
Browse files Browse the repository at this point in the history
The HTTP/2.0 specification requires that the path pseudo header is never
empty for requests unless the request uses the OPTIONS method.

This is currently not correctly enforced.

This patch provides a test and a fix.
  • Loading branch information
carllerche authored Mar 8, 2018
1 parent bbed419 commit 02841eb
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 3 deletions.
8 changes: 6 additions & 2 deletions src/frame/headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,10 +440,14 @@ impl Pseudo {
pub fn request(method: Method, uri: Uri) -> Self {
let parts = uri::Parts::from(uri);

let path = parts
let mut path = parts
.path_and_query
.map(|v| v.into())
.unwrap_or_else(|| Bytes::from_static(b"/"));
.unwrap_or_else(|| Bytes::new());

if path.is_empty() && method != Method::OPTIONS {
path = Bytes::from_static(b"/");
}

let mut pseudo = Pseudo {
method: Some(method),
Expand Down
66 changes: 66 additions & 0 deletions tests/client_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,72 @@ fn recv_too_big_headers() {

}

#[test]
fn request_without_path() {
let _ = ::env_logger::try_init();
let (io, srv) = mock::new();

let srv = srv.assert_client_handshake()
.unwrap()
.recv_settings()
.recv_frame(frames::headers(1).request("GET", "http://example.com/").eos())
.send_frame(frames::headers(1).response(200).eos())
.close();

let client = client::handshake(io)
.expect("handshake")
.and_then(move |(mut client, conn)| {
// Note the lack of trailing slash.
let request = Request::get("http://example.com")
.body(())
.unwrap();

let (response, _) = client.send_request(request, true).unwrap();

conn.drive(response)
});

client.join(srv).wait().unwrap();
}

#[test]
fn request_options_with_star() {
let _ = ::env_logger::try_init();
let (io, srv) = mock::new();

// Note the lack of trailing slash.
let uri = uri::Uri::from_parts({
let mut parts = uri::Parts::default();
parts.scheme = Some(uri::Scheme::HTTP);
parts.authority = Some(uri::Authority::from_shared("example.com".into()).unwrap());
parts.path_and_query = Some(uri::PathAndQuery::from_static("*"));
parts
}).unwrap();

let srv = srv.assert_client_handshake()
.unwrap()
.recv_settings()
.recv_frame(frames::headers(1).request("OPTIONS", uri.clone()).eos())
.send_frame(frames::headers(1).response(200).eos())
.close();

let client = client::handshake(io)
.expect("handshake")
.and_then(move |(mut client, conn)| {
let request = Request::builder()
.method(Method::OPTIONS)
.uri(uri)
.body(())
.unwrap();

let (response, _) = client.send_request(request, true).unwrap();

conn.drive(response)
});

client.join(srv).wait().unwrap();
}

const SETTINGS: &'static [u8] = &[0, 0, 0, 4, 0, 0, 0, 0, 0];
const SETTINGS_ACK: &'static [u8] = &[0, 0, 0, 4, 1, 0, 0, 0, 0];

Expand Down
2 changes: 1 addition & 1 deletion tests/support/prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub use self::futures::{Future, IntoFuture, Sink, Stream};
pub use super::future_ext::{FutureExt, Unwrap};

// Re-export HTTP types
pub use self::http::{HeaderMap, Method, Request, Response, StatusCode, Version};
pub use self::http::{uri, HeaderMap, Method, Request, Response, StatusCode, Version};

pub use self::bytes::{Buf, BufMut, Bytes, BytesMut, IntoBuf};

Expand Down

0 comments on commit 02841eb

Please sign in to comment.