Handle all cachekey regex capture groups

plugins/cachekey/pattern.cc

@@ -152,7 +152,7 @@ Pattern::process(const String &subject, StringVector &result)
   } else {
     /* Replacement was not provided so return all capturing groups except the group zero. */
     StringVector captures;
-    if (capture(subject, captures)) {
+    if (capture(subject, captures) && !captures.empty()) {
       if (captures.size() == 1) {
         result.push_back(captures[0]);
       } else {
@@ -210,7 +210,7 @@ Pattern::capture(const String &subject, StringVector &result)
     return false;
   }
 
-  RegexMatches matches;
+  RegexMatches matches(_captureCount + 1);
   int          matchCount = _re.exec(subject, matches, RE_NOTEMPTY);
   if (matchCount < 0) {
     if (matchCount != RE_ERROR_NOMATCH) {
@@ -219,7 +219,7 @@ Pattern::capture(const String &subject, StringVector &result)
     return false;
   }
 
-  for (int i = 0; i < matchCount; i++) {
+  for (int i = 0; i < matches.size(); i++) {
     std::string_view capture = matches[i];
     String           dst(capture.data(), capture.length());
 
@@ -246,7 +246,7 @@ Pattern::replace(const String &subject, String &result)
     return false;
   }
 
-  RegexMatches matches;
+  RegexMatches matches(_captureCount + 1);
   int          matchCount = _re.exec(subject, matches, RE_NOTEMPTY);
   if (matchCount < 0) {
     if (matchCount != RE_ERROR_NOMATCH) {
@@ -255,18 +255,11 @@ Pattern::replace(const String &subject, String &result)
     return false;
   }
 
-  /* Verify the replacement has the right number of matching groups */
-  for (int i = 0; i < _tokenCount; i++) {
-    if (_tokens[i] >= matchCount) {
-      CacheKeyError("invalid reference in replacement string: $%d", _tokens[i]);
-      return false;
-    }
-  }
-
   int previous = 0;
   for (int i = 0; i < _tokenCount; i++) {
-    int              replIndex = _tokens[i];
-    std::string_view capture   = matches[replIndex];
+    int replIndex = _tokens[i];
+    // Trailing optional groups may not participate in this match.
+    std::string_view capture = (replIndex < matches.size()) ? matches[replIndex] : std::string_view{""};
 
     String src(_replacement, _tokenOffset[i], 2);
     String dst(capture.data(), capture.length());
@@ -304,6 +297,12 @@ Pattern::compile()
     return false;
   }
 
+  _captureCount = _re.get_capture_count();
+  if (_captureCount < 0) {
+    CacheKeyError("failed to get capture count for pattern '%s'", _pattern.c_str());
+    return false;
+  }
+
   if (!_replace) {
     /* No replacement necessary - we are done. */
     return true;
@@ -336,6 +335,16 @@ Pattern::compile()
     }
   }
 
+  if (success) {
+    for (int i = 0; i < _tokenCount; i++) {
+      if (_tokens[i] > _captureCount) {
+        CacheKeyError("invalid reference $%d in replacement '%s': pattern defines only %d group(s)", _tokens[i],
+                      _replacement.c_str(), _captureCount);
+        return false;
+      }
+    }
+  }
+
   return success;
 }
 

plugins/cachekey/pattern.h

@@ -50,7 +50,8 @@ class Pattern
 private:
   bool compile();
 
-  Regex _re; /**< @brief Regex compiled object */
+  Regex   _re;               /**< @brief Regex compiled object */
+  int32_t _captureCount = 0; ///< Number of capture groups defined by the compiled pattern.
 
   String _pattern; /**< @brief Regex pattern string, containing regex patterns and capturing groups. */
   String

plugins/cachekey/unit_tests/pattern_test.cc

@@ -202,13 +202,47 @@ TEST_CASE("Pattern compile and match behavior", "[cachekey][pattern]")
     CHECK(res == "num=123;");
   }
 
-  SECTION("Replacement with invalid group reference")
+  SECTION("Replacement with invalid group reference fails at initialization")
   {
     Pattern p;
-    REQUIRE(p.init("(\\w+)", "$5", true)); // only 2 groups (0 and 1)
+
+    CHECK_FALSE(p.init("(\\w+)", "$5", true));
+    CHECK_FALSE(p.init("(a)(b)?", "$3", true));
+    CHECK_FALSE(p.init("literal", "$1", true));
+  }
+
+  SECTION("Replacement with optional capture groups")
+  {
+    Pattern p;
+
+    REQUIRE(p.init("^(a)(b)?(c)?$", "$1-$2-$3", true));
+    for (const auto &[subject, expected] : {
+           std::pair{"a",   "a--"  },
+           {"ab",  "a-b-" },
+           {"ac",  "a--c" },
+           {"abc", "a-b-c"}
+    }) {
+      String res;
+
+      REQUIRE(p.replace(subject, res));
+      CHECK(res == expected);
+    }
+  }
+
+  SECTION("Capture and replacement beyond the inline match buffer")
+  {
+    Pattern      p;
+    StringVector result;
+
+    REQUIRE(p.init("(a)(b)(c)(d)(e)(f)(g)(h)(i)(j)(k)(l)"));
+    REQUIRE(p.process("abcdefghijkl", result));
+    CHECK(result == StringVector{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l"});
+
+    REQUIRE(p.init("(a)(b)(c)(d)(e)(f)(g)(h)(i)(j)(k)(l)", "$9$1", true));
     String res;
-    // Should fail because $5 doesn't exist
-    CHECK(p.replace("test", res) == false);
+
+    REQUIRE(p.replace("abcdefghijkl", res));
+    CHECK(res == "ia");
   }
 
   SECTION("process() method - capture mode (no replacement)")

tests/gold_tests/pluginTest/cachekey/cachekey_capture.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 cachekey patterns preserve all capture groups.'
+Test.SkipUnless(Condition.PluginExists('cachekey.so'), Condition.PluginExists('xdebug.so'))
+Test.ATSReplayTest(replay_file='capture.replay.yaml')