feat(client): add option to allow misplaced spaces in HTTP/1 responses (

#2506)
hyperium · Apr 20, 2021 · 1134539 · 1134539
1 parent ed2fdb7
commit 1134539
Show file tree

Hide file tree

Showing 7 changed files with 117 additions and 5 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -30,7 +30,7 @@ futures-util = { version = "0.3", default-features = false }
 http = "0.2"
 http-body = "0.4"
 httpdate = "1.0"
-httparse = "1.0"
+httparse = "1.4"
 h2 = { version = "0.3", optional = true }
 itoa = "0.4.1"
 tracing = { version = "0.1", default-features = false, features = ["std"] }

diff --git a/src/client/client.rs b/src/client/client.rs
@@ -961,6 +961,31 @@ impl Builder {
         self
     }
 
+    /// Set whether HTTP/1 connections will accept spaces between header names
+    /// and the colon that follow them in responses.
+    ///
+    /// You probably don't need this, here is what [RFC 7230 Section 3.2.4.] has
+    /// to say about it:
+    ///
+    /// > No whitespace is allowed between the header field-name and colon. In
+    /// > the past, differences in the handling of such whitespace have led to
+    /// > security vulnerabilities in request routing and response handling. A
+    /// > server MUST reject any received request message that contains
+    /// > whitespace between a header field-name and colon with a response code
+    /// > of 400 (Bad Request). A proxy MUST remove any such whitespace from a
+    /// > response message before forwarding the message downstream.
+    ///
+    /// Note that this setting does not affect HTTP/2.
+    ///
+    /// Default is false.
+    ///
+    /// [RFC 7230 Section 3.2.4.]: https://tools.ietf.org/html/rfc7230#section-3.2.4
+    pub fn http1_allow_spaces_after_header_name_in_responses(&mut self, val: bool) -> &mut Self {
+        self.conn_builder
+            .h1_allow_spaces_after_header_name_in_responses(val);
+        self
+    }
+
     /// Set whether HTTP/1 connections will write header names as title case at
     /// the socket level.
     ///

diff --git a/src/client/conn.rs b/src/client/conn.rs
@@ -56,6 +56,7 @@ use std::time::Duration;
 
 use bytes::Bytes;
 use futures_util::future::{self, Either, FutureExt as _};
+use httparse::ParserConfig;
 use pin_project::pin_project;
 use tokio::io::{AsyncRead, AsyncWrite};
 use tower_service::Service;
@@ -123,6 +124,7 @@ where
 pub struct Builder {
     pub(super) exec: Exec,
     h09_responses: bool,
+    h1_parser_config: ParserConfig,
     h1_title_case_headers: bool,
     h1_read_buf_exact_size: Option<usize>,
     h1_max_buf_size: Option<usize>,
@@ -496,6 +498,7 @@ impl Builder {
             exec: Exec::Default,
             h09_responses: false,
             h1_read_buf_exact_size: None,
+            h1_parser_config: Default::default(),
             h1_title_case_headers: false,
             h1_max_buf_size: None,
             #[cfg(feature = "http2")]
@@ -521,6 +524,14 @@ impl Builder {
         self
     }
 
+    pub(crate) fn h1_allow_spaces_after_header_name_in_responses(
+        &mut self,
+        enabled: bool,
+    ) -> &mut Builder {
+        self.h1_parser_config.allow_spaces_after_header_name_in_responses(enabled);
+        self
+    }
+
     pub(super) fn h1_title_case_headers(&mut self, enabled: bool) -> &mut Builder {
         self.h1_title_case_headers = enabled;
         self
@@ -704,6 +715,7 @@ impl Builder {
                 #[cfg(feature = "http1")]
                 Proto::Http1 => {
                     let mut conn = proto::Conn::new(io);
+                    conn.set_h1_parser_config(opts.h1_parser_config);
                     if opts.h1_title_case_headers {
                         conn.set_title_case_headers();
                     }

diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs
@@ -5,6 +5,7 @@ use std::marker::PhantomData;
 use bytes::{Buf, Bytes};
 use http::header::{HeaderValue, CONNECTION};
 use http::{HeaderMap, Method, Version};
+use httparse::ParserConfig;
 use tokio::io::{AsyncRead, AsyncWrite};
 
 use super::io::Buffered;
@@ -44,6 +45,7 @@ where
                 error: None,
                 keep_alive: KA::Busy,
                 method: None,
+                h1_parser_config: ParserConfig::default(),
                 #[cfg(feature = "ffi")]
                 preserve_header_case: false,
                 title_case_headers: false,
@@ -79,6 +81,11 @@ where
         self.state.title_case_headers = true;
     }
 
+    #[cfg(feature = "client")]
+    pub(crate) fn set_h1_parser_config(&mut self, parser_config: ParserConfig) {
+        self.state.h1_parser_config = parser_config;
+    }
+
     #[cfg(feature = "client")]
     pub(crate) fn set_h09_responses(&mut self) {
         self.state.h09_responses = true;
@@ -150,6 +157,7 @@ where
             ParseContext {
                 cached_headers: &mut self.state.cached_headers,
                 req_method: &mut self.state.method,
+                h1_parser_config: self.state.h1_parser_config.clone(),
                 #[cfg(feature = "ffi")]
                 preserve_header_case: self.state.preserve_header_case,
                 h09_responses: self.state.h09_responses,
@@ -284,7 +292,10 @@ where
         ret
     }
 
-    pub(crate) fn poll_read_keep_alive(&mut self, cx: &mut task::Context<'_>) -> Poll<crate::Result<()>> {
+    pub(crate) fn poll_read_keep_alive(
+        &mut self,
+        cx: &mut task::Context<'_>,
+    ) -> Poll<crate::Result<()>> {
         debug_assert!(!self.can_read_head() && !self.can_read_body());
 
         if self.is_read_closed() {
@@ -760,6 +771,7 @@ struct State {
     /// This is used to know things such as if the message can include
     /// a body or not.
     method: Option<Method>,
+    h1_parser_config: ParserConfig,
     #[cfg(feature = "ffi")]
     preserve_header_case: bool,
     title_case_headers: bool,

diff --git a/src/proto/h1/io.rs b/src/proto/h1/io.rs
@@ -159,6 +159,7 @@ where
                 ParseContext {
                     cached_headers: parse_ctx.cached_headers,
                     req_method: parse_ctx.req_method,
+                    h1_parser_config: parse_ctx.h1_parser_config.clone(),
                     #[cfg(feature = "ffi")]
                     preserve_header_case: parse_ctx.preserve_header_case,
                     h09_responses: parse_ctx.h09_responses,
@@ -183,7 +184,10 @@ where
         }
     }
 
-    pub(crate) fn poll_read_from_io(&mut self, cx: &mut task::Context<'_>) -> Poll<io::Result<usize>> {
+    pub(crate) fn poll_read_from_io(
+        &mut self,
+        cx: &mut task::Context<'_>,
+    ) -> Poll<io::Result<usize>> {
         self.read_blocked = false;
         let next = self.read_buf_strategy.next();
         if self.read_buf_remaining_mut() < next {
@@ -378,7 +382,7 @@ impl ReadStrategy {
                         *decrease_now = false;
                     }
                 }
-            },
+            }
             #[cfg(feature = "client")]
             ReadStrategy::Exact(_) => (),
         }
@@ -639,6 +643,7 @@ mod tests {
             let parse_ctx = ParseContext {
                 cached_headers: &mut None,
                 req_method: &mut None,
+                h1_parser_config: Default::default(),
                 #[cfg(feature = "ffi")]
                 preserve_header_case: false,
                 h09_responses: false,

diff --git a/src/proto/h1/mod.rs b/src/proto/h1/mod.rs
@@ -1,5 +1,6 @@
 use bytes::BytesMut;
 use http::{HeaderMap, Method};
+use httparse::ParserConfig;
 
 use crate::body::DecodedLength;
 use crate::proto::{BodyLength, MessageHead};
@@ -70,6 +71,7 @@ pub(crate) struct ParsedMessage<T> {
 pub(crate) struct ParseContext<'a> {
     cached_headers: &'a mut Option<HeaderMap>,
     req_method: &'a mut Option<Method>,
+    h1_parser_config: ParserConfig,
     #[cfg(feature = "ffi")]
     preserve_header_case: bool,
     h09_responses: bool,

diff --git a/src/proto/h1/role.rs b/src/proto/h1/role.rs
@@ -683,7 +683,8 @@ impl Http1Transaction for Client {
                 );
                 let mut res = httparse::Response::new(&mut headers);
                 let bytes = buf.as_ref();
-                match res.parse(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())?;
@@ -1231,6 +1232,7 @@ mod tests {
             ParseContext {
                 cached_headers: &mut None,
                 req_method: &mut method,
+                h1_parser_config: Default::default(),
                 #[cfg(feature = "ffi")]
                 preserve_header_case: false,
                 h09_responses: false,
@@ -1254,6 +1256,7 @@ mod tests {
         let ctx = ParseContext {
             cached_headers: &mut None,
             req_method: &mut Some(crate::Method::GET),
+            h1_parser_config: Default::default(),
             #[cfg(feature = "ffi")]
             preserve_header_case: false,
             h09_responses: false,
@@ -1272,6 +1275,7 @@ mod tests {
         let ctx = ParseContext {
             cached_headers: &mut None,
             req_method: &mut None,
+            h1_parser_config: Default::default(),
             #[cfg(feature = "ffi")]
             preserve_header_case: false,
             h09_responses: false,
@@ -1288,6 +1292,7 @@ mod tests {
         let ctx = ParseContext {
             cached_headers: &mut None,
             req_method: &mut Some(crate::Method::GET),
+            h1_parser_config: Default::default(),
             #[cfg(feature = "ffi")]
             preserve_header_case: false,
             h09_responses: true,
@@ -1306,6 +1311,7 @@ mod tests {
         let ctx = ParseContext {
             cached_headers: &mut None,
             req_method: &mut Some(crate::Method::GET),
+            h1_parser_config: Default::default(),
             #[cfg(feature = "ffi")]
             preserve_header_case: false,
             h09_responses: false,
@@ -1314,6 +1320,48 @@ mod tests {
         assert_eq!(raw, H09_RESPONSE);
     }
 
+    const RESPONSE_WITH_WHITESPACE_BETWEEN_HEADER_NAME_AND_COLON: &'static str =
+        "HTTP/1.1 200 OK\r\nAccess-Control-Allow-Credentials : true\r\n\r\n";
+
+    #[test]
+    fn test_parse_allow_response_with_spaces_before_colons() {
+        use httparse::ParserConfig;
+
+        let _ = pretty_env_logger::try_init();
+        let mut raw = BytesMut::from(RESPONSE_WITH_WHITESPACE_BETWEEN_HEADER_NAME_AND_COLON);
+        let mut h1_parser_config = ParserConfig::default();
+        h1_parser_config.allow_spaces_after_header_name_in_responses(true);
+        let ctx = ParseContext {
+            cached_headers: &mut None,
+            req_method: &mut Some(crate::Method::GET),
+            h1_parser_config,
+            #[cfg(feature = "ffi")]
+            preserve_header_case: false,
+            h09_responses: false,
+        };
+        let msg = Client::parse(&mut raw, ctx).unwrap().unwrap();
+        assert_eq!(raw.len(), 0);
+        assert_eq!(msg.head.subject, crate::StatusCode::OK);
+        assert_eq!(msg.head.version, crate::Version::HTTP_11);
+        assert_eq!(msg.head.headers.len(), 1);
+        assert_eq!(msg.head.headers["Access-Control-Allow-Credentials"], "true");
+    }
+
+    #[test]
+    fn test_parse_reject_response_with_spaces_before_colons() {
+        let _ = pretty_env_logger::try_init();
+        let mut raw = BytesMut::from(RESPONSE_WITH_WHITESPACE_BETWEEN_HEADER_NAME_AND_COLON);
+        let ctx = ParseContext {
+            cached_headers: &mut None,
+            req_method: &mut Some(crate::Method::GET),
+            h1_parser_config: Default::default(),
+            #[cfg(feature = "ffi")]
+            preserve_header_case: false,
+            h09_responses: false,
+        };
+        Client::parse(&mut raw, ctx).unwrap_err();
+    }
+
     #[test]
     fn test_decoder_request() {
         fn parse(s: &str) -> ParsedMessage<RequestLine> {
@@ -1323,6 +1371,7 @@ mod tests {
                 ParseContext {
                     cached_headers: &mut None,
                     req_method: &mut None,
+                    h1_parser_config: Default::default(),
                     #[cfg(feature = "ffi")]
                     preserve_header_case: false,
                     h09_responses: false,
@@ -1339,6 +1388,7 @@ mod tests {
                 ParseContext {
                     cached_headers: &mut None,
                     req_method: &mut None,
+                    h1_parser_config: Default::default(),
                     #[cfg(feature = "ffi")]
                     preserve_header_case: false,
                     h09_responses: false,
@@ -1554,6 +1604,7 @@ mod tests {
                 ParseContext {
                     cached_headers: &mut None,
                     req_method: &mut Some(Method::GET),
+                    h1_parser_config: Default::default(),
                     #[cfg(feature = "ffi")]
                     preserve_header_case: false,
                     h09_responses: false,
@@ -1570,6 +1621,7 @@ mod tests {
                 ParseContext {
                     cached_headers: &mut None,
                     req_method: &mut Some(m),
+                    h1_parser_config: Default::default(),
                     #[cfg(feature = "ffi")]
                     preserve_header_case: false,
                     h09_responses: false,
@@ -1586,6 +1638,7 @@ mod tests {
                 ParseContext {
                     cached_headers: &mut None,
                     req_method: &mut Some(Method::GET),
+                    h1_parser_config: Default::default(),
                     #[cfg(feature = "ffi")]
                     preserve_header_case: false,
                     h09_responses: false,
@@ -1902,6 +1955,7 @@ mod tests {
             ParseContext {
                 cached_headers: &mut None,
                 req_method: &mut Some(Method::GET),
+                h1_parser_config: Default::default(),
                 #[cfg(feature = "ffi")]
                 preserve_header_case: false,
                 h09_responses: false,
@@ -1984,6 +2038,7 @@ mod tests {
                 ParseContext {
                     cached_headers: &mut headers,
                     req_method: &mut None,
+                    h1_parser_config: Default::default(),
                     #[cfg(feature = "ffi")]
                     preserve_header_case: false,
                     h09_responses: false,
@@ -2020,6 +2075,7 @@ mod tests {
                 ParseContext {
                     cached_headers: &mut headers,
                     req_method: &mut None,
+                    h1_parser_config: Default::default(),
                     #[cfg(feature = "ffi")]
                     preserve_header_case: false,
                     h09_responses: false,