From 78f09b29d7a6f04b2647879855e91a3cf3744fb1 Mon Sep 17 00:00:00 2001 From: Wladimir Palant <374261+palant@users.noreply.github.com> Date: Sat, 15 Jun 2024 13:13:07 +0200 Subject: [PATCH] =?UTF-8?q?Fixes=20#229,=20#233=20=E2=80=93=20Set=20proper?= =?UTF-8?q?=20response=20headers=20when=20compression=20is=20enabled?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/protocols/http/compression/mod.rs | 114 +++++++++++++++++- 1 file changed, 113 insertions(+), 1 deletion(-) diff --git a/pingora-core/src/protocols/http/compression/mod.rs b/pingora-core/src/protocols/http/compression/mod.rs index 883c00a68..1ac9e40fb 100644 --- a/pingora-core/src/protocols/http/compression/mod.rs +++ b/pingora-core/src/protocols/http/compression/mod.rs @@ -184,6 +184,13 @@ impl ResponseCompressionCtx { return; } + if depends_on_accept_encoding(resp, *compression_level != 0, *decompress_enable) { + // The response depends on the Accept-Encoding header, make sure to indicate it + // in the Vary response header. + // https://www.rfc-editor.org/rfc/rfc9110#name-vary + add_vary_header(resp, &http::header::ACCEPT_ENCODING); + } + let action = decide_action(resp, accept_encoding); let encoder = match action { Action::Noop => None, @@ -395,6 +402,18 @@ fn test_accept_encoding_req_header() { assert_eq!(ac_list[1], Algorithm::Gzip); } +// test whether the response depends on Accept-Encoding header +fn depends_on_accept_encoding( + resp: &ResponseHeader, + compress_enabled: bool, + decompress_enabled: bool, +) -> bool { + use http::header::CONTENT_ENCODING; + + (decompress_enabled && resp.headers.get(CONTENT_ENCODING).is_some()) + || (compress_enabled && compressible(resp)) +} + // filter response header to see if (de)compression is needed fn decide_action(resp: &ResponseHeader, accept_encoding: &[Algorithm]) -> Action { use http::header::CONTENT_ENCODING; @@ -547,12 +566,94 @@ fn compressible(resp: &ResponseHeader) -> bool { } } +// add Vary header with the specified value or extend an existing Vary header value +fn add_vary_header(resp: &mut ResponseHeader, value: &http::header::HeaderName) { + use http::header::{HeaderValue, VARY}; + + if let Some(existing) = resp.headers.get(VARY) { + let already_present = existing + .as_bytes() + .split(|b| *b == b',') + .map(|mut v| { + // This is equivalent to slice.trim_ascii() which is unstable + while let [first, rest @ ..] = v { + if first.is_ascii_whitespace() { + v = rest; + } else { + break; + } + } + while let [rest @ .., last] = v { + if last.is_ascii_whitespace() { + v = rest; + } else { + break; + } + } + v + }) + .any(|v| v == b"*" || v.eq_ignore_ascii_case(value.as_ref())); + + // Prepend our value to the existing header unless already present + if !already_present { + let mut header = Vec::new(); + header.extend_from_slice(value.as_ref()); + header.extend_from_slice(b", "); + header.extend_from_slice(existing.as_bytes()); + resp.insert_header(&VARY, HeaderValue::from_bytes(&header).unwrap()) + .unwrap(); + } + } else { + resp.insert_header(&VARY, HeaderValue::from_name(value.clone())) + .unwrap(); + } +} + +#[test] +fn test_add_vary_header() { + let mut header = ResponseHeader::build(200, None).unwrap(); + add_vary_header(&mut header, &http::header::ACCEPT_ENCODING); + assert_eq!(header.headers.get("Vary").unwrap(), "accept-encoding"); + + let mut header = ResponseHeader::build(200, None).unwrap(); + header.insert_header("Vary", "Accept-Language").unwrap(); + add_vary_header(&mut header, &http::header::ACCEPT_ENCODING); + assert_eq!( + header.headers.get("Vary").unwrap(), + "accept-encoding, Accept-Language" + ); + + let mut header = ResponseHeader::build(200, None).unwrap(); + header + .insert_header("Vary", "Accept-Language, Accept-Encoding") + .unwrap(); + add_vary_header(&mut header, &http::header::ACCEPT_ENCODING); + assert_eq!( + header.headers.get("Vary").unwrap(), + "Accept-Language, Accept-Encoding" + ); + + let mut header = ResponseHeader::build(200, None).unwrap(); + header.insert_header("Vary", "*").unwrap(); + add_vary_header(&mut header, &http::header::ACCEPT_ENCODING); + assert_eq!(header.headers.get("Vary").unwrap(), "*"); +} + fn adjust_response_header(resp: &mut ResponseHeader, action: &Action) { - use http::header::{HeaderValue, CONTENT_ENCODING, CONTENT_LENGTH, TRANSFER_ENCODING}; + use http::header::{ + HeaderValue, ACCEPT_RANGES, CONTENT_ENCODING, CONTENT_LENGTH, TRANSFER_ENCODING, + }; fn set_stream_headers(resp: &mut ResponseHeader) { // because the transcoding is streamed, content length is not known ahead resp.remove_header(&CONTENT_LENGTH); + + // Even if upstream supports ranged requests, streamed responses cannot. Replace any + // present Accept-Ranges headers. + // https://www.rfc-editor.org/rfc/rfc9110#field.accept-ranges + resp.insert_header(&ACCEPT_RANGES, HeaderValue::from_static("none")) + .unwrap(); + // we stream body now TODO: chunked is for h1 only resp.insert_header(&TRANSFER_ENCODING, HeaderValue::from_static("chunked")) .unwrap(); @@ -581,6 +682,7 @@ fn test_adjust_response_header() { let mut header = ResponseHeader::build(200, None).unwrap(); header.insert_header("content-length", "20").unwrap(); header.insert_header("content-encoding", "gzip").unwrap(); + header.insert_header("accept-ranges", "bytes").unwrap(); adjust_response_header(&mut header, &Noop); assert_eq!( header.headers.get("content-encoding").unwrap().as_bytes(), @@ -596,6 +698,7 @@ fn test_adjust_response_header() { let mut header = ResponseHeader::build(200, None).unwrap(); header.insert_header("content-length", "20").unwrap(); header.insert_header("content-encoding", "gzip").unwrap(); + header.insert_header("accept-ranges", "bytes").unwrap(); adjust_response_header(&mut header, &Decompress(Gzip)); assert!(header.headers.get("content-encoding").is_none()); assert!(header.headers.get("content-length").is_none()); @@ -603,10 +706,15 @@ fn test_adjust_response_header() { header.headers.get("transfer-encoding").unwrap().as_bytes(), b"chunked" ); + assert_eq!( + header.headers.get("accept-ranges").unwrap().as_bytes(), + b"none" + ); // compress let mut header = ResponseHeader::build(200, None).unwrap(); header.insert_header("content-length", "20").unwrap(); + header.insert_header("accept-ranges", "bytes").unwrap(); adjust_response_header(&mut header, &Compress(Gzip)); assert_eq!( header.headers.get("content-encoding").unwrap().as_bytes(), @@ -617,4 +725,8 @@ fn test_adjust_response_header() { header.headers.get("transfer-encoding").unwrap().as_bytes(), b"chunked" ); + assert_eq!( + header.headers.get("accept-ranges").unwrap().as_bytes(), + b"none" + ); }