Bound RegexMatches::operator[] by what the match populated

include/tsutil/Regex.h

@@ -68,7 +68,7 @@ class RegexMatches
 
   /** Get the match at the given index.
    *
-   * @return The match at the given index.
+   * @return The match at the given index, or an empty view.
    */
   std::string_view operator[](size_t index) const;
   /** Get the ovector pointer for the capture groups.  Don't use this unless you know what you are doing.

src/tsutil/Regex.cc

@@ -222,17 +222,19 @@ RegexMatches::size() const
 std::string_view
 RegexMatches::operator[](size_t index) const
 {
-  // check if the index is valid
-  if (index >= pcre2_get_ovector_count(_MatchData::get(_match_data))) {
-    return std::string_view();
+  // The ovector is allocated with a fixed number of pairs, but pcre2_match() writes only as far as
+  // the highest participating group. Every pair past that keeps whatever _buffer happened to hold,
+  // so the allocated count is not a usable bound -- _size is what the match actually populated.
+  if (_size <= 0 || index >= static_cast<size_t>(_size)) {
+    return "";
   }
 
   PCRE2_SIZE *ovector = pcre2_get_ovector_pointer(_MatchData::get(_match_data));
 
-  // A group that did not participate in the match has an unset offset. This happens for an optional
-  // group that precedes a participating one, so a valid index is not enough to guarantee an offset.
+  // Within _size a group can still have not participated, in which case PCRE2 sets both of its
+  // offsets to PCRE2_UNSET. An optional group preceding a participating one is the usual way.
   if (PCRE2_UNSET == ovector[2 * index]) {
-    return std::string_view();
+    return "";
   }
 
   return std::string_view(_subject.data() + ovector[2 * index], ovector[2 * index + 1] - ovector[2 * index]);

src/tsutil/unit_tests/test_Regex.cc

@@ -466,7 +466,7 @@ TEST_CASE("RegexMatches edge cases", "[libts][Regex][RegexMatches]")
     // pcre2_match() returns one past the highest participating group, so an earlier optional group
     // that did not participate is still within that count. Its offsets are unset.
     Regex r;
-    REQUIRE(r.compile("(a)?(b)") == true);
+    REQUIRE(r.compile("(a)?(b)"));
 
     RegexMatches matches;
     int          count = r.exec("b", matches);
@@ -475,7 +475,45 @@ TEST_CASE("RegexMatches edge cases", "[libts][Regex][RegexMatches]")
     CHECK(matches[0] == "b");
     CHECK(matches[1] == "");
     CHECK(matches[2] == "b");
-    CHECK(matches[1].data() == nullptr);
+    CHECK(matches[1].data() != nullptr);
+  }
+
+  SECTION("RegexMatches past what the match populated")
+  {
+    // The ovector holds a fixed number of pairs regardless of the pattern, and pcre2_match() writes
+    // no further than the highest participating group. Reading past that must not build a view over
+    // the uninitialized remainder of the buffer.
+    Regex r;
+    REQUIRE(r.compile("(.*-)(\\d+)(\\?.*)?$"));
+
+    RegexMatches matches;
+    int          count = r.exec("/img-7", matches);
+
+    // Three groups defined, but the trailing optional one did not participate.
+    CHECK(r.get_capture_count() == 3);
+    CHECK(count == 3);
+    CHECK(matches[1] == "/img-");
+    CHECK(matches[2] == "7");
+
+    // A group the pattern defines that the match did not reach, ...
+    CHECK(matches[3] == "");
+    CHECK(matches[3].data() != nullptr);
+
+    // ... and an index past the pattern's groups entirely, still inside the allocated ovector.
+    CHECK(matches[9] == "");
+    CHECK(matches[9].data() != nullptr);
+  }
+
+  SECTION("RegexMatches after a failed match")
+  {
+    Regex r;
+    REQUIRE(r.compile("(a)(b)"));
+
+    RegexMatches matches;
+    CHECK(r.exec("zz", matches) < 0);
+
+    CHECK(matches[0] == "");
+    CHECK(matches[0].data() != nullptr);
   }
 }