bryancall opened a new pull request, #13661: URL: https://github.com/apache/trafficserver/pull/13661
`RegexMatchContext` builds an empty PCRE2 match context, so a caller that supplies its own context silently loses everything the shared context configures. Today that is a 1 MB JIT stack, replaced by PCRE2's 32 KB fallback. `regex_remap` and `esi` are the only two users of the type, and both ran on the fallback. Nobody chose 32 KB. It is what PCRE2 uses when no stack is assigned, and it arrived by omission when the type was introduced in #12575. This builds the context as a copy of the shared one instead, so a caller cannot silently lack what the shared context has, and anything added to the shared context later applies automatically. The shared context now resolves its JIT stack through a callback rather than assigning one directly, because PCRE2 requires a distinct stack per thread and a copied context can be used on another thread. There is no change to `regex_remap.cc`; the plugin keeps its context and the context now starts correct. Issue: https://github.com/apache/trafficserver/issues/13660 ## What this changes in behaviour Measured with `pcre2test` 10.47 against the rule and the 3071 byte URL already in `replay/yts-2819.replay.json`: ``` ~^/alpha/bravo/[?]((?!action=(newsfeed|calendar|contacts|notepad)).)*$~ jitstack=32 Failed: error -46: JIT stack limit reached jitstack=1024 matches ``` That URL now redirects rather than falling through to origin, which is what the rule always intended. The first `-46` threshold for this rule moves from 1377 bytes to 43702 bytes. The default 32 KB fallback lives on the machine stack. An assigned stack is heap allocated. So this reduces thread stack pressure rather than raising it, which is worth stating because the limit this rule has been hitting since 2019 traces back to #5762, where the concern was machine stack exhaustion. ## The #5762 crash guard is preserved, not removed The AuTest run that guards #5762 previously used the 3071 byte URL. Since that URL now matches, the run would no longer have exercised a resource limit. Rather than drop the guard, the run keeps its shape at a subject large enough to still exhaust 1 MB, and the request header limit is raised for it. Verified in the sandbox: ``` Bad regular expression result -46 ("JIT stack limit reached") from "^/alpha/bravo/..." Bad regular expression result -47 ("match limit exceeded") from "^/match_limit/(a+)+$" ``` Two unit tests are added in `test_Regex.cc`: one asserting a caller-supplied context reaches the same verdict as the shared one, and one asserting that resource exhaustion is reported rather than crashing. The second is a direct assertion of the property #5762 was protecting, which the AuTest can only reach indirectly. ## Testing On a dev-asan build: - `test_tsutil` full suite: all pass. `[libts][Regex]` is 386 assertions in 20 cases. - Full `ctest` suite: 134 of 134 pass. This touches shared `tsutil` code, so the whole suite is the relevant blast radius rather than the Regex tests alone. - `regex_remap` and `regex_remap_long_query` AuTests pass. ## A pre-existing AuTest flake, not introduced here The `regex_remap` AuTest is nondeterministic on master today. LeakSanitizer reports a 104 byte leak from `ConfigReloadTask::start_progress_checker()` at `src/mgmt/config/ConfigReloadTrace.cc:394`, which fails the `traffic_server` process exit code. It has nothing to do with regex matching. Five runs of each, same machine, same build settings: | revision | pass | fail | |---|---|---| | unmodified master 7ed34a3d3c | 1 | 4 | | this branch | 3 | 2 | Five runs each is too small to claim this branch is better; the point is only that the flake is present without it. Filed separately. ## Out of scope Whether `regex_remap` should carry a CPU bound at all, and what it should be, is a separate question from whether a caller-supplied context should silently differ from the shared one. Related: https://github.com/apache/trafficserver/issues/13654 -- 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]
