diff --git a/src/core/client/proto/h1/role.rs b/src/core/client/proto/h1/role.rs index b49c337bd..ff953c273 100644 --- a/src/core/client/proto/h1/role.rs +++ b/src/core/client/proto/h1/role.rs @@ -112,8 +112,6 @@ where T::encode(enc, dst) } -// There are 2 main roles, Client and Server. - pub(crate) enum Client {} impl Http1Transaction for Client { @@ -224,10 +222,9 @@ impl Http1Transaction for Client { } if let Some(ref mut header_case_map) = header_case_map { - header_case_map.append( - &name, - OrigHeaderName::Cased(slice.slice(header.name.0..header.name.1)), - ); + let orig_name = + OrigHeaderName::Cased(slice.slice(header.name.0..header.name.1)); + header_case_map.append(&name, orig_name); } headers.append(name, value); @@ -372,7 +369,7 @@ impl Client { // If Transfer-Encoding header is present, and 'chunked' is // not the final encoding, and this is a Request, then it is // malformed. A server should respond with 400 Bad Request. - if inc.version == Version::HTTP_10 { + return if inc.version == Version::HTTP_10 { debug!("HTTP/1.0 cannot have Transfer-Encoding header"); Err(Parse::transfer_encoding_unexpected()) } else if headers::transfer_encoding_is_chunked(&inc.headers) { @@ -380,16 +377,20 @@ impl Client { } else { trace!("not chunked, read till eof"); Ok(Some((DecodedLength::CLOSE_DELIMITED, false))) - } - } else if let Some(len) = headers::content_length_parse_all(&inc.headers) { - Ok(Some((DecodedLength::checked_new(len)?, false))) - } else if inc.headers.contains_key(header::CONTENT_LENGTH) { + }; + } + + if let Some(len) = headers::content_length_parse_all(&inc.headers) { + return Ok(Some((DecodedLength::checked_new(len)?, false))); + } + + if inc.headers.contains_key(header::CONTENT_LENGTH) { debug!("illegal Content-Length header"); - Err(Parse::content_length_invalid()) - } else { - trace!("neither Transfer-Encoding nor Content-Length"); - Ok(Some((DecodedLength::CLOSE_DELIMITED, false))) + return Err(Parse::content_length_invalid()); } + + trace!("neither Transfer-Encoding nor Content-Length"); + Ok(Some((DecodedLength::CLOSE_DELIMITED, false))) } fn set_length(head: &mut RequestHead, body: Option) -> Encoder { let body = if let Some(body) = body { @@ -651,36 +652,28 @@ pub(crate) fn write_headers(headers: &HeaderMap, dst: &mut Vec) { fn write_headers_original_case( headers: &mut HeaderMap, - orig_case: &OrigHeaderMap, + orig_headers: &OrigHeaderMap, dst: &mut Vec, ) { - orig_case.sort_headers(headers); - - // For each header name/value pair, there may be a value in the casemap - // that corresponds to the HeaderValue. So, we iterator all the keys, - // and for each one, try to pair the originally cased name with the value. - // - // TODO: consider adding http::HeaderMap::entries() iterator - for name in headers.keys() { - let mut names = orig_case.get_all(name); - - for value in headers.get_all(name) { - if let Some(orig_name) = names.next() { - extend(dst, orig_name.as_ref()); - } else { - extend(dst, name.as_str().as_bytes()); + orig_headers.sort_headers_for_each(headers, |orig_name, value| { + match orig_name { + OrigHeaderName::Cased(orig_name) => { + extend(dst, orig_name); } - - // Wanted for curl test cases that send `X-Custom-Header:\r\n` - if value.is_empty() { - extend(dst, b":\r\n"); - } else { - extend(dst, b": "); - extend(dst, value.as_bytes()); - extend(dst, b"\r\n"); + OrigHeaderName::Standard(name) => { + extend(dst, name.as_ref()); } } - } + + // Wanted for curl test cases that send `X-Custom-Header:\r\n` + if value.is_empty() { + extend(dst, b":\r\n"); + } else { + extend(dst, b": "); + extend(dst, value.as_bytes()); + extend(dst, b"\r\n"); + } + }); } struct FastWrite<'a>(&'a mut Vec); diff --git a/src/core/client/proto/h2/client.rs b/src/core/client/proto/h2/client.rs index 2be52bcd6..a13406140 100644 --- a/src/core/client/proto/h2/client.rs +++ b/src/core/client/proto/h2/client.rs @@ -564,10 +564,10 @@ where } // Sort headers if we have the original headers - if let Some(orig) = + if let Some(orig_headers) = RequestConfig::::remove(req.extensions_mut()) { - orig.sort_headers(req.headers_mut()); + orig_headers.sort_headers(req.headers_mut()); } let is_connect = req.method() == Method::CONNECT; diff --git a/src/header.rs b/src/header.rs index cd4170234..440d09e45 100644 --- a/src/header.rs +++ b/src/header.rs @@ -132,20 +132,8 @@ impl OrigHeaderMap { self.0.append(name, orig); } - /// Returns a view of all spellings associated with that header name, - /// in the order they were found. - #[inline] - pub(crate) fn get_all<'a>( - &'a self, - name: &HeaderName, - ) -> impl Iterator + 'a> + 'a { - self.0.get_all(name).into_iter() - } - - /// Sorts headers according to the order defined in this map, preserving original casing. - /// - /// Headers specified in this map are placed first in the given order, followed by any - /// remaining headers. This maintains both the desired ordering and original case formatting. + /// Sorts headers by this map, preserving original casing. + /// Headers in the map come first, others follow. pub(crate) fn sort_headers(&self, headers: &mut HeaderMap) { if headers.len() <= 1 || self.0.is_empty() { return; @@ -163,15 +151,15 @@ impl OrigHeaderMap { } // Then insert any remaining headers that were not ordered - let mut prev_header_name: Option = None; + let mut prev_name: Option = None; for (name, value) in headers.drain() { - match (name, &prev_header_name) { + match (name, &prev_name) { (Some(name), _) => { - prev_header_name = Some(name.clone()); + prev_name = Some(name.clone()); sorted_headers.insert(name, value); } - (None, Some(prev)) => { - sorted_headers.append(prev, value); + (None, Some(prev_name)) => { + sorted_headers.append(prev_name, value); } _ => {} } @@ -179,6 +167,37 @@ impl OrigHeaderMap { std::mem::swap(headers, &mut sorted_headers); } + + /// Calls the given function for each header in this map's order, preserving original casing. + /// Headers in the map are processed first, others follow. + pub(crate) fn sort_headers_for_each(&self, headers: &mut HeaderMap, mut dst: F) + where + F: FnMut(&OrigHeaderName, &HeaderValue), + { + // First, sort headers according to the order defined in this map + for (name, orig_name) in self.iter() { + for value in headers.get_all(name) { + dst(orig_name, value); + } + + headers.remove(name); + } + + // After processing all ordered headers, append any remaining headers + let mut prev_name: Option = None; + for (name, value) in headers.drain() { + match (name.map(OrigHeaderName::Standard), &prev_name) { + (Some(name), _) => { + dst(&name, &value); + prev_name = Some(name); + } + (None, Some(prev_name)) => { + dst(prev_name, &value); + } + _ => {} + }; + } + } } impl<'a> IntoIterator for &'a OrigHeaderMap { @@ -300,10 +319,20 @@ mod sealed { #[cfg(test)] mod test { - use http::{HeaderMap, HeaderValue}; + use http::{HeaderMap, HeaderName, HeaderValue}; use super::OrigHeaderMap; + /// Returns a view of all spellings associated with that header name, + /// in the order they were found. + #[inline] + pub(crate) fn get_all<'a>( + orig_headers: &'a OrigHeaderMap, + name: &HeaderName, + ) -> impl Iterator + 'a> + 'a { + orig_headers.0.get_all(name).into_iter() + } + #[test] fn test_header_order() { let mut headers = OrigHeaderMap::new(); @@ -350,7 +379,7 @@ mod test { headers.insert("x-test"); // Check that both headers are stored - let all_x_test: Vec<_> = headers.get_all(&"X-Test".parse().unwrap()).collect(); + let all_x_test: Vec<_> = get_all(&headers, &"X-Test".parse().unwrap()).collect(); assert_eq!(all_x_test.len(), 2); assert!(all_x_test.iter().any(|v| v.as_ref() == b"X-Test")); assert!(all_x_test.iter().any(|v| v.as_ref() == b"x-test")); @@ -366,7 +395,7 @@ mod test { headers.insert("X-test"); // Check that all variations are stored - let all_x_test: Vec<_> = headers.get_all(&"x-test".parse().unwrap()).collect(); + let all_x_test: Vec<_> = get_all(&headers, &"x-test".parse().unwrap()).collect(); assert_eq!(all_x_test.len(), 3); assert!(all_x_test.iter().any(|v| v.as_ref() == b"X-test")); assert!(all_x_test.iter().any(|v| v.as_ref() == b"x-test"));