[compress] Add an option to do not compress partial object (#12121)

Author: masaori335

doc/admin-guide/plugins/compress.en.rst

@@ -108,20 +108,26 @@ range-request
 
 This config controls behavior of this plugin when a client send ``Range`` header and ``Accept-Encoding`` header in the same time.
 
-============== =================================================================
-Value          Description
-============== =================================================================
-ignore-range   Remove ``Range`` header if the request has both headers (Default)
-false          Same as ``ignore-range`` for compatiblity
-no-compression Remove ``Accept-Encoding`` header if the request has both headers
-none           Do nothing
-true           Same as ``none`` for compatibility
-============== =================================================================
+====================== =================================================================
+Value                  Description
+====================== =================================================================
+none                   Do nothing
+true                   Same as ``none`` for compatibility
+remove-range           Remove ``Range`` header if the request has both headers
+remove-accept-encoding Remove ``Accept-Encoding`` header if the request has both headers
+no-compression         Do NOT compress Partial Content (default)
+false                  Same as ``no-compression`` for compatiblity
+====================== =================================================================
 
 .. important::
 
    Do NOT set this to ``none`` (or ``true``) if the cache config is set to ``false``. This combination will deliver corrupted content.
 
+
+.. important::
+
+   Some plugins (like cache_range_request) remove ``Range`` header. If you set ``remove-range`` or ``remove-accept-encoding``, be careful with the order of plugins.
+
 compressible-content-type
 -------------------------
 

plugins/compress/compress.cc

@@ -103,8 +103,8 @@ handle_range_request(TSMBuffer req_buf, TSMLoc req_loc, HostConfiguration *hc)
   debug("Both of Accept-Encoding and Range header are found in the request");
 
   switch (hc->range_request_ctl()) {
-  case RangeRequestCtrl::IGNORE_RANGE: {
-    debug("Remove the Range header by ignore-range config");
+  case RangeRequestCtrl::REMOVE_RANGE: {
+    debug("Remove the Range header by remove-range config");
     while (range_hdr_field) {
       TSMLoc next_dup = TSMimeHdrFieldNextDup(req_buf, req_loc, range_hdr_field);
       TSMimeHdrFieldDestroy(req_buf, req_loc, range_hdr_field);
@@ -113,8 +113,8 @@ handle_range_request(TSMBuffer req_buf, TSMLoc req_loc, HostConfiguration *hc)
     }
     break;
   }
-  case RangeRequestCtrl::NO_COMPRESSION: {
-    debug("Remove the Accept-Encoding header by no-compression config");
+  case RangeRequestCtrl::REMOVE_ACCEPT_ENCODING: {
+    debug("Remove the Accept-Encoding header by remove-accept-encoding config");
     while (accept_encoding_hdr_field) {
       TSMLoc next_dup = TSMimeHdrFieldNextDup(req_buf, req_loc, accept_encoding_hdr_field);
       TSMimeHdrFieldDestroy(req_buf, req_loc, accept_encoding_hdr_field);
@@ -123,6 +123,10 @@ handle_range_request(TSMBuffer req_buf, TSMLoc req_loc, HostConfiguration *hc)
     }
     break;
   }
+  case RangeRequestCtrl::NO_COMPRESSION:
+    // Do NOT touch header - this config is referred by `transformable()` function
+    debug("no header modification by no-compression config");
+    break;
   case RangeRequestCtrl::NONE:
     [[fallthrough]];
   default:
@@ -733,6 +737,33 @@ transformable(TSHttpTxn txnp, bool server, HostConfiguration *host_configuration
     return 0;
   }
 
+  // check Partial Object is transformable
+  if (host_configuration->range_request_ctl() == RangeRequestCtrl::NO_COMPRESSION) {
+    // check Range header in client request
+    // CAVETE: some plugin (- e.g. cache_range_request) tweaks client headers
+    TSMLoc range_hdr_field = TSMimeHdrFieldFind(cbuf, chdr, TS_MIME_FIELD_RANGE, TS_MIME_LEN_RANGE);
+    if (range_hdr_field != TS_NULL_MLOC) {
+      debug("Range header found in the request and range_request is configured as no_compression");
+      TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc);
+      TSHandleMLocRelease(cbuf, chdr, range_hdr_field);
+      TSHandleMLocRelease(cbuf, TS_NULL_MLOC, chdr);
+      return 0;
+    }
+
+    // check Content-Range header in (cached) server response
+    TSMLoc content_range_hdr_field = TSMimeHdrFieldFind(bufp, hdr_loc, TS_MIME_FIELD_CONTENT_RANGE, TS_MIME_LEN_CONTENT_RANGE);
+    if (content_range_hdr_field != TS_NULL_MLOC) {
+      debug("Content-Range header found in the response and range_request is configured as no_compression");
+      TSHandleMLocRelease(bufp, hdr_loc, content_range_hdr_field);
+      TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc);
+      TSHandleMLocRelease(cbuf, TS_NULL_MLOC, chdr);
+      return 0;
+    }
+
+    TSHandleMLocRelease(bufp, hdr_loc, content_range_hdr_field);
+    TSHandleMLocRelease(cbuf, chdr, range_hdr_field);
+  }
+
   // the only compressible method is currently GET.
   int         method_length;
   const char *method = TSHttpHdrMethodGet(cbuf, chdr, &method_length);
@@ -966,8 +997,8 @@ transform_plugin(TSCont contp, TSEvent event, void *edata)
  * 1. Reads the client request header
  * 2. For global plugin, get host configuration from global config
  *    For remap plugin, get host configuration from configs populated through remap
- * 3. Check for Accept encoding
- * 4. Remove Range header
+ * 3. Check for Accept-Encoding header
+ * 4. Check for Range header
  * 5. Schedules TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK and TS_HTTP_TXN_CLOSE_HOOK for
  *    further processing
  */

plugins/compress/configuration.cc

@@ -272,16 +272,22 @@ HostConfiguration::compression_algorithms()
   return compression_algorithms_;
 }
 
+/**
+  "true" and "false" are compatibility with old version, will be removed
+ */
 void
 HostConfiguration::set_range_request(const std::string &token)
 {
-  // "true" and "false" are compatibility with old version, will be removed
-  if (token == "false" || token == "ignore-range") {
-    range_request_ctl_ = RangeRequestCtrl::IGNORE_RANGE;
-  } else if (token == "true" || token == "none") {
+  if (token == "true" || token == "none") {
     range_request_ctl_ = RangeRequestCtrl::NONE;
-  } else if (token == "no-compression") {
+  } else if (token == "false" || token == "no-compression") {
     range_request_ctl_ = RangeRequestCtrl::NO_COMPRESSION;
+  } else if (token == "remove-range") {
+    range_request_ctl_ = RangeRequestCtrl::REMOVE_RANGE;
+  } else if (token == "remove-accept-encoding") {
+    range_request_ctl_ = RangeRequestCtrl::REMOVE_ACCEPT_ENCODING;
+  } else {
+    error("invalid token for range_request: %s", token.c_str());
   }
 }
 

plugins/compress/configuration.h

@@ -42,9 +42,10 @@ enum CompressionAlgorithm {
 };
 
 enum class RangeRequestCtrl : int {
-  IGNORE_RANGE   = 0, ///< Ignore Range Header (default)
-  NO_COMPRESSION = 1, ///< Do NOT compress if it's a range request
-  NONE           = 2, ///< Do nothing
+  NONE                   = 0, ///< Do nothing
+  NO_COMPRESSION         = 1, ///< Do NOT compress Partial Content (default)
+  REMOVE_RANGE           = 2, ///< Remove Range Header
+  REMOVE_ACCEPT_ENCODING = 3, ///< Remove Accept-Encoding Header
 };
 
 class HostConfiguration : private atscppapi::noncopyable
@@ -148,7 +149,7 @@ class HostConfiguration : private atscppapi::noncopyable
   int          compression_algorithms_;
   unsigned int minimum_content_length_;
 
-  RangeRequestCtrl range_request_ctl_{RangeRequestCtrl::IGNORE_RANGE};
+  RangeRequestCtrl range_request_ctl_ = RangeRequestCtrl::NO_COMPRESSION;
   StringContainer  compressible_content_types_;
   StringContainer  allows_;
   // maintain backwards compatibility/usability out of the box

tests/gold_tests/pluginTest/compress/compress-range.test.py

@@ -42,13 +42,23 @@ def setupTS(self):
                 "proxy.config.http.insert_response_via_str": 2,
             })
 
-        self.ts.Setup.Copy("etc/cache-true-ignore-range.config")
+        self.ts.Setup.Copy("etc/cache-true-remove-range.config")
+        self.ts.Setup.Copy("etc/cache-true-remove-accept-encoding.config")
         self.ts.Setup.Copy("etc/cache-true-no-compression.config")
 
         self.ts.Disk.remap_config.AddLines(
             {
-                f"map /cache-true-ignore-range/ http://127.0.0.1:{self.server.Variables.http_port}/ @plugin=compress.so @pparam={Test.RunDirectory}/cache-true-ignore-range.config",
-                f"map /cache-true-no-compression/ http://127.0.0.1:{self.server.Variables.http_port}/ @plugin=compress.so @pparam={Test.RunDirectory}/cache-true-no-compression.config",
+                f"""
+map /cache-true-remove-range/ http://127.0.0.1:{self.server.Variables.http_port}/ \
+    @plugin=compress.so \
+    @pparam={Test.RunDirectory}/cache-true-remove-range.config
+map /cache-true-remove-accept-encoding/ http://127.0.0.1:{self.server.Variables.http_port}/ \
+    @plugin=compress.so \
+    @pparam={Test.RunDirectory}/cache-true-remove-accept-encoding.config
+map /cache-true-no-compression/ http://127.0.0.1:{self.server.Variables.http_port}/ \
+    @plugin=compress.so \
+    @pparam={Test.RunDirectory}/cache-true-no-compression.config
+"""
             })
 
     def run(self):

tests/gold_tests/pluginTest/compress/etc/cache-true-remove-accept-encoding.config

@@ -0,0 +1,5 @@
+cache true
+range-request remove-accept-encoding
+compressible-content-type application/json
+supported-algorithms gzip
+minimum-content-length 0

…press/etc/cache-true-ignore-range.config …press/etc/cache-true-remove-range.configtests/gold_tests/pluginTest/compress/etc/cache-true-ignore-range.config renamed to tests/gold_tests/pluginTest/compress/etc/cache-true-remove-range.config tests/gold_tests/pluginTest/compress/etc/cache-true-ignore-range.config renamed to tests/gold_tests/pluginTest/compress/etc/cache-true-remove-range.config

@@ -1,5 +1,5 @@
 cache true
-range-request ignore-range
+range-request remove-range
 compressible-content-type application/json
 supported-algorithms gzip
 minimum-content-length 0

tests/gold_tests/pluginTest/compress/replay/compress-and-range.replay.yaml

@@ -45,14 +45,14 @@ sessions:
   #
   # ```
   # cache true
-  # range-request ignore-range
+  # range-request remove-range
   #```
   #
   # 1-1: Accept-Encoding only
   - client-request:
       method: "GET"
       version: "1.1"
-      url: /cache-true-ignore-range/
+      url: /cache-true-remove-range/
       headers:
         fields:
           - [ uuid, 1-1]
@@ -73,7 +73,7 @@ sessions:
   - client-request:
       method: "GET"
       version: "1.1"
-      url: /cache-true-ignore-range/
+      url: /cache-true-remove-range/
       headers:
         fields:
           - [ uuid, 1-2]
@@ -98,7 +98,7 @@ sessions:
   - client-request:
       method: "GET"
       version: "1.1"
-      url: /cache-true-ignore-range/
+      url: /cache-true-remove-range/
       headers:
         fields:
           - [ uuid, 1-3]
@@ -125,14 +125,14 @@ sessions:
   #
   # ```
   # cache true
-  # range-request no-compression
+  # range-request remove-accept-encoding
   #```
   #
   # 2-1: Range and Accept-Encoding
   - client-request:
       method: "GET"
       version: "1.1"
-      url: /cache-true-no-compression/
+      url: /cache-true-remove-accept-encoding/
       headers:
         fields:
           - [ uuid, 2-1]
@@ -154,3 +154,37 @@ sessions:
         fields:
           - [ Content-Length, { value: 10, as: equal } ]
 
+  # Test Case 3
+  #
+  # ```
+  # cache true
+  # range-request no-compression
+  #```
+  #
+  # 3-1: Range and Accept-Encoding
+  - client-request:
+      method: "GET"
+      version: "1.1"
+      url: /cache-true-no-compression/
+      headers:
+        fields:
+          - [ uuid, 3-1]
+          - [ Host, example.com ]
+          - [ Range, 0-9 ]
+          - [ Accept-Encoding, gzip ]
+
+    proxy-request:
+      headers:
+        fields:
+          - [ Range, { as: present } ]
+          - [ Accept-Encoding, { as: present } ]
+
+    server-response:
+      <<: *origin-server-response-206
+
+    proxy-response:
+      status: 206
+      headers:
+        fields:
+          - [ Content-Length, { value: 10, as: equal } ]
+