From 11345394d968d4817e1a0ee2550228ac0ae7ce74 Mon Sep 17 00:00:00 2001 From: Anthony Ramine <123095+nox@users.noreply.github.com> Date: Tue, 20 Apr 2021 23:17:48 +0200 Subject: [PATCH] feat(client): add option to allow misplaced spaces in HTTP/1 responses (#2506) --- Cargo.toml | 2 +- src/client/client.rs | 25 +++++++++++++++++++ src/client/conn.rs | 12 +++++++++ src/proto/h1/conn.rs | 14 ++++++++++- src/proto/h1/io.rs | 9 +++++-- src/proto/h1/mod.rs | 2 ++ src/proto/h1/role.rs | 58 +++++++++++++++++++++++++++++++++++++++++++- 7 files changed, 117 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 02d6d333c4..f80170c6cb 100644 --- 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 index 418f3fb4e9..d054d68076 100644 --- 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 index b87600d85a..e65986bbf0 100644 --- 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, h1_max_buf_size: Option, @@ -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 index ce0848ddea..01ec59528d 100644 --- 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> { + pub(crate) fn poll_read_keep_alive( + &mut self, + cx: &mut task::Context<'_>, + ) -> Poll> { 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, + 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 index c7ce48664b..dc6e72c146 100644 --- 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> { + pub(crate) fn poll_read_from_io( + &mut self, + cx: &mut task::Context<'_>, + ) -> Poll> { 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 index 01a9253fa3..3934502e27 100644 --- 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 { pub(crate) struct ParseContext<'a> { cached_headers: &'a mut Option, req_method: &'a mut Option, + 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 index ea9dc96be1..1493943175 100644 --- 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 { @@ -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,