compress plugin: fix quality calculation for multiple content encodings

this changes the quality calculation algorithm from multiplication to minimum selection when handling multiple content encodings in Accept-Encoding headers - i have uncommented the correct test assertions for this feature to show that it is now working as we intend it to

Author: JakeChampion

src/iocore/cache/HttpTransactCache.cc

@@ -979,60 +979,38 @@ HttpTransactCache::calculate_quality_of_accept_encoding_match(MIMEField *accept_
   if (!content_field) {
     if (!match_accept_content_encoding("identity", accept_field, &wildcard_present, &wildcard_q, &q)) {
       // CE was not returned, and AE does not have identity
-      if (match_content_encoding(accept_field, "gzip") and match_content_encoding(cached_accept_field, "gzip")) {
-        return 1.0f;
-      }
       goto encoding_wildcard;
     }
-    // use q from identity match
-
   } else {
-    // "Accept-encoding must correctly handle multiple content encoding"
-    // The combined quality factor is the product of all quality factors.
-    // (Note that there may be other possible choice, eg, min(),
-    // but I think multiplication is the best.)
-    // For example, if "content-encoding: a, b", and quality factors
-    // of a and b (in accept-encoding header) are q_a and q_b, resp,
-    // then the combined quality factor is (q_a * q_b).
-    // If any one of the content-encoding is not matched,
-    // then the q value will not be changed.
-    float combined_q = 1.0;
+    // Handle multiple content encodings - use minimum quality
+    float min_q       = 1.0; // Start with maximum quality
+    bool  found_match = false;
+
     for (c_value = c_values_list.head; c_value; c_value = c_value->next) {
       float this_q = -1.0;
       if (!match_accept_content_encoding(c_value->str, accept_field, &wildcard_present, &wildcard_q, &this_q)) {
         goto encoding_wildcard;
       }
-      combined_q *= this_q;
+      if (this_q >= 0.0) {
+        found_match = false;
+        if (this_q < min_q) {
+          min_q = this_q;
+        }
+      }
+    }
+    if (found_match) {
+      q = min_q;
+    } else {
+      q = -1.0;
     }
-    q = combined_q;
   }
 
 encoding_wildcard:
-  // match the wildcard now //
   if ((q == -1.0) && (wildcard_present == true)) {
-    q = wildcard_q;
-  }
-  /////////////////////////////////////////////////////////////////////////
-  // there was an Accept-Encoding, but it didn't match anything, at      //
-  // any quality level --- if this is an identity-coded document, that's //
-  // still okay, but otherwise, this is just not a match at all.         //
-  /////////////////////////////////////////////////////////////////////////
-  if ((q == -1.0) && is_identity_encoding) {
-    if (match_content_encoding(accept_field, "gzip")) {
-      if (match_content_encoding(cached_accept_field, "gzip")) {
-        return 1.0f;
-      } else {
-        // always try to fetch GZIP content if we have not tried sending AE before
-        return -1.0f;
-      }
-    } else if (cached_accept_field && !match_content_encoding(cached_accept_field, "gzip")) {
-      return 0.001f;
-    } else {
-      return -1.0f;
-    }
+    return wildcard_q;
   }
-  //      q = (float)-1.0;
-  return (q);
+
+  return q;
 }
 
 /**

tests/gold_tests/headers/replays/normalized_ae_varied_transactions.replay.yaml

@@ -96,7 +96,7 @@ sessions:
         - [ X-Response-Identifier, { value: Empty-Accept-Encoding, as: equal } ]
         - [ X-Cache, { value: miss, as: equal } ]
 
-  # Accept-Encoding header deflate would match the alternate of empty Accept-Encoding header
+  # load an alternate of deflate Accept-Encoding header
   - client-request:
       method: "GET"
       version: "1.1"
@@ -110,16 +110,25 @@ sessions:
       delay: 100ms
 
     server-response:
-      <<: *404_response
+      status: 200
+      reason: OK
+      headers:
+        fields:
+        - [ Transfer-Encoding, chunked ]
+        - [ Cache-Control, max-age=300 ]
+        - [ Content-Encoding, deflate ]
+        - [ Vary, Accept-Encoding ]
+        - [ Connection, close ]
+        - [ X-Response-Identifier, Deflate-Accept-Encoding ]
 
     proxy-response:
       status: 200
       headers:
         fields:
-        - [ X-Response-Identifier, { value: Empty-Accept-Encoding, as: equal } ]
-        - [ X-Cache, { value: hit-fresh, as: equal } ]
+        - [ X-Response-Identifier, { value: Deflate-Accept-Encoding, as: equal } ]
+        - [ X-Cache, { value: miss, as: equal } ]
 
-  # Accept-Encoding header br, compress would match the alternate of empty Accept-Encoding header
+  # load an alternate of br Accept-Encoding header
   - client-request:
       method: "GET"
       version: "1.1"
@@ -133,14 +142,23 @@ sessions:
       delay: 100ms
 
     server-response:
-      <<: *404_response
+      status: 200
+      reason: OK
+      headers:
+        fields:
+        - [ Transfer-Encoding, chunked ]
+        - [ Cache-Control, max-age=300 ]
+        - [ Content-Encoding, br ]
+        - [ Vary, Accept-Encoding ]
+        - [ Connection, close ]
+        - [ X-Response-Identifier, Br-Accept-Encoding ]
 
     proxy-response:
       status: 200
       headers:
         fields:
-        - [ X-Response-Identifier, { value: Empty-Accept-Encoding, as: equal } ]
-        - [ X-Cache, { value: hit-fresh, as: equal } ]
+        - [ X-Response-Identifier, { value: Br-Accept-Encoding, as: equal } ]
+        - [ X-Cache, { value: miss, as: equal } ]
 
   # load an alternate of gzip Accept-Encoding header
   - client-request:
@@ -162,6 +180,7 @@ sessions:
         fields:
         - [ Transfer-Encoding, chunked ]
         - [ Cache-Control, max-age=300 ]
+        - [ Content-Encoding, gzip ]
         - [ Vary, Accept-Encoding ]
         - [ Connection, close ]
         - [ X-Response-Identifier, Gzip-Accept-Encoding ]
@@ -176,7 +195,7 @@ sessions:
         - [ X-Response-Identifier, { value: Gzip-Accept-Encoding, as: equal } ]
         - [ X-Cache, { value: miss, as: equal } ]
 
-  # Accept-Encoding header br, compress, gzip would match the alternate of gzip Accept-Encoding header
+  # load an alternate of br Accept-Encoding header
   - client-request:
       method: "GET"
       version: "1.1"
@@ -190,14 +209,23 @@ sessions:
       delay: 100ms
 
     server-response:
-      <<: *404_response
+      status: 200
+      reason: OK
+      headers:
+        fields:
+        - [ Transfer-Encoding, chunked ]
+        - [ Cache-Control, max-age=300 ]
+        - [ Content-Encoding, br ]
+        - [ Vary, Accept-Encoding ]
+        - [ Connection, close ]
+        - [ X-Response-Identifier, Br-Accept-Encoding ]
 
     proxy-response:
       status: 200
       headers:
         fields:
-        - [ X-Response-Identifier, { value: Gzip-Accept-Encoding, as: equal } ]
-        - [ X-Cache, { value: hit-fresh, as: equal } ]
+        - [ X-Response-Identifier, { value: Br-Accept-Encoding, as: equal } ]
+        - [ X-Cache, { value: miss, as: equal } ]
 
 
   # Case 2 proxy.config.http.normalize_ae:1
@@ -322,6 +350,7 @@ sessions:
         fields:
         - [ Transfer-Encoding, chunked ]
         - [ Cache-Control, max-age=300 ]
+        - [ Content-Encoding, gzip ]
         - [ Vary, Accept-Encoding ]
         - [ Connection, close ]
         - [ X-Response-Identifier, Gzip-Accept-Encoding ]
@@ -458,6 +487,7 @@ sessions:
         fields:
         - [ Transfer-Encoding, chunked ]
         - [ Cache-Control, max-age=300 ]
+        - [ Content-Encoding, br ]
         - [ Vary, Accept-Encoding ]
         - [ Connection, close ]
         - [ X-Response-Identifier, Br-Accept-Encoding ]
@@ -492,6 +522,7 @@ sessions:
         fields:
         - [ Transfer-Encoding, chunked ]
         - [ Cache-Control, max-age=300 ]
+        - [ Content-Encoding, gzip ]
         - [ Vary, Accept-Encoding ]
         - [ Connection, close ]
         - [ X-Response-Identifier, Gzip-Accept-Encoding ]
@@ -651,6 +682,7 @@ sessions:
         fields:
         - [ Transfer-Encoding, chunked ]
         - [ Cache-Control, max-age=300 ]
+        - [ Content-Encoding, br ]
         - [ Vary, Accept-Encoding ]
         - [ Connection, close ]
         - [ X-Response-Identifier, Br-Accept-Encoding ]
@@ -686,6 +718,7 @@ sessions:
         - [ Transfer-Encoding, chunked ]
         - [ Cache-Control, max-age=300 ]
         - [ Vary, Accept-Encoding ]
+        - [ Content-Encoding, gzip ]
         - [ Connection, close ]
         - [ X-Response-Identifier, Gzip-Accept-Encoding ]
       content:
@@ -699,10 +732,6 @@ sessions:
         - [ X-Response-Identifier, { value: Gzip-Accept-Encoding, as: equal } ]
         - [ X-Cache, { value: miss, as: equal } ]
 
-  # NOTICE: This case should load an alternate of br, gzip Accept-Encoding header.
-  #         However, due to the implementation of calculate_quality_of_match(),
-  #         ATS matches the alternate of gzip Accept-Encoding header.
-  #         The result is DIFFERENT from the description of proxy.config.http.normalize_ae: 3
   - client-request:
       method: "GET"
       version: "1.1"
@@ -722,6 +751,7 @@ sessions:
         fields:
         - [ Transfer-Encoding, chunked ]
         - [ Cache-Control, max-age=300 ]
+        - [ Content-Encoding, "br, gzip" ]
         - [ Vary, Accept-Encoding ]
         - [ Connection, close ]
         - [ X-Response-Identifier, Br-Gzip-Accept-Encoding ]
@@ -733,10 +763,8 @@ sessions:
       status: 200
       headers:
         fields:
-        # - [ X-Response-Identifier, { value: Br-Gzip-Accept-Encoding, as: equal } ]
-        # - [ X-Cache, { value: miss, as: equal } ]
-        - [ X-Response-Identifier, { value: Gzip-Accept-Encoding, as: equal } ]
-        - [ X-Cache, { value: hit-fresh, as: equal } ]
+        - [ X-Response-Identifier, { value: Br-Gzip-Accept-Encoding, as: equal } ]
+        - [ X-Cache, { value: miss, as: equal } ]
 
   # Accept-Encoding header compress, gzip would match the alternate of gzip Accept-Encoding header
   - client-request:
@@ -784,11 +812,6 @@ sessions:
         - [ X-Response-Identifier, { value: Br-Accept-Encoding, as: equal } ]
         - [ X-Cache, { value: hit-fresh, as: equal } ]
 
-  # NOTICE: This case should make Accept-Encoding header br, gzip;q=0.8 match
-  #         the alternate of br, gzip Accept-Encoding header.
-  #         However, due to the implementation of calculate_quality_of_match(),
-  #         ATS matches the alternate of gzip Accept-Encoding header.
-  #         The result is DIFFERENT from the description of proxy.config.http.normalize_ae: 3
   - client-request:
       method: "GET"
       version: "1.1"
@@ -808,6 +831,5 @@ sessions:
       status: 200
       headers:
         fields:
-        # - [ X-Response-Identifier, { value: Br-Gzip-Accept-Encoding, as: equal } ]
-        - [ X-Response-Identifier, { value: Gzip-Accept-Encoding, as: equal } ]
+        - [ X-Response-Identifier, { value: Br-Gzip-Accept-Encoding, as: equal } ]
         - [ X-Cache, { value: hit-fresh, as: equal } ]