From abd7fca99fb0cbc3ba14fbe2110aa77fe7a20090 Mon Sep 17 00:00:00 2001
From: Sam Landfried <samlland@amazon.com>
Date: Tue, 28 Jul 2026 18:29:19 +0000
Subject: [PATCH] fix(http1): evict pooled conn on request-side Connection:
 close

hyper's client derives connection reuse from the response alone, so a
request carrying `Connection: close` whose backend response omits it
(keeps the socket alive) leaves the connection pooled and reusable.
Disable keep-alive at request-encode time when the outgoing request
carries a `Connection: close` token so the connection is evicted
regardless of the response.

Every `Connection` header line is inspected via a `connection_any_close`
helper (over `get_all`), not just the first, so a `close` on a later
line or within a comma-separated value is honored.

Adds a conn-level regression test (client Conn + write_head) asserting a
`Connection: close` request disables keep-alive -- including
comma-separated and multi-line forms -- while a keep-alive request stays
reusable.
---
 src/headers.rs       | 11 +++++++
 src/proto/h1/conn.rs | 68 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)

diff --git a/src/headers.rs b/src/headers.rs
index 0b36e060f0..89b70fd45e 100644
--- a/src/headers.rs
+++ b/src/headers.rs
@@ -19,6 +19,17 @@ pub(super) fn connection_close(value: &HeaderValue) -> bool {
     connection_has(value, "close")
 }
 
+// Returns true if any `Connection` header field carries a `close` token.
+// A message may have more than one `Connection` header line, so all of them
+// must be inspected (`get`/`connection_close` alone only sees the first).
+#[cfg(feature = "http1")]
+pub(super) fn connection_any_close(headers: &http::HeaderMap) -> bool {
+    headers
+        .get_all(http::header::CONNECTION)
+        .iter()
+        .any(connection_close)
+}
+
 #[cfg(feature = "http1")]
 fn connection_has(value: &HeaderValue, needle: &str) -> bool {
     if let Ok(s) = value.to_str() {
diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs
index dfcadcee56..9a968dd0df 100644
--- a/src/proto/h1/conn.rs
+++ b/src/proto/h1/conn.rs
@@ -613,6 +613,15 @@ where
 
         if !T::should_read_first() {
             self.state.busy();
+            // A client request carrying `Connection: close` must not be pooled or
+            // reused. hyper otherwise derives connection reuse from the response
+            // alone, so a backend that ignores the request-side close (omits
+            // `Connection: close` in its response) would leave the connection in
+            // the pool. Disable keep-alive up front so the connection is evicted
+            // regardless of the response.
+            if headers::connection_any_close(&head.headers) {
+                self.state.disable_keep_alive();
+            }
         }
 
         self.enforce_version(&mut head);
@@ -1196,6 +1205,65 @@ mod tests {
         });
     }
 
+    // A client request carrying `Connection: close` must evict the connection
+    // (disable keep-alive) at request-encode time, so it is never returned to the
+    // pool for reuse — independent of whether the backend response echoes
+    // `Connection: close`. hyper otherwise derives reuse from the response alone.
+    #[test]
+    fn client_request_connection_close_disables_keep_alive() {
+        use super::*;
+        use crate::common::io::Compat;
+        use crate::proto::RequestLine;
+
+        // Encodes a client GET (with the given Connection header lines, one
+        // `append` each) on a fresh client Conn and returns whether the
+        // connection remains reusable.
+        fn remains_reusable_after_get(connection_values: &[&'static str]) -> bool {
+            let io = Compat(tokio_test::io::Builder::new().build());
+            let mut conn = Conn::<_, bytes::Bytes, crate::proto::h1::ClientTransaction>::new(io);
+            assert!(
+                conn.state.wants_keep_alive(),
+                "a fresh client connection should want keep-alive"
+            );
+
+            let mut headers = HeaderMap::new();
+            for value in connection_values {
+                headers.append(CONNECTION, HeaderValue::from_static(value));
+            }
+            let head = MessageHead {
+                version: Version::HTTP_11,
+                subject: RequestLine(Method::GET, "/".parse().unwrap()),
+                headers,
+                extensions: http::Extensions::new(),
+            };
+            conn.write_head(head, None);
+            conn.state.wants_keep_alive()
+        }
+
+        // Control: a request without `Connection: close` leaves the connection reusable.
+        assert!(
+            remains_reusable_after_get(&[]),
+            "a keep-alive request must leave the connection reusable"
+        );
+        // Fix: a `Connection: close` request must disable keep-alive so the
+        // connection is evicted regardless of the response.
+        assert!(
+            !remains_reusable_after_get(&["close"]),
+            "a `Connection: close` request must disable keep-alive (connection evicted)"
+        );
+        // A `close` token in a comma-separated value is honored.
+        assert!(
+            !remains_reusable_after_get(&["keep-alive, close"]),
+            "a `close` token in a comma-separated Connection value must disable keep-alive"
+        );
+        // A `close` in ANY of multiple Connection header lines is honored
+        // (get_all, not just the first line).
+        assert!(
+            !remains_reusable_after_get(&["keep-alive", "close"]),
+            "a `close` in any Connection header line must disable keep-alive"
+        );
+    }
+
     /*
     //TODO: rewrite these using dispatch... someday...
     use futures::{Async, Future, Stream, Sink};