bneradt commented on code in PR #13516:
URL: https://github.com/apache/trafficserver/pull/13516#discussion_r3857381564
##########
src/iocore/net/ConnectionTracker.cc:
##########
@@ -49,11 +51,9 @@ const MgmtConverter ConnectionTracker::MIN_SERVER_CONV(
const MgmtConverter ConnectionTracker::SERVER_MATCH_CONV{
[](const void *data) -> MgmtInt { return
static_cast<MgmtInt>(*static_cast<const decltype(TxnConfig::server_match)
*>(data)); },
[](void *data, MgmtInt i) -> void {
- // Problem - the InkAPITest requires being able to set an arbitrary value,
so this can either
- // correctly clamp or pass the regression tests. Currently it passes the
tests.
- // *static_cast<decltype(TxnConfig::match) *>(data) =
std::clamp(static_cast<decltype(TxnConfig::match)>(i), MATCH_IP,
- // MATCH_BOTH);
- *static_cast<decltype(TxnConfig::server_match) *>(data) =
static_cast<decltype(TxnConfig::server_match)>(i);
+ auto const value = std::clamp(i, static_cast<MgmtInt>(MATCH_IP),
static_cast<MgmtInt>(MATCH_BOTH));
Review Comment:
Yes, this is intentional. `MgmtConverter` store callbacks return `void`, so
`TSHttpTxnConfigIntSet()` reports successful key/type dispatch and cannot
propagate a converter validation error. The string path likewise returns
`TS_SUCCESS` after its converter warns and leaves the value unchanged. I kept
the integer clamp so `server_match` cannot escape the range used by
hashing/equality; rejecting the value would require changing the converter
contract. The updated tests now cover every valid current-value round trip plus
the focused clamp behavior.
--
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]
