bryancall commented on code in PR #13661:
URL: https://github.com/apache/trafficserver/pull/13661#discussion_r3983498957
##########
src/tsutil/unit_tests/test_Regex.cc:
##########
@@ -1050,3 +1050,90 @@ TEST_CASE("Regex end-anchor with alternation",
"[libts][Regex]")
CHECK(r.exec("cdn.example.com.evil.com", matches) == RE_ERROR_NOMATCH);
CHECK(r.exec("prefix.cdn.example.com", matches) == RE_ERROR_NOMATCH);
}
+
+namespace
+{
+/** Does PCRE2 have JIT code for this pattern?
+ *
+ * The two tests below are about the JIT stack, and PCRE2 consults it only
when it
+ * has JIT code to run. Without it both a blank context and the shared one
take the
+ * interpreter and return the same answer, so the tests would pass whether or
not
+ * the behaviour they describe is present. Ask PCRE2 rather than assume.
+ */
+bool
+pattern_has_jit(char const *pattern)
+{
+ int errnum = 0;
+ PCRE2_SIZE erroffset = 0;
+ pcre2_code *code = pcre2_compile(reinterpret_cast<PCRE2_SPTR>(pattern),
PCRE2_ZERO_TERMINATED, 0, &errnum, &erroffset, nullptr);
+ if (code == nullptr) {
+ return false;
+ }
+ pcre2_jit_compile(code, PCRE2_JIT_COMPLETE);
+ size_t jit_size = 0;
+ pcre2_pattern_info(code, PCRE2_INFO_JITSIZE, &jit_size);
+ pcre2_code_free(code);
+ return jit_size > 0;
+}
+} // namespace
+
+// A caller-supplied RegexMatchContext must behave like the shared context that
+// Regex::exec uses when none is supplied. A context built from scratch
silently
+// drops everything the shared one configures, which is how regex_remap came to
+// run with PCRE2's fallback 32KiB JIT stack instead of the 1MiB one.
+TEST_CASE("RegexMatchContext matches the shared context",
"[libts][Regex][RegexMatchContext]")
+{
+ // Quantified alternation of capture groups: every subject character pushes a
+ // backtracking frame, so the JIT stack size is what bounds this.
+ char const *const pattern = R"(^(?:(a)|(b))+$)";
+ if (!pattern_has_jit(pattern)) {
+ SKIP("PCRE2 has no JIT for this pattern, so the JIT stack is never
consulted");
+ }
+
+ Regex re;
+ REQUIRE(re.compile(pattern));
+
+ std::string const subject(1000, 'a');
+
+ RegexMatches shared_matches;
+ RegexMatchContext match_context;
+ RegexMatches own_matches;
+
+ int const shared_rc = re.exec(subject, shared_matches);
+ int const own_rc = re.exec(subject, own_matches, 0, &match_context);
+ CAPTURE(shared_rc, own_rc);
+
+ REQUIRE(shared_rc > 0);
+ REQUIRE(own_rc == shared_rc);
+}
+
+// The guard from #5762: a pattern that backtracks once per character must fail
+// cleanly rather than run the thread out of stack. PCRE1 recursed on the
machine
+// stack and a long enough subject crashed the server; PCRE2 must report an
error
+// instead. If this ever crashes rather than fails, that regression is back.
+TEST_CASE("Regex reports resource exhaustion rather than crashing",
"[libts][Regex][limits]")
+{
+ // Only the JIT path has a bound to exhaust here. PCRE2's interpreter keeps
its
+ // backtracking frames on the heap, so it matches this subject rather than
running
+ // out of anything, and there is no resource error to assert.
+ char const *const pattern =
R"(^/alpha/bravo/[?]((?!action=(newsfeed|calendar|contacts|notepad)).)*$)";
+ if (!pattern_has_jit(pattern)) {
+ SKIP("PCRE2 has no JIT for this pattern, so there is no stack bound to
exhaust");
+ }
+
+ Regex re;
+ REQUIRE(re.compile(pattern));
+
+ // Past what a 1MiB JIT stack holds for this pattern, which starts failing at
+ // roughly 43KiB of subject, so the bound is still exercised.
+ std::string subject{"/alpha/bravo/?"};
+ subject.append(2 * 1024 * 1024, 'x');
+
Review Comment:
Done in db72f15ace, sized to 256KiB.
One correction to the reasoning, since the outcome is right but the stated
cost is not
what I measured. The match time does not scale with the subject here: it
bails at the
JIT stack limit long before traversing it. Measured against libpcre2 10.47:
```
65536 bytes ( 1.5x threshold) -> rc=-46 0.172 ms
131072 bytes ( 3.0x threshold) -> rc=-46 0.138 ms
262144 bytes ( 6.0x threshold) -> rc=-46 0.136 ms
2097152 bytes (48.0x threshold) -> rc=-46 0.148 ms
```
So this was not slower and had no timing sensitivity to remove. What it did
have was a
48 times oversized allocation for no added coverage, which is worth fixing
on its own.
I chose 256KiB rather than something nearer the threshold to keep a six
times margin.
The 43KiB figure is measured, and it came out identical on x86_64 and arm64,
but a
platform with larger JIT frames would push it up, and a test that stops
exercising the
bound would pass silently rather than fail. The margin and that reasoning
are now in a
comment above the line so it does not get trimmed later.
--
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]
