Skip to content

Commit

Permalink
refactor(error): add header parse error details in hyper::Error
Browse files Browse the repository at this point in the history
When a header parse error is because of content-length or
transfer-encoding semantics, include a better error message in the
`hyper::Error`.
  • Loading branch information
seanmonstar committed Jun 11, 2021
1 parent ea8b0cd commit 08b2138
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 11 deletions.
44 changes: 41 additions & 3 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,24 @@ pub(super) enum Parse {
#[cfg(feature = "http1")]
VersionH2,
Uri,
Header,
Header(Header),
TooLarge,
Status,
#[cfg_attr(debug_assertions, allow(unused))]
Internal,
}

#[derive(Debug)]
pub(super) enum Header {
Token,
#[cfg(feature = "http1")]
ContentLengthInvalid,
#[cfg(feature = "http1")]
TransferEncodingInvalid,
#[cfg(feature = "http1")]
TransferEncodingUnexpected,
}

#[derive(Debug)]
pub(super) enum User {
/// Error calling user's HttpBody::poll_data().
Expand Down Expand Up @@ -375,7 +386,19 @@ 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::Header) => "invalid HTTP header parsed",
Kind::Parse(Parse::Header(Header::Token)) => "invalid HTTP header parsed",
#[cfg(feature = "http1")]
Kind::Parse(Parse::Header(Header::ContentLengthInvalid)) => {
"invalid content-length parsed"
}
#[cfg(feature = "http1")]
Kind::Parse(Parse::Header(Header::TransferEncodingInvalid)) => {
"invalid transfer-encoding parsed"
}
#[cfg(feature = "http1")]
Kind::Parse(Parse::Header(Header::TransferEncodingUnexpected)) => {
"unexpected transfer-encoding parsed"
}
Kind::Parse(Parse::TooLarge) => "message head is too large",
Kind::Parse(Parse::Status) => "invalid HTTP status-code parsed",
Kind::Parse(Parse::Internal) => {
Expand Down Expand Up @@ -475,13 +498,28 @@ impl From<Parse> for Error {
}
}

#[cfg(feature = "http1")]
impl Parse {
pub(crate) fn content_length_invalid() -> Self {
Parse::Header(Header::ContentLengthInvalid)
}

pub(crate) fn transfer_encoding_invalid() -> Self {
Parse::Header(Header::TransferEncodingInvalid)
}

pub(crate) fn transfer_encoding_unexpected() -> Self {
Parse::Header(Header::TransferEncodingUnexpected)
}
}

impl From<httparse::Error> for Parse {
fn from(err: httparse::Error) -> Parse {
match err {
httparse::Error::HeaderName
| httparse::Error::HeaderValue
| httparse::Error::NewLine
| httparse::Error::Token => Parse::Header,
| httparse::Error::Token => Parse::Header(Header::Token),
httparse::Error::Status => Parse::Status,
httparse::Error::TooManyHeaders => Parse::TooLarge,
httparse::Error::Version => Parse::Version,
Expand Down
16 changes: 8 additions & 8 deletions src/proto/h1/role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ impl Http1Transaction for Server {
// malformed. A server should respond with 400 Bad Request.
if !is_http_11 {
debug!("HTTP/1.0 cannot have Transfer-Encoding header");
return Err(Parse::Header);
return Err(Parse::transfer_encoding_unexpected());
}
is_te = true;
if headers::is_chunked_(&value) {
Expand All @@ -221,15 +221,15 @@ impl Http1Transaction for Server {
}
let len = value
.to_str()
.map_err(|_| Parse::Header)
.and_then(|s| s.parse().map_err(|_| Parse::Header))?;
.map_err(|_| Parse::content_length_invalid())
.and_then(|s| s.parse().map_err(|_| Parse::content_length_invalid()))?;
if let Some(prev) = con_len {
if prev != len {
debug!(
"multiple Content-Length headers with different values: [{}, {}]",
prev, len,
);
return Err(Parse::Header);
return Err(Parse::content_length_invalid());
}
// we don't need to append this secondary length
continue;
Expand Down Expand Up @@ -267,7 +267,7 @@ impl Http1Transaction for Server {

if is_te && !is_te_chunked {
debug!("request with transfer-encoding header, but not chunked, bad request");
return Err(Parse::Header);
return Err(Parse::transfer_encoding_invalid());
}

let mut extensions = http::Extensions::default();
Expand Down Expand Up @@ -386,7 +386,7 @@ impl Http1Transaction for Server {
use crate::error::Kind;
let status = match *err.kind() {
Kind::Parse(Parse::Method)
| Kind::Parse(Parse::Header)
| Kind::Parse(Parse::Header(_))
| Kind::Parse(Parse::Uri)
| Kind::Parse(Parse::Version) => StatusCode::BAD_REQUEST,
Kind::Parse(Parse::TooLarge) => StatusCode::REQUEST_HEADER_FIELDS_TOO_LARGE,
Expand Down Expand Up @@ -1106,7 +1106,7 @@ impl Client {
// malformed. A server should respond with 400 Bad Request.
if inc.version == Version::HTTP_10 {
debug!("HTTP/1.0 cannot have Transfer-Encoding header");
Err(Parse::Header)
Err(Parse::transfer_encoding_unexpected())
} else if headers::transfer_encoding_is_chunked(&inc.headers) {
Ok(Some((DecodedLength::CHUNKED, false)))
} else {
Expand All @@ -1117,7 +1117,7 @@ impl Client {
Ok(Some((DecodedLength::checked_new(len)?, false)))
} else if inc.headers.contains_key(header::CONTENT_LENGTH) {
debug!("illegal Content-Length header");
Err(Parse::Header)
Err(Parse::content_length_invalid())
} else {
trace!("neither Transfer-Encoding nor Content-Length");
Ok(Some((DecodedLength::CLOSE_DELIMITED, false)))
Expand Down

0 comments on commit 08b2138

Please sign in to comment.