From ec89e6d5e415930b19d9c16b14af18f0a30049bb Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Fri, 2 Dec 2022 20:29:41 +0000 Subject: [PATCH 1/5] prefetch: Add support for cmcd-request nor prefetch handling --- doc/admin-guide/plugins/prefetch.en.rst | 24 ++ plugins/prefetch/common.h | 1 + plugins/prefetch/configs.cc | 12 +- plugins/prefetch/configs.h | 11 +- plugins/prefetch/plugin.cc | 206 ++++++++---- .../pluginTest/prefetch/header_rewrite.conf | 19 ++ .../pluginTest/prefetch/prefetch_cmcd.test.py | 311 ++++++++++++++++++ .../pluginTest/prefetch/prefetch_cmcd0.gold | 13 + .../pluginTest/prefetch/prefetch_cmcd1.gold | 12 + .../prefetch_simple.gold | 0 .../prefetch_simple.test.py | 0 11 files changed, 544 insertions(+), 65 deletions(-) create mode 100644 tests/gold_tests/pluginTest/prefetch/header_rewrite.conf create mode 100644 tests/gold_tests/pluginTest/prefetch/prefetch_cmcd.test.py create mode 100644 tests/gold_tests/pluginTest/prefetch/prefetch_cmcd0.gold create mode 100644 tests/gold_tests/pluginTest/prefetch/prefetch_cmcd1.gold rename tests/gold_tests/pluginTest/{prefetch_simple => prefetch}/prefetch_simple.gold (100%) rename tests/gold_tests/pluginTest/{prefetch_simple => prefetch}/prefetch_simple.test.py (100%) diff --git a/doc/admin-guide/plugins/prefetch.en.rst b/doc/admin-guide/plugins/prefetch.en.rst index 74312d93439..3a23d8dc320 100644 --- a/doc/admin-guide/plugins/prefetch.en.rst +++ b/doc/admin-guide/plugins/prefetch.en.rst @@ -170,6 +170,29 @@ specified with an integer followed by a colon, e.g. ``{8:$2+2}``, causing the resulting number to be padded with leading zeroes if it has fewer digits than the width. +CMCD (Common Media Client Data) CMCD-Request header with nor field +------------------------------------------------------------------ + +If the ``--cmcd-nor`` option is specified the Cmcd-Request header with nor field is handled. + +With setup :: + + map http://example.com http://origin.com \ + @plugin=cachekey.so @pparam=--remove-all-params=true \ + @plugin=prefetch.so \ + @pparam=--cmcd-nor=true + +If the incoming request is :: + + http://example-seed.com/path/someitem.m4a + +with header :: + + Cmcd-Request: nor="otheritem.m4a" + +The following URL will be requested to be prefetched :: + + http://example-seed.com/path/otheritem.m4a Overhead from **next object** prefetch -------------------------------------- @@ -238,6 +261,7 @@ Plugin parameters - ``true`` - configures the plugin run on the **front-tier**, - ``false`` - to be run on the **back-tier**. * ``--api-header`` - the header used by the plugin internally, also used to mark a prefetch request to the next tier in dual-tier usage. +* ``--cmcd-nor`` - prefetch for a Cmcd-Request header with nor field. * ``--fetch-policy`` - fetch policy - ``simple`` - this policy just makes sure there are no same concurrent prefetches triggered (default and always used in combination with any other policy) - ``lru:n`` - this policy uses LRU to identify "hot" objects and triggers prefetch if the object is not found. `n` is the size of the LRU diff --git a/plugins/prefetch/common.h b/plugins/prefetch/common.h index 35bbab0d9c6..8c0ae36ae49 100644 --- a/plugins/prefetch/common.h +++ b/plugins/prefetch/common.h @@ -31,6 +31,7 @@ #include typedef std::string String; +typedef std::string_view StringView; typedef std::set StringSet; typedef std::list StringList; typedef std::vector StringVector; diff --git a/plugins/prefetch/configs.cc b/plugins/prefetch/configs.cc index 21c2ffa811d..3589b2fd12e 100644 --- a/plugins/prefetch/configs.cc +++ b/plugins/prefetch/configs.cc @@ -54,9 +54,11 @@ isTrue(const char *arg) bool PrefetchConfig::init(int argc, char *argv[]) { +<<<<<<< HEAD static const struct option longopt[] = { {const_cast("front"), optional_argument, nullptr, 'f'}, {const_cast("api-header"), optional_argument, nullptr, 'h'}, + {const_cast("cmcd-nor"), optional_argument, nullptr, 'd'}, {const_cast("next-header"), optional_argument, nullptr, 'n'}, {const_cast("fetch-policy"), optional_argument, nullptr, 'p'}, {const_cast("fetch-count"), optional_argument, nullptr, 'c'}, @@ -68,15 +70,15 @@ PrefetchConfig::init(int argc, char *argv[]) {const_cast("metrics-prefix"), optional_argument, nullptr, 'm'}, {const_cast("exact-match"), optional_argument, nullptr, 'y'}, {const_cast("log-name"), optional_argument, nullptr, 'l'}, - {nullptr, 0, nullptr, 0 } + {nullptr, 0, nullptr, 0 }, }; bool status = true; optind = 0; /* argv contains the "to" and "from" URLs. Skip the first so that the second one poses as the program name. */ - argc--; - argv++; + --argc; + ++argv; for (;;) { int opt; @@ -97,6 +99,10 @@ PrefetchConfig::init(int argc, char *argv[]) setApiHeader(optarg); break; + case 'd': /* --cmcd-nor */ + _cmcd_nor = ::isTrue(optarg); + break; + case 'n': /* --next-header */ setNextHeader(optarg); break; diff --git a/plugins/prefetch/configs.h b/plugins/prefetch/configs.h index 1996f4fcef2..c1058c5e969 100644 --- a/plugins/prefetch/configs.h +++ b/plugins/prefetch/configs.h @@ -35,8 +35,8 @@ class PrefetchConfig { public: PrefetchConfig() - : _apiHeader("X-AppleCDN-Prefetch"), - _nextHeader("X-AppleCDN-Prefetch-Next"), + : _apiHeader("X-CDN-Prefetch"), + _nextHeader("X-CDN-Prefetch-Next"), _replaceHost(), _namespace("default"), _metricsPrefix("prefetch.stats") @@ -111,6 +111,12 @@ class PrefetchConfig return _exactMatch; } + bool + isCmcdNor() const + { + return _cmcd_nor; + } + void setFetchCount(const char *optarg) { @@ -208,5 +214,6 @@ class PrefetchConfig unsigned _fetchMax = 0; bool _front = false; bool _exactMatch = false; + bool _cmcd_nor = false; MultiPattern _nextPaths; }; diff --git a/plugins/prefetch/plugin.cc b/plugins/prefetch/plugin.cc index ef1d2991d96..6554afa6cff 100644 --- a/plugins/prefetch/plugin.cc +++ b/plugins/prefetch/plugin.cc @@ -20,9 +20,7 @@ * @file plugin.cc * @brief traffic server plugin entry points. */ - #include -#include #include "ts/ts.h" /* ATS API */ @@ -227,8 +225,12 @@ appendCacheKey(const TSHttpTxn txnp, const TSMBuffer reqBuffer, String &key) TSfree(static_cast(url)); ret = true; } + } else { + PrefetchDebug("Failure lookup up cache url"); } TSHandleMLocRelease(reqBuffer, TS_NULL_MLOC, keyLoc); + } else { + PrefetchDebug("Failure creating url"); } if (!ret) { @@ -348,6 +350,53 @@ getPristineUrlQuery(TSHttpTxn txnp) return pristineQuery; } +static StringView const CmcdHeader{"Cmcd-Request"}; +static StringView const CmcdNorFieldPrefix{"nor="}; + +/** + * @brief Look for and return the nor field of any Cmcd-Request header + * + * @param txnp HTTP transaction structure + * @param buffer request TSMBuffer + * @param hdrloc request TSMLoc + * @return unquoted relative cmcd path from the nor field + * + * sample header: + * Cmcd-Request: mtp=103600,bl=153500,nor="14_176.mp4a" + */ +static String +getCmcdNor(TSMBuffer buffer, TSMLoc hdrloc) +{ + String relpath; + TSMLoc const cmcdloc = TSMimeHdrFieldFind(buffer, hdrloc, CmcdHeader.data(), CmcdHeader.length()); + if (TS_NULL_MLOC != cmcdloc) { + // iterate through the fields + int const cnt = TSMimeHdrFieldValuesCount(buffer, hdrloc, cmcdloc); + for (int ind = 0; ind < cnt; ++ind) { + int flen = 0; + char const *const fval = TSMimeHdrFieldValueStringGet(buffer, hdrloc, cmcdloc, ind, &flen); + + StringView fv(fval, flen); + PrefetchDebug("cmcd-request field: %.*s", (int)fv.length(), fv.data()); + if (0 == fv.compare(0, CmcdNorFieldPrefix.length(), CmcdNorFieldPrefix)) { + fv.remove_prefix(CmcdNorFieldPrefix.length()); + if (fv.front() == '"') { + fv.remove_prefix(1); + } + if (fv.back() == '"') { + fv.remove_suffix(1); + } + relpath = fv; + break; + } + } + TSHandleMLocRelease(buffer, hdrloc, cmcdloc); + } else { + PrefetchDebug("No Cmcd-Request header found"); + } + return relpath; +} + /** * @brief short-cut to set the response . */ @@ -425,19 +474,26 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) TSHttpTxn txnp = static_cast(edata); PrefetchConfig &config = data->_inst->_config; BgFetchState *state = data->_inst->_state; - TSMBuffer reqBuffer; - TSMLoc reqHdrLoc; + TSMBuffer reqBuffer = nullptr; + TSMLoc reqHdrLoc = TS_NULL_MLOC; PrefetchDebug("event: %s (%d)", getEventName(event), event); TSEvent retEvent = TS_EVENT_HTTP_CONTINUE; - if ((event == TS_EVENT_HTTP_POST_REMAP || event == TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE || - event == TS_EVENT_HTTP_SEND_RESPONSE_HDR) && - TS_SUCCESS != TSHttpTxnClientReqGet(txnp, &reqBuffer, &reqHdrLoc)) { - PrefetchError("failed to get client request"); - TSHttpTxnReenable(txnp, TS_EVENT_HTTP_ERROR); - return 0; + // For these cases we need to access the client request + switch (event) { + case TS_EVENT_HTTP_POST_REMAP: + case TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE: + case TS_EVENT_HTTP_SEND_RESPONSE_HDR: + if (TS_SUCCESS != TSHttpTxnClientReqGet(txnp, &reqBuffer, &reqHdrLoc)) { + PrefetchError("failed to get client request"); + TSHttpTxnReenable(txnp, TS_EVENT_HTTP_ERROR); + return 0; + } + break; + default: + break; } switch (event) { @@ -450,7 +506,7 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) } if (!appendCacheKey(txnp, reqBuffer, data->_cachekey)) { PrefetchError("failed to get the cache key"); - TSHttpTxnReenable(txnp, TS_EVENT_HTTP_ERROR); + TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); return 0; } @@ -460,7 +516,7 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) /* first-pass */ if (!config.isExactMatch()) { data->_fetchable = state->acquire(data->_cachekey); - PrefetchDebug("request is %s fetchable", data->_fetchable ? " " : " not "); + PrefetchDebug("request is %s fetchable", data->_fetchable ? "" : "not"); } } } @@ -506,69 +562,99 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) if (data->frontend()) { /* front-end instance */ - String currentPath = getPristineUrlPath(txnp); - String currentQuery = getPristineUrlQuery(txnp); - bool hasValidQuery = false; + String const currentPath = getPristineUrlPath(txnp); + String const currentQuery = getPristineUrlQuery(txnp); + bool hasValidQuery = false; // If there is a --fetch-query defined in the config, and that string is found in the querystring, assume it is // valid, and prefer the --fetch-query over the --fetch-path-pattern(s). - if (!config.getQueryKeyName().empty() && currentQuery.find(config.getQueryKeyName()) != std::string::npos) { + if (!config.getQueryKeyName().empty() && currentQuery.find(config.getQueryKeyName()) != String::npos) { PrefetchDebug("Setting hasValidQuery to true"); hasValidQuery = true; } - if (!hasValidQuery && data->firstPass() && data->_fetchable && !config.getNextPath().empty() && respToTriggerPrefetch(txnp)) { - /* Trigger all necessary background fetches based on the next path pattern */ + if (data->firstPass() && data->_fetchable && respToTriggerPrefetch(txnp)) { + // Configured to handle cmcd-request nor + if (config.isCmcdNor()) { + PrefetchDebug("Considering cmcd nor request"); - if (!currentPath.empty()) { - unsigned total = config.getFetchCount(); - for (unsigned i = 0; i < total; ++i) { - PrefetchDebug("generating prefetch request %d/%d", i + 1, total); - String expandedPath; + TSAssert(nullptr != reqBuffer); + TSAssert(TS_NULL_MLOC != reqHdrLoc); - if (config.getNextPath().replace(currentPath, expandedPath)) { - PrefetchDebug("replaced: %s", expandedPath.c_str()); - expand(expandedPath); - PrefetchDebug("expanded: %s cachekey: %s", expandedPath.c_str(), data->_cachekey.c_str()); + String const relpath = getCmcdNor(reqBuffer, reqHdrLoc); + if (!relpath.empty()) { + PrefetchDebug("Current path: '%s'", currentPath.c_str()); + PrefetchDebug("Parsed cmcd nor relpath: '%s'", relpath.c_str()); - BgFetch::schedule(state, config, /* askPermission */ false, reqBuffer, reqHdrLoc, txnp, expandedPath.c_str(), - expandedPath.length(), data->_cachekey); - } else { - /* We should be here only if the pattern replacement fails (match already checked) */ - PrefetchError("failed to process the pattern"); + String::size_type const lsi = currentPath.find_last_of("/"); + String const nextPath = currentPath.substr(0, lsi + 1) + relpath; - /* If the first or any matches fails there must be something wrong so don't continue */ - break; + PrefetchDebug("Next cmcd nor path: '%s'", nextPath.c_str()); + + // ensure we aren't prefetching the current asset + if (nextPath != currentPath) { + constexpr bool askPermission = false; + BgFetch::schedule(state, config, askPermission, reqBuffer, reqHdrLoc, txnp, nextPath.c_str(), nextPath.length(), + data->_cachekey); + } else { + PrefetchDebug("Next cmcd path same as current path '%s', skipping!", currentPath.c_str()); } - currentPath.assign(expandedPath); } - } else { - PrefetchDebug("failed to get current path"); } - } - if (hasValidQuery && data->firstPass() && data->_fetchable && respToTriggerPrefetch(txnp)) { - /* Trigger all necessary background fetches based on the query string(s) */ - - PrefetchDebug("currentQuery: %s", currentQuery.c_str()); - size_t lastSlashIndex = currentPath.find_last_of("/"); - size_t keyLen = config.getQueryKeyName().size(); - unsigned done = 1; - std::istringstream cStringStream(currentQuery); - std::string param; - - while (getline(cStringStream, param, '&')) { - if (param.find(config.getQueryKeyName()) != 0) { - continue; - } - if (config.getFetchCount() < done++) { - break; + + if (!hasValidQuery && !config.getNextPath().empty()) { + /* Trigger all necessary background fetches based on the next path pattern */ + + if (!currentPath.empty()) { + unsigned const total = config.getFetchCount(); + String workingPath = currentPath; + for (unsigned i = 0; i < total; ++i) { + PrefetchDebug("generating prefetch request %d/%d", i + 1, total); + String expandedPath; + + if (config.getNextPath().replace(workingPath, expandedPath)) { + PrefetchDebug("replaced: %s", expandedPath.c_str()); + expand(expandedPath); + PrefetchDebug("expanded: %s cachekey: %s", expandedPath.c_str(), data->_cachekey.c_str()); + + BgFetch::schedule(state, config, /* askPermission */ false, reqBuffer, reqHdrLoc, txnp, expandedPath.c_str(), + expandedPath.length(), data->_cachekey); + } else { + /* We should be here only if the pattern replacement fails (match already checked) */ + PrefetchError("failed to process the pattern"); + + /* If the first or any matches fails there must be something wrong so don't continue */ + break; + } + workingPath.assign(expandedPath); + } + } else { + PrefetchDebug("failed to get current path"); } - std::string nextFile = param.substr(keyLen + 1); // +1 for the '=' - std::string nextPath = currentPath.substr(0, lastSlashIndex + 1) + nextFile; + } else if (hasValidQuery) { + /* Trigger all necessary background fetches based on the query string(s) */ + + PrefetchDebug("currentQuery: %s", currentQuery.c_str()); + size_t const lastSlashIndex = currentPath.find_last_of("/"); + size_t const keyLen = config.getQueryKeyName().size(); + unsigned done = 1; + std::istringstream cStringStream(currentQuery); + String param; + + while (getline(cStringStream, param, '&')) { + if (param.find(config.getQueryKeyName()) != 0) { + continue; + } + if (config.getFetchCount() < done++) { + break; + } + String nextFile = param.substr(keyLen + 1); // +1 for the '=' + String nextPath = currentPath.substr(0, lastSlashIndex + 1) + nextFile; - PrefetchDebug("nextPath %s, cacheKey %s", nextPath.c_str(), data->_cachekey.c_str()); - BgFetch::schedule(state, config, /* askPermission */ false, reqBuffer, reqHdrLoc, txnp, nextPath.c_str(), - nextPath.length(), data->_cachekey); + PrefetchDebug("nextPath %s, cacheKey %s", nextPath.c_str(), data->_cachekey.c_str()); + BgFetch::schedule(state, config, /* askPermission */ false, reqBuffer, reqHdrLoc, txnp, nextPath.c_str(), + nextPath.length(), data->_cachekey); + } } } } @@ -720,7 +806,7 @@ TSRemapDoRemap(void *instance, TSHttpTxn txnp, TSRemapRequestInfo *rri) /* Make sure we handle only URLs that match the path pattern on the front-end + first-pass, cancel otherwise */ bool handleFetch = true; - if (front && firstPass) { + if (front && firstPass && !config.isCmcdNor()) { /* Front-end plug-in instance + first pass. */ if (config.getNextPath().empty()) { /* No next path pattern specified then pass this request untouched. */ diff --git a/tests/gold_tests/pluginTest/prefetch/header_rewrite.conf b/tests/gold_tests/pluginTest/prefetch/header_rewrite.conf new file mode 100644 index 00000000000..04631fa6a5e --- /dev/null +++ b/tests/gold_tests/pluginTest/prefetch/header_rewrite.conf @@ -0,0 +1,19 @@ +# +# 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. + +cond %{TXN_START_HOOK} +set-header x-debug x-cache-key diff --git a/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd.test.py b/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd.test.py new file mode 100644 index 00000000000..d3feae0fd94 --- /dev/null +++ b/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd.test.py @@ -0,0 +1,311 @@ +''' +''' +# 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. + +import os +Test.Summary = ''' +Test prefetch.so plugin (simple mode). +''' + +origin = Test.MakeOriginServer("origin") + +asset_name = 'request.txt' +prefetch_name = 'prefetch.txt' +prefetch_header = f'Cmcd-Request: foo=12,nor="{prefetch_name}",bar=42' + +request_header = { + "headers": + f"GET /tests/{asset_name} HTTP/1.1\r\n" + "Host: does.not.matter\r\n" # But cant be omitted + f"{prefetch_header}\r\n" + "\r\n", + "timestamp": "1469733493.993", + "body": "" +} +response_header = { + "headers": + "HTTP/1.1 200 OK\r\n" + "Connection: close\r\n" + "Cache-control: max-age=60\r\n" + "\r\n", + "timestamp": "1469733493.993", + "body": f"This is the body for {asset_name}\n" +} +origin.addResponse("sessionlog.json", request_header, response_header) + +# query string +query_name = 'query?this=foo&that' +query_header = f'Cmcd-Request: nor="{query_name}"' + +request_header = { + "headers": + f"GET /tests/query.txt HTTP/1.1\r\n" + "Host: does.not.matter\r\n" # But cant be omitted + f"{query_header}\r\n" + "\r\n", + "timestamp": "1469733493.993", + "body": "" +} +response_header = { + "headers": + "HTTP/1.1 200 OK\r\n" + "Connection: close\r\n" + "Cache-control: max-age=60\r\n" + "\r\n", + "timestamp": "1469733493.993", + "body": f"This is the body for {query_name}\n" +} +origin.addResponse("sessionlog.json", request_header, response_header) + +# fail tests +same_name = 'same.txt' +same_header = f'Cmcd-Request: nor="{same_name}"' + +# setup the prefetched assets +names = [prefetch_name, query_name, same_name] + +for name in names: + request_header = { + "headers": + f"GET /tests/{name} HTTP/1.1\r\n" + "Host: does.not.matter\r\n" # But cant be omitted + "\r\n", + "timestamp": "1469733493.993", + "body": "" + } + response_header = { + "headers": + "HTTP/1.1 200 OK\r\n" + "Connection: close\r\n" + "Cache-control: max-age=60\r\n" + "\r\n", + "timestamp": "1469733493.993", + "body": f"This is the body for {name}\n" + } + origin.addResponse("sessionlog.json", request_header, response_header) + +# prefetch from root +root_name = 'root.txt' +root_header = f'Cmcd-Request: nor="rooted"' + +request_header = { + "headers": + f"GET /{root_name} HTTP/1.1\r\n" + "Host: does.not.matter\r\n" # But cant be omitted + f"{root_header}\r\n" + "\r\n", + "timestamp": "1469733493.993", + "body": "" +} +response_header = { + "headers": + "HTTP/1.1 200 OK\r\n" + "Connection: close\r\n" + "Cache-control: max-age=60\r\n" + "\r\n", + "timestamp": "1469733493.993", + "body": f"This is the body for {root_name}\n" +} +origin.addResponse("sessionlog.json", request_header, response_header) + +request_header = { + "headers": + f"GET /rooted HTTP/1.1\r\n" + "Host: does.not.matter\r\n" # But cant be omitted + "\r\n", + "timestamp": "1469733493.993", + "body": "" +} +response_header = { + "headers": + "HTTP/1.1 200 OK\r\n" + "Connection: close\r\n" + "Cache-control: max-age=60\r\n" + "\r\n", + "timestamp": "1469733493.993", + "body": f"This is the body for rooted\n" +} +origin.addResponse("sessionlog.json", request_header, response_header) + +# allows for multiple ats on localhost +dns = Test.MakeDNServer("dns") + +# next hop trafficserver instance +ts1 = Test.MakeATSProcess("ts1", command="traffic_server 2>trace1.log") +ts1.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'prefetch|http', + 'proxy.config.dns.nameservers': f"127.0.0.1:{dns.Variables.Port}", + 'proxy.config.dns.resolv_conf': "NULL", + 'proxy.config.http.parent_proxy.self_detect': 0, +}) +dns.addRecords(records={f"ts1": ["127.0.0.1"]}) +ts1.Disk.remap_config.AddLine( + f"map / http://127.0.0.1:{origin.Variables.Port}" + + " @plugin=cachekey.so @pparam==--remove-all-params=true" + " @plugin=prefetch.so @pparam==--front=false" +) + +ts1.Disk.logging_yaml.AddLines( + ''' +logging: + formats: + - name: custom + format: '% % % % % %<{X-CDN-Prefetch}cqh>' + logs: + - filename: transaction + format: custom +'''.split("\n") +) + +ts0 = Test.MakeATSProcess("ts0", command="traffic_server 2> trace0.log") +ts0.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'prefetch|http', + 'proxy.config.dns.nameservers': f"127.0.0.1:{dns.Variables.Port}", + 'proxy.config.dns.resolv_conf': "NULL", + 'proxy.config.http.parent_proxy.self_detect': 0, +}) + +dns.addRecords(records={f"ts0": ["127.0.0.1"]}) +ts0.Disk.remap_config.AddLine( + f"map http://ts0 http://ts1:{ts1.Variables.port}" + + " @plugin=cachekey.so @pparam=--remove-all-params=true" + " @plugin=prefetch.so" + + " @pparam=--front=true" + + " @pparam=--fetch-policy=simple" + + " @pparam=--cmcd-nor=true" +) + +ts0.Disk.logging_yaml.AddLines( + ''' +logging: + formats: + - name: custom + format: '% % % % % %<{X-CDN-Prefetch}cqh>' + logs: + - filename: transaction + format: custom +'''.split("\n") +) + + +# start everything up +tr = Test.AddTestRun() +tr.Processes.Default.StartBefore(origin) +tr.Processes.Default.StartBefore(dns) +tr.Processes.Default.StartBefore(ts0) +tr.Processes.Default.StartBefore(ts1) +tr.Processes.Default.Command = 'echo start TS, TSH_N, HTTP origin and DNS.' +tr.Processes.Default.ReturnCode = 0 + +# attempt to get normal asset +tr = Test.AddTestRun() +tr.Processes.Default.Command = ( + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/{asset_name}" +) +tr.Processes.Default.ReturnCode = 0 + +# issue curl form same asset, with prefetch +tr = Test.AddTestRun() +tr.DelayStart = 1 +tr.Processes.Default.Command = ( + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/{asset_name} -H \'{prefetch_header}\'" +) +tr.Processes.Default.ReturnCode = 0 + +# fetch the prefetched asset (only cached on ts1) +tr = Test.AddTestRun() +tr.DelayStart = 1 +tr.Processes.Default.Command = ( + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/{prefetch_name}" +) +tr.Processes.Default.ReturnCode = 0 + +# attempt to prefetch again +tr = Test.AddTestRun() +tr.Processes.Default.Command = ( + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/{asset_name} -H \'{prefetch_header}\'" +) +tr.Processes.Default.ReturnCode = 0 + +# request the prefetched asset +tr = Test.AddTestRun() +tr.Processes.Default.Command = ( + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/{prefetch_name}" +) +tr.Processes.Default.ReturnCode = 0 + +# query path +tr = Test.AddTestRun() +tr.Processes.Default.Command = ( + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/query.txt -H \'{query_header}\'" +) +tr.Processes.Default.ReturnCode = 0 + +# request the prefetched asset +tr = Test.AddTestRun() +tr.Processes.Default.Command = ( + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} \'http://ts0/tests/{query_name}\'" +) +tr.Processes.Default.ReturnCode = 0 + +# same path (reject) +tr = Test.AddTestRun() +tr.Processes.Default.Command = ( + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/{same_name} -H \'{same_header}\'" +) +tr.Processes.Default.ReturnCode = 0 + +# root path +tr = Test.AddTestRun() +tr.Processes.Default.Command = ( + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} \'http://ts0/{root_name}\' -H \'{root_header}\'" +) +tr.Processes.Default.ReturnCode = 0 + +condwaitpath = os.path.join(Test.Variables.AtsTestToolsDir, 'condwait') + +# look for ts transaction log +ts0log = os.path.join(ts0.Variables.LOGDIR, 'transaction.log') +tr = Test.AddTestRun() +ps = tr.Processes.Default +ps.Command = ( + condwaitpath + ' 60 1 -f ' + ts0log +) + +# look for ts1 transaction log +ts1log = os.path.join(ts1.Variables.LOGDIR, 'transaction.log') +tr = Test.AddTestRun() +ps = tr.Processes.Default +ps.Command = ( + condwaitpath + ' 60 1 -f ' + ts1log +) + +tr = Test.AddTestRun() +tr.Processes.Default.Command = ( + f"cat {ts0log}" +) +tr.Streams.stdout = "prefetch_cmcd0.gold" +tr.Processes.Default.ReturnCode = 0 + +tr = Test.AddTestRun() +tr.Processes.Default.Command = ( + f"cat {ts1log}" +) +tr.Streams.stdout = "prefetch_cmcd1.gold" +tr.Processes.Default.ReturnCode = 0 diff --git a/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd0.gold b/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd0.gold new file mode 100644 index 00000000000..59e6e55dc68 --- /dev/null +++ b/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd0.gold @@ -0,0 +1,13 @@ +tests/request.txt 200 TCP_MISS FIN 33 - +tests/request.txt 200 TCP_HIT - 33 - +tests/prefetch.txt 200 TCP_MISS - 16 tests/request.txt +tests/prefetch.txt 200 TCP_MISS FIN 34 - +tests/request.txt 200 TCP_MEM_HIT - 33 - +tests/prefetch.txt 208 TCP_HIT - 20 tests/request.txt +tests/prefetch.txt 200 TCP_MEM_HIT - 34 - +tests/query.txt 200 TCP_MISS FIN 41 - +tests/query 200 TCP_MISS - 16 tests/query.txt +tests/query 200 TCP_MISS FIN 41 - +tests/same.txt 200 TCP_MISS FIN 30 - +root.txt 200 TCP_MISS FIN 30 - +rooted 200 TCP_MISS - 16 root.txt diff --git a/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd1.gold b/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd1.gold new file mode 100644 index 00000000000..f353701a2bd --- /dev/null +++ b/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd1.gold @@ -0,0 +1,12 @@ +tests/request.txt 200 TCP_MISS FIN 33 - +tests/prefetch.txt 200 TCP_MISS - 16 tests/request.txt +tests/prefetch.txt 200 TCP_MISS FIN 34 - +tests/prefetch.txt 200 TCP_HIT - 34 - +tests/query.txt 200 TCP_MISS FIN 41 - +tests/query 200 TCP_MISS - 16 tests/query.txt +tests/query 200 TCP_MISS FIN 41 - +tests/query 200 TCP_HIT - 41 - +tests/same.txt 200 TCP_MISS FIN 30 - +root.txt 200 TCP_MISS FIN 30 - +rooted 200 TCP_MISS - 16 root.txt +rooted 200 TCP_MISS FIN 28 - diff --git a/tests/gold_tests/pluginTest/prefetch_simple/prefetch_simple.gold b/tests/gold_tests/pluginTest/prefetch/prefetch_simple.gold similarity index 100% rename from tests/gold_tests/pluginTest/prefetch_simple/prefetch_simple.gold rename to tests/gold_tests/pluginTest/prefetch/prefetch_simple.gold diff --git a/tests/gold_tests/pluginTest/prefetch_simple/prefetch_simple.test.py b/tests/gold_tests/pluginTest/prefetch/prefetch_simple.test.py similarity index 100% rename from tests/gold_tests/pluginTest/prefetch_simple/prefetch_simple.test.py rename to tests/gold_tests/pluginTest/prefetch/prefetch_simple.test.py From 4454d3e7660d4d1244cb104bc1a475e9a6bdae00 Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Tue, 31 Jan 2023 13:26:46 +0000 Subject: [PATCH 2/5] prefetch: restore failing behavior on missing cachekey --- plugins/prefetch/configs.cc | 1 - plugins/prefetch/plugin.cc | 30 +++++++++++++++--------------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/plugins/prefetch/configs.cc b/plugins/prefetch/configs.cc index 3589b2fd12e..0c762971977 100644 --- a/plugins/prefetch/configs.cc +++ b/plugins/prefetch/configs.cc @@ -54,7 +54,6 @@ isTrue(const char *arg) bool PrefetchConfig::init(int argc, char *argv[]) { -<<<<<<< HEAD static const struct option longopt[] = { {const_cast("front"), optional_argument, nullptr, 'f'}, {const_cast("api-header"), optional_argument, nullptr, 'h'}, diff --git a/plugins/prefetch/plugin.cc b/plugins/prefetch/plugin.cc index 6554afa6cff..5c52ea6da2a 100644 --- a/plugins/prefetch/plugin.cc +++ b/plugins/prefetch/plugin.cc @@ -350,8 +350,8 @@ getPristineUrlQuery(TSHttpTxn txnp) return pristineQuery; } -static StringView const CmcdHeader{"Cmcd-Request"}; -static StringView const CmcdNorFieldPrefix{"nor="}; +static const StringView CmcdHeader{"Cmcd-Request"}; +static const StringView CmcdNorFieldPrefix{"nor="}; /** * @brief Look for and return the nor field of any Cmcd-Request header @@ -365,16 +365,16 @@ static StringView const CmcdNorFieldPrefix{"nor="}; * Cmcd-Request: mtp=103600,bl=153500,nor="14_176.mp4a" */ static String -getCmcdNor(TSMBuffer buffer, TSMLoc hdrloc) +getCmcdNor(const TSMBuffer buffer, const TSMLoc hdrloc) { String relpath; - TSMLoc const cmcdloc = TSMimeHdrFieldFind(buffer, hdrloc, CmcdHeader.data(), CmcdHeader.length()); + const TSMLoc cmcdloc = TSMimeHdrFieldFind(buffer, hdrloc, CmcdHeader.data(), CmcdHeader.length()); if (TS_NULL_MLOC != cmcdloc) { // iterate through the fields - int const cnt = TSMimeHdrFieldValuesCount(buffer, hdrloc, cmcdloc); + const int cnt = TSMimeHdrFieldValuesCount(buffer, hdrloc, cmcdloc); for (int ind = 0; ind < cnt; ++ind) { int flen = 0; - char const *const fval = TSMimeHdrFieldValueStringGet(buffer, hdrloc, cmcdloc, ind, &flen); + const char *const fval = TSMimeHdrFieldValueStringGet(buffer, hdrloc, cmcdloc, ind, &flen); StringView fv(fval, flen); PrefetchDebug("cmcd-request field: %.*s", (int)fv.length(), fv.data()); @@ -506,7 +506,7 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) } if (!appendCacheKey(txnp, reqBuffer, data->_cachekey)) { PrefetchError("failed to get the cache key"); - TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE); + TSHttpTxnReenable(txnp, TS_EVENT_HTTP_ERROR); return 0; } @@ -562,8 +562,8 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) if (data->frontend()) { /* front-end instance */ - String const currentPath = getPristineUrlPath(txnp); - String const currentQuery = getPristineUrlQuery(txnp); + const String currentPath = getPristineUrlPath(txnp); + const String currentQuery = getPristineUrlQuery(txnp); bool hasValidQuery = false; // If there is a --fetch-query defined in the config, and that string is found in the querystring, assume it is @@ -581,13 +581,13 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) TSAssert(nullptr != reqBuffer); TSAssert(TS_NULL_MLOC != reqHdrLoc); - String const relpath = getCmcdNor(reqBuffer, reqHdrLoc); + const String relpath = getCmcdNor(reqBuffer, reqHdrLoc); if (!relpath.empty()) { PrefetchDebug("Current path: '%s'", currentPath.c_str()); PrefetchDebug("Parsed cmcd nor relpath: '%s'", relpath.c_str()); - String::size_type const lsi = currentPath.find_last_of("/"); - String const nextPath = currentPath.substr(0, lsi + 1) + relpath; + const String::size_type lsi = currentPath.find_last_of("/"); + const String nextPath = currentPath.substr(0, lsi + 1) + relpath; PrefetchDebug("Next cmcd nor path: '%s'", nextPath.c_str()); @@ -606,7 +606,7 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) /* Trigger all necessary background fetches based on the next path pattern */ if (!currentPath.empty()) { - unsigned const total = config.getFetchCount(); + const unsigned total = config.getFetchCount(); String workingPath = currentPath; for (unsigned i = 0; i < total; ++i) { PrefetchDebug("generating prefetch request %d/%d", i + 1, total); @@ -635,8 +635,8 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) /* Trigger all necessary background fetches based on the query string(s) */ PrefetchDebug("currentQuery: %s", currentQuery.c_str()); - size_t const lastSlashIndex = currentPath.find_last_of("/"); - size_t const keyLen = config.getQueryKeyName().size(); + const size_t lastSlashIndex = currentPath.find_last_of("/"); + const size_t keyLen = config.getQueryKeyName().size(); unsigned done = 1; std::istringstream cStringStream(currentQuery); String param; From 023e46ed484cd813ee648a8f551df0c780da9103 Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Wed, 8 Feb 2023 15:55:21 +0000 Subject: [PATCH 3/5] prefetch: make cmcd autest more robust --- plugins/prefetch/configs.cc | 1 + tests/gold_tests/pluginTest/prefetch/prefetch_cmcd0.gold | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/prefetch/configs.cc b/plugins/prefetch/configs.cc index 0c762971977..aa869b5638c 100644 --- a/plugins/prefetch/configs.cc +++ b/plugins/prefetch/configs.cc @@ -172,6 +172,7 @@ PrefetchConfig::finalize() PrefetchDebug("front-end: %s", (_front ? "true" : "false")); PrefetchDebug("exact match: %s", (_exactMatch ? "true" : "false")); PrefetchDebug("query key: %s", _queryKey.c_str()); + PrefetchDebug("cncd-nor: %s", (_front ? "true" : "false")); PrefetchDebug("API header name: %s", _apiHeader.c_str()); PrefetchDebug("next object header name: %s", _nextHeader.c_str()); PrefetchDebug("fetch policy parameters: %s", _fetchPolicy.c_str()); diff --git a/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd0.gold b/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd0.gold index 59e6e55dc68..1292bd1500e 100644 --- a/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd0.gold +++ b/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd0.gold @@ -2,9 +2,9 @@ tests/request.txt 200 TCP_MISS FIN 33 - tests/request.txt 200 TCP_HIT - 33 - tests/prefetch.txt 200 TCP_MISS - 16 tests/request.txt tests/prefetch.txt 200 TCP_MISS FIN 34 - -tests/request.txt 200 TCP_MEM_HIT - 33 - +tests/request.txt 200 ``_HIT - 33 - tests/prefetch.txt 208 TCP_HIT - 20 tests/request.txt -tests/prefetch.txt 200 TCP_MEM_HIT - 34 - +tests/prefetch.txt 200 ``_HIT - 34 - tests/query.txt 200 TCP_MISS FIN 41 - tests/query 200 TCP_MISS - 16 tests/query.txt tests/query 200 TCP_MISS FIN 41 - From 84c188ee76588e2a83673e3f1f89b4ac83dede48 Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Thu, 16 Feb 2023 17:47:21 +0000 Subject: [PATCH 4/5] prefetch: cmcd percent decode and scrub query params --- plugins/prefetch/fetch.cc | 16 +++- plugins/prefetch/fetch.h | 5 +- plugins/prefetch/plugin.cc | 51 +++++++---- .../pluginTest/prefetch/prefetch_cmcd.test.py | 84 ++++++++++++------- .../pluginTest/prefetch/prefetch_cmcd0.gold | 26 +++--- .../pluginTest/prefetch/prefetch_cmcd1.gold | 24 +++--- 6 files changed, 133 insertions(+), 73 deletions(-) diff --git a/plugins/prefetch/fetch.cc b/plugins/prefetch/fetch.cc index a114be05553..d1a9b3e98b7 100644 --- a/plugins/prefetch/fetch.cc +++ b/plugins/prefetch/fetch.cc @@ -409,11 +409,12 @@ BgFetch::~BgFetch() bool BgFetch::schedule(BgFetchState *state, const PrefetchConfig &config, bool askPermission, TSMBuffer requestBuffer, - TSMLoc requestHeaderLoc, TSHttpTxn txnp, const char *path, size_t pathLen, const String &cachekey) + TSMLoc requestHeaderLoc, TSHttpTxn txnp, const char *path, size_t pathLen, const String &cachekey, + bool removeQuery) { bool ret = false; BgFetch *fetch = new BgFetch(state, config, askPermission); - if (fetch->init(requestBuffer, requestHeaderLoc, txnp, path, pathLen, cachekey)) { + if (fetch->init(requestBuffer, requestHeaderLoc, txnp, path, pathLen, cachekey, removeQuery)) { fetch->schedule(); ret = true; } else { @@ -451,7 +452,7 @@ BgFetch::addBytes(int64_t b) */ bool BgFetch::init(TSMBuffer reqBuffer, TSMLoc reqHdrLoc, TSHttpTxn txnp, const char *fetchPath, size_t fetchPathLen, - const String &cachekey) + const String &cachekey, bool removeQuery) { TSAssert(TS_NULL_MLOC == _headerLoc); TSAssert(TS_NULL_MLOC == _urlLoc); @@ -506,6 +507,15 @@ BgFetch::init(TSMBuffer reqBuffer, TSMLoc reqHdrLoc, TSHttpTxn txnp, const char return false; } + /* Remove the query string */ + if (removeQuery) { + if (TS_SUCCESS == TSUrlHttpQuerySet(_mbuf, _urlLoc, "", 0)) { + PrefetchDebug("original query string removed"); + } else { + PrefetchError("failed to remove original query string"); + } + } + /* Now set or remove the prefetch API header */ const String &header = _config.getApiHeader(); if (_config.isFront()) { diff --git a/plugins/prefetch/fetch.h b/plugins/prefetch/fetch.h index 0471e1e718f..3f72e4ea8c6 100644 --- a/plugins/prefetch/fetch.h +++ b/plugins/prefetch/fetch.h @@ -169,13 +169,14 @@ class BgFetch { public: static bool schedule(BgFetchState *state, const PrefetchConfig &config, bool askPermission, TSMBuffer requestBuffer, - TSMLoc requestHeaderLoc, TSHttpTxn txnp, const char *path, size_t pathLen, const String &cachekey); + TSMLoc requestHeaderLoc, TSHttpTxn txnp, const char *path, size_t pathLen, const String &cachekey, + bool removeQuery = false); private: BgFetch(BgFetchState *state, const PrefetchConfig &config, bool lock); ~BgFetch(); bool init(TSMBuffer requestBuffer, TSMLoc requestHeaderLoc, TSHttpTxn txnp, const char *fetchPath, size_t fetchPathLen, - const String &cacheKey); + const String &cacheKey, bool removeQuery = false); void schedule(); static int handler(TSCont contp, TSEvent event, void * /* edata ATS_UNUSED */); bool saveIp(TSHttpTxn txnp); diff --git a/plugins/prefetch/plugin.cc b/plugins/prefetch/plugin.cc index 5c52ea6da2a..6ba4c12fe5a 100644 --- a/plugins/prefetch/plugin.cc +++ b/plugins/prefetch/plugin.cc @@ -350,8 +350,9 @@ getPristineUrlQuery(TSHttpTxn txnp) return pristineQuery; } -static const StringView CmcdHeader{"Cmcd-Request"}; -static const StringView CmcdNorFieldPrefix{"nor="}; +static constexpr StringView CmcdHeader{"Cmcd-Request"}; +static constexpr StringView CmcdNorFieldPrefix{"nor="}; +static constexpr StringView CmcdNrrFieldPrefix{"nrr="}; /** * @brief Look for and return the nor field of any Cmcd-Request header @@ -361,6 +362,8 @@ static const StringView CmcdNorFieldPrefix{"nor="}; * @param hdrloc request TSMLoc * @return unquoted relative cmcd path from the nor field * + * If the 'nrr' field is encountered the 'nor' field will be ignored. + * * sample header: * Cmcd-Request: mtp=103600,bl=153500,nor="14_176.mp4a" */ @@ -368,6 +371,8 @@ static String getCmcdNor(const TSMBuffer buffer, const TSMLoc hdrloc) { String relpath; + bool hasnrr = false; // don't prefetch if range request + const TSMLoc cmcdloc = TSMimeHdrFieldFind(buffer, hdrloc, CmcdHeader.data(), CmcdHeader.length()); if (TS_NULL_MLOC != cmcdloc) { // iterate through the fields @@ -377,7 +382,13 @@ getCmcdNor(const TSMBuffer buffer, const TSMLoc hdrloc) const char *const fval = TSMimeHdrFieldValueStringGet(buffer, hdrloc, cmcdloc, ind, &flen); StringView fv(fval, flen); - PrefetchDebug("cmcd-request field: %.*s", (int)fv.length(), fv.data()); + PrefetchDebug("cmcd-request field: '%.*s'", (int)fv.length(), fv.data()); + if (0 == fv.compare(0, CmcdNrrFieldPrefix.length(), CmcdNrrFieldPrefix)) { + PrefetchDebug("cmcd-request nrr field encountered, skipping prefetch!"); + hasnrr = true; + break; + } + if (0 == fv.compare(0, CmcdNorFieldPrefix.length(), CmcdNorFieldPrefix)) { fv.remove_prefix(CmcdNorFieldPrefix.length()); if (fv.front() == '"') { @@ -386,14 +397,29 @@ getCmcdNor(const TSMBuffer buffer, const TSMLoc hdrloc) if (fv.back() == '"') { fv.remove_suffix(1); } - relpath = fv; - break; + + PrefetchDebug("Extracted nor field: '%.*s'", (int)fv.length(), fv.data()); + + // Undo any percent encoding + char buf[8192]; + size_t blen = sizeof(buf); + if (TS_SUCCESS == TSStringPercentDecode(fv.data(), fv.length(), buf, blen, &blen)) { + relpath.assign(buf, blen); + } else { + PrefetchDebug("Error percent decoding nor field: '%.*s'", (int)fv.length(), fv.data()); + } } } TSHandleMLocRelease(buffer, hdrloc, cmcdloc); } else { PrefetchDebug("No Cmcd-Request header found"); } + + // don't prefetch if range request + if (hasnrr) { + relpath.clear(); + } + return relpath; } @@ -574,7 +600,8 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) } if (data->firstPass() && data->_fetchable && respToTriggerPrefetch(txnp)) { - // Configured to handle cmcd-request nor + // If configured to handle Cmcd-Request nor field. + // This still allows other prefetch configurations to work. if (config.isCmcdNor()) { PrefetchDebug("Considering cmcd nor request"); @@ -591,14 +618,10 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) PrefetchDebug("Next cmcd nor path: '%s'", nextPath.c_str()); - // ensure we aren't prefetching the current asset - if (nextPath != currentPath) { - constexpr bool askPermission = false; - BgFetch::schedule(state, config, askPermission, reqBuffer, reqHdrLoc, txnp, nextPath.c_str(), nextPath.length(), - data->_cachekey); - } else { - PrefetchDebug("Next cmcd path same as current path '%s', skipping!", currentPath.c_str()); - } + constexpr bool askPermission = false; + constexpr bool removeQuery = true; + BgFetch::schedule(state, config, askPermission, reqBuffer, reqHdrLoc, txnp, nextPath.c_str(), nextPath.length(), + data->_cachekey, removeQuery); } } diff --git a/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd.test.py b/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd.test.py index d3feae0fd94..43017edc05c 100644 --- a/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd.test.py +++ b/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd.test.py @@ -17,6 +17,8 @@ # limitations under the License. import os +import urllib.parse + Test.Summary = ''' Test prefetch.so plugin (simple mode). ''' @@ -24,14 +26,14 @@ origin = Test.MakeOriginServer("origin") asset_name = 'request.txt' -prefetch_name = 'prefetch.txt' -prefetch_header = f'Cmcd-Request: foo=12,nor="{prefetch_name}",bar=42' +pf_name = 'prefetch.txt' +pf_header = f'Cmcd-Request: foo=12,nor="{pf_name}",bar=42' request_header = { "headers": f"GET /tests/{asset_name} HTTP/1.1\r\n" "Host: does.not.matter\r\n" # But cant be omitted - f"{prefetch_header}\r\n" + f"{pf_header}\r\n" "\r\n", "timestamp": "1469733493.993", "body": "" @@ -49,13 +51,18 @@ # query string query_name = 'query?this=foo&that' -query_header = f'Cmcd-Request: nor="{query_name}"' +query_pf_name = 'query?bar=baz' +query_pf_header = f'Cmcd-Request: nor="{query_pf_name}"' + +# nor field may be percent encoded +query_pf_perc_name = urllib.parse.quote(query_pf_name) +query_pf_perc_header = f'Cmcd-Request: nor="{query_pf_perc_name}"' request_header = { "headers": - f"GET /tests/query.txt HTTP/1.1\r\n" + f"GET /tests/{query_name} HTTP/1.1\r\n" "Host: does.not.matter\r\n" # But cant be omitted - f"{query_header}\r\n" + f"{query_pf_perc_header}\r\n" "\r\n", "timestamp": "1469733493.993", "body": "" @@ -71,12 +78,8 @@ } origin.addResponse("sessionlog.json", request_header, response_header) -# fail tests -same_name = 'same.txt' -same_header = f'Cmcd-Request: nor="{same_name}"' - # setup the prefetched assets -names = [prefetch_name, query_name, same_name] +names = [pf_name, query_pf_name] for name in names: request_header = { @@ -141,11 +144,34 @@ } origin.addResponse("sessionlog.json", request_header, response_header) +# ignore if cmcd-request nrr= found +crr_name = 'crr.txt' +crr_header = f'Cmcd-Request: foo=12,nor="{crr_name}",bar=42,nrr="0-"' +request_header = { + "headers": + f"GET /tests/{crr_name} HTTP/1.1\r\n" + "Host: does.not.matter\r\n" # But cant be omitted + f"{crr_header}\r\n" + "\r\n", + "timestamp": "1469733493.993", + "body": "" +} +response_header = { + "headers": + "HTTP/1.1 200 OK\r\n" + "Connection: close\r\n" + "Cache-control: max-age=60\r\n" + "\r\n", + "timestamp": "1469733493.993", + "body": f"This is the body for {crr_name}\n" +} +origin.addResponse("sessionlog.json", request_header, response_header) + # allows for multiple ats on localhost dns = Test.MakeDNServer("dns") # next hop trafficserver instance -ts1 = Test.MakeATSProcess("ts1", command="traffic_server 2>trace1.log") +ts1 = Test.MakeATSProcess("ts1") ts1.Disk.records_config.update({ 'proxy.config.diags.debug.enabled': 1, 'proxy.config.diags.debug.tags': 'prefetch|http', @@ -156,7 +182,7 @@ dns.addRecords(records={f"ts1": ["127.0.0.1"]}) ts1.Disk.remap_config.AddLine( f"map / http://127.0.0.1:{origin.Variables.Port}" + - " @plugin=cachekey.so @pparam==--remove-all-params=true" + " @plugin=cachekey.so @pparam==--sort-params=true" " @plugin=prefetch.so @pparam==--front=false" ) @@ -165,14 +191,14 @@ logging: formats: - name: custom - format: '% % % % % %<{X-CDN-Prefetch}cqh>' + format: '% % % % % %<{X-CDN-Prefetch}cqh>' logs: - filename: transaction format: custom '''.split("\n") ) -ts0 = Test.MakeATSProcess("ts0", command="traffic_server 2> trace0.log") +ts0 = Test.MakeATSProcess("ts0") ts0.Disk.records_config.update({ 'proxy.config.diags.debug.enabled': 1, 'proxy.config.diags.debug.tags': 'prefetch|http', @@ -184,7 +210,7 @@ dns.addRecords(records={f"ts0": ["127.0.0.1"]}) ts0.Disk.remap_config.AddLine( f"map http://ts0 http://ts1:{ts1.Variables.port}" + - " @plugin=cachekey.so @pparam=--remove-all-params=true" + " @plugin=cachekey.so @pparam=--sort-params=true" " @plugin=prefetch.so" + " @pparam=--front=true" + " @pparam=--fetch-policy=simple" + @@ -196,7 +222,7 @@ logging: formats: - name: custom - format: '% % % % % %<{X-CDN-Prefetch}cqh>' + format: '% % % % % %<{X-CDN-Prefetch}cqh>' logs: - filename: transaction format: custom @@ -224,7 +250,7 @@ tr = Test.AddTestRun() tr.DelayStart = 1 tr.Processes.Default.Command = ( - f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/{asset_name} -H \'{prefetch_header}\'" + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/{asset_name} -H \'{pf_header}\'" ) tr.Processes.Default.ReturnCode = 0 @@ -232,49 +258,49 @@ tr = Test.AddTestRun() tr.DelayStart = 1 tr.Processes.Default.Command = ( - f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/{prefetch_name}" + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/{pf_name}" ) tr.Processes.Default.ReturnCode = 0 # attempt to prefetch again tr = Test.AddTestRun() tr.Processes.Default.Command = ( - f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/{asset_name} -H \'{prefetch_header}\'" + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/{asset_name} -H \'{pf_header}\'" ) tr.Processes.Default.ReturnCode = 0 # request the prefetched asset tr = Test.AddTestRun() tr.Processes.Default.Command = ( - f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/{prefetch_name}" + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/{pf_name}" ) tr.Processes.Default.ReturnCode = 0 -# query path +# prefetch using query params with query prefetch perc encoded tr = Test.AddTestRun() tr.Processes.Default.Command = ( - f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/query.txt -H \'{query_header}\'" + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} \'http://ts0/tests/{query_name}\' -H \'{query_pf_perc_header}\'" ) tr.Processes.Default.ReturnCode = 0 -# request the prefetched asset +# request the prefetched asset without perc encoding tr = Test.AddTestRun() tr.Processes.Default.Command = ( - f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} \'http://ts0/tests/{query_name}\'" + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} \'http://ts0/tests/{query_pf_name}\'" ) tr.Processes.Default.ReturnCode = 0 -# same path (reject) +# ensure root path prefetch works tr = Test.AddTestRun() tr.Processes.Default.Command = ( - f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} http://ts0/tests/{same_name} -H \'{same_header}\'" + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} \'http://ts0/{root_name}\' -H \'{root_header}\'" ) tr.Processes.Default.ReturnCode = 0 -# root path +# ensure request with nrr= field is skipped tr = Test.AddTestRun() tr.Processes.Default.Command = ( - f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} \'http://ts0/{root_name}\' -H \'{root_header}\'" + f"curl --verbose --proxy 127.0.0.1:{ts0.Variables.port} \'http://ts0/{crr_name}\' -H \'{crr_header}\'" ) tr.Processes.Default.ReturnCode = 0 diff --git a/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd0.gold b/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd0.gold index 1292bd1500e..84aeaf45fa1 100644 --- a/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd0.gold +++ b/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd0.gold @@ -1,13 +1,13 @@ -tests/request.txt 200 TCP_MISS FIN 33 - -tests/request.txt 200 TCP_HIT - 33 - -tests/prefetch.txt 200 TCP_MISS - 16 tests/request.txt -tests/prefetch.txt 200 TCP_MISS FIN 34 - -tests/request.txt 200 ``_HIT - 33 - -tests/prefetch.txt 208 TCP_HIT - 20 tests/request.txt -tests/prefetch.txt 200 ``_HIT - 34 - -tests/query.txt 200 TCP_MISS FIN 41 - -tests/query 200 TCP_MISS - 16 tests/query.txt -tests/query 200 TCP_MISS FIN 41 - -tests/same.txt 200 TCP_MISS FIN 30 - -root.txt 200 TCP_MISS FIN 30 - -rooted 200 TCP_MISS - 16 root.txt +http://ts0/tests/request.txt 200 TCP_MISS FIN 33 - +http://ts0/tests/request.txt 200 TCP_HIT - 33 - +http://ts0/tests/prefetch.txt 200 TCP_MISS - 16 tests/request.txt +http://ts0/tests/prefetch.txt 200 TCP_MISS FIN 34 - +http://ts0/tests/request.txt 200 ``_HIT - 33 - +http://ts0/tests/prefetch.txt 208 TCP_HIT - 20 tests/request.txt +http://ts0/tests/prefetch.txt 200 ``_HIT - 34 - +http://ts0/tests/query?this=foo&that 200 TCP_MISS FIN 41 - +http://ts0/tests/query?bar=baz 200 TCP_MISS - 16 tests/query +http://ts0/tests/query?bar=baz 200 TCP_MISS FIN 35 - +http://ts0/root.txt 200 TCP_MISS FIN 30 - +http://ts0/rooted 200 TCP_MISS - 16 root.txt +http://ts0/crr.txt 404 TCP_MISS - 5 - diff --git a/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd1.gold b/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd1.gold index f353701a2bd..1d2b7fceacd 100644 --- a/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd1.gold +++ b/tests/gold_tests/pluginTest/prefetch/prefetch_cmcd1.gold @@ -1,12 +1,12 @@ -tests/request.txt 200 TCP_MISS FIN 33 - -tests/prefetch.txt 200 TCP_MISS - 16 tests/request.txt -tests/prefetch.txt 200 TCP_MISS FIN 34 - -tests/prefetch.txt 200 TCP_HIT - 34 - -tests/query.txt 200 TCP_MISS FIN 41 - -tests/query 200 TCP_MISS - 16 tests/query.txt -tests/query 200 TCP_MISS FIN 41 - -tests/query 200 TCP_HIT - 41 - -tests/same.txt 200 TCP_MISS FIN 30 - -root.txt 200 TCP_MISS FIN 30 - -rooted 200 TCP_MISS - 16 root.txt -rooted 200 TCP_MISS FIN 28 - +http://ts1:``/tests/request.txt 200 TCP_MISS FIN 33 - +http://ts1:``/tests/prefetch.txt 200 TCP_MISS - 16 tests/request.txt +http://ts1:``/tests/prefetch.txt 200 TCP_MISS FIN 34 - +http://ts1:``/tests/prefetch.txt 200 TCP_HIT - 34 - +http://ts1:``/tests/query?this=foo&that 200 TCP_MISS FIN 41 - +http://ts1:``/tests/query?bar=baz 200 TCP_MISS - 16 tests/query +http://ts1:``/tests/query?bar=baz 200 TCP_MISS FIN 35 - +http://ts1:``/tests/query?bar=baz 200 TCP_HIT - 35 - +http://ts1:``/root.txt 200 TCP_MISS FIN 30 - +http://ts1:``/rooted 200 TCP_MISS - 16 root.txt +http://ts1:``/rooted 200 TCP_MISS FIN 28 - +http://ts1:``/crr.txt 404 TCP_MISS - 5 - From ead6c152ac94853ac91a73598169fa046c1fc3b9 Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Fri, 10 Mar 2023 13:21:33 +0000 Subject: [PATCH 5/5] revert debug statement change --- plugins/prefetch/plugin.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/prefetch/plugin.cc b/plugins/prefetch/plugin.cc index 6ba4c12fe5a..7306cfbb795 100644 --- a/plugins/prefetch/plugin.cc +++ b/plugins/prefetch/plugin.cc @@ -542,7 +542,7 @@ contHandleFetch(const TSCont contp, TSEvent event, void *edata) /* first-pass */ if (!config.isExactMatch()) { data->_fetchable = state->acquire(data->_cachekey); - PrefetchDebug("request is %s fetchable", data->_fetchable ? "" : "not"); + PrefetchDebug("request is %s fetchable", data->_fetchable ? " " : " not "); } } }