From 02841ebd77c0bfea1939a097847f7bcaab552690 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Wed, 7 Mar 2018 20:48:54 -0800 Subject: [PATCH] Normalize HTTP request path. (#228) 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. --- src/frame/headers.rs | 8 +++-- tests/client_request.rs | 66 ++++++++++++++++++++++++++++++++++++++++ tests/support/prelude.rs | 2 +- 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/src/frame/headers.rs b/src/frame/headers.rs index 1ffa743..3b6a9bd 100644 --- a/src/frame/headers.rs +++ b/src/frame/headers.rs @@ -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), diff --git a/tests/client_request.rs b/tests/client_request.rs index 86297d8..a2c09d9 100644 --- a/tests/client_request.rs +++ b/tests/client_request.rs @@ -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]; diff --git a/tests/support/prelude.rs b/tests/support/prelude.rs index 272fd98..5204009 100644 --- a/tests/support/prelude.rs +++ b/tests/support/prelude.rs @@ -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};