refactor Regex::captureCount(), add unit tests

Author: traeak

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;

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;

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<TSHttpStatus>(0);
+  Regex                    _rex;
+  RegexMatchContext const *_match_context = nullptr; // owned by RemapInstance
+  RemapRegex              *_next          = nullptr;
+  TSHttpStatus             _status        = static_cast<TSHttpStatus>(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();

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;

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<int32_t>(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<int32_t>(refs);
 }
 
 //----------------------------------------------------------------------------

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<submatch_t> 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_t> 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_t> 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);
+  }
+}