diff --git a/include/tsutil/Regex.h b/include/tsutil/Regex.h index 03ce7a32e33..5b913bece06 100644 --- a/include/tsutil/Regex.h +++ b/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. diff --git a/src/tsutil/Regex.cc b/src/tsutil/Regex.cc index 2c84b3fa08e..3281622b1ec 100644 --- a/src/tsutil/Regex.cc +++ b/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)) { + 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]); diff --git a/src/tsutil/unit_tests/test_Regex.cc b/src/tsutil/unit_tests/test_Regex.cc index 76273e23226..f1bd0a7c866 100644 --- a/src/tsutil/unit_tests/test_Regex.cc +++ b/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); } }