Copilot commented on code in PR #12985:
URL: https://github.com/apache/trafficserver/pull/12985#discussion_r2978068396
##########
src/proxy/ParentSelection.cc:
##########
@@ -556,6 +556,11 @@ ParentRecord::ProcessParents(char *val, bool isPrimary)
errPtr = "Parent string is empty";
goto MERROR;
}
+ if (tmp3 && strlen(tmp3) > MAXDNAME) {
+ errPtr = "Parent hash_string is too long";
+ goto MERROR;
Review Comment:
The new length guard is off by one because `tmp3` points at the '&'
separator while the destination buffer is `hash_string[MAXDNAME + 1]` and the
copy size is `strlen(tmp3)` (which includes the '&'). As written, a hash string
of exactly `MAXDNAME` bytes (valid for `MAXDNAME + 1` buffer including NUL) is
rejected. Consider validating against `strlen(tmp3) > MAXDNAME + 1` (copy size)
or checking `strlen(tmp3 + 1) > MAXDNAME` (hash-string payload), and reuse the
computed length for the subsequent `memcpy`.
##########
src/proxy/ParentSelection.cc:
##########
@@ -556,6 +556,11 @@ ParentRecord::ProcessParents(char *val, bool isPrimary)
errPtr = "Parent string is empty";
goto MERROR;
}
+ if (tmp3 && strlen(tmp3) > MAXDNAME) {
+ errPtr = "Parent hash_string is too long";
Review Comment:
Error message uses the internal field name `hash_string` with an underscore,
while the surrounding parse errors are phrased as normal words (e.g., "Parent
hostname is too long"). Consider changing this to "Parent hash string is too
long" for consistency and readability.
```suggestion
errPtr = "Parent hash string is too long";
```
##########
src/proxy/ParentSelection.cc:
##########
@@ -556,6 +556,11 @@ ParentRecord::ProcessParents(char *val, bool isPrimary)
errPtr = "Parent string is empty";
goto MERROR;
}
+ if (tmp3 && strlen(tmp3) > MAXDNAME) {
+ errPtr = "Parent hash_string is too long";
+ goto MERROR;
+ }
Review Comment:
This adds new input validation for '&hash' tokens but there’s no regression
coverage for the new failure mode or the boundary condition (hash length ==
MAXDNAME). Since this file already contains
EXCLUSIVE_REGRESSION_TEST(PARENTSELECTION), please add a test that exercises an
overlong hash string (expecting the parse to fail with the new error) and a
max-length hash string (expecting it to be accepted).
--
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]
