[compress] Add range request control option

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

@@ -106,8 +106,21 @@ by the origin. Enabled by default.
 range-request
 -------------
 
-When set to ``true``, causes |TS| to compress responses to Range Requests.
-Disabled by default. Setting this to true while setting cache to false leads to delivering corrupted content.
+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
+============== =================================================================
+
+.. important::
+
+   Do NOT set this to ``none`` (or ``true``) if the cache config is set to ``false``. This combination will deliver corrupted content.
 
 compressible-content-type
 -------------------------

plugins/compress/compress.cc

@@ -24,6 +24,7 @@
 #include <cstring>
 #include <zlib.h>
 
+#include "ts/apidefs.h"
 #include "tscore/ink_config.h"
 
 #if HAVE_BROTLI_ENCODE_H
@@ -76,6 +77,57 @@ static TSMutex compress_config_mutex = nullptr;
 Configuration *cur_config  = nullptr;
 Configuration *prev_config = nullptr;
 
+namespace
+{
+/**
+  If client request has both of Range and Accept-Encoding header, follow range-request config.
+ */
+void
+handle_range_request(TSMBuffer req_buf, TSMLoc req_loc, HostConfiguration *hc)
+{
+  TSMLoc accept_encoding_hdr_field =
+    TSMimeHdrFieldFind(req_buf, req_loc, TS_MIME_FIELD_ACCEPT_ENCODING, TS_MIME_LEN_ACCEPT_ENCODING);
+  if (accept_encoding_hdr_field == TS_NULL_MLOC) {
+    return;
+  }
+
+  TSMLoc range_hdr_field = TSMimeHdrFieldFind(req_buf, req_loc, TS_MIME_FIELD_RANGE, TS_MIME_LEN_RANGE);
+  if (range_hdr_field == TS_NULL_MLOC) {
+    return;
+  }
+
+  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");
+    while (range_hdr_field) {
+      TSMLoc next_dup = TSMimeHdrFieldNextDup(req_buf, req_loc, range_hdr_field);
+      TSMimeHdrFieldDestroy(req_buf, req_loc, range_hdr_field);
+      TSHandleMLocRelease(req_buf, req_loc, range_hdr_field);
+      range_hdr_field = next_dup;
+    }
+    break;
+  }
+  case RangeRequestCtrl::NO_COMPRESSION: {
+    debug("Remove the Accept-Encoding header by no-compression 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);
+      TSHandleMLocRelease(req_buf, req_loc, accept_encoding_hdr_field);
+      accept_encoding_hdr_field = next_dup;
+    }
+    break;
+  }
+  case RangeRequestCtrl::NONE:
+    [[fallthrough]];
+  default:
+    debug("Do nothing by none config");
+    break;
+  }
+}
+} // namespace
+
 static Data *
 data_alloc(int compression_type, int compression_algorithms)
 {
@@ -635,7 +687,7 @@ transformable(TSHttpTxn txnp, bool server, HostConfiguration *host_configuration
   /* Client request header */
   TSMBuffer cbuf;
   TSMLoc    chdr;
-  TSMLoc    cfield, rfield;
+  TSMLoc    cfield;
 
   const char  *value;
   int          len;
@@ -677,17 +729,6 @@ transformable(TSHttpTxn txnp, bool server, HostConfiguration *host_configuration
     return 0;
   }
 
-  // check if Range Requests are cacheable
-  bool range_request = host_configuration->range_request();
-  rfield             = TSMimeHdrFieldFind(cbuf, chdr, TS_MIME_FIELD_RANGE, TS_MIME_LEN_RANGE);
-  if (rfield != TS_NULL_MLOC && !range_request) {
-    debug("Range header found in the request and range_request is configured as false, not compressible");
-    TSHandleMLocRelease(cbuf, chdr, rfield);
-    TSHandleMLocRelease(cbuf, TS_NULL_MLOC, chdr);
-    TSHandleMLocRelease(bufp, TS_NULL_MLOC, hdr_loc);
-    return 0;
-  }
-
   // the only compressible method is currently GET.
   int         method_length;
   const char *method = TSHttpHdrMethodGet(cbuf, chdr, &method_length);
@@ -922,7 +963,8 @@ transform_plugin(TSCont contp, TSEvent event, void *edata)
  * 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. Schedules TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK and TS_HTTP_TXN_CLOSE_HOOK for
+ * 4. Remove Range header
+ * 5. Schedules TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK and TS_HTTP_TXN_CLOSE_HOOK for
  *    further processing
  */
 static void
@@ -957,6 +999,7 @@ handle_request(TSHttpTxn txnp, Configuration *config)
 
       info("Kicking off compress plugin for request");
       normalize_accept_encoding(txnp, req_buf, req_loc);
+      handle_range_request(req_buf, req_loc, hc);
       TSHttpTxnHookAdd(txnp, TS_HTTP_CACHE_LOOKUP_COMPLETE_HOOK, transform_contp);
       TSHttpTxnHookAdd(txnp, TS_HTTP_TXN_CLOSE_HOOK, transform_contp); // To release the config
     }

plugins/compress/configuration.cc

@@ -270,6 +270,19 @@ HostConfiguration::compression_algorithms()
   return compression_algorithms_;
 }
 
+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") {
+    range_request_ctl_ = RangeRequestCtrl::NONE;
+  } else if (token == "no-compression") {
+    range_request_ctl_ = RangeRequestCtrl::NO_COMPRESSION;
+  }
+}
+
 Configuration *
 Configuration::Parse(const char *path)
 {
@@ -383,7 +396,7 @@ Configuration::Parse(const char *path)
         state = kParseStart;
         break;
       case kParseRangeRequest:
-        current_host_configuration->set_range_request(token == "true");
+        current_host_configuration->set_range_request(token);
         state = kParseStart;
         break;
       case kParseFlush:
@@ -406,8 +419,8 @@ Configuration::Parse(const char *path)
   current_host_configuration->update_defaults();
 
   // Check combination of configs
-  if (!current_host_configuration->cache() && current_host_configuration->range_request()) {
-    warning("Combination of 'cache false' and 'range-request true' might deliver corrupted content");
+  if (!current_host_configuration->cache() && current_host_configuration->range_request_ctl() == RangeRequestCtrl::NONE) {
+    warning("Combination of 'cache false' and 'range-request none' might deliver corrupted content");
   }
 
   if (state != kParseStart) {

plugins/compress/configuration.h

@@ -41,14 +41,19 @@ enum CompressionAlgorithm {
   ALGORITHM_BROTLI  = 4 // For bit manipulations
 };
 
+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
+};
+
 class HostConfiguration : private atscppapi::noncopyable
 {
 public:
   explicit HostConfiguration(const std::string &host)
     : host_(host),
       enabled_(true),
       cache_(true),
-      range_request_(false),
       remove_accept_encoding_(false),
       flush_(false),
       compression_algorithms_(ALGORITHM_GZIP),
@@ -66,15 +71,10 @@ class HostConfiguration : private atscppapi::noncopyable
   {
     enabled_ = x;
   }
-  bool
-  range_request()
-  {
-    return range_request_;
-  }
-  void
-  set_range_request(bool x)
+  RangeRequestCtrl
+  range_request_ctl()
   {
-    range_request_ = x;
+    return range_request_ctl_;
   }
   bool
   cache()
@@ -137,19 +137,20 @@ class HostConfiguration : private atscppapi::noncopyable
   bool is_status_code_compressible(const TSHttpStatus status_code) const;
   void add_compression_algorithms(std::string &algorithms);
   int  compression_algorithms();
+  void set_range_request(const std::string &token);
 
 private:
   std::string  host_;
   bool         enabled_;
   bool         cache_;
-  bool         range_request_;
   bool         remove_accept_encoding_;
   bool         flush_;
   int          compression_algorithms_;
   unsigned int minimum_content_length_;
 
-  StringContainer compressible_content_types_;
-  StringContainer allows_;
+  RangeRequestCtrl range_request_ctl_;
+  StringContainer  compressible_content_types_;
+  StringContainer  allows_;
   // maintain backwards compatibility/usability out of the box
   std::set<TSHttpStatus> compressible_status_codes_ = {TS_HTTP_STATUS_OK, TS_HTTP_STATUS_PARTIAL_CONTENT,
                                                        TS_HTTP_STATUS_NOT_MODIFIED};

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

@@ -0,0 +1,64 @@
+'''
+Test compress plugin with range request
+'''
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the #  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+Test.Summary = '''
+Test compress plugin with range request
+'''
+
+Test.SkipUnless(Condition.PluginExists('compress.so'))
+
+
+class CompressPluginTest:
+    replayFile = "replay/compress-and-range.replay.yaml"
+
+    def __init__(self):
+        self.setupOriginServer()
+        self.setupTS()
+
+    def setupOriginServer(self):
+        self.server = Test.MakeVerifierServerProcess("verifier-server", self.replayFile)
+
+    def setupTS(self):
+        self.ts = Test.MakeATSProcess("ts")
+        self.ts.Disk.records_config.update(
+            {
+                "proxy.config.diags.debug.enabled": 1,
+                "proxy.config.diags.debug.tags": "http|compress",
+                "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-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",
+            })
+
+    def run(self):
+        tr = Test.AddTestRun()
+        tr.AddVerifierClientProcess(
+            "verifier-client", self.replayFile, http_ports=[self.ts.Variables.port], other_args='--thread-limit 1')
+        tr.Processes.Default.StartBefore(self.ts)
+        tr.Processes.Default.StartBefore(self.server)
+        tr.StillRunningAfter = self.ts
+        tr.StillRunningAfter = self.server
+
+
+CompressPluginTest().run()

tests/gold_tests/pluginTest/compress/etc/cache-true-ignore-range.config

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

tests/gold_tests/pluginTest/compress/etc/cache-true-no-compression.config

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