Inherit the shared match context configuration in RegexMatchContext

include/tsutil/Regex.h

@@ -113,12 +113,13 @@ class RegexMatchContext
   RegexMatchContext();
   ~RegexMatchContext();
 
-  /// uses pcre2_match_context_copy for a deep copy.
-  RegexMatchContext(RegexMatchContext const &orig);
-  RegexMatchContext &operator=(RegexMatchContext const &orig);
-
-  RegexMatchContext(RegexMatchContext &&)            = default;
-  RegexMatchContext &operator=(RegexMatchContext &&) = default;
+  /// Not copyable or movable. Nothing copies one, the defaulted move copied the
+  /// raw pointer and left both objects freeing it, and the copy constructor could
+  /// leave the pointer null for its own destructor to assert on.
+  RegexMatchContext(RegexMatchContext const &)            = delete;
+  RegexMatchContext &operator=(RegexMatchContext const &) = delete;
+  RegexMatchContext(RegexMatchContext &&)                 = delete;
+  RegexMatchContext &operator=(RegexMatchContext &&)      = delete;
 
   /** Limits the amount of backtracking that can take place.
    * Any regex exec call that fails will return PCRE2_ERROR_MATCHLIMIT(-47)

src/tsutil/Regex.cc

@@ -26,6 +26,7 @@
 
 #define PCRE2_CODE_UNIT_WIDTH 8
 #include <pcre2.h>
+#include <pthread.h>
 
 #include <array>
 #include <vector>
@@ -79,6 +80,54 @@ my_free(void *ptr, void * /*caller*/)
   free(ptr);
 }
 
+//----------------------------------------------------------------------------
+// One match context is shared by every thread that matches through it, and PCRE2
+// requires a distinct JIT stack per thread, so the stack comes from a callback
+// invoked at match time rather than a pointer baked in when the context is built.
+//
+// The per thread stack is held in a pthread key rather than a thread_local. A
+// thread_local with a destructor registers it through __cxa_thread_atexit, which
+// takes the dynamic loader lock; doing that from a match would invert lock order
+// against a dlopen caller running a plugin's static initialization. See the same
+// hazard described at Diags::tag_activated. A pthread key registers its destructor
+// once, at key creation, and never from the matching path.
+pthread_key_t  jit_stack_key;
+bool           jit_stack_key_valid = false;
+pthread_once_t jit_stack_key_once  = PTHREAD_ONCE_INIT;
+
+void
+destroy_jit_stack(void *stack)
+{
+  if (stack != nullptr) {
+    pcre2_jit_stack_free(static_cast<pcre2_jit_stack *>(stack));
+  }
+}
+
+void
+make_jit_stack_key()
+{
+  jit_stack_key_valid = pthread_key_create(&jit_stack_key, destroy_jit_stack) == 0;
+}
+
+pcre2_jit_stack *
+jit_stack_for_this_thread(void *)
+{
+  pthread_once(&jit_stack_key_once, make_jit_stack_key);
+  if (!jit_stack_key_valid) {
+    // Without a key there is nowhere to keep a stack, and jit_stack_key holds a
+    // default value that may name an unrelated key. Returning null tells PCRE2 to
+    // use its own default stack, which pcre2jit documents as thread safe.
+    return nullptr;
+  }
+
+  auto *stack = static_cast<pcre2_jit_stack *>(pthread_getspecific(jit_stack_key));
+  if (stack == nullptr) {
+    stack = pcre2_jit_stack_create(4096, 1024 * 1024, nullptr); // 1 page min and 1MB max
+    pthread_setspecific(jit_stack_key, stack);
+  }
+  return stack;
+}
+
 //----------------------------------------------------------------------------
 class RegexContext
 {
@@ -100,9 +149,6 @@ class RegexContext
     if (_match_context != nullptr) {
       pcre2_match_context_free(_match_context);
     }
-    if (_jit_stack != nullptr) {
-      pcre2_jit_stack_free(_jit_stack);
-    }
   }
   pcre2_general_context *
   get_general_context()
@@ -126,13 +172,11 @@ class RegexContext
     _general_context = pcre2_general_context_create(my_malloc, my_free, nullptr);
     _compile_context = pcre2_compile_context_create(_general_context);
     _match_context   = pcre2_match_context_create(_general_context);
-    _jit_stack       = pcre2_jit_stack_create(4096, 1024 * 1024, nullptr); // 1 page min and 1MB max
-    pcre2_jit_stack_assign(_match_context, nullptr, _jit_stack);
+    pcre2_jit_stack_assign(_match_context, jit_stack_for_this_thread, nullptr);
   }
   pcre2_general_context *_general_context = nullptr;
   pcre2_compile_context *_compile_context = nullptr;
   pcre2_match_context   *_match_context   = nullptr;
-  pcre2_jit_stack       *_jit_stack       = nullptr;
 };
 
 } // namespace
@@ -257,37 +301,15 @@ struct RegexMatchContext::_MatchContext {
 //----------------------------------------------------------------------------
 RegexMatchContext::RegexMatchContext()
 {
-  auto ctx = pcre2_match_context_create(nullptr);
-  debug_assert_message(ctx, "Failed to allocate custom pcre2 match context");
+  // Copy the shared context rather than building a blank one. A blank context
+  // silently drops everything the shared context configures, which is how this
+  // type came to run with PCRE2's fallback 32KiB JIT stack instead of the 1MiB
+  // one every other caller gets. Callers override only the fields they mean to.
+  auto ctx = pcre2_match_context_copy(RegexContext::get_instance()->get_match_context());
+  debug_assert_message(ctx, "Failed to copy the shared pcre2 match context");
   _MatchContext::set(_match_context, ctx);
 }
 
-//----------------------------------------------------------------------------
-RegexMatchContext::RegexMatchContext(RegexMatchContext const &other)
-{
-  auto ptr = _MatchContext::get(other._match_context);
-  if (nullptr != ptr) {
-    pcre2_match_context *const ctx = pcre2_match_context_copy(ptr);
-    _MatchContext::set(_match_context, ctx);
-  }
-}
-
-//----------------------------------------------------------------------------
-RegexMatchContext &
-RegexMatchContext::operator=(RegexMatchContext const &other)
-{
-  if (&other != this) {
-    auto ptr = _MatchContext::get(other._match_context);
-    if (nullptr != ptr) {
-      pcre2_match_context *const ctx = pcre2_match_context_copy(ptr);
-      _MatchContext::set(_match_context, ctx);
-    } else {
-      _MatchContext::set(_match_context, nullptr);
-    }
-  }
-  return *this;
-}
-
 //----------------------------------------------------------------------------
 RegexMatchContext::~RegexMatchContext()
 {

src/tsutil/unit_tests/test_Regex.cc

@@ -1050,3 +1050,92 @@ 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));
+
+  // This pattern starts failing at roughly 43KiB of subject against a 1MiB JIT
+  // stack, measured identically on x86_64 and arm64. 256KiB keeps a six times
+  // margin for a platform whose JIT frames are larger, without allocating more
+  // than the bound needs. Do not trim this to just above 43KiB.
+  std::string subject{"/alpha/bravo/?"};
+  subject.append(256 * 1024, 'x');
+
+  RegexMatches matches;
+  int const    rc = re.exec(subject, matches);
+  CAPTURE(rc);
+
+  // Reaching this line at all is the crash assertion.
+  REQUIRE(rc < 0);
+  REQUIRE(rc != RE_ERROR_NOMATCH);
+}

tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py

@@ -88,7 +88,9 @@
         'proxy.config.diags.debug.enabled': 1,
         'proxy.config.diags.debug.tags': 'http|regex_remap',
         'proxy.config.dns.nameservers': f"127.0.0.1:{nameserver.Variables.Port}",
-        'proxy.config.dns.resolv_conf': 'NULL'
+        'proxy.config.dns.resolv_conf': 'NULL',
+        # The crash-guard run below needs a request larger than the 32 KB default.
+        'proxy.config.http.request_header_max_size': 131072
     })
 
 # 0 Test - Load cache (miss) (path1)
@@ -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)
 tr.Processes.Default.ReturnCode = 0
 tr.Processes.Default.Streams.stdout = "gold/regex_remap_crash.gold"
 ts.Disk.diags_log.Content += Testers.ContainsExpression(