From 0ae6b3494280a52443792937b5289a3ac86c91f9 Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Thu, 16 Oct 2025 12:48:20 -0600 Subject: [PATCH 1/9] regex_remap: convert from pcre to Regex --- plugins/regex_remap/CMakeLists.txt | 2 +- plugins/regex_remap/regex_remap.cc | 102 ++++++++++------------------- 2 files changed, 34 insertions(+), 70 deletions(-) diff --git a/plugins/regex_remap/CMakeLists.txt b/plugins/regex_remap/CMakeLists.txt index a69af3590da..2a233eedc3a 100644 --- a/plugins/regex_remap/CMakeLists.txt +++ b/plugins/regex_remap/CMakeLists.txt @@ -17,6 +17,6 @@ add_atsplugin(regex_remap regex_remap.cc) -target_link_libraries(regex_remap PRIVATE PCRE::PCRE libswoc::libswoc) +target_link_libraries(regex_remap PRIVATE libswoc::libswoc) verify_remap_plugin(regex_remap) diff --git a/plugins/regex_remap/regex_remap.cc b/plugins/regex_remap/regex_remap.cc index de38ba23787..2a156734756 100644 --- a/plugins/regex_remap/regex_remap.cc +++ b/plugins/regex_remap/regex_remap.cc @@ -45,17 +45,13 @@ #include "tscore/ink_time.h" #include "tscore/ink_inet.h" -#ifdef HAVE_PCRE_PCRE_H -#include -#else -#include -#endif +#include "tsutil/Regex.h" static const char *PLUGIN_NAME = "regex_remap"; // Constants -static const int OVECCOUNT = 30; // 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 int MATCHCOUNT = 15; +static const int MAX_SUBS = 32; // No more than 32 substitution variables in the subst string // Substitutions other than regex matches enum ExtraSubstitutions { @@ -117,13 +113,6 @@ class RemapRegex Dbg(dbg_ctl, "Calling destructor"); TSfree(_rex_string); TSfree(_subst); - - if (_rex) { - pcre_free(_rex); - } - if (_extra) { - pcre_free(_extra); - } } bool initialize(const std::string ®, const std::string &sub, const std::string &opt); @@ -140,25 +129,17 @@ class RemapRegex fprintf(stderr, "[%s]: Regex %d ( %s ): %.2f%%\n", now, ix, _rex_string, 100.0 * _hits / max); } - int compile(const char *&error, int &erroffset); + int compile(std::string &error, int &erroffset); - // Perform the regular expression matching against a string. int - match(const char *str, int len, int ovector[]) + match(std::string_view const str, RegexMatches &matches) const { - return pcre_exec(_rex, // the compiled pattern - _extra, // Extra data from study (maybe) - str, // the subject string - len, // the length of the subject - 0, // start at offset 0 in the subject - 0, // default options - ovector, // output vector for substring information - OVECCOUNT); // number of elements in the output vector + return _rex.exec(str, matches); } // Substitutions - int get_lengths(const int ovector[], int lengths[], TSRemapRequestInfo *rri, UrlComponents *req_url); - int substitute(char dest[], const char *src, const int ovector[], const int lengths[], TSHttpTxn txnp, TSRemapRequestInfo *rri, + int get_lengths(RegexMatches const &matches, int lengths[], TSRemapRequestInfo *rri, UrlComponents *req_url); + int substitute(char dest[], RegexMatches const &matches, const int lengths[], TSHttpTxn txnp, TSRemapRequestInfo *rri, UrlComponents *req_url, bool lowercase_substitutions); // setter / getters for members the linked list. @@ -263,8 +244,7 @@ class RemapRegex bool _lowercase_substitutions = false; - pcre *_rex = nullptr; - pcre_extra *_extra = nullptr; + Regex _rex; RemapRegex *_next = nullptr; TSHttpStatus _status = static_cast(0); @@ -319,7 +299,7 @@ RemapRegex::initialize(const std::string ®, const std::string &sub, const std // These take an option 0|1 value, without value it implies 1 if (opt.compare(start, 8, "caseless") == 0) { - _options |= PCRE_CASELESS; + _options |= RE_CASE_INSENSITIVE; } else if (opt.compare(start, 23, "lowercase_substitutions") == 0) { _lowercase_substitutions = true; } else if (opt.compare(start, 8, "strategy") == 0) { @@ -386,36 +366,17 @@ RemapRegex::initialize(const std::string ®, const std::string &sub, const std // Compile and study the regular expression. int -RemapRegex::compile(const char *&error, int &erroffset) +RemapRegex::compile(std::string &error, int &erroffset) { char *str; - int ccount; + // int ccount; // Initialize these in case they are not set. error = "unknown error"; erroffset = -1; - _rex = pcre_compile(_rex_string, // the pattern - _options, // options - &error, // for error message - &erroffset, // for error offset - nullptr); // use default character tables - - if (nullptr == _rex) { - return -1; - } - - _extra = pcre_study(_rex, PCRE_STUDY_EXTRA_NEEDED, &error); - if (error != nullptr) { - return -1; - } - - // POOMA - also dependent on actual stack size. Crashes with previous value of 2047, - _extra->match_limit_recursion = 1750; - _extra->flags |= PCRE_EXTRA_MATCH_LIMIT_RECURSION; - - if (pcre_fullinfo(_rex, _extra, PCRE_INFO_CAPTURECOUNT, &ccount) != 0) { - error = "call to pcre_fullinfo() failed"; + bool const restat = _rex.compile(_rex_string, error, erroffset, _options); + if (!restat) { return -1; } @@ -464,10 +425,12 @@ RemapRegex::compile(const char *&error, int &erroffset) } if (ix > -1) { - if ((ix < 10) && (ix > ccount)) { - error = "using unavailable captured substring ($n) in substitution"; - return -1; - } + /* +if ((ix < 10) && (ix > matches.size())) { +error = "using unavailable captured substring ($n) in substitution"; +return -1; +} + */ _sub_ix[_num_subs] = ix; _sub_pos[_num_subs] = (str - _subst); @@ -487,7 +450,7 @@ RemapRegex::compile(const char *&error, int &erroffset) // We also calculate a total length for the new string, which is the max length the // substituted string can have (use it to allocate a buffer before calling substitute() ). int -RemapRegex::get_lengths(const int ovector[], int lengths[], TSRemapRequestInfo *rri, UrlComponents *req_url) +RemapRegex::get_lengths(RegexMatches const &matches, int lengths[], TSRemapRequestInfo *rri, UrlComponents *req_url) { int len = _subst_len + 1; // Bigger then necessary @@ -495,7 +458,7 @@ RemapRegex::get_lengths(const int ovector[], int lengths[], TSRemapRequestInfo * int ix = _sub_ix[i]; if (ix < 10) { - lengths[ix] = ovector[2 * ix + 1] - ovector[2 * ix]; // -1 - -1 == 0 + lengths[ix] = matches[ix].length(); len += lengths[ix]; } else { int tmp_len; @@ -541,8 +504,8 @@ RemapRegex::get_lengths(const int ovector[], int lengths[], TSRemapRequestInfo * // regex that was matches, while $1 - $9 are the corresponding groups. Return the final // length of the string as written to dest (not including the trailing '0'). int -RemapRegex::substitute(char dest[], const char *src, const int ovector[], const int lengths[], TSHttpTxn txnp, - TSRemapRequestInfo *rri, UrlComponents *req_url, bool lowercase_substitutions) +RemapRegex::substitute(char dest[], RegexMatches const &matches, const int lengths[], TSHttpTxn txnp, TSRemapRequestInfo *rri, + UrlComponents *req_url, bool lowercase_substitutions) { if (_num_subs > 0) { char *p1 = dest; @@ -556,7 +519,7 @@ RemapRegex::substitute(char dest[], const char *src, const int ovector[], const memcpy(p1, p2, _sub_pos[i] - prev); p1 += (_sub_pos[i] - prev); if (ix < 10) { - memcpy(p1, src + ovector[2 * ix], lengths[ix]); + memcpy(p1, matches[ix].data(), matches[ix].length()); p1 += lengths[ix]; } else { char buff[INET6_ADDRSTRLEN]; @@ -783,9 +746,9 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE continue; } - const char *error; + std::string error; int erroffset; - if (cur->compile(error, erroffset) < 0) { + if (!cur->compile(error, erroffset)) { std::ostringstream oss; oss << '[' << PLUGIN_NAME << "] PCRE failed in " << (ri->filename).c_str() << " (line " << lineno << ')'; if (erroffset > 0) { @@ -915,8 +878,7 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) UrlComponents req_url; req_url.populate(src_url.bufp, src_url.loc); - int ovector[OVECCOUNT]; - int lengths[OVECCOUNT / 2 + 1]; + int lengths[MATCHCOUNT + 1]; int dest_len; TSRemapStatus retval = TSREMAP_DID_REMAP; RemapRegex *re = ri->first; @@ -963,12 +925,14 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) match_buf[match_len] = '\0'; // NULL terminate the match string Dbg(dbg_ctl, "Target match string is `%s'", match_buf); + RegexMatches matches(MATCHCOUNT); + // Apply the regular expressions, in order. First one wins. while (re) { // Since we check substitutions on parse time, we don't need to reset ovector - auto match_result = re->match(match_buf, match_len, ovector); + auto match_result = re->match(match_buf, matches); if (match_result >= 0) { - int new_len = re->get_lengths(ovector, lengths, rri, &req_url); + int new_len = re->get_lengths(matches, lengths, rri, &req_url); // Set timeouts if (re->active_timeout_option() > (-1)) { @@ -1040,7 +1004,7 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) char *dest; dest = static_cast(alloca(new_len + 8)); - dest_len = re->substitute(dest, match_buf, ovector, lengths, txnp, rri, &req_url, lowercase_substitutions); + dest_len = re->substitute(dest, matches, lengths, txnp, rri, &req_url, lowercase_substitutions); Dbg(dbg_ctl, "New URL is estimated to be %d bytes long, or less", new_len); Dbg(dbg_ctl, "New URL is %s (length %d)", dest, dest_len); From 430708b9af935743f8b8c11065dc122c80cf3d15 Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Tue, 28 Oct 2025 08:11:41 -0600 Subject: [PATCH 2/9] get RegexMatchContext helper class working --- include/tsutil/Regex.h | 48 +++++++- plugins/regex_remap/regex_remap.cc | 57 ++++++--- src/tsutil/Regex.cc | 111 +++++++++++++++++- .../regex_remap/regex_remap.test.py | 2 +- 4 files changed, 197 insertions(+), 21 deletions(-) diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index cd8d7c1cb49..aa2739e607f 100644 --- a/include/tsutil/Regex.h +++ b/include/tsutil/Regex.h @@ -94,6 +94,50 @@ class RegexMatches _MatchDataPtr _match_data; }; +/// @brief Wrapper for PCRE2 match context +class RegexMatchContext +{ + friend class Regex; + +public: + /** Construct a new RegexMatchContext object. + */ + RegexMatchContext(); + ~RegexMatchContext(); + + /// uses pcre2_match_context_copy to duplicate. + RegexMatchContext(RegexMatchContext const &orig); + RegexMatchContext &operator=(RegexMatchContext const &orig); + + RegexMatchContext(RegexMatchContext &&) = default; + RegexMatchContext &operator=(RegexMatchContext &&) = default; + + /** maximum amount of heap memory (KiB) used to hold backtracking information. + */ + void setHeapLimit(uint32_t limit); + + /** Limits the amount of backtracking that can take place. + */ + void setMatchLimit(uint32_t limit); + + /** Limits the depth of nested backtracking. + */ + void setDepthLimit(uint32_t limit); + + /** Limits how far an unanchored search can advance in the subject string. + */ + void setOffsetLimit(uint32_t limit); + +private: + /// @internal This wraps a void* so to avoid requiring a pcre2 include. + struct _MatchContext; + struct _MatchContextPtr { + void *_ptr = nullptr; + }; + + _MatchContextPtr _match_context; +}; + /// @brief Wrapper for PCRE2 regular expression. class Regex { @@ -179,6 +223,7 @@ class Regex * @param subject String to match against. * @param matches Place to store the capture groups. * @param flags Match flags (e.g., RE_NOTEMPTY). + * @param optional context Match context (set matching limits). * @return @c The number of capture groups. < 0 if an error occurred. 0 if the number of Matches is too small. * * It is safe to call this method concurrently on the same instance of @a this. @@ -186,7 +231,8 @@ class Regex * Each capture group takes 3 elements of @a ovector, therefore @a ovecsize must * be a multiple of 3 and at least three times the number of desired capture groups. */ - int exec(std::string_view subject, RegexMatches &matches, uint32_t flags) const; + int exec(std::string_view subject, RegexMatches &matches, uint32_t flags, + RegexMatchContext const *const matchContext = nullptr) const; /// @return The number of capture groups in the compiled pattern. int get_capture_count(); diff --git a/plugins/regex_remap/regex_remap.cc b/plugins/regex_remap/regex_remap.cc index 2a156734756..0ca95f70661 100644 --- a/plugins/regex_remap/regex_remap.cc +++ b/plugins/regex_remap/regex_remap.cc @@ -129,12 +129,21 @@ class RemapRegex fprintf(stderr, "[%s]: Regex %d ( %s ): %.2f%%\n", now, ix, _rex_string, 100.0 * _hits / max); } + // Returns '0' on success int compile(std::string &error, int &erroffset); + // number of matches, or '0' if failed int match(std::string_view const str, RegexMatches &matches) const { - return _rex.exec(str, matches); + TSAssert(nullptr != _match_context); + bool const stat = _rex.exec(str, matches, 0, _match_context); + if (stat) { + return matches.size(); + } else { + Dbg(dbg_ctl, "Regex match failure: %.*s", (int)str.length(), str.data()); + } + return 0; } // Substitutions @@ -154,6 +163,12 @@ class RemapRegex return _next; } + inline void + set_match_context(RegexMatchContext *const ctx) + { + _match_context = ctx; + } + // setter / getters for order number within the linked list inline void set_order(int order) @@ -244,9 +259,10 @@ class RemapRegex bool _lowercase_substitutions = false; - Regex _rex; - RemapRegex *_next = nullptr; - TSHttpStatus _status = static_cast(0); + Regex _rex; + RegexMatchContext *_match_context = nullptr; // owned by RemapInstance + RemapRegex *_next = nullptr; + TSHttpStatus _status = static_cast(0); int _active_timeout = -1; int _no_activity_timeout = -1; @@ -377,6 +393,7 @@ RemapRegex::compile(std::string &error, int &erroffset) bool const restat = _rex.compile(_rex_string, error, erroffset, _options); if (!restat) { + TSError("[%s] Error compiling : %s", PLUGIN_NAME, _rex_string); return -1; } @@ -593,17 +610,18 @@ RemapRegex::substitute(char dest[], RegexMatches const &matches, const int lengt struct RemapInstance { RemapInstance() : filename("unknown") {} - RemapRegex *first = nullptr; - RemapRegex *last = nullptr; - 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; + RemapRegex *first = nullptr; + RemapRegex *last = nullptr; + 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; }; /////////////////////////////////////////////////////////////////////////////// @@ -748,9 +766,10 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE std::string error; int erroffset; - if (!cur->compile(error, erroffset)) { + Dbg(dbg_ctl, "Compiling regex: %s", regex.c_str()); + if (0 != cur->compile(error, erroffset)) { std::ostringstream oss; - oss << '[' << PLUGIN_NAME << "] PCRE failed in " << (ri->filename).c_str() << " (line " << lineno << ')'; + oss << '[' << PLUGIN_NAME << "] Regex compile failed in " << (ri->filename).c_str() << " (line " << lineno << ')'; if (erroffset > 0) { oss << " at offset " << erroffset; } @@ -764,6 +783,7 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE } else { Dbg(dbg_ctl, "Added regex=%s with subs=%s and options `%s'", regex.c_str(), subst.c_str(), options.c_str()); cur->set_order(++count); + cur->set_match_context(&ri->match_context); auto tmp = cur.get(); if (ri->first == nullptr) { ri->first = cur.release(); @@ -774,6 +794,8 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE } } + ri->match_context.setMatchLimit(1750); + // Make sure we got something... if (ri->first == nullptr) { TSError("[%s] no regular expressions from the maps", PLUGIN_NAME); @@ -786,6 +808,7 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE void TSRemapDeleteInstance(void *ih) { + Dbg(dbg_ctl, "TSRemapDeleteInstance"); RemapInstance *ri = static_cast(ih); RemapRegex *re; RemapRegex *tmp; diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index c40d64491be..1702cd5dea0 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -208,6 +208,103 @@ RegexMatches::operator[](size_t index) const return std::string_view(_subject.data() + ovector[2 * index], ovector[2 * index + 1] - ovector[2 * index]); } +//---------------------------------------------------------------------------- +struct RegexMatchContext::_MatchContext { + static pcre2_match_context * + get(_MatchContextPtr const &p) + { + return static_cast(p._ptr); + } + static void + set(_MatchContextPtr &p, pcre2_match_context *ptr) + { + p._ptr = ptr; + } +}; + +//---------------------------------------------------------------------------- +RegexMatchContext::RegexMatchContext() +{ + auto ctx = pcre2_match_context_create(nullptr); + debug_assert_message(ctx, "Failed to allocate custom 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() +{ + auto ptr = _MatchContext::get(_match_context); + if (ptr != nullptr) { + pcre2_match_context_free(ptr); + } +} + +//---------------------------------------------------------------------------- +void +RegexMatchContext::setHeapLimit(uint32_t limit) +{ + auto ptr = _MatchContext::get(_match_context); + if (ptr != nullptr) { + pcre2_set_heap_limit(ptr, limit); + } +} + +//---------------------------------------------------------------------------- +void +RegexMatchContext::setMatchLimit(uint32_t limit) +{ + auto ptr = _MatchContext::get(_match_context); + if (ptr != nullptr) { + pcre2_set_match_limit(ptr, limit); + } +} + +//---------------------------------------------------------------------------- +void +RegexMatchContext::setDepthLimit(uint32_t limit) +{ + auto ptr = _MatchContext::get(_match_context); + if (ptr != nullptr) { + pcre2_set_depth_limit(ptr, limit); + } +} + +//---------------------------------------------------------------------------- +void +RegexMatchContext::setOffsetLimit(uint32_t limit) +{ + auto ptr = _MatchContext::get(_match_context); + if (ptr != nullptr) { + pcre2_set_offset_limit(ptr, limit); + } +} + //---------------------------------------------------------------------------- struct Regex::_Code { static pcre2_code * @@ -355,7 +452,7 @@ Regex::exec(std::string_view subject, RegexMatches &matches) const //---------------------------------------------------------------------------- int32_t -Regex::exec(std::string_view subject, RegexMatches &matches, uint32_t flags) const +Regex::exec(std::string_view subject, RegexMatches &matches, uint32_t flags, RegexMatchContext const *const matchContext) const { auto code = _Code::get(_code); @@ -363,8 +460,18 @@ Regex::exec(std::string_view subject, RegexMatches &matches, uint32_t flags) con if (code == nullptr) { return PCRE2_ERROR_NULL; } + + // Use the provided or the thread global context? + auto const match_context = [&]() -> pcre2_match_context * { + if (nullptr == matchContext) { + return RegexContext::get_instance()->get_match_context(); + } else { + return RegexMatchContext::_MatchContext::get(matchContext->_match_context); + } + }(); + int count = pcre2_match(code, reinterpret_cast(subject.data()), subject.size(), 0, flags, - RegexMatches::_MatchData::get(matches._match_data), RegexContext::get_instance()->get_match_context()); + RegexMatches::_MatchData::get(matches._match_data), match_context); matches._size = count; 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 12e3b9867e8..7f8714f16f2 100644 --- a/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py +++ b/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py @@ -127,5 +127,5 @@ 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 -21', "Resource limit exceeded") + 'ERROR: .regex_remap. Bad regular expression result -46', "Recursion limit exceeded") tr.StillRunningAfter = ts From 27582d7a3b73a892d9f8d47fe5c8f0dbe86f1dce Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Tue, 28 Oct 2025 08:15:44 -0600 Subject: [PATCH 3/9] cleanup --- plugins/regex_remap/regex_remap.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/plugins/regex_remap/regex_remap.cc b/plugins/regex_remap/regex_remap.cc index 0ca95f70661..1543706c37b 100644 --- a/plugins/regex_remap/regex_remap.cc +++ b/plugins/regex_remap/regex_remap.cc @@ -384,9 +384,6 @@ RemapRegex::initialize(const std::string ®, const std::string &sub, const std int RemapRegex::compile(std::string &error, int &erroffset) { - char *str; - // int ccount; - // Initialize these in case they are not set. error = "unknown error"; erroffset = -1; @@ -398,7 +395,7 @@ RemapRegex::compile(std::string &error, int &erroffset) } // Get some info for the string substitutions - str = _subst; + char *str = _subst; _num_subs = 0; while (str && *str) { From a6b287fab0c2ea4e36bf307a8438737b59d67a4c Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Tue, 28 Oct 2025 08:32:39 -0600 Subject: [PATCH 4/9] remove pcre2 match context features not in centos --- include/tsutil/Regex.h | 8 -------- src/tsutil/Regex.cc | 20 -------------------- 2 files changed, 28 deletions(-) diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index aa2739e607f..4946f032282 100644 --- a/include/tsutil/Regex.h +++ b/include/tsutil/Regex.h @@ -112,18 +112,10 @@ class RegexMatchContext RegexMatchContext(RegexMatchContext &&) = default; RegexMatchContext &operator=(RegexMatchContext &&) = default; - /** maximum amount of heap memory (KiB) used to hold backtracking information. - */ - void setHeapLimit(uint32_t limit); - /** Limits the amount of backtracking that can take place. */ void setMatchLimit(uint32_t limit); - /** Limits the depth of nested backtracking. - */ - void setDepthLimit(uint32_t limit); - /** Limits how far an unanchored search can advance in the subject string. */ void setOffsetLimit(uint32_t limit); diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index 1702cd5dea0..f0f2bd864f6 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -265,16 +265,6 @@ RegexMatchContext::~RegexMatchContext() } } -//---------------------------------------------------------------------------- -void -RegexMatchContext::setHeapLimit(uint32_t limit) -{ - auto ptr = _MatchContext::get(_match_context); - if (ptr != nullptr) { - pcre2_set_heap_limit(ptr, limit); - } -} - //---------------------------------------------------------------------------- void RegexMatchContext::setMatchLimit(uint32_t limit) @@ -285,16 +275,6 @@ RegexMatchContext::setMatchLimit(uint32_t limit) } } -//---------------------------------------------------------------------------- -void -RegexMatchContext::setDepthLimit(uint32_t limit) -{ - auto ptr = _MatchContext::get(_match_context); - if (ptr != nullptr) { - pcre2_set_depth_limit(ptr, limit); - } -} - //---------------------------------------------------------------------------- void RegexMatchContext::setOffsetLimit(uint32_t limit) From a45e4443d84b2b21c5c3fb2c6211aed49f5f5bd0 Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Tue, 28 Oct 2025 13:25:00 -0600 Subject: [PATCH 5/9] add back in comments from the pcre plugin --- plugins/regex_remap/regex_remap.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/plugins/regex_remap/regex_remap.cc b/plugins/regex_remap/regex_remap.cc index 1543706c37b..37ab9104a29 100644 --- a/plugins/regex_remap/regex_remap.cc +++ b/plugins/regex_remap/regex_remap.cc @@ -50,8 +50,9 @@ static const char *PLUGIN_NAME = "regex_remap"; // Constants -static const int MATCHCOUNT = 15; -static const int MAX_SUBS = 32; // No more than 32 substitution variables in the subst string +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 // Substitutions other than regex matches enum ExtraSubstitutions { @@ -791,7 +792,7 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE } } - ri->match_context.setMatchLimit(1750); + ri->match_context.setMatchLimit(REGEX_MATCH_LIMIT); // Make sure we got something... if (ri->first == nullptr) { From 46816fc99448cc40ec9eb2447977c566f67d887c Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Wed, 29 Oct 2025 08:27:05 -0600 Subject: [PATCH 6/9] refactor Regex::captureCount(), add unit tests --- include/tsutil/Regex.h | 11 ++- .../experimental/cookie_remap/cookie_remap.cc | 4 +- plugins/regex_remap/regex_remap.cc | 28 ++++--- src/proxy/http/remap/RemapConfig.cc | 4 +- src/tsutil/Regex.cc | 27 ++++--- src/tsutil/unit_tests/test_Regex.cc | 81 ++++++++++++++++++- 6 files changed, 117 insertions(+), 38 deletions(-) diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index 4946f032282..2f585ea3005 100644 --- a/include/tsutil/Regex.h +++ b/include/tsutil/Regex.h @@ -116,10 +116,6 @@ class RegexMatchContext */ void setMatchLimit(uint32_t limit); - /** Limits how far an unanchored search can advance in the subject string. - */ - void setOffsetLimit(uint32_t limit); - private: /// @internal This wraps a void* so to avoid requiring a pcre2 include. struct _MatchContext; @@ -226,8 +222,11 @@ class Regex int exec(std::string_view subject, RegexMatches &matches, uint32_t flags, RegexMatchContext const *const matchContext = nullptr) const; - /// @return The number of capture groups in the compiled pattern. - int get_capture_count(); + /// @return The number of capture groups in the compiled pattern, -1 for fail. + int32_t captureCount() const; + + /// @return number of highest back references, -1 for fail. + int32_t backrefMax() const; /// @return Is the compiled pattern empty? bool empty() const; diff --git a/plugins/experimental/cookie_remap/cookie_remap.cc b/plugins/experimental/cookie_remap/cookie_remap.cc index bcaa4d207ec..cc4f657b27e 100644 --- a/plugins/experimental/cookie_remap/cookie_remap.cc +++ b/plugins/experimental/cookie_remap/cookie_remap.cc @@ -385,7 +385,7 @@ class subop return false; } - regex_ccount = regex->get_capture_count(); + regex_ccount = regex->captureCount(); if (regex_ccount < 0) { delete regex; regex = nullptr; @@ -445,7 +445,7 @@ class subop Regex *regex = nullptr; std::string regex_string; - int regex_ccount = 0; + int32_t regex_ccount = 0; std::string bucket; unsigned int how_many = 0; diff --git a/plugins/regex_remap/regex_remap.cc b/plugins/regex_remap/regex_remap.cc index 37ab9104a29..13def2e9a12 100644 --- a/plugins/regex_remap/regex_remap.cc +++ b/plugins/regex_remap/regex_remap.cc @@ -165,7 +165,7 @@ class RemapRegex } inline void - set_match_context(RegexMatchContext *const ctx) + set_match_context(RegexMatchContext const *const ctx) { _match_context = ctx; } @@ -260,10 +260,10 @@ class RemapRegex bool _lowercase_substitutions = false; - Regex _rex; - RegexMatchContext *_match_context = nullptr; // owned by RemapInstance - RemapRegex *_next = nullptr; - TSHttpStatus _status = static_cast(0); + Regex _rex; + RegexMatchContext const *_match_context = nullptr; // owned by RemapInstance + RemapRegex *_next = nullptr; + TSHttpStatus _status = static_cast(0); int _active_timeout = -1; int _no_activity_timeout = -1; @@ -395,6 +395,12 @@ RemapRegex::compile(std::string &error, int &erroffset) return -1; } + int32_t const ccount = _rex.captureCount(); + if (ccount < 0) { + error = "Failure to get capture count for Regex"; + return -1; + } + // Get some info for the string substitutions char *str = _subst; _num_subs = 0; @@ -440,12 +446,10 @@ RemapRegex::compile(std::string &error, int &erroffset) } if (ix > -1) { - /* -if ((ix < 10) && (ix > matches.size())) { -error = "using unavailable captured substring ($n) in substitution"; -return -1; -} - */ + if ((ix < 10) && (ix > ccount)) { + error = "using unavailable captured substring ($n) in substitution"; + return -1; + } _sub_ix[_num_subs] = ix; _sub_pos[_num_subs] = (str - _subst); @@ -781,7 +785,7 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE } else { Dbg(dbg_ctl, "Added regex=%s with subs=%s and options `%s'", regex.c_str(), subst.c_str(), options.c_str()); cur->set_order(++count); - cur->set_match_context(&ri->match_context); + cur->set_match_context(&(ri->match_context)); auto tmp = cur.get(); if (ri->first == nullptr) { ri->first = cur.release(); diff --git a/src/proxy/http/remap/RemapConfig.cc b/src/proxy/http/remap/RemapConfig.cc index 73d74b8bf42..0231cf9d19d 100644 --- a/src/proxy/http/remap/RemapConfig.cc +++ b/src/proxy/http/remap/RemapConfig.cc @@ -974,7 +974,7 @@ process_regex_mapping_config(const char *from_host_lower, url_mapping *new_mappi std::string_view to_host{}; int to_host_len; int substitution_id; - int captures; + int32_t captures; reg_map->to_url_host_template = nullptr; reg_map->to_url_host_template_len = 0; @@ -989,7 +989,7 @@ process_regex_mapping_config(const char *from_host_lower, url_mapping *new_mappi goto lFail; } - captures = reg_map->regular_expression.get_capture_count(); + captures = reg_map->regular_expression.captureCount(); if (captures == -1) { Warning("pcre_fullinfo failed!"); goto lFail; diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index f0f2bd864f6..895717b6589 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -275,16 +275,6 @@ RegexMatchContext::setMatchLimit(uint32_t limit) } } -//---------------------------------------------------------------------------- -void -RegexMatchContext::setOffsetLimit(uint32_t limit) -{ - auto ptr = _MatchContext::get(_match_context); - if (ptr != nullptr) { - pcre2_set_offset_limit(ptr, limit); - } -} - //---------------------------------------------------------------------------- struct Regex::_Code { static pcre2_code * @@ -470,13 +460,24 @@ Regex::exec(std::string_view subject, RegexMatches &matches, uint32_t flags, Reg //---------------------------------------------------------------------------- int32_t -Regex::get_capture_count() +Regex::captureCount() const { - int captures = -1; + uint32_t captures = 0; if (pcre2_pattern_info(_Code::get(_code), PCRE2_INFO_CAPTURECOUNT, &captures) != 0) { return -1; } - return captures; + return static_cast(captures); +} + +//---------------------------------------------------------------------------- +int32_t +Regex::backrefMax() const +{ + uint32_t refs = 0; + if (pcre2_pattern_info(_Code::get(_code), PCRE2_INFO_BACKREFMAX, &refs) != 0) { + return -1; + } + return static_cast(refs); } //---------------------------------------------------------------------------- diff --git a/src/tsutil/unit_tests/test_Regex.cc b/src/tsutil/unit_tests/test_Regex.cc index 679117f3153..2a3ae49576d 100644 --- a/src/tsutil/unit_tests/test_Regex.cc +++ b/src/tsutil/unit_tests/test_Regex.cc @@ -81,7 +81,7 @@ struct submatch_t { struct submatch_test_t { std::string_view regex; - int capture_count; + int32_t capture_count; std::vector tests; }; @@ -129,7 +129,7 @@ TEST_CASE("Regex", "[libts][Regex]") for (auto &item : submatch_test_data) { Regex r; REQUIRE(r.compile(item.regex.data()) == true); - REQUIRE(r.get_capture_count() == item.capture_count); + REQUIRE(r.captureCount() == item.capture_count); for (auto &test : item.tests) { RegexMatches matches; @@ -146,7 +146,7 @@ TEST_CASE("Regex", "[libts][Regex]") for (auto &item : submatch_test_data) { Regex r; REQUIRE(r.compile(item.regex.data()) == true); - REQUIRE(r.get_capture_count() == item.capture_count); + REQUIRE(r.captureCount() == item.capture_count); for (auto &test : item.tests) { RegexMatches matches; @@ -489,3 +489,78 @@ TEST_CASE("Regex copy with RE_NOTEMPTY flag", "[libts][Regex][copy][flags]") CHECK(copy.exec(std::string_view(""), RE_NOTEMPTY) == false); } } + +struct backref_test_t { + std::string_view regex; + bool valid; + int32_t backref_max; +}; + +std::vector backref_test_data{ + { + {""}, + true, 0, + }, + { + {R"(\b(\w+)\s+\1\b)"}, + true, 1, + }, + { + {R"((.)\1)"}, + true,1, + }, + { + {R"((.)(.).\2\1)"}, + true, 2, + }, + { + {R"((.\2\1)"}, + false, -1, + }, +}; + +TEST_CASE("Regex back reference counting", "[libts][Regex][backrefMax]") +{ + // case sensitive test + for (auto &item : backref_test_data) { + Regex r; + REQUIRE(r.compile(item.regex) == item.valid); + REQUIRE(r.backrefMax() == item.backref_max); + } +} + +struct match_context_test_t { + std::string_view regex; + std::string_view str; + bool valid; + int32_t rcode; +}; + +std::vector match_context_test_data{ + { + {"abc"}, + {"abc"}, + true, 1, + }, + {{"a+b"}, {"aaaaaab"}, true, 1}, + {{"(a+)+b"}, {"aaaaab"}, true, -47}, // PCRE2_ERROR_MATCHLIMIT + { + {"(."}, + {"a"}, + false, -1, + }, +}; + +TEST_CASE("RegexMatchContext", "[libts][Regex][RegexMatchContext]") +{ + RegexMatchContext match_context; + match_context.setMatchLimit(5); + + // case sensitive test + for (auto &item : match_context_test_data) { + Regex r; + REQUIRE(r.compile(item.regex) == item.valid); + RegexMatches matches; + REQUIRE(r.exec(item.str, matches, 0, &match_context) == item.rcode); + } +} From 1132e7adecb070dac450346980da05d28a2a0bea Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Wed, 29 Oct 2025 11:25:24 -0600 Subject: [PATCH 7/9] change Regex api function convention, add to unit test --- include/tsutil/Regex.h | 11 ++-- .../experimental/cookie_remap/cookie_remap.cc | 2 +- plugins/regex_remap/regex_remap.cc | 4 +- src/proxy/http/remap/RemapConfig.cc | 2 +- src/tsutil/Regex.cc | 25 ++++----- src/tsutil/unit_tests/test_Regex.cc | 53 ++++++------------- 6 files changed, 39 insertions(+), 58 deletions(-) diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index 2f585ea3005..3fea4ee9cdb 100644 --- a/include/tsutil/Regex.h +++ b/include/tsutil/Regex.h @@ -95,6 +95,8 @@ class RegexMatches }; /// @brief Wrapper for PCRE2 match context +/// +/// @internal This instance is not tied to any Regex and can be used with one of the Regex::exec overloads. class RegexMatchContext { friend class Regex; @@ -105,7 +107,7 @@ class RegexMatchContext RegexMatchContext(); ~RegexMatchContext(); - /// uses pcre2_match_context_copy to duplicate. + /// uses pcre2_match_context_copy for a deep copy. RegexMatchContext(RegexMatchContext const &orig); RegexMatchContext &operator=(RegexMatchContext const &orig); @@ -113,8 +115,9 @@ class RegexMatchContext RegexMatchContext &operator=(RegexMatchContext &&) = default; /** Limits the amount of backtracking that can take place. + * Any regex exec call that fails will return PCRE2_ERROR_MATCHLIMIT(-47) */ - void setMatchLimit(uint32_t limit); + void set_match_limit(uint32_t limit); private: /// @internal This wraps a void* so to avoid requiring a pcre2 include. @@ -223,10 +226,10 @@ class Regex RegexMatchContext const *const matchContext = nullptr) const; /// @return The number of capture groups in the compiled pattern, -1 for fail. - int32_t captureCount() const; + int32_t get_capture_count() const; /// @return number of highest back references, -1 for fail. - int32_t backrefMax() const; + int32_t get_backref_max() const; /// @return Is the compiled pattern empty? bool empty() const; diff --git a/plugins/experimental/cookie_remap/cookie_remap.cc b/plugins/experimental/cookie_remap/cookie_remap.cc index cc4f657b27e..9c889f8e4a7 100644 --- a/plugins/experimental/cookie_remap/cookie_remap.cc +++ b/plugins/experimental/cookie_remap/cookie_remap.cc @@ -385,7 +385,7 @@ class subop return false; } - regex_ccount = regex->captureCount(); + regex_ccount = regex->get_capture_count(); if (regex_ccount < 0) { delete regex; regex = nullptr; diff --git a/plugins/regex_remap/regex_remap.cc b/plugins/regex_remap/regex_remap.cc index 13def2e9a12..4ed8c900405 100644 --- a/plugins/regex_remap/regex_remap.cc +++ b/plugins/regex_remap/regex_remap.cc @@ -395,7 +395,7 @@ RemapRegex::compile(std::string &error, int &erroffset) return -1; } - int32_t const ccount = _rex.captureCount(); + int32_t const ccount = _rex.get_capture_count(); if (ccount < 0) { error = "Failure to get capture count for Regex"; return -1; @@ -796,7 +796,7 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE } } - ri->match_context.setMatchLimit(REGEX_MATCH_LIMIT); + ri->match_context.set_match_limit(REGEX_MATCH_LIMIT); // Make sure we got something... if (ri->first == nullptr) { diff --git a/src/proxy/http/remap/RemapConfig.cc b/src/proxy/http/remap/RemapConfig.cc index 0231cf9d19d..a1ceac4e0ee 100644 --- a/src/proxy/http/remap/RemapConfig.cc +++ b/src/proxy/http/remap/RemapConfig.cc @@ -989,7 +989,7 @@ process_regex_mapping_config(const char *from_host_lower, url_mapping *new_mappi goto lFail; } - captures = reg_map->regular_expression.captureCount(); + captures = reg_map->regular_expression.get_capture_count(); if (captures == -1) { Warning("pcre_fullinfo failed!"); goto lFail; diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index 895717b6589..fcb468bd76f 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -260,6 +260,7 @@ RegexMatchContext::operator=(RegexMatchContext const &other) RegexMatchContext::~RegexMatchContext() { auto ptr = _MatchContext::get(_match_context); + debug_assert_message(ptr, "Failed to get the match context"); if (ptr != nullptr) { pcre2_match_context_free(ptr); } @@ -267,9 +268,10 @@ RegexMatchContext::~RegexMatchContext() //---------------------------------------------------------------------------- void -RegexMatchContext::setMatchLimit(uint32_t limit) +RegexMatchContext::set_match_limit(uint32_t limit) { auto ptr = _MatchContext::get(_match_context); + debug_assert_message(ptr, "Failed to get the match context"); if (ptr != nullptr) { pcre2_set_match_limit(ptr, limit); } @@ -432,16 +434,15 @@ Regex::exec(std::string_view subject, RegexMatches &matches, uint32_t flags, Reg } // Use the provided or the thread global context? - auto const match_context = [&]() -> pcre2_match_context * { - if (nullptr == matchContext) { - return RegexContext::get_instance()->get_match_context(); - } else { - return RegexMatchContext::_MatchContext::get(matchContext->_match_context); - } - }(); + pcre2_match_context *match_context; + if (nullptr == matchContext) { + match_context = RegexContext::get_instance()->get_match_context(); + } else { + match_context = RegexMatchContext::_MatchContext::get(matchContext->_match_context); + } - int count = pcre2_match(code, reinterpret_cast(subject.data()), subject.size(), 0, flags, - RegexMatches::_MatchData::get(matches._match_data), match_context); + int const count = pcre2_match(code, reinterpret_cast(subject.data()), subject.size(), 0, flags, + RegexMatches::_MatchData::get(matches._match_data), match_context); matches._size = count; @@ -460,7 +461,7 @@ Regex::exec(std::string_view subject, RegexMatches &matches, uint32_t flags, Reg //---------------------------------------------------------------------------- int32_t -Regex::captureCount() const +Regex::get_capture_count() const { uint32_t captures = 0; if (pcre2_pattern_info(_Code::get(_code), PCRE2_INFO_CAPTURECOUNT, &captures) != 0) { @@ -471,7 +472,7 @@ Regex::captureCount() const //---------------------------------------------------------------------------- int32_t -Regex::backrefMax() const +Regex::get_backref_max() const { uint32_t refs = 0; if (pcre2_pattern_info(_Code::get(_code), PCRE2_INFO_BACKREFMAX, &refs) != 0) { diff --git a/src/tsutil/unit_tests/test_Regex.cc b/src/tsutil/unit_tests/test_Regex.cc index 2a3ae49576d..b1b2c1609d9 100644 --- a/src/tsutil/unit_tests/test_Regex.cc +++ b/src/tsutil/unit_tests/test_Regex.cc @@ -129,7 +129,7 @@ TEST_CASE("Regex", "[libts][Regex]") for (auto &item : submatch_test_data) { Regex r; REQUIRE(r.compile(item.regex.data()) == true); - REQUIRE(r.captureCount() == item.capture_count); + REQUIRE(r.get_capture_count() == item.capture_count); for (auto &test : item.tests) { RegexMatches matches; @@ -146,7 +146,7 @@ TEST_CASE("Regex", "[libts][Regex]") for (auto &item : submatch_test_data) { Regex r; REQUIRE(r.compile(item.regex.data()) == true); - REQUIRE(r.captureCount() == item.capture_count); + REQUIRE(r.get_capture_count() == item.capture_count); for (auto &test : item.tests) { RegexMatches matches; @@ -497,35 +497,20 @@ struct backref_test_t { }; std::vector backref_test_data{ - { - {""}, - true, 0, - }, - { - {R"(\b(\w+)\s+\1\b)"}, - true, 1, - }, - { - {R"((.)\1)"}, - true,1, - }, - { - {R"((.)(.).\2\1)"}, - true, 2, - }, - { - {R"((.\2\1)"}, - false, -1, - }, + {{""}, true, 0 }, + {{R"(\b(\w+)\s+\1\b)"}, true, 1 }, + {{R"((.)\1)"}, true, 1 }, + {{R"((.)(.).\2\1)"}, true, 2 }, + {{R"((.\2\1)"}, false, -1}, }; -TEST_CASE("Regex back reference counting", "[libts][Regex][backrefMax]") +TEST_CASE("Regex back reference counting", "[libts][Regex][get_backref_max]") { // case sensitive test for (auto &item : backref_test_data) { Regex r; REQUIRE(r.compile(item.regex) == item.valid); - REQUIRE(r.backrefMax() == item.backref_max); + REQUIRE(r.get_backref_max() == item.backref_max); } } @@ -537,30 +522,22 @@ struct match_context_test_t { }; std::vector match_context_test_data{ - { - {"abc"}, - {"abc"}, - true, 1, - }, - {{"a+b"}, {"aaaaaab"}, true, 1}, - {{"(a+)+b"}, {"aaaaab"}, true, -47}, // PCRE2_ERROR_MATCHLIMIT - { - {"(."}, - {"a"}, - false, -1, - }, + {{"abc"}, {"abc"}, true, 1 }, + {{"abc"}, {"a"}, true, -1 }, + {{R"(^(\d{3})-(\d{3})-(\d{4})$)"}, {"123-456-7890"}, true, -47}, + {{"(."}, {"a"}, false, -51}, }; TEST_CASE("RegexMatchContext", "[libts][Regex][RegexMatchContext]") { RegexMatchContext match_context; - match_context.setMatchLimit(5); + match_context.set_match_limit(2); + RegexMatches matches; // case sensitive test for (auto &item : match_context_test_data) { Regex r; REQUIRE(r.compile(item.regex) == item.valid); - RegexMatches matches; REQUIRE(r.exec(item.str, matches, 0, &match_context) == item.rcode); } } From 664ff219595203989f414a9e267c2c473144e47c Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Mon, 3 Nov 2025 08:06:39 -0700 Subject: [PATCH 8/9] better handling of regex exec call failure --- include/tsutil/Regex.h | 6 ++ plugins/regex_remap/regex_remap.cc | 11 ++- src/tsutil/Regex.cc | 30 ++++++-- .../regex_remap/regex_remap.test.py | 9 ++- .../regex_remap/replay/yts-2819.replay.json | 74 +++++++++++++++++++ 5 files changed, 113 insertions(+), 17 deletions(-) diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index 3fea4ee9cdb..cc5260c24f5 100644 --- a/include/tsutil/Regex.h +++ b/include/tsutil/Regex.h @@ -225,6 +225,12 @@ class Regex int exec(std::string_view subject, RegexMatches &matches, uint32_t flags, RegexMatchContext const *const matchContext = nullptr) const; + /** Error string for exec failure. + * + * @param int return code from exec call. + */ + static std::string get_error_string(int rc); + /// @return The number of capture groups in the compiled pattern, -1 for fail. int32_t get_capture_count() const; diff --git a/plugins/regex_remap/regex_remap.cc b/plugins/regex_remap/regex_remap.cc index 4ed8c900405..7c467d8a099 100644 --- a/plugins/regex_remap/regex_remap.cc +++ b/plugins/regex_remap/regex_remap.cc @@ -133,18 +133,17 @@ class RemapRegex // Returns '0' on success int compile(std::string &error, int &erroffset); - // number of matches, or '0' if failed + // number of matches, or negative if failed int match(std::string_view const str, RegexMatches &matches) const { TSAssert(nullptr != _match_context); - bool const stat = _rex.exec(str, matches, 0, _match_context); - if (stat) { + 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(); - } else { - Dbg(dbg_ctl, "Regex match failure: %.*s", (int)str.length(), str.data()); } - return 0; + return stat; } // Substitutions diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index fcb468bd76f..0e76c50ce18 100644 --- a/src/tsutil/Regex.cc +++ b/src/tsutil/Regex.cc @@ -383,7 +383,7 @@ Regex::compile(std::string_view pattern, std::string &error, int &erroroffset, u // get pcre2 error message PCRE2_UCHAR buffer[256]; pcre2_get_error_message(error_code, buffer, sizeof(buffer)); - error.assign((char *)buffer); + error.assign((char const *)buffer); return false; } @@ -441,22 +441,38 @@ Regex::exec(std::string_view subject, RegexMatches &matches, uint32_t flags, Reg match_context = RegexMatchContext::_MatchContext::get(matchContext->_match_context); } - int const count = pcre2_match(code, reinterpret_cast(subject.data()), subject.size(), 0, flags, - RegexMatches::_MatchData::get(matches._match_data), match_context); + int const rc = pcre2_match(code, reinterpret_cast(subject.data()), subject.size(), 0, flags, + RegexMatches::_MatchData::get(matches._match_data), match_context); - matches._size = count; + matches._size = rc; // match was successful - if (count >= 0) { + if (rc >= 0) { matches._subject = subject; // match but the output vector was too small, adjust the size of the matches - if (count == 0) { + if (rc == 0) { matches._size = pcre2_get_ovector_count(RegexMatches::_MatchData::get(matches._match_data)); } } - return count; + return rc; +} + +//---------------------------------------------------------------------------- +// static +std::string +Regex::get_error_string(int rc) +{ + std::string res; + + if (rc < 0) { + PCRE2_UCHAR buffer[256]; + pcre2_get_error_message(rc, buffer, sizeof(buffer)); + res.assign((char const *)buffer); + } + + return res; } //---------------------------------------------------------------------------- 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 7f8714f16f2..086481f03e8 100644 --- a/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py +++ b/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py @@ -60,6 +60,7 @@ [ "# regex_remap configuration\n" "^/alpha/bravo/[?]((?!action=(newsfeed|calendar|contacts|notepad)).)*$ https://redirect.com/ @status=301\n" + "^/match_limit/(a+)+$ https://redirect.com/ @status=301\n" ]) ts.Disk.File( @@ -119,13 +120,13 @@ tr.Processes.Default.Streams.stdout = "gold/regex_remap_simple.gold" tr.StillRunningAfter = ts -# 3 Test - Crash test. -tr = Test.AddTestRun("crash test") -creq = replay_txns[1]['client-request'] +# 3 Test - Match limit test +tr = Test.AddTestRun("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 -46', "Recursion limit exceeded") + 'ERROR: .regex_remap. Bad regular expression result -47', "Match limit exceeded") tr.StillRunningAfter = ts 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 5083a134e54..4361a9800ff 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 @@ -156,6 +156,80 @@ ] } } + }, + { + "uuid": "match_limit", + "client-request": { + "version": "1.1", + "scheme": "http", + "method": "GET", + "url": "http://example.one/match_limit/aaaaaaaaaaaaaaaaaaaf", + "headers": { + "fields": [ + [ + "Host", + "example.one" + ], + [ + "uuid", + "match_limit" + ] + ] + } + }, + "proxy-request": { + "version": "1.1", + "scheme": "http", + "method": "GET", + "url": "http://example.one/", + "headers": { + "fields": [ + [ + "uuid", + "match_limit" + ] + ] + } + }, + "server-response": { + "status": 200, + "reason": "OK", + "content": { + "size": 6128 + }, + "headers": { + "fields": [ + [ + "Host", + "example.one" + ], + [ + "uuid", + "180" + ], + [ + "Content-Length", + "6128" + ], + [ + "Connection", + "close" + ] + ] + } + }, + "proxy-response": { + "status": 200, + "reason": "OK", + "content": { + "size": 6128 + }, + "headers": { + "fields": [ + [ "Content-Length", 6128 ] + ] + } + } } ] } From 1de5d5cb89092a954ca3d9335a87d8d216b07d1d Mon Sep 17 00:00:00 2001 From: Brian Olsen Date: Tue, 4 Nov 2025 05:49:45 -0700 Subject: [PATCH 9/9] use errmsg in diags.log, restore older match limit test --- plugins/regex_remap/regex_remap.cc | 5 +++-- .../pluginTest/regex_remap/regex_remap.test.py | 15 +++++++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/plugins/regex_remap/regex_remap.cc b/plugins/regex_remap/regex_remap.cc index 7c467d8a099..18838291e29 100644 --- a/plugins/regex_remap/regex_remap.cc +++ b/plugins/regex_remap/regex_remap.cc @@ -1063,8 +1063,9 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) } } else if (match_result != -1) { ink_atomic_increment(&(ri->failures), 1); - TSError(R"([%s] Bad regular expression result %d from "%s" in file "%s".)", PLUGIN_NAME, match_result, re->regex(), - ri->filename.c_str()); + std::string const errmsg = Regex::get_error_string(match_result); + TSError(R"([%s] Bad regular expression result %d ("%s") from "%s" in file "%s".)", PLUGIN_NAME, match_result, errmsg.c_str(), + re->regex(), ri->filename.c_str()); } // Try the next regex 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 086481f03e8..1c98e109090 100644 --- a/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py +++ b/tests/gold_tests/pluginTest/regex_remap/regex_remap.test.py @@ -120,8 +120,19 @@ tr.Processes.Default.Streams.stdout = "gold/regex_remap_simple.gold" tr.StillRunningAfter = ts -# 3 Test - Match limit test -tr = Test.AddTestRun("match limit") +# 3 Test - Match limit test 0 +tr = Test.AddTestRun("match limit 0") +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.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.StillRunningAfter = ts + +# 4 Test - Match limit test 1 +tr = Test.AddTestRun("match limit 1") creq = replay_txns[2]['client-request'] tr.MakeCurlCommand(curl_and_args + \ '--header "uuid: {}" '.format(creq["headers"]["fields"][1][1]) + '"{}"'.format(creq["url"]), ts=ts)