Copilot commented on code in PR #12643:
URL: https://github.com/apache/trafficserver/pull/12643#discussion_r2496391159
##########
src/records/RecUtils.cc:
##########
@@ -387,26 +390,49 @@ recordRegexCheck(const char *pattern, const char *value)
return regex.compile(pattern) && regex.exec(value);
}
+// Helper to parse integer from string_view using std::from_chars
+// Requires the ENTIRE string to be a valid integer (strict parsing)
+bool
+parse_int(std::string_view sv, int &out)
+{
+ auto [ptr, ec] = std::from_chars(sv.data(), sv.data() + sv.size(), out);
+ // Success only if no error AND entire string was consumed
+ return ec == std::errc{} && ptr == sv.data() + sv.size();
+}
+
bool
recordRangeCheck(const char *pattern, const char *value)
{
- char *p = const_cast<char *>(pattern);
- Tokenizer dashTok("-");
-
- if (recordRegexCheck("^[0-9]+$", value)) {
- while (*p != '[') {
- p++;
- } // skip to '['
- if (dashTok.Initialize(++p, COPY_TOKS) == 2) {
- int l_limit = atoi(dashTok[0]);
- int u_limit = atoi(dashTok[1]);
- int val = atoi(value);
- if (val >= l_limit && val <= u_limit) {
- return true;
- }
- }
+ std::string_view sv_pattern(pattern);
+
+ // Find '[' and ']'
+ auto start = sv_pattern.find('[');
+ if (start == std::string_view::npos) {
+ return false; // No '[' found
}
- return false;
+
+ auto end = sv_pattern.find(']', start);
+ if (end == std::string_view::npos) {
+ return false; // No ']' found
+ }
+
+ // Extract range portion between brackets: "[0-10]" -> "0-10"
+ std::string_view range = sv_pattern.substr(start + 1, end - start - 1);
+
+ // Find the dash separator
+ auto dash_pos = range.find('-');
+ if (dash_pos == std::string_view::npos) {
+ return false; // No dash found
+ }
Review Comment:
The implementation will incorrectly find the first dash in patterns with
negative numbers like `[-10-0]`, resulting in parsing an empty string before
the dash as the lower bound. While the test correctly validates that negative
ranges should be rejected, this bug could cause silent failures or unexpected
behavior. Consider adding a comment explaining that negative ranges are not
supported, or add an explicit check to reject patterns starting with `-` after
the opening bracket.
```suggestion
// Negative ranges are not supported. Explicitly reject if lower or upper
bound starts with '-'.
auto dash_pos = range.find('-');
if (dash_pos == std::string_view::npos) {
return false; // No dash found
}
if (range.size() > 0 && range[0] == '-') {
return false; // Lower bound is negative, not supported
}
if (dash_pos + 1 < range.size() && range[dash_pos + 1] == '-') {
return false; // Upper bound is negative, not supported
}
```
--
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]
