Copilot commented on code in PR #13441:
URL: https://github.com/apache/trafficserver/pull/13441#discussion_r3666944753
##########
src/tsutil/Regex.cc:
##########
@@ -205,6 +205,13 @@ RegexMatches::operator[](size_t index) const
}
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.
+ if (PCRE2_UNSET == ovector[2 * index]) {
+ return std::string_view();
+ }
+
return std::string_view(_subject.data() + ovector[2 * index], ovector[2 *
index + 1] - ovector[2 * index]);
Review Comment:
This only checks the start offset for `PCRE2_UNSET`, but the end offset can
also be unset; additionally, if offsets are ever inconsistent (e.g., `end <
start`), `ovector[2 * index + 1] - ovector[2 * index]` can underflow and
produce a huge length. Consider reading both `start` and `end` into locals and
returning `{}` when `start == PCRE2_UNSET || end == PCRE2_UNSET || end < start`.
##########
src/tsutil/unit_tests/test_Regex.cc:
##########
@@ -460,6 +460,23 @@ TEST_CASE("RegexMatches edge cases",
"[libts][Regex][RegexMatches]")
CHECK(count >= 2); // At least whole match + first group
CHECK(matches[1] == "foo");
}
+
+ SECTION("RegexMatches with a non-participating group before a participating
one")
+ {
+ // 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);
Review Comment:
In Catch2, `REQUIRE(expr)` already asserts truthiness, so
`REQUIRE(r.compile(\"(a)?(b)\"));` is equivalent and avoids an unnecessary `==
true`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
