fix(http1): return 414 when URI contains more than 65534 characters (#2706)

Previous behavior returned a 404 Bad Request. Conforms to HTTP 1.1 RFC.

Closes #2701
This commit is contained in:
Rajin Gill
2021-11-29 12:31:41 -08:00
committed by GitHub
parent 1010614a0d
commit 5f938fffa6
3 changed files with 30 additions and 2 deletions

View File

@@ -71,6 +71,7 @@ pub(super) enum Parse {
#[cfg(feature = "http1")] #[cfg(feature = "http1")]
VersionH2, VersionH2,
Uri, Uri,
UriTooLong,
Header(Header), Header(Header),
TooLarge, TooLarge,
Status, Status,
@@ -152,7 +153,10 @@ impl Error {
/// Returns true if this was an HTTP parse error caused by a message that was too large. /// 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 { 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 /// Returns true if this was an HTTP parse error caused by an invalid response status code or
@@ -398,6 +402,7 @@ impl Error {
#[cfg(feature = "http1")] #[cfg(feature = "http1")]
Kind::Parse(Parse::VersionH2) => "invalid HTTP version parsed (found HTTP2 preface)", Kind::Parse(Parse::VersionH2) => "invalid HTTP version parsed (found HTTP2 preface)",
Kind::Parse(Parse::Uri) => "invalid URI", Kind::Parse(Parse::Uri) => "invalid URI",
Kind::Parse(Parse::UriTooLong) => "URI too long",
Kind::Parse(Parse::Header(Header::Token)) => "invalid HTTP header parsed", Kind::Parse(Parse::Header(Header::Token)) => "invalid HTTP header parsed",
#[cfg(feature = "http1")] #[cfg(feature = "http1")]
Kind::Parse(Parse::Header(Header::ContentLengthInvalid)) => { Kind::Parse(Parse::Header(Header::ContentLengthInvalid)) => {

View File

@@ -25,6 +25,7 @@ use crate::proto::{BodyLength, MessageHead, RequestHead, RequestLine};
const MAX_HEADERS: usize = 100; const MAX_HEADERS: usize = 100;
const AVERAGE_HEADER_SIZE: usize = 30; // totally scientific const AVERAGE_HEADER_SIZE: usize = 30; // totally scientific
const MAX_URI_LEN: usize = (u16::MAX - 1) as usize;
macro_rules! header_name { macro_rules! header_name {
($bytes:expr) => {{ ($bytes:expr) => {{
@@ -148,9 +149,13 @@ impl Http1Transaction for Server {
Ok(httparse::Status::Complete(parsed_len)) => { Ok(httparse::Status::Complete(parsed_len)) => {
trace!("Request.parse Complete({})", parsed_len); trace!("Request.parse Complete({})", parsed_len);
len = parsed_len; len = parsed_len;
let uri = req.path.unwrap();
if uri.len() > MAX_URI_LEN {
return Err(Parse::UriTooLong);
}
subject = RequestLine( subject = RequestLine(
Method::from_bytes(req.method.unwrap().as_bytes())?, Method::from_bytes(req.method.unwrap().as_bytes())?,
req.path.unwrap().parse()?, uri.parse()?,
); );
version = if req.version.unwrap() == 1 { version = if req.version.unwrap() == 1 {
keep_alive = true; keep_alive = true;
@@ -408,6 +413,7 @@ impl Http1Transaction for Server {
| Kind::Parse(Parse::Uri) | Kind::Parse(Parse::Uri)
| Kind::Parse(Parse::Version) => StatusCode::BAD_REQUEST, | Kind::Parse(Parse::Version) => StatusCode::BAD_REQUEST,
Kind::Parse(Parse::TooLarge) => StatusCode::REQUEST_HEADER_FIELDS_TOO_LARGE, Kind::Parse(Parse::TooLarge) => StatusCode::REQUEST_HEADER_FIELDS_TOO_LARGE,
Kind::Parse(Parse::UriTooLong) => StatusCode::URI_TOO_LONG,
_ => return None, _ => return None,
}; };

View File

@@ -1025,6 +1025,23 @@ fn http_10_request_receives_http_10_response() {
assert_eq!(s(&buf[..expected.len()]), expected); 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] #[tokio::test]
async fn disable_keep_alive_mid_request() { async fn disable_keep_alive_mid_request() {
let listener = tcp_bind(&"127.0.0.1:0".parse().unwrap()).unwrap(); let listener = tcp_bind(&"127.0.0.1:0".parse().unwrap()).unwrap();