Fixes cloudflare#229, cloudflare#233 – Set proper response headers wh…

…en compression is enabled
CodSpeedHQ · Jun 15, 2024 · 78f09b2 · 78f09b2
1 parent 31d7b63
commit 78f09b2
Showing 1 changed file with 113 additions and 1 deletion.
diff --git 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,17 +698,23 @@ 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());
     assert_eq!(
         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"
+    );
 }