Skip to content

Commit

Permalink
fix(http1): return 414 when URI contains more than 65534 characters
Browse files Browse the repository at this point in the history
Previous behavior returned a 404 Bad Request. Conforms to HTTP 1.1 RFC.

Closes hyperium#2701
  • Loading branch information
rajing committed Nov 25, 2021
1 parent 1010614 commit f1ea9e4
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 2 deletions.
4 changes: 3 additions & 1 deletion src/error.rs
Expand Up @@ -71,6 +71,7 @@ pub(super) enum Parse {
#[cfg(feature = "http1")]
VersionH2,
Uri,
UriTooLong,
Header(Header),
TooLarge,
Status,
Expand Down Expand Up @@ -152,7 +153,7 @@ impl Error {

/// Returns true if this was an HTTP parse error caused by a message that was too large.
pub fn is_parse_too_large(&self) -> bool {
matches!(self.inner.kind, Kind::Parse(Parse::TooLarge))
matches!(self.inner.kind, Kind::Parse(Parse::TooLarge) | Kind::Parse(Parse::UriTooLong))
}

/// Returns true if this was an HTTP parse error caused by an invalid response status code or
Expand Down Expand Up @@ -398,6 +399,7 @@ impl Error {
#[cfg(feature = "http1")]
Kind::Parse(Parse::VersionH2) => "invalid HTTP version parsed (found HTTP2 preface)",
Kind::Parse(Parse::Uri) => "invalid URI",
Kind::Parse(Parse::UriTooLong) => "URI too long",
Kind::Parse(Parse::Header(Header::Token)) => "invalid HTTP header parsed",
#[cfg(feature = "http1")]
Kind::Parse(Parse::Header(Header::ContentLengthInvalid)) => {
Expand Down
8 changes: 7 additions & 1 deletion src/proto/h1/role.rs
Expand Up @@ -25,6 +25,7 @@ use crate::proto::{BodyLength, MessageHead, RequestHead, RequestLine};

const MAX_HEADERS: usize = 100;
const AVERAGE_HEADER_SIZE: usize = 30; // totally scientific
const MAX_URI_LEN: usize = (u16::MAX - 1) as usize;

macro_rules! header_name {
($bytes:expr) => {{
Expand Down Expand Up @@ -148,9 +149,13 @@ impl Http1Transaction for Server {
Ok(httparse::Status::Complete(parsed_len)) => {
trace!("Request.parse Complete({})", parsed_len);
len = parsed_len;
let uri= req.path.unwrap();
if uri.len() > MAX_URI_LEN {
return Err(Parse::UriTooLong);
}
subject = RequestLine(
Method::from_bytes(req.method.unwrap().as_bytes())?,
req.path.unwrap().parse()?,
uri.parse()?,
);
version = if req.version.unwrap() == 1 {
keep_alive = true;
Expand Down Expand Up @@ -407,6 +412,7 @@ impl Http1Transaction for Server {
| Kind::Parse(Parse::Header(_))
| Kind::Parse(Parse::Uri)
| Kind::Parse(Parse::Version) => StatusCode::BAD_REQUEST,
Kind::Parse(Parse::UriTooLong) => StatusCode::URI_TOO_LONG,
Kind::Parse(Parse::TooLarge) => StatusCode::REQUEST_HEADER_FIELDS_TOO_LARGE,
_ => return None,
};
Expand Down
17 changes: 17 additions & 0 deletions tests/server.rs
Expand Up @@ -1025,6 +1025,23 @@ fn http_10_request_receives_http_10_response() {
assert_eq!(s(&buf[..expected.len()]), expected);
}

#[test]
fn http_11_uri_too_long() {
let server = serve();

let long_path = "a".repeat(65534);
let request_line = format!("GET /{} HTTP/1.1\r\n\r\n", long_path);

let mut req = connect(server.addr());
req.write_all(request_line.as_bytes()).unwrap();

let expected = "HTTP/1.1 414 URI Too Long\r\ncontent-length: 0\r\n";
let mut buf = [0; 256];
let n = req.read(&mut buf).unwrap();
assert!(n >= expected.len(), "read: {:?} >= {:?}", n, expected.len());
assert_eq!(s(&buf[..expected.len()]), expected);
}

#[tokio::test]
async fn disable_keep_alive_mid_request() {
let listener = tcp_bind(&"127.0.0.1:0".parse().unwrap()).unwrap();
Expand Down

0 comments on commit f1ea9e4

Please sign in to comment.