From 55d9a584b19a553b105842f6bf954edf46a007ee Mon Sep 17 00:00:00 2001 From: bensadiku <43443348+bensadiku@users.noreply.github.com> Date: Sat, 5 Jun 2021 00:17:37 +0200 Subject: [PATCH] refactor(http1): return Parse::Internal error if there's an illegal header name or value (#2544) --- src/error.rs | 5 +++++ src/proto/h1/role.rs | 42 +++++++++++++++--------------------------- 2 files changed, 20 insertions(+), 27 deletions(-) diff --git a/src/error.rs b/src/error.rs index dd577b99a6..c0d9622d50 100644 --- a/src/error.rs +++ b/src/error.rs @@ -73,6 +73,8 @@ pub(super) enum Parse { Header, TooLarge, Status, + #[cfg_attr(debug_assertions, allow(unused))] + Internal, } #[derive(Debug, PartialEq)] @@ -374,6 +376,9 @@ impl Error { Kind::Parse(Parse::Header) => "invalid HTTP header parsed", Kind::Parse(Parse::TooLarge) => "message head is too large", Kind::Parse(Parse::Status) => "invalid HTTP status-code parsed", + Kind::Parse(Parse::Internal) => { + "internal error inside Hyper and/or its dependencies, please report" + } Kind::IncompleteMessage => "connection closed before message completed", #[cfg(feature = "http1")] Kind::UnexpectedMessage => "received unexpected message from connection", diff --git a/src/proto/h1/role.rs b/src/proto/h1/role.rs index 84dc091147..0d59edf2d6 100644 --- a/src/proto/h1/role.rs +++ b/src/proto/h1/role.rs @@ -8,9 +8,9 @@ use std::mem; #[cfg(any(test, feature = "server", feature = "ffi"))] use bytes::Bytes; use bytes::BytesMut; -use http::header::{self, Entry, HeaderName, HeaderValue}; #[cfg(feature = "server")] use http::header::ValueIter; +use http::header::{self, Entry, HeaderName, HeaderValue}; use http::{HeaderMap, Method, StatusCode, Version}; use crate::body::DecodedLength; @@ -29,22 +29,10 @@ const AVERAGE_HEADER_SIZE: usize = 30; // totally scientific macro_rules! header_name { ($bytes:expr) => {{ - #[cfg(debug_assertions)] { match HeaderName::from_bytes($bytes) { Ok(name) => name, - Err(_) => panic!( - "illegal header name from httparse: {:?}", - ::bytes::Bytes::copy_from_slice($bytes) - ), - } - } - - #[cfg(not(debug_assertions))] - { - match HeaderName::from_bytes($bytes) { - Ok(name) => name, - Err(_) => panic!("illegal header name from httparse: {:?}", $bytes), + Err(e) => maybe_panic!(e), } } }}; @@ -52,23 +40,24 @@ macro_rules! header_name { macro_rules! header_value { ($bytes:expr) => {{ - #[cfg(debug_assertions)] { - let __hvb: ::bytes::Bytes = $bytes; - match HeaderValue::from_maybe_shared(__hvb.clone()) { - Ok(name) => name, - Err(_) => panic!("illegal header value from httparse: {:?}", __hvb), - } - } - - #[cfg(not(debug_assertions))] - { - // Unsafe: httparse already validated header value unsafe { HeaderValue::from_maybe_shared_unchecked($bytes) } } }}; } +macro_rules! maybe_panic { + ($($arg:tt)*) => ({ + let _err = ($($arg)*); + if cfg!(debug_assertions) { + panic!("{:?}", _err); + } else { + error!("Internal Hyper error, please report {:?}", _err); + return Err(Parse::Internal) + } + }) +} + pub(super) fn parse_headers( bytes: &mut BytesMut, ctx: ParseContext<'_>, @@ -891,8 +880,7 @@ impl Http1Transaction for Client { ); let mut res = httparse::Response::new(&mut headers); let bytes = buf.as_ref(); - match ctx.h1_parser_config.parse_response(&mut res, bytes) - { + match ctx.h1_parser_config.parse_response(&mut res, bytes) { Ok(httparse::Status::Complete(len)) => { trace!("Response.parse Complete({})", len); let status = StatusCode::from_u16(res.code.unwrap())?;