cmcfarlen commented on code in PR #13506:
URL: https://github.com/apache/trafficserver/pull/13506#discussion_r3730335911
##########
src/records/RecordsConfig.cc:
##########
@@ -395,7 +395,7 @@ static constexpr RecordElement RecordsConfig[] =
,
{RECT_CONFIG, "proxy.config.http.per_server.connection.min", RECD_INT, "0",
RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
,
- {RECT_CONFIG, "proxy.config.http.per_server.connection.metric_enabled",
RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "[0-1]", RECA_NULL}
+ {RECT_CONFIG, "proxy.config.http.per_server.connection.metric_enabled",
RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_STR, "[0-2]", RECA_NULL}
Review Comment:
Good catch, fixed to `^[0-2]$`.
Confirmed the mechanism: `recordRegexCheck` in `RecUtils.cc` does
`regex.compile(pattern) && regex.exec(value)`, which is an unanchored search,
so `10` matched on the `0`. The anchored form is also the dominant convention
in this file (124 entries use `^...$` versus 6 that do not, including a
`^[0-1]$`).
For completeness: the looseness was pre-existing rather than introduced
here, since the previous value was `[0-1]`, equally unanchored, and it was
never a functional bug because the config update handler clamps the value. Two
other unanchored `[0-2]` entries remain elsewhere in the file; happy to anchor
those too, though it seemed out of scope for this PR.
--
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]