fix(http_server source): Conditionally send Connection: Close header …

…based on HTTP version (#19801) * fix(http_server source): Conditionally send Connection: Close header based on HTTP version Previously this header was sent when `default_max_connection_age` expired for all HTTP requests regardless of HTTP version, but this header is not applicable to HTTP/2 and HTTP/3 requests. Instead only send this header for other HTTP versions. HTTP/2 and HTTP/3 support GOAWAY frames for this but I think this may need to be implemented at a different level. Fixes: #19643 Signed-off-by: Jesse Szwedko <[email protected]> * Add changelog entry Signed-off-by: Jesse Szwedko <[email protected]> --------- Signed-off-by: Jesse Szwedko <[email protected]>
vectordotdev · Feb 5, 2024 · 86c5e54 · 86c5e54
1 parent 43b96ba
commit 86c5e54
Show file tree

Hide file tree

Showing 11 changed files with 93 additions and 13 deletions.
diff --git a/changelog.d/19643_only_set_connection_close_http1.fix.md b/changelog.d/19643_only_set_connection_close_http1.fix.md
@@ -0,0 +1,3 @@
+When terminating idle HTTP connections using the configured `max_connection_age`, only send
+`Connection: Close` for HTTP/0.9, HTTP/1.0, and HTTP/1.1 requests. This header is not supported on
+HTTP/2 and HTTP/3 requests. This may be supported on these HTTP versions in the future.
diff --git a/src/http.rs b/src/http.rs
@@ -10,6 +10,7 @@ use futures::future::BoxFuture;
 use headers::{Authorization, HeaderMapExt};
 use http::{
     header::HeaderValue, request::Builder, uri::InvalidUri, HeaderMap, Request, Response, Uri,
+    Version,
 };
 use hyper::{
     body::{Body, HttpBody},
@@ -401,6 +402,8 @@ pub struct KeepaliveConfig {
     /// The maximum amount of time a connection may exist before it is closed
     /// by sending a `Connection: close` header on the HTTP response.
     ///
+    /// Only applies to HTTP/0.9, HTTP/1.0, and HTTP/1.1 requests.
+    ///
     /// A random jitter configured by `max_connection_age_jitter_factor` is added
     /// to the specified duration to spread out connection storms.
     #[serde(default = "default_max_connection_age")]
@@ -524,22 +527,32 @@ where
         let start_reference = self.start_reference;
         let max_connection_age = self.max_connection_age;
         let peer_addr = self.peer_addr;
+        let version = req.version();
         let future = self.service.call(req);
         Box::pin(async move {
             let mut response = future.await?;
-            if start_reference.elapsed() >= max_connection_age {
-                debug!(
-                    message = "Closing connection due to max connection age.",
-                    ?max_connection_age,
-                    connection_age = ?start_reference.elapsed(),
-                    ?peer_addr,
-                );
-                // Tell the client to close this connection.
-                // Hyper will automatically close the connection after the response is sent.
-                response.headers_mut().insert(
-                    hyper::header::CONNECTION,
-                    hyper::header::HeaderValue::from_static("close"),
-                );
+            match version {
+                Version::HTTP_09 | Version::HTTP_10 | Version::HTTP_11 => {
+                    if start_reference.elapsed() >= max_connection_age {
+                        debug!(
+                            message = "Closing connection due to max connection age.",
+                            ?max_connection_age,
+                            connection_age = ?start_reference.elapsed(),
+                            ?peer_addr,
+                        );
+                        // Tell the client to close this connection.
+                        // Hyper will automatically close the connection after the response is sent.
+                        response.headers_mut().insert(
+                            hyper::header::CONNECTION,
+                            hyper::header::HeaderValue::from_static("close"),
+                        );
+                    }
+                }
+                // TODO need to send GOAWAY frame
+                Version::HTTP_2 => (),
+                // TODO need to send GOAWAY frame
+                Version::HTTP_3 => (),
+                _ => (),
             }
             Ok(response)
         })
@@ -666,6 +679,52 @@ mod tests {
         );
     }
 
+    #[tokio::test]
+    async fn test_max_connection_age_service_http2() {
+        tokio::time::pause();
+
+        let start_reference = Instant::now();
+        let max_connection_age = Duration::from_secs(0);
+        let mut service = MaxConnectionAgeService {
+            service: tower::service_fn(|_req: Request<Body>| async {
+                Ok::<Response<Body>, hyper::Error>(Response::new(Body::empty()))
+            }),
+            start_reference,
+            max_connection_age,
+            peer_addr: "1.2.3.4:1234".parse().unwrap(),
+        };
+
+        let mut req = Request::get("http://example.com")
+            .body(Body::empty())
+            .unwrap();
+        *req.version_mut() = Version::HTTP_2;
+        let response = service.call(req).await.unwrap();
+        assert_eq!(response.headers().get("Connection"), None);
+    }
+
+    #[tokio::test]
+    async fn test_max_connection_age_service_http3() {
+        tokio::time::pause();
+
+        let start_reference = Instant::now();
+        let max_connection_age = Duration::from_secs(0);
+        let mut service = MaxConnectionAgeService {
+            service: tower::service_fn(|_req: Request<Body>| async {
+                Ok::<Response<Body>, hyper::Error>(Response::new(Body::empty()))
+            }),
+            start_reference,
+            max_connection_age,
+            peer_addr: "1.2.3.4:1234".parse().unwrap(),
+        };
+
+        let mut req = Request::get("http://example.com")
+            .body(Body::empty())
+            .unwrap();
+        *req.version_mut() = Version::HTTP_3;
+        let response = service.call(req).await.unwrap();
+        assert_eq!(response.headers().get("Connection"), None);
+    }
+
     #[tokio::test]
     async fn test_max_connection_age_service_zero_duration() {
         tokio::time::pause();

diff --git a/website/cue/reference/components/sources/base/aws_kinesis_firehose.cue b/website/cue/reference/components/sources/base/aws_kinesis_firehose.cue
@@ -342,6 +342,8 @@ base: components: sources: aws_kinesis_firehose: configuration: {
 					The maximum amount of time a connection may exist before it is closed
 					by sending a `Connection: close` header on the HTTP response.
 
+					Only applies to HTTP/0.9, HTTP/1.0, and HTTP/1.1 requests.
+
 					A random jitter configured by `max_connection_age_jitter_factor` is added
 					to the specified duration to spread out connection storms.
 					"""

diff --git a/website/cue/reference/components/sources/base/datadog_agent.cue b/website/cue/reference/components/sources/base/datadog_agent.cue
@@ -339,6 +339,8 @@ base: components: sources: datadog_agent: configuration: {
 					The maximum amount of time a connection may exist before it is closed
 					by sending a `Connection: close` header on the HTTP response.
 
+					Only applies to HTTP/0.9, HTTP/1.0, and HTTP/1.1 requests.
+
 					A random jitter configured by `max_connection_age_jitter_factor` is added
 					to the specified duration to spread out connection storms.
 					"""

diff --git a/website/cue/reference/components/sources/base/heroku_logs.cue b/website/cue/reference/components/sources/base/heroku_logs.cue
@@ -336,6 +336,8 @@ base: components: sources: heroku_logs: configuration: {
 					The maximum amount of time a connection may exist before it is closed
 					by sending a `Connection: close` header on the HTTP response.
 
+					Only applies to HTTP/0.9, HTTP/1.0, and HTTP/1.1 requests.
+
 					A random jitter configured by `max_connection_age_jitter_factor` is added
 					to the specified duration to spread out connection storms.
 					"""

diff --git a/website/cue/reference/components/sources/base/http.cue b/website/cue/reference/components/sources/base/http.cue
@@ -364,6 +364,8 @@ base: components: sources: http: configuration: {
 					The maximum amount of time a connection may exist before it is closed
 					by sending a `Connection: close` header on the HTTP response.
 
+					Only applies to HTTP/0.9, HTTP/1.0, and HTTP/1.1 requests.
+
 					A random jitter configured by `max_connection_age_jitter_factor` is added
 					to the specified duration to spread out connection storms.
 					"""

diff --git a/website/cue/reference/components/sources/base/http_server.cue b/website/cue/reference/components/sources/base/http_server.cue
@@ -364,6 +364,8 @@ base: components: sources: http_server: configuration: {
 					The maximum amount of time a connection may exist before it is closed
 					by sending a `Connection: close` header on the HTTP response.
 
+					Only applies to HTTP/0.9, HTTP/1.0, and HTTP/1.1 requests.
+
 					A random jitter configured by `max_connection_age_jitter_factor` is added
 					to the specified duration to spread out connection storms.
 					"""

diff --git a/website/cue/reference/components/sources/base/opentelemetry.cue b/website/cue/reference/components/sources/base/opentelemetry.cue
@@ -177,6 +177,8 @@ base: components: sources: opentelemetry: configuration: {
 																The maximum amount of time a connection may exist before it is closed
 																by sending a `Connection: close` header on the HTTP response.
 
+																Only applies to HTTP/0.9, HTTP/1.0, and HTTP/1.1 requests.
+
 																A random jitter configured by `max_connection_age_jitter_factor` is added
 																to the specified duration to spread out connection storms.
 																"""

diff --git a/website/cue/reference/components/sources/base/prometheus_pushgateway.cue b/website/cue/reference/components/sources/base/prometheus_pushgateway.cue
@@ -76,6 +76,8 @@ base: components: sources: prometheus_pushgateway: configuration: {
 					The maximum amount of time a connection may exist before it is closed
 					by sending a `Connection: close` header on the HTTP response.
 
+					Only applies to HTTP/0.9, HTTP/1.0, and HTTP/1.1 requests.
+
 					A random jitter configured by `max_connection_age_jitter_factor` is added
 					to the specified duration to spread out connection storms.
 					"""

diff --git a/website/cue/reference/components/sources/base/prometheus_remote_write.cue b/website/cue/reference/components/sources/base/prometheus_remote_write.cue
@@ -66,6 +66,8 @@ base: components: sources: prometheus_remote_write: configuration: {
 					The maximum amount of time a connection may exist before it is closed
 					by sending a `Connection: close` header on the HTTP response.
 
+					Only applies to HTTP/0.9, HTTP/1.0, and HTTP/1.1 requests.
+
 					A random jitter configured by `max_connection_age_jitter_factor` is added
 					to the specified duration to spread out connection storms.
 					"""

diff --git a/website/cue/reference/components/sources/base/splunk_hec.cue b/website/cue/reference/components/sources/base/splunk_hec.cue
@@ -91,6 +91,8 @@ base: components: sources: splunk_hec: configuration: {
 					The maximum amount of time a connection may exist before it is closed
 					by sending a `Connection: close` header on the HTTP response.
 
+					Only applies to HTTP/0.9, HTTP/1.0, and HTTP/1.1 requests.
+
 					A random jitter configured by `max_connection_age_jitter_factor` is added
 					to the specified duration to spread out connection storms.
 					"""