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 94927a3 Treat objects with negative max-age CC directives as stale.
(#7260)
94927a3 is described below
commit 94927a319675eb12cd51cbc282cc02be5b39c72a
Author: Brian Neradt <brian.ner...@gmail.com>
AuthorDate: Thu Oct 8 17:46:12 2020 -0500
Treat objects with negative max-age CC directives as stale. (#7260)
Per RFC 7234, section-1.2.1, max-age values should be a non-negative
value. If it is negative, therefore, the value is invalid. Per RFC
7234, section-4.2.1, invalid freshness specifications should be
considered stale.
Before this change, negative max-age values were considered non-existent
and treated with a default max-age value. This fixes that so that if the
max-age is negative, it is considered stale.
(cherry picked from commit 718bef406aacdb14564cb111c3ad14cb9bc1466d)
---
proxy/http/HttpTransact.cc | 23 +-
tests/gold_tests/cache/cache-control.test.py | 24 ++
.../cache/replay/cache-control-max-age.replay.yaml | 360 +++++++++++++++++++++
3 files changed, 405 insertions(+), 2 deletions(-)
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 79cca0e..c9a1c47 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -7187,12 +7187,31 @@ HttpTransact::get_max_age(HTTPHdr *response)
int max_age = -1;
uint32_t cc_mask = response->get_cooked_cc_mask();
+ bool max_age_is_present = false;
if (cc_mask & MIME_COOKED_MASK_CC_S_MAXAGE) {
// Precedence to s-maxage
- max_age = static_cast<int>(response->get_cooked_cc_s_maxage());
+ max_age = static_cast<int>(response->get_cooked_cc_s_maxage());
+ max_age_is_present = true;
} else if (cc_mask & MIME_COOKED_MASK_CC_MAX_AGE) {
// If s-maxage isn't set, try max-age
- max_age = static_cast<int>(response->get_cooked_cc_max_age());
+ max_age = static_cast<int>(response->get_cooked_cc_max_age());
+ max_age_is_present = true;
+ }
+
+ // Negative max-age values:
+ //
+ // Per RFC 7234, section-1.2.1, max-age values should be a non-negative
+ // value. If it is negative, therefore, the value is invalid. Per RFC 7234,
+ // section-4.2.1, invalid freshness specifications should be considered
+ // stale.
+ //
+ // Negative return values from this function are used to indicate that the
+ // max-age value was not present, resulting in a default value likely being
+ // used. If the max-age is negative, therefore, we return 0 to indicate to
+ // the caller that the max-age directive was present and indicates that the
+ // object should be considered stale.
+ if (max_age_is_present && max_age < 0) {
+ max_age = 0;
}
return max_age;
diff --git a/tests/gold_tests/cache/cache-control.test.py
b/tests/gold_tests/cache/cache-control.test.py
index 3f59659..11964ad 100644
--- a/tests/gold_tests/cache/cache-control.test.py
+++ b/tests/gold_tests/cache/cache-control.test.py
@@ -118,3 +118,27 @@ tr.Processes.Default.Command = "printf 'GET
/no_cache_control HTTP/1.1\r\n''Cach
tr.Processes.Default.ReturnCode = 0
tr.Processes.Default.Streams.stdout = "gold/cache_no_cache.gold"
tr.StillRunningAfter = ts
+
+#
+# Verify correct handling of various max-age directives in both clients and
+# responses.
+#
+ts = Test.MakeATSProcess("ts-for-proxy-verifier")
+replay_file = "replay/cache-control-max-age.replay.yaml"
+server = Test.MakeVerifierServerProcess("proxy-verifier-server", replay_file)
+ts.Disk.records_config.update({
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http',
+ 'proxy.config.http.insert_age_in_response': 0,
+
+ # Disable ignoring max-age in the client request so we can test that
+ # behavior too.
+ 'proxy.config.http.cache.ignore_client_cc_max_age': 0,
+})
+ts.Disk.remap_config.AddLine(
+ 'map / http://127.0.0.1:{0}'.format(server.Variables.http_port)
+)
+tr = Test.AddTestRun("Verify correct max-age cache-control behavior.")
+tr.Processes.Default.StartBefore(server)
+tr.Processes.Default.StartBefore(ts)
+tr.AddVerifierClientProcess("proxy-verifier-client", replay_file,
http_ports=[ts.Variables.port])
diff --git a/tests/gold_tests/cache/replay/cache-control-max-age.replay.yaml
b/tests/gold_tests/cache/replay/cache-control-max-age.replay.yaml
new file mode 100644
index 0000000..ba6305d
--- /dev/null
+++ b/tests/gold_tests/cache/replay/cache-control-max-age.replay.yaml
@@ -0,0 +1,360 @@
+# 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.
+
+#
+# This replay file assumes that caching is enabled and
+# proxy.config.http.cache.ignore_client_cc_max_age is set to 0 so that we can
+# test max-age in the client requests.
+#
+
+meta:
+ version: "1.0"
+
+ blocks:
+ - request_for_positive_max_age: &request_for_positive_max_age
+ client-request:
+ method: "GET"
+ version: "1.1"
+ scheme: "http"
+ url: /path/200_positive_max_age
+ headers:
+ fields:
+ - [ Host, example.com ]
+
+ - request_for_zero_max_age: &request_for_zero_max_age
+ client-request:
+ method: "GET"
+ version: "1.1"
+ scheme: "http"
+ url: /path/200_zero_max_age
+ headers:
+ fields:
+ - [ Host, example.com ]
+
+ - request_for_negative_max_age: &request_for_negative_max_age
+ client-request:
+ method: "GET"
+ version: "1.1"
+ scheme: "http"
+ url: /path/200_negative_max_age
+ headers:
+ fields:
+ - [ Host, example.com ]
+
+ - request_for_non_number_max_age: &request_for_non_number_max_age
+ client-request:
+ method: "GET"
+ version: "1.1"
+ scheme: "http"
+ url: /path/200_non_number_max_age
+ headers:
+ fields:
+ - [ Host, example.com ]
+
+ - 200_ok_response: &200_ok_response
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [ Content-Length, 16 ]
+ - [ Cache-Control, max-age=300 ]
+
+sessions:
+- transactions:
+
+ #
+ # Test 1: Verify that a 200 response with a positive max-age is cached.
+ #
+ - all: { headers: { fields: [[ uuid, 1 ]]}}
+ <<: *request_for_positive_max_age
+
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [ Content-Length, 16 ]
+ - [ Cache-Control, max-age=300 ]
+
+ proxy-response:
+ status: 200
+
+ - all: { headers: { fields: [[ uuid, 2 ]]}}
+ <<: *request_for_positive_max_age
+
+ # This should not go through to the server. Return a non-200 response to
+ # verify it is served from cache.
+ server-response:
+ status: 400
+ reason: "Bad Request"
+ headers:
+ fields:
+ - [ Content-Length, 0 ]
+
+ # Expect the cached 200 response.
+ proxy-response:
+ status: 200
+
+ #
+ # Test 2: Verify that a 200 response with a 0 max-age is considered stale.
+ #
+ - all: { headers: { fields: [[ uuid, 3 ]]}}
+ <<: *request_for_zero_max_age
+
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [ Content-Length, 16 ]
+ - [ Cache-Control, max-age=0 ]
+
+ proxy-response:
+ status: 200
+
+ - all: { headers: { fields: [[ uuid, 4 ]]}}
+ <<: *request_for_zero_max_age
+
+ # This should go through to the server because the response's max-age was 0
+ # and therefore object should be considered stale.
+ server-response:
+ status: 400
+ reason: "Bad Request"
+ headers:
+ fields:
+ - [ Content-Length, 0 ]
+
+ # Expect the 400 response from the server because the 200 response should
+ # be considered stale.
+ proxy-response:
+ status: 400
+
+ #
+ # Test 3: Verify that a 200 response with a negative max age is not served
+ # from the cache. Since it is invalid, the item should be considered stale.
+ #
+ - all: { headers: { fields: [[ uuid, 5 ]]}}
+ <<: *request_for_negative_max_age
+
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [ Content-Length, 16 ]
+ # Notice the negative max-age.
+ - [ Cache-Control, max-age=-300 ]
+
+ proxy-response:
+ status: 200
+
+ - all: { headers: { fields: [[ uuid, 6 ]]}}
+ <<: *request_for_negative_max_age
+
+ # This should go through to the server because the above should not be
+ # served from the cache.
+ server-response:
+ status: 400
+ reason: "Bad Request"
+ headers:
+ fields:
+ - [ Content-Length, 0 ]
+
+ # Expect the 400 response from the server because the 200 response should
+ # not be served from the cache.
+ proxy-response:
+ status: 400
+
+ #
+ # Test 4: Verify that a 200 response with a non-integer max-age is not served
+ # from the cache. Since it is invalid, it should be considered stale.
+ #
+ - all: { headers: { fields: [[ uuid, 7 ]]}}
+ <<: *request_for_non_number_max_age
+
+ server-response:
+ status: 200
+ reason: OK
+ headers:
+ fields:
+ - [ Content-Length, 16 ]
+ # Notice the invalid, non-integer max-age value.
+ - [ Cache-Control, max-age=not_a_number ]
+
+ proxy-response:
+ status: 200
+
+ - all: { headers: { fields: [[ uuid, 8 ]]}}
+ <<: *request_for_non_number_max_age
+
+ # This should go through to the server because the above should not be
+ # cached.
+ server-response:
+ status: 400
+ reason: "Bad Request"
+ headers:
+ fields:
+ - [ Content-Length, 0 ]
+
+ # Expect the 400 response from the server because the 200 response should
+ # not be cached.
+ proxy-response:
+ status: 400
+
+ #
+ # Test 5: Verify that a request with a positive max-age for the future is
+ # replied to out of the cache.
+ #
+ - all: { headers: { fields: [[ uuid, 9 ]]}}
+
+ # For the first request, simply populate the cache with the object.
+ client-request:
+ method: "GET"
+ version: "1.1"
+ scheme: "http"
+ url: /path/test_request_with_positive_max_age
+ headers:
+ fields:
+ - [ Host, example.com ]
+
+ <<: *200_ok_response
+
+ proxy-response:
+ status: 200
+
+ - all: { headers: { fields: [[ uuid, 10 ]]}}
+
+ # Request the item again, with a max-age specification in the future.
+ client-request:
+ method: "GET"
+ version: "1.1"
+ scheme: "http"
+ url: /path/test_request_with_positive_max_age
+ headers:
+ fields:
+ - [ Host, example.com ]
+ - [ Cache-Control, max-age=300 ]
+
+ # This should be replied to out of the cache, therefore this 400 response
+ # should not make it to the client.
+ server-response:
+ status: 400
+ reason: "Bad Request"
+ headers:
+ fields:
+ - [ Content-Length, 0 ]
+
+ # Expect the cached 200 response.
+ proxy-response:
+ status: 200
+
+ #
+ # Test 6: Verify that a request with a 0 max-age results in the object being
+ # considered stale.
+ #
+ - all: { headers: { fields: [[ uuid, 11 ]]}}
+
+ # For the first request, simply populate the cache with the object.
+ client-request:
+ method: "GET"
+ version: "1.1"
+ scheme: "http"
+ url: /path/test_request_with_zero_max_age
+ headers:
+ fields:
+ - [ Host, example.com ]
+
+ <<: *200_ok_response
+
+ proxy-response:
+ status: 200
+
+ - all: { headers: { fields: [[ uuid, 12 ]]}}
+
+ # Request the item again, with a max-age specification in the future.
+ client-request:
+ method: "GET"
+ version: "1.1"
+ scheme: "http"
+ url: /path/test_request_with_zero_max_age
+ headers:
+ fields:
+ - [ Host, example.com ]
+ - [ Cache-Control, max-age=0 ]
+
+ # This should go through to the server because the 0 max-age
+ # directive in the request should make ATS consider it stale.
+ server-response:
+ status: 400
+ reason: "Bad Request"
+ headers:
+ fields:
+ - [ Content-Length, 0 ]
+
+ # Expect the 400 response from the server because the 200 response should
+ # not be cached.
+ proxy-response:
+ status: 400
+
+ #
+ # Test 7: Verify that a request with a negative max-age directive
+ # results in the object being treated as stale.
+ #
+ - all: { headers: { fields: [[ uuid, 13 ]]}}
+
+ # For the first request, simply populate the cache with the object.
+ client-request:
+ method: "GET"
+ version: "1.1"
+ scheme: "http"
+ url: /path/test_request_with_negative_max_age
+ headers:
+ fields:
+ - [ Host, example.com ]
+
+ <<: *200_ok_response
+
+ proxy-response:
+ status: 200
+
+ - all: { headers: { fields: [[ uuid, 14 ]]}}
+
+ # For the second request, specify a negative max-age value. This is
+ # invalid, and thus the freshness should be considered stale.
+ client-request:
+ method: "GET"
+ version: "1.1"
+ scheme: "http"
+ url: /path/test_request_with_negative_max_age
+ headers:
+ fields:
+ - [ Host, example.com ]
+ - [ Cache-Control, max-age=-300 ]
+
+ # This should go through to the server because the negative max-age
+ # directive in the request should make ATS consider it stale.
+ server-response:
+ status: 400
+ reason: "Bad Request"
+ headers:
+ fields:
+ - [ Content-Length, 0 ]
+
+ # Expect the 400 response from the server because the 200 response should
+ # not be cached.
+ proxy-response:
+ status: 400
