Skip to content

Commit 52688fe

Browse files
cmcfarlenclaude
andauthored
Fix MATCH_SET parsing of quoted items and add tests (#13024)
* 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 <noreply@anthropic.com> * link cripts to test if enabled * nevermind, that test is cursed --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 4f9f3ea commit 52688fe

5 files changed

Lines changed: 92 additions & 8 deletions

File tree

plugins/header_rewrite/CMakeLists.txt

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,15 @@ if(BUILD_TESTING)
9999
endif()
100100
endif()
101101

102-
# This test has linker issue when cripts is enabled, so its commented for now
103-
# add_executable(test_matcher matcher_tests.cc matcher.cc lulu.cc regex_helper.cc resources.cc)
104-
# add_catch2_test(NAME test_matcher COMMAND $<TARGET_FILE:test_matcher>)
102+
# add_executable(test_matcher matcher_tests.cc matcher.cc lulu.cc regex_helper.cc)
103+
# add_catch2_test(NAME test_matcher COMMAND $<TARGET_FILE:test_matcher>)
105104
#
106-
# target_link_libraries(test_matcher PRIVATE Catch2::Catch2WithMain ts::tscore libswoc::libswoc PkgConfig::PCRE2)
105+
# target_link_libraries(test_matcher PRIVATE Catch2::Catch2WithMain ts::tscore libswoc::libswoc PkgConfig::PCRE2)
106+
#
107+
# if(ENABLE_CRIPTS)
108+
# target_link_libraries(test_matcher PRIVATE ts::cripts)
109+
# target_compile_definitions(test_matcher PRIVATE TS_HAS_CRIPTS=1)
110+
# endif()
107111

108112
endif()
109113
verify_global_plugin(header_rewrite)

plugins/header_rewrite/matcher.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ template <class T> class Matchers : public Matcher
209209

210210
field.ltrim_if(&isspace).rtrim_if(&isspace);
211211
values.insert(convert(std::string(field)));
212-
start = ++cur + skip_quotes;
212+
start = ++cur;
213213
skip_quotes = 0;
214214
} else {
215215
++cur;

plugins/header_rewrite/matcher_tests.cc

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,17 @@ TSHttpTxnServerRespGet(TSHttpTxn, TSMBuffer *, TSMLoc *)
8383
return TS_SUCCESS;
8484
}
8585

86+
TSHttpSsn
87+
TSHttpTxnSsnGet(TSHttpTxn)
88+
{
89+
return nullptr;
90+
}
91+
92+
void
93+
Resources::destroy()
94+
{
95+
}
96+
8697
ClassAllocator<ProxyMutex, false> mutexAllocator("mutexAllocator");
8798

8899
TEST_CASE("Matcher", "[plugins][header_rewrite]")
@@ -106,3 +117,16 @@ TEST_CASE("MatcherSet", "[plugins][header_rewrite]")
106117
foo.set("foo, bar, baz", CondModifiers::MOD_NOCASE);
107118
REQUIRE(foo.test("FOO", res) == true);
108119
}
120+
121+
TEST_CASE("MatcherSetQuoted", "[plugins][header_rewrite]")
122+
{
123+
Matchers<std::string> foo(MATCH_SET);
124+
TSHttpTxn txn = nullptr;
125+
TSCont c = nullptr;
126+
Resources res(txn, c);
127+
128+
foo.set("\"foo\",\"bar\"", CondModifiers::MOD_NOCASE);
129+
REQUIRE(foo.test("FOO", res) == true);
130+
REQUIRE(foo.test("BAR", res) == true);
131+
REQUIRE(foo.test("BAZ", res) == false);
132+
}

tests/gold_tests/pluginTest/header_rewrite/header_rewrite_bundle.replay.yaml

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,57 @@ sessions:
321321
- [ X-Testing, { value: "elif", as: equal } ]
322322
- [ X-Pre-Else, { as: absent } ]
323323

324-
# Test 6: cond method GET
324+
# Test 6: quoted set matching - second item "bar" should match
325+
- transactions:
326+
- client-request:
327+
method: "GET"
328+
version: "1.1"
329+
url: /from_1/hrw-sets.png
330+
headers:
331+
fields:
332+
- [ Host, www.example.com ]
333+
- [ X-Quoted-Set, "bar" ]
334+
- [ uuid, quoted-set-match ]
335+
336+
server-response:
337+
status: 200
338+
reason: OK
339+
headers:
340+
fields:
341+
- [ Connection, close ]
342+
343+
proxy-response:
344+
status: 200
345+
headers:
346+
fields:
347+
- [ X-Quoted-Set, { value: "Yes", as: equal } ]
348+
349+
# Test 7: quoted set matching - non-member should not match
350+
- transactions:
351+
- client-request:
352+
method: "GET"
353+
version: "1.1"
354+
url: /from_1/hrw-sets.png
355+
headers:
356+
fields:
357+
- [ Host, www.example.com ]
358+
- [ X-Quoted-Set, "baz" ]
359+
- [ uuid, quoted-set-nomatch ]
360+
361+
server-response:
362+
status: 200
363+
reason: OK
364+
headers:
365+
fields:
366+
- [ Connection, close ]
367+
368+
proxy-response:
369+
status: 200
370+
headers:
371+
fields:
372+
- [ X-Quoted-Set, { value: "No", as: equal } ]
373+
374+
# Test 8: cond method GET
325375
- transactions:
326376
- client-request:
327377
method: "GET"
@@ -345,7 +395,7 @@ sessions:
345395
fields:
346396
- [ Via, { as: present } ]
347397

348-
# Test 7: cond method DELETE
398+
# Test 9: cond method DELETE
349399
- transactions:
350400
- client-request:
351401
method: "DELETE"
@@ -369,7 +419,7 @@ sessions:
369419
fields:
370420
- [ Via, { as: present } ]
371421

372-
# Test 8: End [L] #5423
422+
# Test 10: End [L] #5423
373423
- transactions:
374424
- client-request:
375425
method: "GET"

tests/gold_tests/pluginTest/header_rewrite/rules/rule_client.conf

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,9 @@ elif
3939
set-header X-Testing "elif"
4040
else
4141
set-header X-Testing "No"
42+
43+
cond %{SEND_RESPONSE_HDR_HOOK}
44+
cond %{CLIENT-HEADER:X-Quoted-Set} ("foo","bar")
45+
set-header X-Quoted-Set "Yes"
46+
else
47+
set-header X-Quoted-Set "No"

0 commit comments

Comments
 (0)