perf(http1): write during header sorting

src/core/client/proto/h1/role.rs

@@ -112,9 +112,7 @@ where
     T::encode(enc, dst)
 }
 
-// There are 2 main roles, Client and Server.
-
-pub(crate) enum Client {}
+pub(crate) struct Client;
 
 impl Http1Transaction for Client {
     type Incoming = StatusCode;
@@ -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,24 +369,28 @@ 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) {
                 Ok(Some((DecodedLength::CHUNKED, false)))
             } 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<BodyLength>) -> Encoder {
         let body = if let Some(body) = body {
@@ -651,36 +652,25 @@ pub(crate) fn write_headers(headers: &HeaderMap, dst: &mut Vec<u8>) {
 
 fn write_headers_original_case(
     headers: &mut HeaderMap,
-    orig_case: &OrigHeaderMap,
+    orig_headers: &OrigHeaderMap,
     dst: &mut Vec<u8>,
 ) {
-    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, |name, orig_name, value| {
+        if let Some(orig_name) = orig_name {
+            extend(dst, orig_name.as_ref());
+        } else {
+            extend(dst, name.as_str().as_bytes());
+        }
 
-            // 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");
-            }
+        // 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<u8>);

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::<RequestOrigHeaderMap>::remove(req.extensions_mut())
                     {
-                        orig.sort_headers(req.headers_mut());
+                        orig_headers.sort_headers(req.headers_mut());
                     }
 
                     let is_connect = req.method() == Method::CONNECT;

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<Item = impl AsRef<[u8]> + '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,22 +151,53 @@ impl OrigHeaderMap {
         }
 
         // Then insert any remaining headers that were not ordered
-        let mut prev_header_name: Option<HeaderName> = None;
+        let mut prev_name: Option<HeaderName> = 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);
                 }
                 _ => {}
             }
         }
 
         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<F>(&self, headers: &mut HeaderMap, mut dst: F)
+    where
+        F: FnMut(&HeaderName, Option<&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(name, Some(orig_name), value);
+            }
+
+            headers.remove(name);
+        }
+
+        // After processing all ordered headers, append any remaining headers
+        let mut prev_name: Option<HeaderName> = None;
+        for (name, value) in headers.drain() {
+            let name = match (&name, &prev_name) {
+                (Some(name), _) => {
+                    prev_name = Some(name.clone());
+                    name
+                }
+                (None, Some(prev_name)) => prev_name,
+                _ => continue,
+            };
+
+            dst(name, None, &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<Item = impl AsRef<[u8]> + '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"));