From 86c5e5475a6a6ee213198851251b8b035481a011 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Mon, 5 Feb 2024 14:55:52 -0800 Subject: [PATCH] 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 * Add changelog entry Signed-off-by: Jesse Szwedko --------- Signed-off-by: Jesse Szwedko --- ...643_only_set_connection_close_http1.fix.md | 3 + src/http.rs | 85 ++++++++++++++++--- .../sources/base/aws_kinesis_firehose.cue | 2 + .../components/sources/base/datadog_agent.cue | 2 + .../components/sources/base/heroku_logs.cue | 2 + .../components/sources/base/http.cue | 2 + .../components/sources/base/http_server.cue | 2 + .../components/sources/base/opentelemetry.cue | 2 + .../sources/base/prometheus_pushgateway.cue | 2 + .../sources/base/prometheus_remote_write.cue | 2 + .../components/sources/base/splunk_hec.cue | 2 + 11 files changed, 93 insertions(+), 13 deletions(-) create mode 100644 changelog.d/19643_only_set_connection_close_http1.fix.md diff --git a/changelog.d/19643_only_set_connection_close_http1.fix.md b/changelog.d/19643_only_set_connection_close_http1.fix.md new file mode 100644 index 0000000000000..b649ed777d231 --- /dev/null +++ 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 index 11d427bee7e47..cdd38d157420e 100644 --- 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| async { + Ok::, 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| async { + Ok::, 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 index e3f63bf250500..1c2a08dfc3bc6 100644 --- 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 index 6725e69052367..7d36b47a7119f 100644 --- 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 index 3d1913e53fe91..98ba81eb59a7a 100644 --- 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 index 20090193d42ea..b837bbf914a2f 100644 --- 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 index 2aa63b778582d..a71dfac637670 100644 --- 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 index d0abfa8454752..5b57a2a53d05a 100644 --- 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 index 109e15cfb7e04..d862fb7b1e289 100644 --- 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 index 600dde089fcb1..9a58b2b58a525 100644 --- 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 index 935c626501045..a002596901c65 100644 --- 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. """