[compress] Fix range-request false

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,38 @@ 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;
+  }
+
+  if (!hc->range_request()) {
+    debug("Both of Accept-Encoding and Range header are found in the request and range-request is configured as false, remove the "
+          "Range header");
+    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;
+    }
+  }
+}
+} // namespace
+
 static Data *
 data_alloc(int compression_type, int compression_algorithms)
 {
@@ -635,7 +668,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 +710,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 +944,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 +980,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

@@ -402,6 +402,10 @@ Configuration::Parse(const char *path)
     }
   }
 
+  if (!current_host_configuration->cache() && current_host_configuration->range_request()) {
+    warning("Combination of 'cache false' and 'range-request true' might deliver corrupt content.");
+  }
+
   // Update the defaults for the last host configuration too, if needed.
   current_host_configuration->update_defaults();
 

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

@@ -0,0 +1,60 @@
+'''
+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-range-false.config")
+        self.ts.Disk.remap_config.AddLines(
+            {
+                f"map /cache-true-range-false/ http://127.0.0.1:{self.server.Variables.http_port}/ @plugin=compress.so @pparam={Test.RunDirectory}/cache-true-range-false.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-range-false.config

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

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

@@ -0,0 +1,116 @@
+#  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.
+
+meta:
+  version: "1.0"
+  blocks:
+  - origin-server-response-200: &origin-server-response-200
+      status: 200
+      headers:
+        fields:
+          - [ Cache-Control, public;max-age=3600 ]
+          - [ Content-Type, application/json ]
+          - [ Content-Length, 1024 ]
+        content:
+          encoding: plain
+          size: 1024
+
+  - origin-server-response-206: &origin-server-response-206
+      status: 206
+      headers:
+        fields:
+          - [ Cache-Control, public;max-age=3600 ]
+          - [ Content-Type, application/json ]
+          - [ Content-Length, 10 ]
+        content:
+          encoding: plain
+          size: 10
+
+sessions:
+- transactions:
+  # cache-true-range-false.config
+  # 1-1: Accept-Encoding only
+  - client-request:
+      method: "GET"
+      version: "1.1"
+      url: /cache-true-range-false/
+      headers:
+        fields:
+          - [ uuid, 1-1]
+          - [ Host, example.com ]
+          - [ Accept-Encoding, gzip ]
+
+    server-response:
+      <<: *origin-server-response-200
+
+    proxy-response:
+      status: 200
+      headers:
+        fields:
+          - [ Content-Encoding, { value: gzip, as: equal } ]
+          - [ Content-Length, { value: 223, as: equal } ]
+
+  # 1-2: Range and Accept-Encoding
+  - client-request:
+      method: "GET"
+      version: "1.1"
+      url: /cache-true-range-false/
+      headers:
+        fields:
+          - [ uuid, 1-2]
+          - [ Host, example.com ]
+          - [ Range, 0-9 ]
+          - [ Accept-Encoding, gzip ]
+
+    proxy-request:
+      headers:
+        fields:
+          - [ Range, { as: absent } ]
+
+    server-response:
+      <<: *origin-server-response-200
+
+    proxy-response:
+      status: 200
+      headers:
+        fields:
+          - [ Content-Encoding, { value: gzip, as: equal } ]
+          - [ Content-Length, { value: 223, as: equal } ]
+
+  # 1-3: Range only
+  - client-request:
+      method: "GET"
+      version: "1.1"
+      url: /cache-true-range-false/
+      headers:
+        fields:
+          - [ uuid, 1-3]
+          - [ Host, example.com ]
+          - [ Range, 0-9 ]
+
+    proxy-request:
+      headers:
+        fields:
+          - [ Range, { as: present } ]
+
+    server-response:
+      <<: *origin-server-response-206
+
+    proxy-response:
+      status: 206
+      headers:
+        fields:
+          - [ Content-Length, { value: 10, as: equal } ]