From 8b40074bdbff8d62e3a363866700db0f1900e85f Mon Sep 17 00:00:00 2001 From: bneradt Date: Mon, 27 Jul 2026 20:21:56 -0500 Subject: [PATCH] Fix exact URL cache config matching Exact URL entries in cache.config are dropped after successful parsing, so their cache actions never apply. Failed entries are inserted instead, which also defeats duplicate detection. Correct the success check and add unit and replay coverage for matching, nonmatching, invalid, and duplicate rules. Extend ATSReplayTest to accept cache.config entries with dynamic server ports. Fixes: #13421 --- doc/developer-guide/testing/autests.en.rst | 1 + src/proxy/ControlMatcher.cc | 2 +- src/proxy/unit_tests/CMakeLists.txt | 9 +- src/proxy/unit_tests/test_ControlMatcher.cc | 118 ++++++++++++++++ .../autest-site/ats_replay.test.ext | 7 + .../gold_tests/cache/cache-exact-url.test.py | 24 ++++ .../cache/replay/cache-exact-url.replay.yaml | 126 ++++++++++++++++++ 7 files changed, 285 insertions(+), 2 deletions(-) create mode 100644 src/proxy/unit_tests/test_ControlMatcher.cc create mode 100644 tests/gold_tests/cache/cache-exact-url.test.py create mode 100644 tests/gold_tests/cache/replay/cache-exact-url.replay.yaml diff --git a/doc/developer-guide/testing/autests.en.rst b/doc/developer-guide/testing/autests.en.rst index 04a02e57126..9d11b525035 100644 --- a/doc/developer-guide/testing/autests.en.rst +++ b/doc/developer-guide/testing/autests.en.rst @@ -356,6 +356,7 @@ The ``autest`` section configures the test environment: - **process_config**: Parameters passed to ``MakeATSProcess`` (e.g., ``enable_cache``) - **records_config**: Dictionary of records.config settings - **remap_config**: List of remap rules (string or dict format) + - **cache_config**: List of cache.config rules - **copy_to_config_dir**: List of files/directories to copy to ATS config directory - **log_validation**: Log validation rules for ``traffic_out`` and ``diags_log`` - **metric_checks**: List of metric name/value pairs to verify after traffic completes diff --git a/src/proxy/ControlMatcher.cc b/src/proxy/ControlMatcher.cc index 4be7cf1880a..d24d539988b 100644 --- a/src/proxy/ControlMatcher.cc +++ b/src/proxy/ControlMatcher.cc @@ -313,7 +313,7 @@ UrlMatcher::NewEntry(matcher_line *line_info) // Fill in the parameter info cur_d = data_array + num_el; error = cur_d->Init(line_info); - if (error.failed()) { + if (!error.failed()) { url_str[num_el] = ats_strdup(pattern); url_value[num_el] = num_el; url_ht.emplace(url_str[num_el], url_value[num_el]); diff --git a/src/proxy/unit_tests/CMakeLists.txt b/src/proxy/unit_tests/CMakeLists.txt index ba196cdf9b2..acfd23724cc 100644 --- a/src/proxy/unit_tests/CMakeLists.txt +++ b/src/proxy/unit_tests/CMakeLists.txt @@ -16,7 +16,14 @@ ####################### add_executable( - test_proxy main.cc test_ControlBase.cc test_FetchSM.cc test_ParentHashConfig.cc test_PluginYAML.cc stub.cc + test_proxy + main.cc + test_ControlBase.cc + test_ControlMatcher.cc + test_FetchSM.cc + test_ParentHashConfig.cc + test_PluginYAML.cc + stub.cc ) target_link_libraries(test_proxy PRIVATE Catch2::Catch2WithMain ts::http ts::proxy ts::tscore ts::records ts::inkevent) diff --git a/src/proxy/unit_tests/test_ControlMatcher.cc b/src/proxy/unit_tests/test_ControlMatcher.cc new file mode 100644 index 00000000000..5fa08804349 --- /dev/null +++ b/src/proxy/unit_tests/test_ControlMatcher.cc @@ -0,0 +1,118 @@ +/** @file + + Unit tests for ControlMatcher. + + @section license License + + 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. + */ + +#include "proxy/CacheControl.h" +#include "proxy/ControlMatcher.h" +#include "tscore/MatcherUtils.h" +#include "tscore/ink_memory.h" + +#include + +#include + +namespace +{ +class TestRequestData : public HttpRequestData +{ +public: + explicit TestRequestData(std::string_view url) : _url(url) {} + + char * + get_string() override + { + return ats_strndup(_url.data(), _url.size()); + } + +private: + std::string_view _url; +}; + +void +parse_line(char *text, matcher_line &line, int line_number) +{ + REQUIRE(parseConfigLine(text, &line, &http_dest_tags) == nullptr); + line.line_num = line_number; +} +} // namespace + +TEST_CASE("UrlMatcher inserts and matches exact URLs", "[ControlMatcher]") +{ + UrlMatcher matcher{"CacheControl", "cache.config"}; + char config[] = "url=http://example.com/exact action=never-cache"; + matcher_line line; + + matcher.AllocateSpace(1); + parse_line(config, line, 1); + + Result result = matcher.NewEntry(&line); + + REQUIRE_FALSE(result.failed()); + REQUIRE(matcher.num_el == 1); + + TestRequestData exact_request{"http://example.com/exact"}; + CacheControlResult exact_result; + + matcher.Match(&exact_request, &exact_result); + CHECK(exact_result.never_cache); + + TestRequestData other_request{"http://example.com/other"}; + CacheControlResult other_result; + + matcher.Match(&other_request, &other_result); + CHECK_FALSE(other_result.never_cache); +} + +TEST_CASE("UrlMatcher does not insert invalid records", "[ControlMatcher]") +{ + UrlMatcher matcher{"CacheControl", "cache.config"}; + char config[] = "url=http://example.com/exact action=invalid"; + matcher_line line; + + matcher.AllocateSpace(1); + parse_line(config, line, 1); + + Result result = matcher.NewEntry(&line); + + CHECK(result.failed()); + CHECK(matcher.num_el == 0); +} + +TEST_CASE("UrlMatcher rejects duplicate URLs", "[ControlMatcher]") +{ + UrlMatcher matcher{"CacheControl", "cache.config"}; + char first_config[] = "url=http://example.com/exact action=never-cache"; + char second_config[] = "url=http://example.com/exact action=standard-cache"; + matcher_line first_line; + matcher_line second_line; + + matcher.AllocateSpace(2); + parse_line(first_config, first_line, 1); + parse_line(second_config, second_line, 2); + + Result first_result = matcher.NewEntry(&first_line); + Result second_result = matcher.NewEntry(&second_line); + + CHECK_FALSE(first_result.failed()); + CHECK(second_result.failed()); + CHECK(matcher.num_el == 1); +} diff --git a/tests/gold_tests/autest-site/ats_replay.test.ext b/tests/gold_tests/autest-site/ats_replay.test.ext index 327062f7884..205fac37a84 100644 --- a/tests/gold_tests/autest-site/ats_replay.test.ext +++ b/tests/gold_tests/autest-site/ats_replay.test.ext @@ -150,6 +150,13 @@ def configure_ats(obj: 'TestRun', server: 'Process', ats_config: dict, dns: Opti parent_line = parent_line.replace('{SERVER_HTTPS_PORT}', str(server.Variables.https_port)) ts.Disk.parent_config.AddLine(parent_line) + # Configure cache_config if specified. + cache_config = ats_config.get('cache_config', []) + for cache_line in cache_config: + cache_line = cache_line.replace('{SERVER_HTTP_PORT}', str(server.Variables.http_port)) + cache_line = cache_line.replace('{SERVER_HTTPS_PORT}', str(server.Variables.https_port)) + ts.Disk.cache_config.AddLine(cache_line) + # Configure logging.yaml if specified. logging_yaml = ats_config.get('logging_yaml') if logging_yaml != None: diff --git a/tests/gold_tests/cache/cache-exact-url.test.py b/tests/gold_tests/cache/cache-exact-url.test.py new file mode 100644 index 00000000000..42a7434016f --- /dev/null +++ b/tests/gold_tests/cache/cache-exact-url.test.py @@ -0,0 +1,24 @@ +''' +Test exact URL rules in cache.config. +''' +# 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 that exact URL cache.config rules match only the configured URL. +''' + +Test.ATSReplayTest(replay_file="replay/cache-exact-url.replay.yaml") diff --git a/tests/gold_tests/cache/replay/cache-exact-url.replay.yaml b/tests/gold_tests/cache/replay/cache-exact-url.replay.yaml new file mode 100644 index 00000000000..da0efda6714 --- /dev/null +++ b/tests/gold_tests/cache/replay/cache-exact-url.replay.yaml @@ -0,0 +1,126 @@ +# 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" + +autest: + description: 'Verify exact URL cache.config matching' + + server: + name: 'server' + + client: + name: 'client' + + ats: + name: 'ts' + process_config: + enable_cache: true + records_config: + proxy.config.diags.debug.enabled: 1 + proxy.config.diags.debug.tags: 'cache_control|matcher' + remap_config: + - from: "http://example.com/" + to: "http://127.0.0.1:{SERVER_HTTP_PORT}/" + cache_config: + - "url=http://127.0.0.1:{SERVER_HTTP_PORT}/exact action=never-cache" + +sessions: +- transactions: + - client-request: + method: GET + url: /exact + version: '1.1' + headers: + fields: + - [Host, example.com] + - [uuid, exact-first] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Length, "0"] + - [Cache-Control, "max-age=300"] + + proxy-response: + status: 200 + + - client-request: + delay: 100ms + method: GET + url: /exact + version: '1.1' + headers: + fields: + - [Host, example.com] + - [uuid, exact-second] + + server-response: + status: 201 + reason: Created + headers: + fields: + - [Content-Length, "0"] + - [Cache-Control, "max-age=300"] + + proxy-response: + status: 201 + + - client-request: + method: GET + url: /other + version: '1.1' + headers: + fields: + - [Host, example.com] + - [uuid, other-first] + + server-response: + status: 202 + reason: Accepted + headers: + fields: + - [Content-Length, "0"] + - [Cache-Control, "max-age=300"] + + proxy-response: + status: 202 + + - client-request: + delay: 100ms + method: GET + url: /other + version: '1.1' + headers: + fields: + - [Host, example.com] + - [uuid, other-second] + + proxy-request: + expect: absent + + server-response: + status: 404 + reason: Not Found + headers: + fields: + - [Content-Length, "0"] + + proxy-response: + status: 202