Fix Pingora forwarding Connection: closed to upstream, causing the co…

pingora-core/src/protocols/tls/boringssl_openssl/stream.rs

@@ -203,7 +203,11 @@ impl SslDigest {
             None => (Vec::new(), None, None),
         };
 
-        SslDigest::new(cipher, ssl.version_str(), org, sn, cert_digest)
+        let sni = ssl
+            .servername(ssl::NameType::HOST_NAME)
+            .map(|name| name.to_string());
+
+        SslDigest::with_sni(cipher, ssl.version_str(), org, sn, cert_digest, sni)
     }
 }
 

pingora-core/src/protocols/tls/digest.rs

@@ -29,6 +29,8 @@ pub struct SslDigest {
     pub serial_number: Option<String>,
     /// The digest of the peer's certificate
     pub cert_digest: Vec<u8>,
+    /// The server name indicated by the client (SNI)
+    pub sni: Option<String>,
 }
 
 impl SslDigest {
@@ -49,6 +51,29 @@ impl SslDigest {
             organization,
             serial_number,
             cert_digest,
+            sni: None,
+        }
+    }
+
+    /// Create a new SslDigest with SNI
+    pub fn with_sni<S>(
+        cipher: S,
+        version: S,
+        organization: Option<String>,
+        serial_number: Option<String>,
+        cert_digest: Vec<u8>,
+        sni: Option<String>,
+    ) -> Self
+    where
+        S: Into<Cow<'static, str>>,
+    {
+        SslDigest {
+            cipher: cipher.into(),
+            version: version.into(),
+            organization,
+            serial_number,
+            cert_digest,
+            sni,
         }
     }
 }

pingora-core/src/protocols/tls/rustls/stream.rs

@@ -384,7 +384,16 @@ impl SslDigest {
             .map(|(organization, serial)| (organization, Some(serial)))
             .unwrap_or_default();
 
-        SslDigest::new(cipher, version, organization, serial_number, cert_digest)
+        let sni = match stream {
+            pingora_rustls::TlsStream::Server(server_stream) => server_stream
+                .get_ref()
+                .1
+                .server_name()
+                .map(|name| name.to_string()),
+            _ => None,
+        };
+
+        SslDigest::with_sni(cipher, version, organization, serial_number, cert_digest, sni)
     }
 }
 

pingora-core/src/protocols/tls/s2n/stream.rs

@@ -306,12 +306,17 @@ impl SslDigest {
             }
         }
 
-        SslDigest::new(
+        // TODO: S2N API for reading SNI should be implemented when available
+        // Currently S2N doesn't expose server name / SNI through public API
+        let sni: Option<String> = None;
+
+        SslDigest::with_sni(
             cipher,
             version,
             organization,
             serial_number,
             cert_digest.unwrap_or_default(),
+            sni,
         )
     }
 }

pingora-http/src/lib.rs

@@ -289,6 +289,78 @@ impl RequestHeader {
     pub fn as_owned_parts(&self) -> ReqParts {
         clone_req_parts(&self.base)
     }
+
+    /// Removes all hop-by-hop headers as defined in RFC 7230 Section 6.1
+    /// and RFC 7540 Section 8.1.2
+    ///
+    /// Hop-by-hop headers are only meaningful for a single connection and
+    /// should not be forwarded by proxies. This method removes:
+    /// - Connection
+    /// - Keep-Alive
+    /// - Proxy-Authenticate
+    /// - Proxy-Authorization
+    /// - TE
+    /// - Trailer
+    /// - Transfer-Encoding
+    /// - Upgrade
+    ///
+    /// Additionally, RFC 7230 allows the Connection header to declare
+    /// other headers as hop-by-hop (e.g., "Connection: close, Custom-Header").
+    /// This method also removes any headers declared in the Connection header.
+    ///
+    /// # Example
+    ///
+    /// ```ignore
+    /// let mut req = RequestHeader::build("GET", "/", None)?;
+    /// req.insert_header("Connection", "close")?;
+    /// req.insert_header("Host", "example.com")?;
+    ///
+    /// req.remove_hop_by_hop_headers();
+    ///
+    /// assert!(req.get_header("Connection").is_none());
+    /// assert_eq!(req.get_header("Host").unwrap(), "example.com");
+    /// ```
+    pub fn remove_hop_by_hop_headers(&mut self) {
+        // RFC 7230 also allows the Connection header to declare
+        // additional headers as hop-by-hop. We need to handle this first, before we remove Connection itself.
+        // e.g., "Connection: X-Custom-Header, close" means X-Custom-Header is also hop-by-hop
+        let mut tokens_to_remove = Vec::new();
+        if let Some(conn_value) = self.headers.get("connection") {
+            if let Ok(conn_str) = std::str::from_utf8(conn_value.as_bytes()) {
+                for token in conn_str.split(',').map(|s| s.trim()).filter(|s| !s.is_empty()) {
+                    // Skip known Connection values, only collect other headers to remove
+                    if !token.eq_ignore_ascii_case("close")
+                        && !token.eq_ignore_ascii_case("keep-alive")
+                        && !token.eq_ignore_ascii_case("upgrade")
+                    {
+                        tokens_to_remove.push(token.to_string());
+                    }
+                }
+            }
+        }
+
+        // Now remove the standard hop-by-hop headers
+        // See: https://tools.ietf.org/html/rfc7230#section-6.1
+        const HOP_BY_HOP: &[&str] = &[
+            "connection",
+            "keep-alive",
+            "proxy-authenticate",
+            "proxy-authorization",
+            "te",
+            "trailer",
+            "transfer-encoding",
+            "upgrade",
+        ];
+
+        for header_name in HOP_BY_HOP {
+            self.remove_header(*header_name);
+        }
+
+        // Finally, remove the headers declared in Connection
+        for token in tokens_to_remove {
+            self.remove_header(&token);
+        }
+    }
 }
 
 impl Clone for RequestHeader {
@@ -910,4 +982,107 @@ mod tests {
                 .unwrap()
         );
     }
+
+    #[test]
+    fn test_remove_hop_by_hop_headers() {
+        let mut req = RequestHeader::build("GET", b"/", None).unwrap();
+
+        // Add standard hop-by-hop headers
+        req.insert_header("Connection", "close").unwrap();
+        req.insert_header("Keep-Alive", "timeout=5").unwrap();
+        req.insert_header("Transfer-Encoding", "chunked").unwrap();
+        req.insert_header("Upgrade", "websocket").unwrap();
+        req.insert_header("Proxy-Authorization", "Basic xyz").unwrap();
+        req.insert_header("TE", "trailers").unwrap();
+        req.insert_header("Trailer", "X-Trailer").unwrap();
+
+        // Add regular headers that should stay
+        req.insert_header("Host", "example.com").unwrap();
+        req.insert_header("User-Agent", "test").unwrap();
+        req.insert_header("Accept", "*/*").unwrap();
+
+        req.remove_hop_by_hop_headers();
+
+        // Verify hop-by-hop headers are removed
+        assert!(req.headers.get("Connection").is_none());
+        assert!(req.headers.get("Keep-Alive").is_none());
+        assert!(req.headers.get("Transfer-Encoding").is_none());
+        assert!(req.headers.get("Upgrade").is_none());
+        assert!(req.headers.get("Proxy-Authorization").is_none());
+        assert!(req.headers.get("TE").is_none());
+        assert!(req.headers.get("Trailer").is_none());
+
+        // Verify regular headers remain
+        assert_eq!(
+            req.headers.get("Host").map(|v| v.as_bytes()),
+            Some(b"example.com" as &[u8])
+        );
+        assert_eq!(
+            req.headers.get("User-Agent").map(|v| v.as_bytes()),
+            Some(b"test" as &[u8])
+        );
+        assert_eq!(
+            req.headers.get("Accept").map(|v| v.as_bytes()),
+            Some(b"*/*" as &[u8])
+        );
+    }
+
+    #[test]
+    fn test_connection_declared_headers() {
+        let mut req = RequestHeader::build("GET", b"/", None).unwrap();
+
+        // Connection header can declare other headers as hop-by-hop
+        // Per RFC 7230: "Connection: X-Custom-Hop, close" means X-Custom-Hop is also hop-by-hop
+        req.insert_header("Connection", "X-Custom-Hop, close").unwrap();
+        req.insert_header("X-Custom-Hop", "custom-value").unwrap();
+        req.insert_header("X-Regular-Header", "keep-me").unwrap();
+        req.insert_header("Host", "example.com").unwrap();
+
+        req.remove_hop_by_hop_headers();
+
+        // Both Connection and declared headers should be removed
+        assert!(req.headers.get("Connection").is_none());
+        assert!(
+            req.headers.get("X-Custom-Hop").is_none(),
+            "X-Custom-Hop should be removed as it was declared in Connection header"
+        );
+
+        // Regular headers stay
+        assert_eq!(
+            req.headers.get("X-Regular-Header").map(|v| v.as_bytes()),
+            Some(b"keep-me" as &[u8])
+        );
+        assert_eq!(
+            req.headers.get("Host").map(|v| v.as_bytes()),
+            Some(b"example.com" as &[u8])
+        );
+    }
+
+    #[test]
+    fn test_remove_hop_by_hop_headers_case_insensitive() {
+        let mut req = RequestHeader::build("GET", b"/", None).unwrap();
+
+        // Test case insensitivity
+        req.insert_header("CONNECTION", "CLOSE").unwrap();
+        req.insert_header("keep-alive", "timeout=5").unwrap();
+        req.insert_header("TRANSFER-ENCODING", "chunked").unwrap();
+
+        req.insert_header("Host", "example.com").unwrap();
+
+        req.remove_hop_by_hop_headers();
+
+        // All hop-by-hop headers should be removed regardless of case
+        assert!(req.headers.get("CONNECTION").is_none());
+        assert!(req.headers.get("connection").is_none());
+        assert!(req.headers.get("keep-alive").is_none());
+        assert!(req.headers.get("KEEP-ALIVE").is_none());
+        assert!(req.headers.get("TRANSFER-ENCODING").is_none());
+        assert!(req.headers.get("transfer-encoding").is_none());
+
+        // Regular headers should stay
+        assert_eq!(
+            req.headers.get("Host").map(|v| v.as_bytes()),
+            Some(b"example.com" as &[u8])
+        );
+    }
 }

pingora-proxy/src/proxy_h1.rs

@@ -36,6 +36,10 @@ impl<SV> HttpProxy<SV> {
 
         let mut req = session.req_header().clone();
 
+        // Remove hop-by-hop headers before forwarding to upstream
+        // as per RFC 7230 Section 6.1
+        req.remove_hop_by_hop_headers();
+
         // Convert HTTP2 headers to H1
         if req.version == Version::HTTP_2 {
             req.set_version(Version::HTTP_11);

pingora-proxy/src/proxy_h2.rs

@@ -83,13 +83,10 @@ impl<SV> HttpProxy<SV> {
         let mut req = session.req_header().clone();
 
         if req.version != Version::HTTP_2 {
-            /* remove H1 specific headers */
-            // https://github.com/hyperium/h2/blob/d3b9f1e36aadc1a7a6804e2f8e86d3fe4a244b4f/src/proto/streams/send.rs#L72
-            req.remove_header(&http::header::TRANSFER_ENCODING);
-            req.remove_header(&http::header::CONNECTION);
-            req.remove_header(&http::header::UPGRADE);
-            req.remove_header("keep-alive");
-            req.remove_header("proxy-connection");
+            /* remove hop-by-hop headers as per RFC 7230 and RFC 7540 */
+            // https://tools.ietf.org/html/rfc7230#section-6.1
+            // https://tools.ietf.org/html/rfc7540#section-8.1.2
+            req.remove_hop_by_hop_headers();
         }
 
         /* turn it into h2 */