You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2020/09/28 19:30:18 UTC

[trafficserver] branch 9.0.x updated: Follow redirection responses when refreshing stale cache objects. (#7213)

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

zwoop pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/9.0.x by this push:
     new 8bc4d63  Follow redirection responses when refreshing stale cache objects. (#7213)
8bc4d63 is described below

commit 8bc4d63b624da172738310adf19446e45ebc2aca
Author: Walt Karas <wk...@verizonmedia.com>
AuthorDate: Thu Sep 24 10:56:02 2020 -0500

    Follow redirection responses when refreshing stale cache objects. (#7213)
    
    (cherry picked from commit f3647619921ada5214b78d6ddb594a10fbc99a15)
---
 proxy/http/HttpSM.cc                               |  2 +-
 proxy/http/HttpTransact.cc                         |  8 +-
 proxy/http/HttpTransact.h                          |  1 +
 tests/gold_tests/redirect/gold/redirect_stale.gold | 10 +++
 tests/gold_tests/redirect/redirect_stale.test.py   | 95 ++++++++++++++++++++++
 5 files changed, 114 insertions(+), 2 deletions(-)

diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 84dc4f4..9e853ea 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -8092,7 +8092,7 @@ inline bool
 HttpSM::is_redirect_required()
 {
   bool redirect_required = (enable_redirection && (redirection_tries <= t_state.txn_conf->number_of_redirections) &&
-                            !HttpTransact::is_cache_hit(t_state.cache_lookup_result));
+                            !HttpTransact::is_fresh_cache_hit(t_state.cache_lookup_result));
 
   SMDebug("http_redirect", "is_redirect_required %u", redirect_required);
 
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 7482f7a..7134cd1 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -7699,9 +7699,15 @@ HttpTransact::is_request_likely_cacheable(State *s, HTTPHdr *request)
 }
 
 bool
+HttpTransact::is_fresh_cache_hit(CacheLookupResult_t r)
+{
+  return (r == CACHE_LOOKUP_HIT_FRESH || r == CACHE_LOOKUP_HIT_WARNING);
+}
+
+bool
 HttpTransact::is_cache_hit(CacheLookupResult_t r)
 {
-  return (r == CACHE_LOOKUP_HIT_FRESH || r == CACHE_LOOKUP_HIT_WARNING || r == CACHE_LOOKUP_HIT_STALE);
+  return (is_fresh_cache_hit(r) || r == CACHE_LOOKUP_HIT_STALE);
 }
 
 void
diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h
index 9ae15e3..966bb7f 100644
--- a/proxy/http/HttpTransact.h
+++ b/proxy/http/HttpTransact.h
@@ -1038,6 +1038,7 @@ public:
   static bool will_this_request_self_loop(State *s);
   static bool is_request_likely_cacheable(State *s, HTTPHdr *request);
   static bool is_cache_hit(CacheLookupResult_t r);
+  static bool is_fresh_cache_hit(CacheLookupResult_t r);
 
   static void build_request(State *s, HTTPHdr *base_request, HTTPHdr *outgoing_request, HTTPVersion outgoing_version);
   static void build_response(State *s, HTTPHdr *base_response, HTTPHdr *outgoing_response, HTTPVersion outgoing_version,
diff --git a/tests/gold_tests/redirect/gold/redirect_stale.gold b/tests/gold_tests/redirect/gold/redirect_stale.gold
new file mode 100644
index 0000000..fe4f3b6
--- /dev/null
+++ b/tests/gold_tests/redirect/gold/redirect_stale.gold
@@ -0,0 +1,10 @@
+HTTP/1.1 200 OK
+X-Obj: obj2
+Cache-Control: max-age=2
+Content-Length: 0
+
+HTTP/1.1 200 OK
+X-Obj: obj2
+Cache-Control: max-age=2
+Content-Length: 0
+
diff --git a/tests/gold_tests/redirect/redirect_stale.test.py b/tests/gold_tests/redirect/redirect_stale.test.py
new file mode 100644
index 0000000..5ca975e
--- /dev/null
+++ b/tests/gold_tests/redirect/redirect_stale.test.py
@@ -0,0 +1,95 @@
+'''
+Test that redirects will be followed when refreshing stale cache objects.
+'''
+#  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 that redirects will be followed when refreshing stale cache objects.
+'''
+
+server = Test.MakeOriginServer("server")
+
+ArbitraryTimestamp = '12345678'
+
+request_header = {
+    'headers': ('GET /obj HTTP/1.1\r\n'
+                'Host: *\r\n\r\n'),
+    'timestamp': ArbitraryTimestamp,
+    'body': ''}
+response_header = {
+    'headers': ('HTTP/1.1 302 Found\r\n'
+                'Location: http://127.0.0.1:{}/obj2\r\n\r\n'.format(server.Variables.Port)),
+    'timestamp': ArbitraryTimestamp,
+    'body': ''}
+server.addResponse('sessionfile.log', request_header, response_header)
+
+request_header = {
+    'headers': ('GET /obj2 HTTP/1.1\r\n'
+                'Host: *\r\n\r\n'),
+    'timestamp': ArbitraryTimestamp,
+    'body': ''}
+response_header = {
+    'headers': ('HTTP/1.1 200 OK\r\n'
+                'X-Obj: obj2\r\n'
+                'Cache-Control: max-age=2\r\n'
+                'Content-Length: 0\r\n\r\n'),
+    'timestamp': ArbitraryTimestamp,
+    'body': ''}
+server.addResponse('sessionfile.log', request_header, response_header)
+
+ts = Test.MakeATSProcess("ts")
+
+ts.Disk.records_config.update({
+    'proxy.config.diags.debug.enabled': 1,
+    'proxy.config.diags.debug.tags': 'http|dns|cache|redirect',
+    'proxy.config.http.cache.required_headers': 0,  # Only Content-Length header required for caching.
+    'proxy.config.http.push_method_enabled': 1,
+    'proxy.config.url_remap.remap_required': 0,
+    'proxy.config.http.redirect.actions': 'routable:follow,loopback:follow,self:follow',
+    'proxy.config.http.number_of_redirections': 1
+})
+
+# Set up to check the output after the tests have run.
+#
+log_id = Test.Disk.File("log2.txt")
+log_id.Content = "gold/redirect_stale.gold"
+
+tr = Test.AddTestRun()
+tr.Processes.Default.StartBefore(ts)
+tr.Processes.Default.StartBefore(server)
+tr.Processes.Default.Command = (
+    r"printf 'GET /obj HTTP/1.1\r\nHost: 127.0.0.1:{}\r\n\r\n' | nc localhost {} >> log.txt".format(
+        server.Variables.Port, ts.Variables.port)
+)
+tr.Processes.Default.ReturnCode = 0
+
+# Wait for the response in cache to become stale, then GET it again.
+#
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = (
+    r"sleep 4 ; printf 'GET /obj HTTP/1.1\r\nHost: 127.0.0.1:{}\r\n\r\n' | nc localhost {} >> log.txt".format(
+        server.Variables.Port, ts.Variables.port)
+)
+tr.Processes.Default.ReturnCode = 0
+
+# Filter out inconsistent content in test output.
+#
+tr = Test.AddTestRun()
+tr.Processes.Default.Command = (
+    r"grep -v -e '^Date: ' -e '^Age: ' -e '^Connection: ' -e '^Server: ATS/' log.txt | tr -d '\r'> log2.txt"
+)
+tr.Processes.Default.ReturnCode = 0