You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by bn...@apache.org on 2021/07/14 13:03:42 UTC

[trafficserver] branch master updated: crr: change global hook to POST_REMAP, optimize cachekey creation logic (#8034)

This is an automated email from the ASF dual-hosted git repository.

bnolsen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 8a948af  crr: change global hook to POST_REMAP, optimize cachekey creation logic (#8034)
8a948af is described below

commit 8a948afc6caafdbd4d28e63b377610f66ee8ea5b
Author: Brian Olsen <bn...@gmail.com>
AuthorDate: Wed Jul 14 07:03:30 2021 -0600

    crr: change global hook to POST_REMAP, optimize cachekey creation logic (#8034)
---
 .../cache_range_requests/cache_range_requests.cc   |  36 +++--
 .../cache_range_requests_cachekey_global.test.py   | 176 +++++++++++++++++++++
 2 files changed, 197 insertions(+), 15 deletions(-)

diff --git a/plugins/cache_range_requests/cache_range_requests.cc b/plugins/cache_range_requests/cache_range_requests.cc
index cb294e8..b3f397a 100644
--- a/plugins/cache_range_requests/cache_range_requests.cc
+++ b/plugins/cache_range_requests/cache_range_requests.cc
@@ -203,21 +203,27 @@ range_header_check(TSHttpTxn txnp, pluginconfig *const pc)
           std::string &rv = txn_state->range_value;
           rv.assign(hdr_value, length);
           DEBUG_LOG("length: %d, txn_state->range_value: %s", length, rv.c_str());
-          req_url              = TSHttpTxnEffectiveUrlStringGet(txnp, &url_length);
-          cache_key_url_length = snprintf(cache_key_url, 8192, "%s-%s", req_url, rv.c_str());
-          DEBUG_LOG("Rewriting cache URL for %s to %s", req_url, cache_key_url);
-          if (req_url != nullptr) {
-            TSfree(req_url);
-          }
-
           if (nullptr != pc) {
+            if (pc->modify_cache_key || PS_DEFAULT != pc->ps_mode) {
+              req_url              = TSHttpTxnEffectiveUrlStringGet(txnp, &url_length);
+              cache_key_url_length = snprintf(cache_key_url, 8192, "%s-%s", req_url, rv.c_str());
+              DEBUG_LOG("Forming new cache URL for '%s': '%s'", req_url, cache_key_url);
+              if (req_url != nullptr) {
+                TSfree(req_url);
+              }
+            }
+
             // set the cache key if configured to.
-            if (pc->modify_cache_key && TS_SUCCESS != TSCacheUrlSet(txnp, cache_key_url, cache_key_url_length)) {
-              ERROR_LOG("failed to change the cache url to %s.", cache_key_url);
-              ERROR_LOG("Disabling cache for this transaction to avoid cache poisoning.");
-              TSHttpTxnServerRespNoStoreSet(txnp, 1);
-              TSHttpTxnRespCacheableSet(txnp, 0);
-              TSHttpTxnReqCacheableSet(txnp, 0);
+            if (pc->modify_cache_key) {
+              if (TS_SUCCESS == TSCacheUrlSet(txnp, cache_key_url, cache_key_url_length)) {
+                DEBUG_LOG("Setting cache key to '%s'", cache_key_url);
+              } else {
+                ERROR_LOG("failed to change the cache url to '%s'", cache_key_url);
+                ERROR_LOG("Disabling cache for this transaction to avoid cache poisoning.");
+                TSHttpTxnServerRespNoStoreSet(txnp, 1);
+                TSHttpTxnRespCacheableSet(txnp, 0);
+                TSHttpTxnReqCacheableSet(txnp, 0);
+              }
             }
 
             // Optionally set the parent_selection_url to the cache_key url or path
@@ -230,7 +236,7 @@ range_header_check(TSHttpTxn txnp, pluginconfig *const pc)
                 if (TS_SUCCESS == TSUrlCreate(hdr_buf, &ps_loc) &&
                     TS_PARSE_DONE == TSUrlParse(hdr_buf, ps_loc, &start, end) && // This should always succeed.
                     TS_SUCCESS == TSHttpTxnParentSelectionUrlSet(txnp, hdr_buf, ps_loc)) {
-                  DEBUG_LOG("Set Parent Selection URL to cache_key_url: %s", cache_key_url);
+                  DEBUG_LOG("Setting Parent Selection URL to '%s'", cache_key_url);
                   TSHandleMLocRelease(hdr_buf, TS_NULL_MLOC, ps_loc);
                 }
               }
@@ -638,6 +644,6 @@ TSPluginInit(int argc, const char *argv[])
     ERROR_LOG("failed to create the transaction continuation handler.");
     return;
   } else {
-    TSHttpHookAdd(TS_HTTP_READ_REQUEST_HDR_HOOK, txnp_cont);
+    TSHttpHookAdd(TS_HTTP_POST_REMAP_HOOK, txnp_cont);
   }
 }
diff --git a/tests/gold_tests/pluginTest/cache_range_requests/cache_range_requests_cachekey_global.test.py b/tests/gold_tests/pluginTest/cache_range_requests/cache_range_requests_cachekey_global.test.py
new file mode 100644
index 0000000..13baca6
--- /dev/null
+++ b/tests/gold_tests/pluginTest/cache_range_requests/cache_range_requests_cachekey_global.test.py
@@ -0,0 +1,176 @@
+'''
+'''
+#  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 = '''
+cache_range_requests with cachekey as a global plugin
+'''
+
+# Test description:
+# Preload the cache with the entire asset to be range requested.
+# Reload remap rule with cache_range_requests plugin
+# Request content through the cache_range_requests plugin
+
+Test.SkipUnless(
+    Condition.PluginExists('cache_range_requests.so'),
+    Condition.PluginExists('cachekey.so'),
+    Condition.PluginExists('xdebug.so'),
+)
+Test.ContinueOnFail = False
+Test.testName = "cache_range_requests_cachekey_global"
+
+# Define and configure ATS, enable traffic_ctl config reload
+ts = Test.MakeATSProcess("ts", command="traffic_server")
+
+# Define and configure origin server
+server = Test.MakeOriginServer("server", lookup_key="{%uuid}")
+
+# default root
+req_chk = {"headers":
+           "GET / HTTP/1.1\r\n" +
+           "Host: www.example.com\r\n" +
+           "uuid: none\r\n" +
+           "\r\n",
+           "timestamp": "1469733493.993",
+           "body": ""
+           }
+
+res_chk = {"headers":
+           "HTTP/1.1 200 OK\r\n" +
+           "Connection: close\r\n" +
+           "\r\n",
+           "timestamp": "1469733493.993",
+           "body": ""
+           }
+
+server.addResponse("sessionlog.json", req_chk, res_chk)
+
+body = "lets go surfin now"
+bodylen = len(body)
+
+# this request should work
+req_full = {"headers":
+            "GET /path HTTP/1.1\r\n" +
+            "Host: www.example.com\r\n" +
+            "Accept: */*\r\n" +
+            "uuid: full\r\n" +
+            "\r\n",
+            "timestamp": "1469733493.993",
+            "body": ""
+            }
+
+res_full = {"headers":
+            "HTTP/1.1 206 Partial Content\r\n" +
+            "Accept-Ranges: bytes\r\n" +
+            'Etag: "foo"\r\n' +
+            "Cache-Control: public, max-age=500\r\n" +
+            "Connection: close\r\n" +
+            "\r\n",
+            "timestamp": "1469733493.993",
+            "body": body
+            }
+
+server.addResponse("sessionlog.json", req_full, res_full)
+
+# this request should work
+req_good = {"headers":
+            "GET /path HTTP/1.1\r\n" +
+            "Host: www.example.com\r\n" +
+            "Accept: */*\r\n" +
+            "Range: bytes=0-\r\n" +
+            "uuid: range_full\r\n" +
+            "\r\n",
+            "timestamp": "1469733493.993",
+            "body": ""
+            }
+
+res_good = {"headers":
+            "HTTP/1.1 206 Partial Content\r\n" +
+            "Accept-Ranges: bytes\r\n" +
+            'Etag: "foo"\r\n' +
+            "Cache-Control: public, max-age=500\r\n" +
+            "Content-Range: bytes 0-{0}/{0}\r\n".format(bodylen) +
+            "Connection: close\r\n" +
+            "\r\n",
+            "timestamp": "1469733493.993",
+            "body": body
+            }
+
+server.addResponse("sessionlog.json", req_good, res_good)
+
+# this request should fail with a cache_range_requests asset
+req_fail = {"headers":
+            "GET /path HTTP/1.1\r\n" +
+            "Host: www.fail.com\r\n" +
+            "Accept: */*\r\n" +
+            "Range: bytes=0-\r\n" +
+            "uuid: range_fail\r\n" +
+            "\r\n",
+            "timestamp": "1469733493.993",
+            "body": ""
+            }
+
+res_fail = {"headers":
+            "HTTP/1.1 206 Partial Content\r\n" +
+            "Accept-Ranges: bytes\r\n" +
+            'Etag: "foo"\r\n' +
+            "Cache-Control: public, max-age=500\r\n" +
+            "Content-Range: bytes 0-{0}/{0}\r\n".format(bodylen) +
+            "Connection: close\r\n" +
+            "\r\n",
+            "timestamp": "1469733493.993",
+            "body": body
+            }
+
+server.addResponse("sessionlog.json", req_fail, res_fail)
+
+# cache range requests plugin remap, working config
+ts.Disk.remap_config.AddLine(
+    'map http://www.example.com http://127.0.0.1:{}'.format(server.Variables.Port)
+)
+
+# improperly configured cache_range_requests with cachekey
+ts.Disk.remap_config.AddLine(
+    'map http://www.fail.com http://127.0.0.1:{}'.format(server.Variables.Port)
+)
+
+# cache debug
+ts.Disk.plugin_config.AddLines([
+    'cachekey.so --include-headers=Range --static-prefix=foo',
+    'cache_range_requests.so --no-modify-cachekey',
+    'xdebug.so',
+])
+
+# minimal configuration
+ts.Disk.records_config.update({
+    'proxy.config.diags.debug.enabled': 1,
+    'proxy.config.diags.debug.tags': 'cachekey|cache_range_requests',
+})
+
+curl_and_args = 'curl -s -D /dev/stdout -o /dev/stderr -x localhost:{} -H "x-debug: x-cache-key"'.format(ts.Variables.port)
+
+# 0 Test - Fetch full asset via range
+tr = Test.AddTestRun("asset fetch via range")
+ps = tr.Processes.Default
+ps.StartBefore(server, ready=When.PortOpen(server.Variables.Port))
+ps.StartBefore(Test.Processes.ts)
+ps.Command = curl_and_args + ' http://www.example.com/path -r0- -H "uuid: full"'
+ps.ReturnCode = 0
+ps.Streams.stdout.Content = Testers.ContainsExpression(
+    "X-Cache-Key: /foo/Range:bytes=0-/path",
+    "expected cachekey style range request in cachekey")
+tr.StillRunningAfter = ts