bneradt commented on code in PR #13519:
URL: https://github.com/apache/trafficserver/pull/13519#discussion_r3738788561
##########
src/iocore/net/quic/QUICTypes.cc:
##########
@@ -723,17 +723,22 @@ QUICConnectionId::ZERO()
return QUICConnectionId(zero, 0);
}
-QUICConnectionId::QUICConnectionId()
-{
- this->randomize();
-}
-
QUICConnectionId::QUICConnectionId(const uint8_t *buf, uint8_t len) : _len(len)
{
ink_assert(len <= QUICConnectionId::MAX_LENGTH);
memcpy(this->_id, buf, std::min(static_cast<int>(len),
QUICConnectionId::MAX_LENGTH));
}
+QUICConnectionId
+QUICConnectionId::random()
+{
+ uint8_t id[MAX_LENGTH] = {0};
+
+ ink_release_assert(SCID_LEN <= MAX_LENGTH);
+ ink_release_assert(RAND_bytes(id, SCID_LEN) == 1);
+ return {id, SCID_LEN};
+}
Review Comment:
I considered making random() fallible, but kept the fail-closed assertion
intentionally. The factory promises a valid randomized CID, and all current
callers require one; several construction paths cannot propagate an optional
result without a broader API redesign. Returning a zero or invalid CID would
violate the safety invariant this PR adds. I think recoverable RNG failure
handling would be better handled as a separate change that redesigns those call
chains consistently.
--
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]
