diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index 5b913bece06..c9c148ef181 100644 --- a/include/tsutil/Regex.h +++ b/include/tsutil/Regex.h @@ -144,7 +144,8 @@ class Regex * * Creates a new Regex object with a deep copy of the compiled pattern. * Uses pcre2_code_copy() to duplicate the compiled pattern without - * requiring the original pattern string. + * requiring the original pattern string, then compiles the copy for the + * just-in-time engine, which pcre2_code_copy() cannot carry over. * * @param other The Regex object to copy from. */ diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index 3281622b1ec..70b7e990d84 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -330,7 +330,17 @@ Regex::Regex(Regex const &other) if (other_code != nullptr) { // Use PCRE2's built-in function to deep copy the compiled pattern auto *copied_code = pcre2_code_copy(other_code); - _Code::set(_code, copied_code); + + // Null when pcre2 could not obtain memory. Leave the object empty rather than + // holding a null pattern. + if (copied_code != nullptr) { + // The copy does not carry the machine code the JIT produced, so without this it + // matches on the interpreter, under different resource limits than the original. + // Unchecked for the same reason compile() does not check it. + pcre2_jit_compile(copied_code, PCRE2_JIT_COMPLETE); + + _Code::set(_code, copied_code); + } } } diff --git a/src/tsutil/unit_tests/test_Regex.cc b/src/tsutil/unit_tests/test_Regex.cc index f1bd0a7c866..5819131994e 100644 --- a/src/tsutil/unit_tests/test_Regex.cc +++ b/src/tsutil/unit_tests/test_Regex.cc @@ -1050,3 +1050,67 @@ 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); } + +// pcre2_code_copy() copies the compiled pattern but not the machine code the JIT produced +// for it, because that code is position dependent. A copy that is not passed back through +// pcre2_jit_compile() therefore matches on the interpreter: the same answers, far more +// slowly, and under a different set of resource limits, so a subject one of them reports +// as too expensive the other quietly matches. +// +// The subject below is sized past the JIT engine's stack bound for this pattern, which is +// what makes the two engines disagree. The assertion is that a copy answers the same as +// its original, whatever that answer is, so the test needs no knowledge of whether this +// build has a JIT. +TEST_CASE("Regex copies answer the same as their original", "[libts][Regex][copy]") +{ + Regex original; + REQUIRE(original.compile(R"(^/alpha/bravo/[?]((?!action=(newsfeed|calendar|contacts|notepad)).)*$)")); + + std::string subject{"/alpha/bravo/?"}; + subject.append(256 * 1024, 'x'); + + RegexMatches original_matches; + int const original_rc = original.exec(subject, original_matches); + CAPTURE(original_rc); + + SECTION("copy constructor") + { + Regex copy(original); + RegexMatches matches; + int const rc = copy.exec(subject, matches); + CAPTURE(rc); + CHECK(rc == original_rc); + } + + SECTION("copy assignment") + { + Regex copy; + REQUIRE(copy.compile("unrelated")); + copy = original; + + RegexMatches matches; + int const rc = copy.exec(subject, matches); + CAPTURE(rc); + CHECK(rc == original_rc); + } + + SECTION("a copy of a copy") + { + Regex first(original); + Regex second(first); + RegexMatches matches; + int const rc = second.exec(subject, matches); + CAPTURE(rc); + CHECK(rc == original_rc); + } + + SECTION("a copy still matches what the original matches") + { + Regex copy(original); + std::string const ordinary{"/alpha/bravo/?action=weather"}; + + RegexMatches original_ordinary; + RegexMatches copy_ordinary; + CHECK(original.exec(ordinary, original_ordinary) == copy.exec(ordinary, copy_ordinary)); + } +}