From 67ebe515de9fb5dc4614d31c2717dec836c450d7 Mon Sep 17 00:00:00 2001 From: Chris McFarlen Date: Thu, 26 Mar 2026 16:12:54 -0500 Subject: [PATCH 1/3] Fix MATCH_SET parsing of quoted items and add tests The set parser was incorrectly advancing 'start' by skip_quotes after a comma, causing the second and subsequent quoted items to be parsed with their leading characters truncated. Add a unit test for quoted set parsing and enable the test_matcher build target (fix linker issue by removing resources.cc and adding stubs). Add autest coverage for quoted sets in header_rewrite bundle. Co-Authored-By: Claude Opus 4.6 --- plugins/header_rewrite/CMakeLists.txt | 9 ++- plugins/header_rewrite/matcher.h | 2 +- plugins/header_rewrite/matcher_tests.cc | 24 ++++++++ .../header_rewrite_bundle.replay.yaml | 56 ++++++++++++++++++- .../header_rewrite/rules/rule_client.conf | 6 ++ 5 files changed, 88 insertions(+), 9 deletions(-) diff --git a/plugins/header_rewrite/CMakeLists.txt b/plugins/header_rewrite/CMakeLists.txt index 90ff5156c75..39e2d9ede0f 100644 --- a/plugins/header_rewrite/CMakeLists.txt +++ b/plugins/header_rewrite/CMakeLists.txt @@ -99,11 +99,10 @@ if(BUILD_TESTING) endif() endif() - # This test has linker issue when cripts is enabled, so its commented for now - # add_executable(test_matcher matcher_tests.cc matcher.cc lulu.cc regex_helper.cc resources.cc) - # add_catch2_test(NAME test_matcher COMMAND $) - # - # target_link_libraries(test_matcher PRIVATE Catch2::Catch2WithMain ts::tscore libswoc::libswoc PkgConfig::PCRE2) + add_executable(test_matcher matcher_tests.cc matcher.cc lulu.cc regex_helper.cc) + add_catch2_test(NAME test_matcher COMMAND $) + + target_link_libraries(test_matcher PRIVATE Catch2::Catch2WithMain ts::tscore libswoc::libswoc PkgConfig::PCRE2) endif() verify_global_plugin(header_rewrite) diff --git a/plugins/header_rewrite/matcher.h b/plugins/header_rewrite/matcher.h index 1f4b52e143f..94868a20b67 100644 --- a/plugins/header_rewrite/matcher.h +++ b/plugins/header_rewrite/matcher.h @@ -209,7 +209,7 @@ template class Matchers : public Matcher field.ltrim_if(&isspace).rtrim_if(&isspace); values.insert(convert(std::string(field))); - start = ++cur + skip_quotes; + start = ++cur; skip_quotes = 0; } else { ++cur; diff --git a/plugins/header_rewrite/matcher_tests.cc b/plugins/header_rewrite/matcher_tests.cc index ea0ae607ecb..83c8c1bc6ad 100644 --- a/plugins/header_rewrite/matcher_tests.cc +++ b/plugins/header_rewrite/matcher_tests.cc @@ -83,6 +83,17 @@ TSHttpTxnServerRespGet(TSHttpTxn, TSMBuffer *, TSMLoc *) return TS_SUCCESS; } +TSHttpSsn +TSHttpTxnSsnGet(TSHttpTxn) +{ + return nullptr; +} + +void +Resources::destroy() +{ +} + ClassAllocator mutexAllocator("mutexAllocator"); TEST_CASE("Matcher", "[plugins][header_rewrite]") @@ -106,3 +117,16 @@ TEST_CASE("MatcherSet", "[plugins][header_rewrite]") foo.set("foo, bar, baz", CondModifiers::MOD_NOCASE); REQUIRE(foo.test("FOO", res) == true); } + +TEST_CASE("MatcherSetQuoted", "[plugins][header_rewrite]") +{ + Matchers foo(MATCH_SET); + TSHttpTxn txn = nullptr; + TSCont c = nullptr; + Resources res(txn, c); + + foo.set("\"foo\",\"bar\"", CondModifiers::MOD_NOCASE); + REQUIRE(foo.test("FOO", res) == true); + REQUIRE(foo.test("BAR", res) == true); + REQUIRE(foo.test("BAZ", res) == false); +} diff --git a/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bundle.replay.yaml b/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bundle.replay.yaml index 81245d5b0d6..65be89b4536 100644 --- a/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bundle.replay.yaml +++ b/tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bundle.replay.yaml @@ -314,7 +314,57 @@ sessions: - [ X-Testing, { value: "elif", as: equal } ] - [ X-Pre-Else, { as: absent } ] - # Test 6: cond method GET + # Test 6: quoted set matching - second item "bar" should match +- transactions: + - client-request: + method: "GET" + version: "1.1" + url: /from_1/hrw-sets.png + headers: + fields: + - [ Host, www.example.com ] + - [ X-Quoted-Set, "bar" ] + - [ uuid, quoted-set-match ] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Connection, close ] + + proxy-response: + status: 200 + headers: + fields: + - [ X-Quoted-Set, { value: "Yes", as: equal } ] + + # Test 7: quoted set matching - non-member should not match +- transactions: + - client-request: + method: "GET" + version: "1.1" + url: /from_1/hrw-sets.png + headers: + fields: + - [ Host, www.example.com ] + - [ X-Quoted-Set, "baz" ] + - [ uuid, quoted-set-nomatch ] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [ Connection, close ] + + proxy-response: + status: 200 + headers: + fields: + - [ X-Quoted-Set, { value: "No", as: equal } ] + + # Test 8: cond method GET - transactions: - client-request: method: "GET" @@ -338,7 +388,7 @@ sessions: fields: - [ Via, { as: present } ] - # Test 7: cond method DELETE + # Test 9: cond method DELETE - transactions: - client-request: method: "DELETE" @@ -362,7 +412,7 @@ sessions: fields: - [ Via, { as: present } ] - # Test 8: End [L] #5423 + # Test 10: End [L] #5423 - transactions: - client-request: method: "GET" diff --git a/tests/gold_tests/pluginTest/header_rewrite/rules/rule_client.conf b/tests/gold_tests/pluginTest/header_rewrite/rules/rule_client.conf index e298a8bed1c..6562aaa1f24 100644 --- a/tests/gold_tests/pluginTest/header_rewrite/rules/rule_client.conf +++ b/tests/gold_tests/pluginTest/header_rewrite/rules/rule_client.conf @@ -39,3 +39,9 @@ elif set-header X-Testing "elif" else set-header X-Testing "No" + +cond %{SEND_RESPONSE_HDR_HOOK} +cond %{CLIENT-HEADER:X-Quoted-Set} ("foo","bar") + set-header X-Quoted-Set "Yes" +else + set-header X-Quoted-Set "No" From e17796198902b740ab99b40b34bd1b803dd0d45e Mon Sep 17 00:00:00 2001 From: Chris McFarlen Date: Thu, 26 Mar 2026 16:34:01 -0500 Subject: [PATCH 2/3] link cripts to test if enabled --- plugins/header_rewrite/CMakeLists.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/header_rewrite/CMakeLists.txt b/plugins/header_rewrite/CMakeLists.txt index 39e2d9ede0f..2594743f229 100644 --- a/plugins/header_rewrite/CMakeLists.txt +++ b/plugins/header_rewrite/CMakeLists.txt @@ -104,6 +104,11 @@ if(BUILD_TESTING) target_link_libraries(test_matcher PRIVATE Catch2::Catch2WithMain ts::tscore libswoc::libswoc PkgConfig::PCRE2) + if(ENABLE_CRIPTS) + target_link_libraries(test_matcher PRIVATE ts::cripts) + target_compile_definitions(test_matcher PRIVATE TS_HAS_CRIPTS=1) + endif() + endif() verify_global_plugin(header_rewrite) verify_remap_plugin(header_rewrite) From 617f789d34caedfa4244b2c0ec1b5c78d8b3424e Mon Sep 17 00:00:00 2001 From: Chris McFarlen Date: Thu, 26 Mar 2026 16:44:02 -0500 Subject: [PATCH 3/3] nevermind, that test is cursed --- plugins/header_rewrite/CMakeLists.txt | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/plugins/header_rewrite/CMakeLists.txt b/plugins/header_rewrite/CMakeLists.txt index 2594743f229..619c0a1191e 100644 --- a/plugins/header_rewrite/CMakeLists.txt +++ b/plugins/header_rewrite/CMakeLists.txt @@ -99,15 +99,15 @@ if(BUILD_TESTING) endif() endif() - add_executable(test_matcher matcher_tests.cc matcher.cc lulu.cc regex_helper.cc) - add_catch2_test(NAME test_matcher COMMAND $) - - target_link_libraries(test_matcher PRIVATE Catch2::Catch2WithMain ts::tscore libswoc::libswoc PkgConfig::PCRE2) - - if(ENABLE_CRIPTS) - target_link_libraries(test_matcher PRIVATE ts::cripts) - target_compile_definitions(test_matcher PRIVATE TS_HAS_CRIPTS=1) - endif() + # add_executable(test_matcher matcher_tests.cc matcher.cc lulu.cc regex_helper.cc) + # add_catch2_test(NAME test_matcher COMMAND $) + # + # target_link_libraries(test_matcher PRIVATE Catch2::Catch2WithMain ts::tscore libswoc::libswoc PkgConfig::PCRE2) + # + # if(ENABLE_CRIPTS) + # target_link_libraries(test_matcher PRIVATE ts::cripts) + # target_compile_definitions(test_matcher PRIVATE TS_HAS_CRIPTS=1) + # endif() endif() verify_global_plugin(header_rewrite)