Copilot commented on code in PR #12607:
URL: https://github.com/apache/trafficserver/pull/12607#discussion_r2460980169
##########
plugins/cachekey/pattern.cc:
##########
@@ -299,18 +270,11 @@ Pattern::replace(const String &subject, String &result)
int previous = 0;
for (int i = 0; i < _tokenCount; i++) {
- int replIndex = _tokens[i];
- int start = ovector[2 * replIndex];
- int length = ovector[2 * replIndex + 1] - ovector[2 * replIndex];
-
- /* Handle the case when no match / a group capture result in an empty
string */
- if (start < 0) {
- start = 0;
- length = 0;
- }
+ int replIndex = _tokens[i];
Review Comment:
Missing bounds check for `replIndex`. If the replacement string contains a
group reference like `$5` that exceeds the number of captured groups
(`matchCount`), accessing `matches[replIndex]` will be out of bounds. Add
validation: `if (replIndex >= matchCount) { return false; }`
```suggestion
int replIndex = _tokens[i];
if (replIndex >= matchCount) {
CacheKeyError("invalid reference in replacement string: $%d",
replIndex);
return false;
}
```
##########
plugins/cachekey/pattern.cc:
##########
@@ -224,66 +201,60 @@ Pattern::match(const String &subject)
}
/**
- * @brief Return all PCRE capture groups that matched in the subject string
- * @param subject PCRE subject string
+ * @brief Return all Regex capture groups that matched in the subject string
+ * @param subject Regex subject string
* @param result reference to vector of strings containing all capture groups
*/
bool
Pattern::capture(const String &subject, StringVector &result)
{
- int matchCount;
- int ovector[OVECOUNT];
-
CacheKeyDebug("capturing '%s' from '%s'", _pattern.c_str(), subject.c_str());
- if (!_re) {
+ if (_re.empty()) {
CacheKeyError("regular expression not initialized");
return false;
}
- matchCount = pcre_exec(_re, nullptr, subject.c_str(), subject.length(), 0,
PCRE_NOTEMPTY, ovector, OVECOUNT);
+ RegexMatches matches;
+ int matchCount = _re.exec(subject, matches, RE_NOTEMPTY);
if (matchCount < 0) {
- if (matchCount != PCRE_ERROR_NOMATCH) {
+ if (matchCount != RE_ERROR_NOMATCH) {
CacheKeyError("matching error %d", matchCount);
}
return false;
}
for (int i = 0; i < matchCount; i++) {
- int start = ovector[2 * i];
- int length = ovector[2 * i + 1] - ovector[2 * i];
+ std::string_view capture = matches[i];
+ String dst(capture.data(), capture.length());
Review Comment:
[nitpick] Inconsistent construction of String from string_view. Line 277
uses the same pattern. Consider extracting this to a helper function or using a
consistent pattern like `String dst(capture)` if String supports string_view
construction.
```suggestion
String dst(capture);
```
--
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]
