From dccd8f0658f85060705cb82522c07144c20d9885 Mon Sep 17 00:00:00 2001 From: bneradt Date: Tue, 8 Sep 2026 15:29:05 -0500 Subject: [PATCH 1/2] Use the shared Regex context in regex_remap Valid long URLs can miss regex_remap redirects. The old PCRE matcher used recursive calls for backtracking, so its recursion limit was reduced from 2047 to 1750 after stack crashes in #6819. The PCRE2 conversion in #12575 accidentally reused 1750 as a matching-work limit. Its per-instance context also bypassed ATS's 1 MiB thread-local JIT stack, leaving a 32 KiB fallback that rejects a valid 3 KB query. PCRE2 still has a depth limit, but since 10.30 its interpreter stores backtracking frames on the heap rather than using recursive calls. Depth and heap limits control that storage; JIT ignores the depth limit and uses a separately bounded stack. The old stack-derived 1750 value is therefore neither a suitable work budget nor a JIT safeguard. This patch uses ATS's shared thread-local Regex context, restoring PCRE2's default work limit and ATS's 1 MiB JIT stack while retaining depth and heap limits. The normal work default of 10 million permits more worst-case CPU time per match than 1750, but still bounds excessive backtracking. This patch covers both long-query redirects and the remaining work limit, preserves independent log assertions, and removes ESI's obsolete cross-reference without changing its limit. Backport: 10.2.x only. The 10.1.x and 9.2.x branches still use PCRE's correct recursion-depth limit and must retain it. Fixes: #13651 Reported-by: Vinith Bindiganavale Co-authored-by: Codex Astra Medium --- plugins/esi/lib/IncludeUrlValidator.cc | 1 - plugins/regex_remap/regex_remap.cc | 43 ++-- .../pluginTest/regex_remap/long_query.conf | 20 ++ .../regex_remap/regex_remap.test.py | 30 ++- .../regex_remap_long_query.test.py | 19 ++ .../regex_remap/replay/long_query.replay.yaml | 230 ++++++++++++++++++ .../regex_remap/replay/yts-2819.replay.json | 2 +- 7 files changed, 309 insertions(+), 36 deletions(-) create mode 100644 tests/gold_tests/pluginTest/regex_remap/long_query.conf create mode 100644 tests/gold_tests/pluginTest/regex_remap/regex_remap_long_query.test.py create mode 100644 tests/gold_tests/pluginTest/regex_remap/replay/long_query.replay.yaml diff --git a/plugins/esi/lib/IncludeUrlValidator.cc b/plugins/esi/lib/IncludeUrlValidator.cc index 5bb9c0c51fe..33834b06fc4 100644 --- a/plugins/esi/lib/IncludeUrlValidator.cc +++ b/plugins/esi/lib/IncludeUrlValidator.cc @@ -39,7 +39,6 @@ namespace // Backtracking limit for the allowlist match. PCRE2 stops and reports // PCRE2_ERROR_MATCHLIMIT once this many match steps are taken, bounding // worst-case CPU per validation against attacker-influenced hostnames. - // Matches the value used by the regex_remap plugin. constexpr uint32_t ALLOW_REGEX_MATCH_LIMIT = 1750; bool diff --git a/plugins/regex_remap/regex_remap.cc b/plugins/regex_remap/regex_remap.cc index 86b2b2d2ba7..ecf186ab58a 100644 --- a/plugins/regex_remap/regex_remap.cc +++ b/plugins/regex_remap/regex_remap.cc @@ -55,9 +55,8 @@ static const char *PLUGIN_NAME = "regex_remap"; // Constants -static const int MATCHCOUNT = 15; // We support $0 - $9 x2 ints, and this needs to be 1.5x that -static const int MAX_SUBS = 32; // No more than 32 substitution variables in the subst string -static const int32_t REGEX_MATCH_LIMIT = 1750; // POOMA - also dependent on actual stack size. Crashes with previous value of 2047 +static const int MATCHCOUNT = 15; // We support $0 - $9 x2 ints, and this needs to be 1.5x that +static const int MAX_SUBS = 32; // No more than 32 substitution variables in the subst string // Substitutions other than regex matches enum ExtraSubstitutions { @@ -116,10 +115,10 @@ struct UrlComponents { // is const, and it is read concurrently and without locks by every ET_NET // thread of every RemapInstance that loaded the same rule file. Nothing here // may be mutated after that point, and nothing per-instance or per-transaction -// may be stored here. That is why the match context and profiling hit counts -// are passed in as arguments rather than kept as members: they belong to -// RemapInstance. Put new per-instance state on RemapInstance, indexed in -// lockstep with RuleSet::rules(), never on this class. +// may be stored here. Profiling hit counts belong to RemapInstance and are +// passed in as arguments rather than kept as members. Put new per-instance +// state on RemapInstance, indexed in lockstep with RuleSet::rules(), never on +// this class. // class RemapRegex { @@ -159,9 +158,9 @@ class RemapRegex // number of matches, or negative if failed int - match(std::string_view const str, RegexMatches &matches, RegexMatchContext const *match_context) const + match(std::string_view const str, RegexMatches &matches) const { - int const stat = _rex.exec(str, matches, 0, match_context); + int const stat = _rex.exec(str, matches); if (0 <= stat) { Dbg(dbg_ctl, "Regex match (%d): %.*s", stat, (int)str.length(), str.data()); return matches.size(); @@ -806,18 +805,17 @@ class RuleSetCache struct RemapInstance { RemapInstance() : filename("unknown") {} - SharedRuleSet rule_set; - std::vector rule_hits; - RegexMatchContext match_context = {}; - bool pristine_url = false; - bool profile = false; - bool method = false; - bool query_string = true; - bool host = false; - int hits = 0; - int misses = 0; - int failures = 0; - std::string filename; + SharedRuleSet rule_set; + std::vector rule_hits; + bool pristine_url = false; + bool profile = false; + bool method = false; + bool query_string = true; + bool host = false; + int hits = 0; + int misses = 0; + int failures = 0; + std::string filename; }; /////////////////////////////////////////////////////////////////////////////// @@ -917,7 +915,6 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE if (!ri->rule_set) { return TS_ERROR; } - ri->match_context.set_match_limit(REGEX_MATCH_LIMIT); if (ri->profile) { ri->rule_hits.resize(ri->rule_set->rules().size()); } @@ -1059,7 +1056,7 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) auto const &re = rules[rule_ix]; // Since we check substitutions on parse time, we don't need to reset ovector - auto match_result = re->match(match_buf.data(), matches, &(ri->match_context)); + auto match_result = re->match(match_buf.data(), matches); if (match_result >= 0) { int new_len = re->get_lengths(matches, lengths, rri, &req_url); diff --git a/tests/gold_tests/pluginTest/regex_remap/long_query.conf b/tests/gold_tests/pluginTest/regex_remap/long_query.conf new file mode 100644 index 00000000000..d151b5b2541 --- /dev/null +++ b/tests/gold_tests/pluginTest/regex_remap/long_query.conf @@ -0,0 +1,20 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Match a token within the query and preserve both capture groups. +^/cms(\?.*)TOKEN(.*)$ https://redirect.example/cms$1TOKEN$2 @status=302 +# Make a skipped rule observable without contacting an origin. +^/cms.*$ https://fallback.example/ @status=307 diff --git a/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py b/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py index 91d4cb7ab3a..49f2805e964 100644 --- a/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py +++ b/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py @@ -45,6 +45,12 @@ # Define ATS and configure ts = Test.MakeATSProcess("ts", enable_cache=False) +# This test deliberately exhausts matching work for one rule. Replace the +# default blanket error exclusion once, then append all rule-specific checks. +ts.Disk.diags_log.Content = Testers.ExcludesExpression( + r'ERROR: (?!\[regex_remap\] Bad regular expression result -47 \("match limit exceeded"\) from "\^/match_limit/)', + "Only the deliberate excessive-backtracking error is allowed") + testName = "regex_remap" regex_remap_conf_path = os.path.join(ts.Variables.CONFIGDIR, 'regex_remap.conf') @@ -116,26 +122,28 @@ tr.Processes.Default.Streams.stdout = "gold/regex_remap_simple.gold" tr.StillRunningAfter = ts -# 3 Test - Match limit test 0 -tr = Test.AddTestRun("match limit 0") +# 3 Test - A valid long query needs more than PCRE2's fallback 32 KiB JIT stack. +tr = Test.AddTestRun("long query with nested captures redirects") creq = replay_txns[1]['client-request'] -tr.MakeCurlCommand(curl_and_args + \ - '--header "uuid: {}" '.format(creq["headers"]["fields"][1][1]) + '"{}"'.format(creq["url"]), ts=ts) +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_crash.gold" -ts.Disk.diags_log.Content = Testers.ContainsExpression( - 'ERROR: .regex_remap. Bad regular expression result -47', "Match limit exceeded") +tr.Processes.Default.Streams.stdout = "gold/regex_remap_redirect.gold" +ts.Disk.diags_log.Content += Testers.ExcludesExpression( + r'Bad regular expression result .*alpha/bravo', "The valid long query must not fail regex matching") tr.StillRunningAfter = ts -# 4 Test - Match limit test 1 -tr = Test.AddTestRun("match limit 1") +# 4 Test - The nested quantifiers must exceed PCRE2's default matching-work limit. +tr = Test.AddTestRun("excessive backtracking reaches the match limit") creq = replay_txns[2]['client-request'] tr.MakeCurlCommand(curl_and_args + \ '--header "uuid: {}" '.format(creq["headers"]["fields"][1][1]) + '"{}"'.format(creq["url"]), ts=ts) tr.Processes.Default.ReturnCode = 0 tr.Processes.Default.Streams.stdout = "gold/regex_remap_crash.gold" -ts.Disk.diags_log.Content = Testers.ContainsExpression( - 'ERROR: .regex_remap. Bad regular expression result -47', "Match limit exceeded") +ts.Disk.diags_log.Content += Testers.ContainsExpression( + r'ERROR: \[regex_remap\] Bad regular expression result -47.*\^/match_limit/', + "The excessive-backtracking rule must reach the match limit") tr.StillRunningAfter = ts diff --git a/tests/gold_tests/pluginTest/regex_remap/regex_remap_long_query.test.py b/tests/gold_tests/pluginTest/regex_remap/regex_remap_long_query.test.py new file mode 100644 index 00000000000..b430530da4e --- /dev/null +++ b/tests/gold_tests/pluginTest/regex_remap/regex_remap_long_query.test.py @@ -0,0 +1,19 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +Test.Summary = 'Verify regex_remap redirects with long query strings.' +Test.SkipUnless(Condition.PluginExists('regex_remap.so')) +Test.ATSReplayTest(replay_file='replay/long_query.replay.yaml') diff --git a/tests/gold_tests/pluginTest/regex_remap/replay/long_query.replay.yaml b/tests/gold_tests/pluginTest/regex_remap/replay/long_query.replay.yaml new file mode 100644 index 00000000000..2f6092ae426 --- /dev/null +++ b/tests/gold_tests/pluginTest/regex_remap/replay/long_query.replay.yaml @@ -0,0 +1,230 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +meta: + version: "1.0" + +autest: + description: 'Valid long query strings must not exhaust the match-work limit' + server: + name: 'server' + client: + name: 'client' + ats: + name: 'ts' + copy_to_config_dir: + - 'long_query.conf' + remap_config: + - from: 'http://example.com/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + plugins: + - name: 'regex_remap.so' + args: ['long_query.conf'] + log_validation: + diags_log: + excludes: + - expression: 'Bad regular expression result' + description: 'Simple query matching must not report any regex error' + +sessions: +- transactions: + - client-request: + method: GET + version: "1.1" + url: "/cms?partner=TOKEN&x=abc" + headers: + fields: + - [Host, example.com] + - [uuid, short-match] + proxy-request: + expect: absent + server-response: + status: 500 + headers: + fields: + - [Content-Length, "0"] + proxy-response: + status: 302 + headers: + fields: + - [Location, {value: "https://redirect.example/cms?partner=TOKEN&x=abc", as: equal}] + + # Regression for #13651: the 2021-byte subject exceeds the old 1750-work + # limit. Keep the 2000-byte suffix: shortening it can hide the regression. + # Only this case requires extensive backtracking; the other requests are + # controls for token placement and absence, not additional stress cases. + - client-request: + method: GET + version: "1.1" + url: "/cms?partner=TOKEN&x=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaa" + headers: + fields: + - [Host, example.com] + - [uuid, long-match] + proxy-request: + expect: absent + server-response: + status: 500 + headers: + fields: + - [Content-Length, "0"] + proxy-response: + status: 302 + headers: + fields: + - [Location, {value: "https://redirect.example/cms?partner=TOKEN&x=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", as: equal}] + + - client-request: + method: GET + version: "1.1" + url: "/cms?x=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaa&partner=TOKEN" + headers: + fields: + - [Host, example.com] + - [uuid, token-at-end] + proxy-request: + expect: absent + server-response: + status: 500 + headers: + fields: + - [Content-Length, "0"] + proxy-response: + status: 302 + headers: + fields: + - [Location, {value: "https://redirect.example/cms?x=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa&partner=TOKEN", as: equal}] + + - client-request: + method: GET + version: "1.1" + url: "/cms?partner=OTHER&x=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\ + aaaaaaaaaaaaaaaaaaaaa" + headers: + fields: + - [Host, example.com] + - [uuid, no-token] + proxy-request: + expect: absent + server-response: + status: 500 + headers: + fields: + - [Content-Length, "0"] + proxy-response: + status: 307 + headers: + fields: + - [Location, {value: "https://fallback.example/", as: equal}] diff --git a/tests/gold_tests/pluginTest/regex_remap/replay/yts-2819.replay.json b/tests/gold_tests/pluginTest/regex_remap/replay/yts-2819.replay.json index 4361a9800ff..d492eb0ba9c 100644 --- a/tests/gold_tests/pluginTest/regex_remap/replay/yts-2819.replay.json +++ b/tests/gold_tests/pluginTest/regex_remap/replay/yts-2819.replay.json @@ -163,7 +163,7 @@ "version": "1.1", "scheme": "http", "method": "GET", - "url": "http://example.one/match_limit/aaaaaaaaaaaaaaaaaaaf", + "url": "http://example.one/match_limit/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaf", "headers": { "fields": [ [ From 1dee7010515f10dd416008e5613770a7dec425d3 Mon Sep 17 00:00:00 2001 From: bneradt Date: Wed, 9 Sep 2026 10:34:13 -0500 Subject: [PATCH 2/2] Restore the plugin match context and crash guard Moving regex_remap to the shared Regex context changes JIT stack behavior beyond the matching-work regression. The original 3 KB lookahead request already failed before the PCRE2 conversion and is part of the crash guard from #5762, not a new redirect regression. This patch restores the per-instance context and non-redirecting crash expectation while retaining the removal of the 1750 work limit. The independent log assertions identify resource exhaustion for the crash guard and match-work exhaustion for the separate nested-quantifier rule. The ordinary long-query coverage and other review improvements remain in place; the shared-context change belongs in a separate PR. Co-authored-by: Codex Astra Medium --- plugins/regex_remap/regex_remap.cc | 37 ++++++++++--------- .../regex_remap/regex_remap.test.py | 26 +++++++------ 2 files changed, 33 insertions(+), 30 deletions(-) diff --git a/plugins/regex_remap/regex_remap.cc b/plugins/regex_remap/regex_remap.cc index ecf186ab58a..6b2d20d1636 100644 --- a/plugins/regex_remap/regex_remap.cc +++ b/plugins/regex_remap/regex_remap.cc @@ -115,10 +115,10 @@ struct UrlComponents { // is const, and it is read concurrently and without locks by every ET_NET // thread of every RemapInstance that loaded the same rule file. Nothing here // may be mutated after that point, and nothing per-instance or per-transaction -// may be stored here. Profiling hit counts belong to RemapInstance and are -// passed in as arguments rather than kept as members. Put new per-instance -// state on RemapInstance, indexed in lockstep with RuleSet::rules(), never on -// this class. +// may be stored here. That is why the match context and profiling hit counts +// are passed in as arguments rather than kept as members: they belong to +// RemapInstance. Put new per-instance state on RemapInstance, indexed in +// lockstep with RuleSet::rules(), never on this class. // class RemapRegex { @@ -158,9 +158,9 @@ class RemapRegex // number of matches, or negative if failed int - match(std::string_view const str, RegexMatches &matches) const + match(std::string_view const str, RegexMatches &matches, RegexMatchContext const *match_context) const { - int const stat = _rex.exec(str, matches); + int const stat = _rex.exec(str, matches, 0, match_context); if (0 <= stat) { Dbg(dbg_ctl, "Regex match (%d): %.*s", stat, (int)str.length(), str.data()); return matches.size(); @@ -805,17 +805,18 @@ class RuleSetCache struct RemapInstance { RemapInstance() : filename("unknown") {} - SharedRuleSet rule_set; - std::vector rule_hits; - bool pristine_url = false; - bool profile = false; - bool method = false; - bool query_string = true; - bool host = false; - int hits = 0; - int misses = 0; - int failures = 0; - std::string filename; + SharedRuleSet rule_set; + std::vector rule_hits; + RegexMatchContext match_context = {}; + bool pristine_url = false; + bool profile = false; + bool method = false; + bool query_string = true; + bool host = false; + int hits = 0; + int misses = 0; + int failures = 0; + std::string filename; }; /////////////////////////////////////////////////////////////////////////////// @@ -1056,7 +1057,7 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) auto const &re = rules[rule_ix]; // Since we check substitutions on parse time, we don't need to reset ovector - auto match_result = re->match(match_buf.data(), matches); + auto match_result = re->match(match_buf.data(), matches, &(ri->match_context)); if (match_result >= 0) { int new_len = re->get_lengths(matches, lengths, rri, &req_url); diff --git a/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py b/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py index 49f2805e964..c6c30830127 100644 --- a/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py +++ b/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py @@ -45,11 +45,12 @@ # Define ATS and configure ts = Test.MakeATSProcess("ts", enable_cache=False) -# This test deliberately exhausts matching work for one rule. Replace the -# default blanket error exclusion once, then append all rule-specific checks. +# These two rules deliberately exercise resource limits. Replace the blanket +# error exclusion once, then append independent checks for each rule below. ts.Disk.diags_log.Content = Testers.ExcludesExpression( - r'ERROR: (?!\[regex_remap\] Bad regular expression result -47 \("match limit exceeded"\) from "\^/match_limit/)', - "Only the deliberate excessive-backtracking error is allowed") + r'ERROR: (?!\[regex_remap\] Bad regular expression result ' + r'(?:-(?:46|47|53|63) .* from "\^/alpha/bravo/|-47 .* from "\^/match_limit/))', + "Only the deliberate resource-limit errors are allowed") testName = "regex_remap" @@ -122,16 +123,17 @@ tr.Processes.Default.Streams.stdout = "gold/regex_remap_simple.gold" tr.StillRunningAfter = ts -# 3 Test - A valid long query needs more than PCRE2's fallback 32 KiB JIT stack. -tr = Test.AddTestRun("long query with nested captures redirects") +# 3 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). +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']}'" + " | grep -e '^HTTP/' -e '^Location'", - ts=ts) +tr.MakeCurlCommand(curl_and_args + f"--header 'uuid: {creq['headers']['fields'][1][1]}' '{creq['url']}'", ts=ts) tr.Processes.Default.ReturnCode = 0 -tr.Processes.Default.Streams.stdout = "gold/regex_remap_redirect.gold" -ts.Disk.diags_log.Content += Testers.ExcludesExpression( - r'Bad regular expression result .*alpha/bravo', "The valid long query must not fail regex matching") +tr.Processes.Default.Streams.stdout = "gold/regex_remap_crash.gold" +ts.Disk.diags_log.Content += Testers.ContainsExpression( + r'ERROR: \[regex_remap\] Bad regular expression result -(?:46|47|53|63).*"\^/alpha/bravo/', + "The crash-guard rule must report resource exhaustion") tr.StillRunningAfter = ts # 4 Test - The nested quantifiers must exceed PCRE2's default matching-work limit.