fix(http1): preserve hop-by-hop when setting close or keep-alive (#4196)

This adjusts #4110 to use `append` instead of `insert` when a nonzero number of "connection" headers are already present, and only if none of the existing headers already include the value being added.

Fixes #4195

Author: dtolnay

src/proto/h1/conn.rs

@@ -11,7 +11,7 @@ use std::time::Duration;
 use crate::rt::{Read, Write};
 use bytes::{Buf, Bytes};
 use futures_core::ready;
-use http::header::{HeaderValue, CONNECTION};
+use http::header::{Entry, HeaderValue, CONNECTION};
 use http::{HeaderMap, Method, Version};
 use http_body::Frame;
 use httparse::ParserConfig;
@@ -664,10 +664,11 @@ where
 
     // Fix keep-alive when Connection: keep-alive header is not present
     fn fix_keep_alive(&mut self, head: &mut MessageHead<T::Outgoing>) {
-        let outgoing_is_keep_alive = head
-            .headers
-            .get(CONNECTION)
-            .map_or(false, headers::connection_keep_alive);
+        let connection_entry = head.headers.entry(CONNECTION);
+        let outgoing_is_keep_alive = match &connection_entry {
+            Entry::Occupied(entry) => entry.iter().any(headers::connection_keep_alive),
+            Entry::Vacant(_) => false,
+        };
 
         if !outgoing_is_keep_alive {
             match head.version {
@@ -676,10 +677,14 @@ where
                 Version::HTTP_10 => self.state.disable_keep_alive(),
                 // If response is version 1.1 and keep-alive is wanted, add
                 // Connection: keep-alive header when not present
-                Version::HTTP_11 if self.state.wants_keep_alive() => {
-                    head.headers
-                        .insert(CONNECTION, HeaderValue::from_static("keep-alive"));
-                }
+                Version::HTTP_11 if self.state.wants_keep_alive() => match connection_entry {
+                    Entry::Occupied(mut entry) => {
+                        entry.append(HeaderValue::from_static("keep-alive"));
+                    }
+                    Entry::Vacant(entry) => {
+                        entry.insert(HeaderValue::from_static("keep-alive"));
+                    }
+                },
                 _ => (),
             }
         }
@@ -698,8 +703,16 @@ where
             }
             Version::HTTP_11 => {
                 if let KA::Disabled = self.state.keep_alive.status() {
-                    head.headers
-                        .insert(CONNECTION, HeaderValue::from_static("close"));
+                    match head.headers.entry(CONNECTION) {
+                        Entry::Occupied(mut entry) => {
+                            if !entry.iter().any(headers::connection_close) {
+                                entry.append(HeaderValue::from_static("close"));
+                            }
+                        }
+                        Entry::Vacant(entry) => {
+                            entry.insert(HeaderValue::from_static("close"));
+                        }
+                    }
                 }
             }
             _ => (),

tests/client.rs

@@ -1558,6 +1558,35 @@ test! {
             body: None,
 }
 
+// https://github.com/hyperium/hyper/issues/4195
+test! {
+    name: client_hop_by_hop_headers,
+
+    server:
+        expected: "\
+            GET / HTTP/1.1\r\n\
+            connection: close, x-hop\r\n\
+            x-hop: ...\r\n\
+            host: {addr}\r\n\
+            \r\n\
+            ",
+        reply: REPLY_OK,
+
+    client:
+        request: {
+            method: GET,
+            url: "http://{addr}/",
+            headers: {
+                "connection" => "close, x-hop",
+                "x-hop" => "...",
+            },
+        },
+        response:
+            status: OK,
+            headers: {},
+            body: None,
+}
+
 mod conn {
     use std::error::Error;
     use std::io::{self, Read, Write};