From 2ade7445d2788cdb1da7cbaa86378ad6b136b810 Mon Sep 17 00:00:00 2001 From: NeonPhantom123 Date: Sat, 27 Jun 2026 08:10:57 +0530 Subject: [PATCH 01/10] server: omit Vary: Accept-Encoding when NoCompression extension is set --- dropshot/src/server.rs | 52 ++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs index 9e4fa2b4..88dcb8ef 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -988,35 +988,37 @@ async fn http_request_handle( } } } - }; +}; - if matches!(server.config.compression, CompressionConfig::Gzip) - && is_compressible_content_type(response.headers()) - { - // Add Vary: Accept-Encoding header for all compressible content - // types. This needs to be there even if the response ends up not being - // compressed because it tells caches (like browsers and CDNs) that the - // response content depends on the value of the Accept-Encoding header. - // Without this, a cache might mistakenly serve a compressed response to - // a client that cannot decompress it, or serve an uncompressed response - // to a client that could have benefited from compression. +if matches!(server.config.compression, CompressionConfig::Gzip) + && is_compressible_content_type(response.headers()) +{ + if should_compress_response( + &method, + &request_headers, + response.status(), + response.headers(), + response.extensions(), + ) { + // Add Vary: Accept-Encoding and compress. The Vary header is added + // here so caches know they must not serve a compressed response to + // a client that didn't accept it. + add_vary_header(response.headers_mut()); + response = apply_gzip_compression(response); + } else if response.extensions().get::().is_none() { + // Add Vary: Accept-Encoding for compressible responses that are + // skipped this time (e.g. body too small, wrong method, partial + // content) but that could be compressed for a different request. + // Omit Vary when NoCompression is set because that response will + // never be compressed regardless of Accept-Encoding. add_vary_header(response.headers_mut()); - - if should_compress_response( - &method, - &request_headers, - response.status(), - response.headers(), - response.extensions(), - ) { - response = apply_gzip_compression(response); - } } +} - response.headers_mut().insert( - HEADER_REQUEST_ID, - http::header::HeaderValue::from_str(&request_id).unwrap(), - ); +response.headers_mut().insert( + HEADER_REQUEST_ID, + http::header::HeaderValue::from_str(&request_id).unwrap(), +); Ok(response) } From 3186313716ef99fd0430d037776b32f0b9ca117a Mon Sep 17 00:00:00 2001 From: NeonPhantom123 Date: Sat, 27 Jun 2026 08:12:28 +0530 Subject: [PATCH 02/10] compression: test that NoCompression suppresses should_compress_response --- dropshot/src/compression.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/dropshot/src/compression.rs b/dropshot/src/compression.rs index 1a2f1dd7..7fe8eb80 100644 --- a/dropshot/src/compression.rs +++ b/dropshot/src/compression.rs @@ -811,4 +811,33 @@ mod tests { ); } } + + #[test] + fn test_no_compression_extension_returns_false_for_should_compress() { + // When a handler sets NoCompression, should_compress_response must + // return false so that callers can suppress Vary: Accept-Encoding. + // A response that will never be compressed regardless of + // Accept-Encoding must not carry Vary: Accept-Encoding, as that + // would cause caches to split unnecessarily. + let method = http::Method::GET; + let mut request_headers = HeaderMap::new(); + request_headers.insert(ACCEPT_ENCODING, v("gzip")); + let status = http::StatusCode::OK; + let mut response_headers = HeaderMap::new(); + response_headers.insert(CONTENT_TYPE, v("application/json")); + let mut extensions = http::Extensions::new(); + extensions.insert(NoCompression); + + assert!( + !should_compress_response( + &method, + &request_headers, + status, + &response_headers, + &extensions, + ), + "should_compress_response must return false when NoCompression \ + is set so callers can omit Vary: Accept-Encoding" + ); + } } From 8356f8572e7e4fa1aecd7b4288ea28b4f5781e45 Mon Sep 17 00:00:00 2001 From: NeonPhantom123 Date: Sat, 27 Jun 2026 20:18:28 +0530 Subject: [PATCH 03/10] address review: update comments, move assert to integration tests --- dropshot/src/server.rs | 56 +++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs index 88dcb8ef..6d652a1d 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -988,37 +988,37 @@ async fn http_request_handle( } } } -}; + }; -if matches!(server.config.compression, CompressionConfig::Gzip) - && is_compressible_content_type(response.headers()) -{ - if should_compress_response( - &method, - &request_headers, - response.status(), - response.headers(), - response.extensions(), - ) { - // Add Vary: Accept-Encoding and compress. The Vary header is added - // here so caches know they must not serve a compressed response to - // a client that didn't accept it. - add_vary_header(response.headers_mut()); - response = apply_gzip_compression(response); - } else if response.extensions().get::().is_none() { - // Add Vary: Accept-Encoding for compressible responses that are - // skipped this time (e.g. body too small, wrong method, partial - // content) but that could be compressed for a different request. - // Omit Vary when NoCompression is set because that response will - // never be compressed regardless of Accept-Encoding. - add_vary_header(response.headers_mut()); + if matches!(server.config.compression, CompressionConfig::Gzip) + && is_compressible_content_type(response.headers()) + { + if should_compress_response( + &method, + &request_headers, + response.status(), + response.headers(), + response.extensions(), + ) { + // Add Vary: Accept-Encoding and compress. The Vary header is added + // here so caches know they must not serve a compressed response to + // a client that didn't accept it. + add_vary_header(response.headers_mut()); + response = apply_gzip_compression(response); + } else if response.extensions().get::().is_none() { + // Add Vary: Accept-Encoding for compressible responses that are + // skipped this time (e.g. body too small, wrong method, partial + // content) but that could be compressed for a different request. + // Omit Vary when NoCompression is set because that response will + // never be compressed regardless of Accept-Encoding. + add_vary_header(response.headers_mut()); + } } -} -response.headers_mut().insert( - HEADER_REQUEST_ID, - http::header::HeaderValue::from_str(&request_id).unwrap(), -); + response.headers_mut().insert( + HEADER_REQUEST_ID, + http::header::HeaderValue::from_str(&request_id).unwrap(), + ); Ok(response) } From 97654daca99ea54a69a5b7ed8a5424d8652f3d58 Mon Sep 17 00:00:00 2001 From: NeonPhantom123 Date: Sat, 27 Jun 2026 21:11:13 +0530 Subject: [PATCH 04/10] fix: import NoCompression in server.rs --- dropshot/src/server.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs index 6d652a1d..82dd650c 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -1,6 +1,7 @@ // Copyright 2024 Oxide Computer Company //! Generic server-wide state and facilities +use crate::compression::NoCompression; use super::ProbeRegistration; use super::api_description::ApiDescription; use super::body::Body; From fc91a5e299283cc47e0baae9adabd4da55643f60 Mon Sep 17 00:00:00 2001 From: NeonPhantom123 Date: Sat, 27 Jun 2026 21:14:18 +0530 Subject: [PATCH 05/10] fix: use super:: for NoCompression import to match file convention --- dropshot/src/server.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs index 82dd650c..7f12f9e7 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -1,8 +1,7 @@ // Copyright 2024 Oxide Computer Company //! Generic server-wide state and facilities -use crate::compression::NoCompression; -use super::ProbeRegistration; +use super::compression::NoCompression; use super::api_description::ApiDescription; use super::body::Body; use super::compression::add_vary_header; From fe3c71fa3c9d8d51520f85fbcb2e8520a2be7f20 Mon Sep 17 00:00:00 2001 From: NeonPhantom123 Date: Sat, 27 Jun 2026 21:35:47 +0530 Subject: [PATCH 06/10] fix: remove unit test, add Vary asserts to integration tests --- dropshot/tests/integration-tests/gzip.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/dropshot/tests/integration-tests/gzip.rs b/dropshot/tests/integration-tests/gzip.rs index cecf7c4e..9f95c6da 100644 --- a/dropshot/tests/integration-tests/gzip.rs +++ b/dropshot/tests/integration-tests/gzip.rs @@ -524,6 +524,15 @@ async fn test_compression_disabled_with_extension() { assert_eq!(response.headers().get(header::CONTENT_ENCODING), None); + // A NoCompression response will never be compressed regardless of + // Accept-Encoding, so it must not carry Vary: Accept-Encoding (which would + // cause caches to split entries on a header the body doesn't depend on). + assert_eq!( + response.headers().get(header::VARY), + None, + "NoCompression response should omit Vary: Accept-Encoding" + ); + get_response_bytes(&mut response).await; testctx.teardown().await; @@ -540,6 +549,20 @@ async fn test_no_compression_below_size_threshold() { assert_eq!(response.headers().get(header::CONTENT_ENCODING), None); + // Unlike the NoCompression case, a compressible response skipped only + // because it's below the size threshold must still carry Vary: a different + // request to the same resource could be compressed. + let vary = response + .headers() + .get(header::VARY) + .expect("compressible-but-skipped response should keep Vary") + .to_str() + .unwrap(); + assert!( + vary.to_lowercase().contains("accept-encoding"), + "Vary should include Accept-Encoding, got: {vary}" + ); + testctx.teardown().await; } From ed17e94e552bdb8157535a796d963fc870e29580 Mon Sep 17 00:00:00 2001 From: NeonPhantom123 Date: Sat, 27 Jun 2026 21:37:40 +0530 Subject: [PATCH 07/10] fix: remove unit test from compression.rs --- dropshot/src/compression.rs | 30 +----------------------------- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/dropshot/src/compression.rs b/dropshot/src/compression.rs index 7fe8eb80..b006fa4e 100644 --- a/dropshot/src/compression.rs +++ b/dropshot/src/compression.rs @@ -812,32 +812,4 @@ mod tests { } } - #[test] - fn test_no_compression_extension_returns_false_for_should_compress() { - // When a handler sets NoCompression, should_compress_response must - // return false so that callers can suppress Vary: Accept-Encoding. - // A response that will never be compressed regardless of - // Accept-Encoding must not carry Vary: Accept-Encoding, as that - // would cause caches to split unnecessarily. - let method = http::Method::GET; - let mut request_headers = HeaderMap::new(); - request_headers.insert(ACCEPT_ENCODING, v("gzip")); - let status = http::StatusCode::OK; - let mut response_headers = HeaderMap::new(); - response_headers.insert(CONTENT_TYPE, v("application/json")); - let mut extensions = http::Extensions::new(); - extensions.insert(NoCompression); - - assert!( - !should_compress_response( - &method, - &request_headers, - status, - &response_headers, - &extensions, - ), - "should_compress_response must return false when NoCompression \ - is set so callers can omit Vary: Accept-Encoding" - ); - } -} + } \ No newline at end of file From aa55d9c130f19815a8330c0f1e24bb58ed8459e3 Mon Sep 17 00:00:00 2001 From: NeonPhantom123 Date: Sun, 28 Jun 2026 20:44:12 +0530 Subject: [PATCH 08/10] fix: restore ProbeRegistration import and fix extra brace in compression.rs --- dropshot/src/server.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs index 7f12f9e7..472380a5 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -1,6 +1,7 @@ // Copyright 2024 Oxide Computer Company //! Generic server-wide state and facilities +use super::ProbeRegistration; use super::compression::NoCompression; use super::api_description::ApiDescription; use super::body::Body; From da71f20df858e628b2641d44a5ecb517d06cd4ca Mon Sep 17 00:00:00 2001 From: NeonPhantom123 Date: Sun, 28 Jun 2026 20:47:34 +0530 Subject: [PATCH 09/10] fix: correct closing brace indentation in compression.rs --- dropshot/src/compression.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dropshot/src/compression.rs b/dropshot/src/compression.rs index b006fa4e..1a2f1dd7 100644 --- a/dropshot/src/compression.rs +++ b/dropshot/src/compression.rs @@ -811,5 +811,4 @@ mod tests { ); } } - - } \ No newline at end of file +} From 9e942b9917003c855c6f2efa0b736bda345f583b Mon Sep 17 00:00:00 2001 From: NeonPhantom123 Date: Mon, 29 Jun 2026 10:08:56 +0530 Subject: [PATCH 10/10] fix: sort NoCompression import alphabetically per cargo fmt --- dropshot/src/server.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dropshot/src/server.rs b/dropshot/src/server.rs index 472380a5..8c24c46a 100644 --- a/dropshot/src/server.rs +++ b/dropshot/src/server.rs @@ -2,9 +2,9 @@ //! Generic server-wide state and facilities use super::ProbeRegistration; -use super::compression::NoCompression; use super::api_description::ApiDescription; use super::body::Body; +use super::compression::NoCompression; use super::compression::add_vary_header; use super::compression::apply_gzip_compression; use super::compression::is_compressible_content_type;