bryancall commented on PR #13683: URL: https://github.com/apache/trafficserver/pull/13683#issuecomment-6019585402
@JosiahWI Sorry this one sat so long. The test was enforcing a bug. This PR doesn't change any documented behavior. The rule is `^/alpha/bravo/[?]((?!action=(newsfeed|calendar|contacts|notepad)).)*$ https://redirect.com/ @status=301`. The query is `action=newsfed;...` (newsfed, not newsfeed), so the lookahead never trips. The pattern matches that URL, and the docs say a matched rule redirects. The old run 3 got a 200 only because the match never finished. The rule backtracks once per subject character. The plugin's match context carried no JIT stack, so PCRE2 fell back to its 32 KiB default. A 3 KB query used that up, the match returned an error, and the rule was skipped (#13660). You can see the test knew this: next to the 200 it required `Bad regular expression result -46/-47/-53/-63` in diags.log, so it passed only when the regex failed. With the context now inheriting the shared 1 MB stack, the same request matches and returns the 301. Run 3 is what fails if the fix is reverted. The #5762 crash guard is still there as run 3b. It sends a 64,000-byte query, past the roughly 44 KB that the 1 MB stack can handle, and asserts the same 200 plus resource-exhaustion error as before. It's gated on `TS_HAS_PCRE2_JIT`, because the interpreter keeps its frames on the heap and just matches. test_Regex.cc covers the same property directly by bounding the stack. If that answers it, could you take another look? Your review from 9/15 still shows changes requested, and I believe every thread from it is addressed. -- 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]
