Copilot commented on code in PR #13352:
URL: https://github.com/apache/trafficserver/pull/13352#discussion_r3617367586
##########
plugins/prefetch/pattern.cc:
##########
@@ -253,22 +181,18 @@ 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) {
- PrefetchError("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 dst = matches[replIndex];
+ int replIndex = _tokens[i];
- String src(_replacement, _tokenOffset[i], 2);
+ /* $replIndex was validated at config-load time against the number of
groups the pattern defines, but
+ * the group may still not have participated in *this* match (e.g. a
trailing optional group such as
+ * "(\?.*)?" when the subject has no query string). pcre2_match() returns
one past the highest
+ * participating group, so substitute an empty string for a group at or
beyond that -- the documented
+ * PCRE2 semantics for an unmatched group -- rather than failing the whole
replacement. */
+ std::string_view dst = (replIndex < matchCount) ? matches[replIndex] :
std::string_view{};
Review Comment:
`Pattern::replace()` uses `matches[replIndex]` when `replIndex <
matchCount`, but a capture group can still be *unset* even with `replIndex <
matchCount` (e.g. pattern `(a)?(b)` on subject `b`: group 1 is PCRE2_UNSET,
group 2 matches). `RegexMatches::operator[]` does not check for PCRE2_UNSET and
will compute `subject.data() + ovector[...]` with a `size_t(-1)` offset, which
is undefined behavior and can crash. To implement the intended PCRE2 semantics
(“unmatched group expands to empty”) safely, inspect the ovector for UNSET
before constructing the view.
--
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]
