Skip to content

Commit

Permalink
Return an error rather than panicking when HeaderName is too long (hy…
Browse files Browse the repository at this point in the history
…perium#433)

Fixes hyperium#432.

This eliminates an undocumented panic from the `HeaderName` creation functions, returning instead an
`InvalidHeaderName` when the name length exceeds `MAX_HEADER_NAME_LEN`.

I considered making `InvalidHeaderName` a richer error type, but since it was already used for
another type of length error (rejecting zero-length names) I figured it was okay to reuse.

I also redefined `MAX_HEADER_NAME_LEN` slightly, so that it is equal to the largest allowed header
length, rather than one past that value. This was motivated by discovering a bug in my comparison
logic when I went to write the new test exercising the wrong-length error conditions.
  • Loading branch information
acfoltzer authored and Benxiang Ge committed Jul 26, 2021
1 parent f3d85f1 commit 9f671af
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/header/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,4 +169,4 @@ pub use self::name::{
/// Generally, 64kb for a header name is WAY too much than would ever be needed
/// in practice. Restricting it to this size enables using `u16` values to
/// represent offsets when dealing with header names.
const MAX_HEADER_NAME_LEN: usize = 1 << 16;
const MAX_HEADER_NAME_LEN: usize = (1 << 16) - 1;
39 changes: 26 additions & 13 deletions src/header/name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1116,10 +1116,6 @@ fn parse_hdr<'a>(
($d:ident, $src:ident, 35) => { to_lower!($d, $src, 34); $d[34] = table[$src[34] as usize]; };
}

assert!(len < super::MAX_HEADER_NAME_LEN,
"header name too long -- max length is {}",
super::MAX_HEADER_NAME_LEN);

match len {
0 => Err(InvalidHeaderName::new()),
2 => {
Expand Down Expand Up @@ -1509,17 +1505,16 @@ fn parse_hdr<'a>(
validate(b, len)
}
}
_ => {
if len < 64 {
for i in 0..len {
b[i] = table[data[i] as usize];
}

validate(b, len)
} else {
Ok(HdrName::custom(data, false))
len if len < 64 => {
for i in 0..len {
b[i] = table[data[i] as usize];
}
validate(b, len)
}
len if len <= super::MAX_HEADER_NAME_LEN => {
Ok(HdrName::custom(data, false))
}
_ => Err(InvalidHeaderName::new()),
}
}

Expand Down Expand Up @@ -2156,6 +2151,24 @@ mod tests {
}
}

#[test]
fn test_invalid_name_lengths() {
assert!(
HeaderName::from_bytes(&[]).is_err(),
"zero-length header name is an error",
);
let mut long = vec![b'a'; super::super::MAX_HEADER_NAME_LEN];
assert!(
HeaderName::from_bytes(long.as_slice()).is_ok(),
"max header name length is ok",
);
long.push(b'a');
assert!(
HeaderName::from_bytes(long.as_slice()).is_err(),
"longer than max header name length is an error",
);
}

#[test]
fn test_from_hdr_name() {
use self::StandardHeader::Vary;
Expand Down

0 comments on commit 9f671af

Please sign in to comment.