bryancall commented on code in PR #13683:
URL: https://github.com/apache/trafficserver/pull/13683#discussion_r4017439356
##########
tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py:
##########
@@ -123,12 +125,29 @@
tr.Processes.Default.Streams.stdout = "gold/regex_remap_simple.gold"
tr.StillRunningAfter = ts
-# 3 Test - Preserve the original crash guard from #5762. This request must
+# 3 Test - A 3 KB query redirects. This rule backtracks once per subject
+# character, so it used to exhaust the 32 KB stack PCRE2 falls back to when a
+# match context carries none, and the rule was skipped. The plugin's context
now
+# inherits the shared 1 MB stack, so the rule matches and the redirect fires.
+tr = Test.AddTestRun("long query redirects rather than exhausting the JIT
stack")
+creq = replay_txns[1]['client-request']
+tr.MakeCurlCommand(
+ curl_and_args + f"--header 'uuid: {creq['headers']['fields'][1][1]}'
'{creq['url']}'" + " | grep -e '^HTTP/' -e '^Location'",
+ ts=ts)
+tr.Processes.Default.ReturnCode = 0
+tr.Processes.Default.Streams.stdout = "gold/regex_remap_redirect.gold"
+tr.StillRunningAfter = ts
+
+# 3b Test - Preserve the original crash guard from #5762. This request must
# survive resource exhaustion without redirecting, regardless of which matching
-# resource limit is reached (JIT stack, match work, depth, or heap).
+# resource limit is reached (JIT stack, match work, depth, or heap). Against
the
+# shared 1 MB stack this rule needs a subject past 43 KB to exhaust it, which
is
+# why the request header limit is raised above. Shortening this query silently
+# turns the run into a plain redirect test.
+crash_guard_query = 'x' * 64000
tr = Test.AddTestRun("resource exhaustion does not crash ATS")
-creq = replay_txns[1]['client-request']
-tr.MakeCurlCommand(curl_and_args + f"--header 'uuid:
{creq['headers']['fields'][1][1]}' '{creq['url']}'", ts=ts)
+tr.MakeCurlCommand(
+ curl_and_args + "--header 'uuid: 180' " +
f"'http://example.one/alpha/bravo/?action=newsfed;{crash_guard_query}'", ts=ts)
Review Comment:
Fixed in 6efbf77f3e, and you were right that the unit tests were the tell:
they skip on `pattern_has_jit()` and this run did not, so a PCRE2 built without
JIT would have failed the suite on a rule that behaves exactly as designed.
The run now gates on a new `TS_HAS_PCRE2_JIT` feature, reported by
`traffic_layout info` from `pcre2_config(PCRE2_CONFIG_JIT, ...)`.
`traffic_layout` already includes `pcre2.h` and gets PCRE2 through `tsutil`'s
PUBLIC link, so this needed no build change. The gate reads the way the QUIC
and Brotli runs read.
I checked the gate is wired rather than decorative. On a JIT build the suite
executes 14 test runs and `diags.log` carries both `-46` and `-47`. With the
condition pointed at a feature that does not exist, it executes 13, `diags.log`
carries only `-47`, and nothing fails for a missing assertion, because the
`ContainsExpression` is scoped inside the same branch.
Nothing else in the file needs the gate. The 3 KB redirect run gets the same
answer from either engine, so it passes without a JIT, and I said so in a
comment rather than leaving the next reader to work it out. The crash property
is not lost on a build without JIT either: the match-limit run reaches `-47`
through the interpreter, and `test_Regex.cc` asserts the property directly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
