diff --git a/doc/admin-guide/plugins/compress.en.rst b/doc/admin-guide/plugins/compress.en.rst index c2dd22009b1..846212f1425 100644 --- a/doc/admin-guide/plugins/compress.en.rst +++ b/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 ------------------------- diff --git a/plugins/compress/compress.cc b/plugins/compress/compress.cc index 2f5f5d5b8ab..0e02be267b0 100644 --- a/plugins/compress/compress.cc +++ b/plugins/compress/compress.cc @@ -24,6 +24,7 @@ #include #include +#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 } diff --git a/plugins/compress/configuration.cc b/plugins/compress/configuration.cc index 82cb15d75de..cbc37c990d8 100644 --- a/plugins/compress/configuration.cc +++ b/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) { diff --git a/plugins/compress/configuration.h b/plugins/compress/configuration.h index 80169461734..cc348a12d55 100644 --- a/plugins/compress/configuration.h +++ b/plugins/compress/configuration.h @@ -41,6 +41,12 @@ 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: @@ -48,7 +54,6 @@ class HostConfiguration : private atscppapi::noncopyable : 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 compressible_status_codes_ = {TS_HTTP_STATUS_OK, TS_HTTP_STATUS_PARTIAL_CONTENT, TS_HTTP_STATUS_NOT_MODIFIED}; diff --git a/tests/gold_tests/pluginTest/compress/compress-range.test.py b/tests/gold_tests/pluginTest/compress/compress-range.test.py new file mode 100644 index 00000000000..42c1e19e8b0 --- /dev/null +++ b/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() diff --git a/tests/gold_tests/pluginTest/compress/etc/cache-true-ignore-range.config b/tests/gold_tests/pluginTest/compress/etc/cache-true-ignore-range.config new file mode 100644 index 00000000000..c961699cc03 --- /dev/null +++ b/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 diff --git a/tests/gold_tests/pluginTest/compress/etc/cache-true-no-compression.config b/tests/gold_tests/pluginTest/compress/etc/cache-true-no-compression.config new file mode 100644 index 00000000000..eb93956fa50 --- /dev/null +++ b/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 diff --git a/tests/gold_tests/pluginTest/compress/replay/compress-and-range.replay.yaml b/tests/gold_tests/pluginTest/compress/replay/compress-and-range.replay.yaml new file mode 100644 index 00000000000..85829975042 --- /dev/null +++ b/tests/gold_tests/pluginTest/compress/replay/compress-and-range.replay.yaml @@ -0,0 +1,156 @@ +# 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: + # Test Case 1 + # + # ``` + # cache true + # range-request ignore-range + #``` + # + # 1-1: Accept-Encoding only + - client-request: + method: "GET" + version: "1.1" + url: /cache-true-ignore-range/ + 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 only + - client-request: + method: "GET" + version: "1.1" + url: /cache-true-ignore-range/ + headers: + fields: + - [ uuid, 1-2] + - [ 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 } ] + + # 1-3: Range and Accept-Encoding + - client-request: + method: "GET" + version: "1.1" + url: /cache-true-ignore-range/ + headers: + fields: + - [ uuid, 1-3] + - [ 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 } ] + + # Test Case 2 + # + # ``` + # cache true + # range-request no-compression + #``` + # + # 2-1: Range and Accept-Encoding + - client-request: + method: "GET" + version: "1.1" + url: /cache-true-no-compression/ + headers: + fields: + - [ uuid, 2-1] + - [ Host, example.com ] + - [ Range, 0-9 ] + - [ Accept-Encoding, gzip ] + + 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 } ] +