From a8b5970f9376f7fb2d287e63d79e45bb01ef866e Mon Sep 17 00:00:00 2001 From: Serris Lew Date: Thu, 8 Dec 2022 16:38:27 -0800 Subject: [PATCH 1/4] Add slice plugin config to strip Range header for HEAD requests --- doc/admin-guide/plugins/slice.en.rst | 4 + plugins/experimental/slice/Config.cc | 4 + plugins/experimental/slice/Config.h | 5 +- plugins/experimental/slice/server.cc | 8 ++ plugins/experimental/slice/util.cc | 17 ++-- .../slice/replay/slice_range.replay.yaml | 82 +++++++++++++++++++ .../pluginTest/slice/slice_rm_range.test.py | 69 ++++++++++++++++ 7 files changed, 181 insertions(+), 8 deletions(-) create mode 100644 tests/gold_tests/pluginTest/slice/replay/slice_range.replay.yaml create mode 100644 tests/gold_tests/pluginTest/slice/slice_rm_range.test.py diff --git a/doc/admin-guide/plugins/slice.en.rst b/doc/admin-guide/plugins/slice.en.rst index 8426c18b031..cea69fe62a1 100644 --- a/doc/admin-guide/plugins/slice.en.rst +++ b/doc/admin-guide/plugins/slice.en.rst @@ -135,6 +135,10 @@ The slice plugin supports the following options:: can improve cache miss latency. -f for short + --enable-strip-range (optional) + Enable slice plugin to strip Range header for HEAD requests. + -h for short + Examples:: @plugin=slice.so @pparam=--blockbytes=1000000 @plugin=cache_range_requests.so diff --git a/plugins/experimental/slice/Config.cc b/plugins/experimental/slice/Config.cc index 5cd71d47339..9b719571e68 100644 --- a/plugins/experimental/slice/Config.cc +++ b/plugins/experimental/slice/Config.cc @@ -124,6 +124,7 @@ Config::fromArgs(int const argc, char const *const argv[]) {const_cast("skip-header"), required_argument, nullptr, 's'}, {const_cast("blockbytes-test"), required_argument, nullptr, 't'}, {const_cast("prefetch-count"), required_argument, nullptr, 'f'}, + {const_cast("enable-strip-range"), no_argument, nullptr, 'h'}, {nullptr, 0, nullptr, 0}, }; @@ -226,6 +227,9 @@ Config::fromArgs(int const argc, char const *const argv[]) case 'f': { m_prefetchcount = atoi(optarg); } break; + case 'h': { + m_head_strip_range = true; + } break; default: break; } diff --git a/plugins/experimental/slice/Config.h b/plugins/experimental/slice/Config.h index d1dceb99e5b..c83badc52b5 100644 --- a/plugins/experimental/slice/Config.h +++ b/plugins/experimental/slice/Config.h @@ -44,8 +44,9 @@ struct Config { int m_paceerrsecs{0}; // -1 disable logging, 0 no pacing, max 60s int m_prefetchcount{0}; // 0 disables prefetching enum RefType { First, Relative }; - RefType m_reftype{First}; // reference slice is relative to request - bool m_head_req{false}; // HEAD request + RefType m_reftype{First}; // reference slice is relative to request + bool m_head_req{false}; // HEAD request + bool m_head_strip_range{false}; // strip range header for head requests std::string m_skip_header; std::string m_crr_ims_header; diff --git a/plugins/experimental/slice/server.cc b/plugins/experimental/slice/server.cc index d79425921ff..7861c3c12b4 100644 --- a/plugins/experimental/slice/server.cc +++ b/plugins/experimental/slice/server.cc @@ -111,6 +111,14 @@ handleFirstServerHeader(Data *const data, TSCont const contp) // Should run TSVIONSetBytes(output_io, hlen + bodybytes); int64_t const hlen = TSHttpHdrLengthGet(header.m_buffer, header.m_lochdr); int64_t const clen = contentLengthFrom(header); + if (data->m_config->m_head_req && TS_HTTP_STATUS_OK == header.status()) { + DEBUG_LOG("HEAD request stripped Range header: expects 200"); + TSVIONBytesSet(output_vio, hlen); + TSHttpHdrPrint(header.m_buffer, header.m_lochdr, output_buf); + data->m_bytessent = hlen; + TSVIOReenable(output_vio); + return HeaderState::Good; + } DEBUG_LOG("Passthru bytes: header: %" PRId64 " body: %" PRId64, hlen, clen); if (clen != INT64_MAX) { TSVIONBytesSet(output_vio, hlen + clen); diff --git a/plugins/experimental/slice/util.cc b/plugins/experimental/slice/util.cc index 60b7a175de9..b91636b02ca 100644 --- a/plugins/experimental/slice/util.cc +++ b/plugins/experimental/slice/util.cc @@ -79,12 +79,17 @@ request_block(TSCont contp, Data *const data) // reuse the incoming client header, just change the range HttpHeader header(data->m_req_hdrmgr.m_buffer, data->m_req_hdrmgr.m_lochdr); - // add/set sub range key and add slicer tag - bool const rangestat = header.setKeyVal(TS_MIME_FIELD_RANGE, TS_MIME_LEN_RANGE, rangestr, rangelen); - - if (!rangestat) { - ERROR_LOG("Error trying to set range request header %s", rangestr); - return false; + // if configured, remove range header from head requests + if (data->m_config->m_head_req && data->m_config->m_head_strip_range) { + header.removeKey(TS_MIME_FIELD_RANGE, TS_MIME_LEN_RANGE); + } else { + // add/set sub range key and add slicer tag + bool const rangestat = header.setKeyVal(TS_MIME_FIELD_RANGE, TS_MIME_LEN_RANGE, rangestr, rangelen); + + if (!rangestat) { + ERROR_LOG("Error trying to set range request header %s", rangestr); + return false; + } } header.removeKey(SLICE_CRR_HEADER.data(), SLICE_CRR_HEADER.size()); diff --git a/tests/gold_tests/pluginTest/slice/replay/slice_range.replay.yaml b/tests/gold_tests/pluginTest/slice/replay/slice_range.replay.yaml new file mode 100644 index 00000000000..2fd685b2417 --- /dev/null +++ b/tests/gold_tests/pluginTest/slice/replay/slice_range.replay.yaml @@ -0,0 +1,82 @@ +# 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" + +sessions: +- transactions: + - client-request: + method: "HEAD" + version: "1.1" + url: /no/range + headers: + fields: + - [ Host, example.com ] + - [ X-Request, request ] + - [ Range, bytes=0-5 ] + - [ uuid, 1 ] + + proxy-request: + headers: + fields: + - [ Range, { as: absent } ] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Content-Length, 16 ] + - [ X-Response, response ] + + proxy-response: + status: 200 + headers: + fields: + - [ X-Response, { value: response, as: equal } ] + - [ Content-Length, 16 ] + + - client-request: + method: "HEAD" + version: "1.1" + url: /with/range + headers: + fields: + - [ Host, example.com ] + - [ X-Request, request ] + - [ Range, bytes=0-5 ] + - [ uuid, 2 ] + + proxy-request: + headers: + fields: + - [ Range, { as: present } ] + + server-response: + status: 206 + headers: + fields: + - [ Content-Length, 6 ] + - [ X-Response, response ] + - [ Content-Range, bytes 0-5/16 ] + + proxy-response: + status: 206 + headers: + fields: + - [ Content-Length, 6 ] + - [ X-Response, { value: response, as: equal } ] \ No newline at end of file diff --git a/tests/gold_tests/pluginTest/slice/slice_rm_range.test.py b/tests/gold_tests/pluginTest/slice/slice_rm_range.test.py new file mode 100644 index 00000000000..3bb72677357 --- /dev/null +++ b/tests/gold_tests/pluginTest/slice/slice_rm_range.test.py @@ -0,0 +1,69 @@ +''' +Verify ATS slice plugin config: @pparam=--enable-strip-range +''' +# 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 = ''' +Verify ATS slice plugin config: @pparam=--enable-strip-range +''' +Test.SkipUnless( + Condition.PluginExists('slice.so'), + Condition.PluginExists('cache_range_requests.so'), +) + +class SliceStripRangeForHeadRequestTest: + replay_file = "replay/slice_range.replay.yaml" + + def __init__(self): + """Initialize the Test processes for the test runs.""" + self._server = Test.MakeVerifierServerProcess("server", SliceStripRangeForHeadRequestTest.replay_file) + self._configure_trafficserver() + + + def _configure_trafficserver(self): + """Configure Traffic Server.""" + self._ts = Test.MakeATSProcess("ts", enable_cache=False) + + self._ts.Disk.remap_config.AddLines([ + f"map /no/range http://127.0.0.1:{self._server.Variables.http_port} \ + @plugin=slice.so @pparam=--blockbytes-test=10 @pparam=--enable-strip-range \ + @plugin=cache_range_requests.so", + f"map /with/range http://127.0.0.1:{self._server.Variables.http_port} \ + @plugin=slice.so @pparam=--blockbytes-test=10 \ + @plugin=cache_range_requests.so", + ]) + + self._ts.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http|slice', + }) + + def _test_head_request_range_header(self): + tr = Test.AddTestRun() + + tr.AddVerifierClientProcess( + "client", + SliceStripRangeForHeadRequestTest.replay_file, + http_ports=[self._ts.Variables.port]) + + tr.Processes.Default.StartBefore(self._server) + tr.Processes.Default.StartBefore(self._ts) + + def run(self): + self._test_head_request_range_header() + +SliceStripRangeForHeadRequestTest().run() From 233362249a9d28b15c1220c7741309a2b4a93fa4 Mon Sep 17 00:00:00 2001 From: Serris Lew Date: Fri, 9 Dec 2022 10:24:59 -0800 Subject: [PATCH 2/4] Remove trailing whitespaces --- tests/gold_tests/pluginTest/slice/slice_rm_range.test.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/gold_tests/pluginTest/slice/slice_rm_range.test.py b/tests/gold_tests/pluginTest/slice/slice_rm_range.test.py index 3bb72677357..65abb31ea32 100644 --- a/tests/gold_tests/pluginTest/slice/slice_rm_range.test.py +++ b/tests/gold_tests/pluginTest/slice/slice_rm_range.test.py @@ -25,6 +25,7 @@ Condition.PluginExists('cache_range_requests.so'), ) + class SliceStripRangeForHeadRequestTest: replay_file = "replay/slice_range.replay.yaml" @@ -33,7 +34,6 @@ def __init__(self): self._server = Test.MakeVerifierServerProcess("server", SliceStripRangeForHeadRequestTest.replay_file) self._configure_trafficserver() - def _configure_trafficserver(self): """Configure Traffic Server.""" self._ts = Test.MakeATSProcess("ts", enable_cache=False) @@ -59,11 +59,12 @@ def _test_head_request_range_header(self): "client", SliceStripRangeForHeadRequestTest.replay_file, http_ports=[self._ts.Variables.port]) - + tr.Processes.Default.StartBefore(self._server) tr.Processes.Default.StartBefore(self._ts) def run(self): self._test_head_request_range_header() + SliceStripRangeForHeadRequestTest().run() From 0af101ff6f2e22900616cdfd66d31ad2c7f8024d Mon Sep 17 00:00:00 2001 From: Serris Lew Date: Tue, 27 Dec 2022 22:20:15 -0800 Subject: [PATCH 3/4] Add Head to config name for clarity --- doc/admin-guide/plugins/slice.en.rst | 2 +- plugins/experimental/slice/Config.cc | 2 +- tests/gold_tests/pluginTest/slice/slice_rm_range.test.py | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/admin-guide/plugins/slice.en.rst b/doc/admin-guide/plugins/slice.en.rst index cea69fe62a1..c3a875eebc4 100644 --- a/doc/admin-guide/plugins/slice.en.rst +++ b/doc/admin-guide/plugins/slice.en.rst @@ -135,7 +135,7 @@ The slice plugin supports the following options:: can improve cache miss latency. -f for short - --enable-strip-range (optional) + --strip-range-for-head (optional) Enable slice plugin to strip Range header for HEAD requests. -h for short diff --git a/plugins/experimental/slice/Config.cc b/plugins/experimental/slice/Config.cc index 9b719571e68..65ca648ba8f 100644 --- a/plugins/experimental/slice/Config.cc +++ b/plugins/experimental/slice/Config.cc @@ -124,7 +124,7 @@ Config::fromArgs(int const argc, char const *const argv[]) {const_cast("skip-header"), required_argument, nullptr, 's'}, {const_cast("blockbytes-test"), required_argument, nullptr, 't'}, {const_cast("prefetch-count"), required_argument, nullptr, 'f'}, - {const_cast("enable-strip-range"), no_argument, nullptr, 'h'}, + {const_cast("strip-range-for-head"), no_argument, nullptr, 'h'}, {nullptr, 0, nullptr, 0}, }; diff --git a/tests/gold_tests/pluginTest/slice/slice_rm_range.test.py b/tests/gold_tests/pluginTest/slice/slice_rm_range.test.py index 65abb31ea32..9f7b887c2e6 100644 --- a/tests/gold_tests/pluginTest/slice/slice_rm_range.test.py +++ b/tests/gold_tests/pluginTest/slice/slice_rm_range.test.py @@ -1,5 +1,5 @@ ''' -Verify ATS slice plugin config: @pparam=--enable-strip-range +Verify ATS slice plugin config: @pparam=--strip-range-for-head ''' # Licensed to the Apache Software Foundation (ASF) under one # or more contributor license agreements. See the NOTICE file @@ -18,7 +18,7 @@ # limitations under the License. Test.Summary = ''' -Verify ATS slice plugin config: @pparam=--enable-strip-range +Verify ATS slice plugin config: @pparam=--strip-range-for-head ''' Test.SkipUnless( Condition.PluginExists('slice.so'), @@ -40,7 +40,7 @@ def _configure_trafficserver(self): self._ts.Disk.remap_config.AddLines([ f"map /no/range http://127.0.0.1:{self._server.Variables.http_port} \ - @plugin=slice.so @pparam=--blockbytes-test=10 @pparam=--enable-strip-range \ + @plugin=slice.so @pparam=--blockbytes-test=10 @pparam=--strip-range-for-head \ @plugin=cache_range_requests.so", f"map /with/range http://127.0.0.1:{self._server.Variables.http_port} \ @plugin=slice.so @pparam=--blockbytes-test=10 \ From d681949e1d6b976772e500fb875995504f557de8 Mon Sep 17 00:00:00 2001 From: Serris Lew Date: Tue, 3 Jan 2023 15:19:48 -0800 Subject: [PATCH 4/4] Add verification directives to autest replay --- .../pluginTest/slice/replay/slice_range.replay.yaml | 5 +++-- tests/gold_tests/pluginTest/slice/slice_rm_range.test.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/gold_tests/pluginTest/slice/replay/slice_range.replay.yaml b/tests/gold_tests/pluginTest/slice/replay/slice_range.replay.yaml index 2fd685b2417..793ec66c57d 100644 --- a/tests/gold_tests/pluginTest/slice/replay/slice_range.replay.yaml +++ b/tests/gold_tests/pluginTest/slice/replay/slice_range.replay.yaml @@ -48,7 +48,7 @@ sessions: headers: fields: - [ X-Response, { value: response, as: equal } ] - - [ Content-Length, 16 ] + - [ Content-Length, { value: "16", as: equal} ] - client-request: method: "HEAD" @@ -78,5 +78,6 @@ sessions: status: 206 headers: fields: - - [ Content-Length, 6 ] + - [ Content-Length, { value: "6", as: equal} ] + - [ Content-Range, { value: "bytes 0-5/16", as: equal} ] - [ X-Response, { value: response, as: equal } ] \ No newline at end of file diff --git a/tests/gold_tests/pluginTest/slice/slice_rm_range.test.py b/tests/gold_tests/pluginTest/slice/slice_rm_range.test.py index 9f7b887c2e6..ab8101a3443 100644 --- a/tests/gold_tests/pluginTest/slice/slice_rm_range.test.py +++ b/tests/gold_tests/pluginTest/slice/slice_rm_range.test.py @@ -49,7 +49,7 @@ def _configure_trafficserver(self): self._ts.Disk.records_config.update({ 'proxy.config.diags.debug.enabled': 1, - 'proxy.config.diags.debug.tags': 'http|slice', + 'proxy.config.diags.debug.tags': 'http|slice|cache_range_requests', }) def _test_head_request_range_header(self):