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

bneradt 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 d05506e2c3 Fix exact URL cache config matching (#13438)
d05506e2c3 is described below

commit d05506e2c32081fb7c0d96355fab24ae974cf9f8
Author: Brian Neradt <[email protected]>
AuthorDate: Fri Jul 31 18:16:05 2026 -0500

    Fix exact URL cache config matching (#13438)
    
    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 +++++++++++++++++++
 tests/gold_tests/autest-site/ats_replay.test.ext   |   7 ++
 tests/gold_tests/cache/cache-exact-url.test.py     |  24 ++++
 .../cache/replay/cache-exact-url.replay.yaml       | 126 +++++++++++++++++++++
 7 files changed, 285 insertions(+), 2 deletions(-)

diff --git a/doc/developer-guide/testing/autests.en.rst 
b/doc/developer-guide/testing/autests.en.rst
index 04a02e5712..9d11b52503 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 4be7cf1880..d24d539988 100644
--- a/src/proxy/ControlMatcher.cc
+++ b/src/proxy/ControlMatcher.cc
@@ -313,7 +313,7 @@ UrlMatcher<Data, MatchResult>::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 ba196cdf9b..acfd23724c 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 0000000000..5fa0880434
--- /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 <catch2/catch_test_macros.hpp>
+
+#include <string_view>
+
+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<CacheControlRecord, CacheControlResult> 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<CacheControlRecord, CacheControlResult> 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<CacheControlRecord, CacheControlResult> 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 327062f788..205fac37a8 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 0000000000..42a7434016
--- /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 0000000000..da0efda671
--- /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

Reply via email to