remove constraints of 4-bytes minimum len for connection-id
Summary:
New QUIC draft no longer has this limitation for connection-id, and allows
connid of len 0 -> 20.
This diff removes the constraints. I still kept the requirement for
*server-chosen* conn-id
MODIFY
quic/client/QuicClientTransport.cpp
quic/client/QuicClientTransport.cpp
@@ -39,8 +39,8 @@ QuicClientTransport::QuicClientTransport(
DCHECK(handshakeFactory);
// TODO(T53612743) Only enforce that the initial destination connection id
// is at least kMinInitialDestinationConnIdLength.
- // All subsequent connection ids should be in between
- // [kMinConnectionIdSize, kMaxConnectionIdSize]
+ // All subsequent destination connection ids should be in between
+ // [kMinSelfConnectionIdSize, kMaxConnectionIdSize]
DCHECK(
connectionIdSize == 0 ||
(connectionIdSize >= kMinInitialDestinationConnIdLength &&

MODIFY
quic/codec/Decode.cpp
quic/codec/Decode.cpp
@@ -500,7 +500,7 @@ NewConnectionIdFrame decodeNewConnectionIdFrame(folly::io::Cursor& cursor) {
quic::TransportErrorCode::FRAME_ENCODING_ERROR,
quic::FrameType::NEW_CONNECTION_ID);
}
- if (connIdLen < kMinConnectionIdSize || connIdLen > kMaxConnectionIdSize) {
+ if (connIdLen > kMaxConnectionIdSize) {
throw QuicTransportException(
"ConnectionId invalid length",
quic::TransportErrorCode::FRAME_ENCODING_ERROR,

MODIFY
quic/codec/DefaultConnectionIdAlgo.cpp
quic/codec/DefaultConnectionIdAlgo.cpp
@@ -31,7 +31,7 @@ constexpr uint8_t kShortVersionBitsMask = 0xc0;
* Sets the short version id bits (0 - 1) into the given ConnectionId
*/
void setVersionBitsInConnId(quic::ConnectionId& connId, uint8_t version) {
- if (connId.size() < quic::kMinConnectionIdSize) {
+ if (connId.size() < quic::kMinSelfConnectionIdSize) {
throw quic::QuicInternalException(
"ConnectionId is too small for version",
quic::LocalErrorCode::INTERNAL_ERROR);
@@ -45,7 +45,7 @@ void setVersionBitsInConnId(quic::ConnectionId& connId, uint8_t version) {
* Extract the version id bits (0 - 1) from the given ConnectionId
*/
uint8_t getVersionBitsFromConnId(const quic::ConnectionId& connId) {
- if (connId.size() < quic::kMinConnectionIdSize) {
+ if (connId.size() < quic::kMinSelfConnectionIdSize) {
throw quic::QuicInternalException(
"ConnectionId is too small for version",
quic::LocalErrorCode::INTERNAL_ERROR);
@@ -59,7 +59,7 @@ uint8_t getVersionBitsFromConnId(const quic::ConnectionId& connId) {
* Sets the host id bits [2 - 17] bits into the given ConnectionId
*/
void setHostIdBitsInConnId(quic::ConnectionId& connId, uint16_t hostId) {
- if (connId.size() < quic::kMinConnectionIdSize) {
+ if (connId.size() < quic::kMinSelfConnectionIdSize) {
throw quic::QuicInternalException(
"ConnectionId is too small for hostid",
quic::LocalErrorCode::INTERNAL_ERROR);
@@ -83,7 +83,7 @@ void setHostIdBitsInConnId(quic::ConnectionId& connId, uint16_t hostId) {
* Extract the host id bits [2 - 17] bits from the given ConnectionId
*/
uint16_t getHostIdBitsInConnId(const quic::ConnectionId& connId) {
- if (connId.size() < quic::kMinConnectionIdSize) {
+ if (connId.size() < quic::kMinSelfConnectionIdSize) {
throw quic::QuicInternalException(
"ConnectionId is too small for hostid",
quic::LocalErrorCode::INTERNAL_ERROR);
@@ -104,7 +104,7 @@ uint16_t getHostIdBitsInConnId(const quic::ConnectionId& connId) {
* Sets the given 8-bit workerId into the given connectionId's 18-25 bits
*/
void setWorkerIdBitsInConnId(quic::ConnectionId& connId, uint8_t workerId) {
- if (connId.size() < quic::kMinConnectionIdSize) {
+ if (connId.size() < quic::kMinSelfConnectionIdSize) {
throw quic::QuicInternalException(
"ConnectionId is too small for workerid",
quic::LocalErrorCode::INTERNAL_ERROR);
@@ -123,7 +123,7 @@ void setWorkerIdBitsInConnId(quic::ConnectionId& connId, uint8_t workerId) {
* Extracts the 'workerId' bits from the given ConnectionId
*/
uint8_t getWorkerIdFromConnId(const quic::ConnectionId& connId) {
- if (connId.size() < quic::kMinConnectionIdSize) {
+ if (connId.size() < quic::kMinSelfConnectionIdSize) {
throw quic::QuicInternalException(
"ConnectionId is too small for workerid",
quic::LocalErrorCode::INTERNAL_ERROR);
@@ -139,7 +139,7 @@ uint8_t getWorkerIdFromConnId(const quic::ConnectionId& connId) {
* Sets the server id bit (at 26th bit) into the given ConnectionId
*/
void setProcessIdBitsInConnId(quic::ConnectionId& connId, uint8_t processId) {
- if (connId.size() < quic::kMinConnectionIdSize) {
+ if (connId.size() < quic::kMinSelfConnectionIdSize) {
throw quic::QuicInternalException(
"ConnectionId is too small for processid",
quic::LocalErrorCode::INTERNAL_ERROR);
@@ -153,7 +153,7 @@ void setProcessIdBitsInConnId(quic::ConnectionId& connId, uint8_t processId) {
* Extract the server id bit (at 26th bit) from the given ConnectionId
*/
uint8_t getProcessIdBitsFromConnId(const quic::ConnectionId& connId) {
- if (connId.size() < quic::kMinConnectionIdSize) {
+ if (connId.size() < quic::kMinSelfConnectionIdSize) {
throw quic::QuicInternalException(
"ConnectionId is too small for processid",
quic::LocalErrorCode::INTERNAL_ERROR);
@@ -167,7 +167,7 @@ uint8_t getProcessIdBitsFromConnId(const quic::ConnectionId& connId) {
namespace quic {
bool DefaultConnectionIdAlgo::canParse(const ConnectionId& id) const {
- if (id.size() < kMinConnectionIdSize) {
+ if (id.size() < kMinSelfConnectionIdSize) {
return false;
}
return getVersionBitsFromConnId(id) == kShortVersionId;

MODIFY
quic/codec/QuicConnectionId.cpp
quic/codec/QuicConnectionId.cpp
@@ -32,9 +32,7 @@ ConnectionId::ConnectionId(const std::vector<uint8_t>& connidIn) {
static_assert(
std::numeric_limits<uint8_t>::max() > kMaxConnectionIdSize,
"Max connection size is too big");
- if (connidIn.size() != 0 &&
- (connidIn.size() < kMinConnectionIdSize ||
- connidIn.size() > kMaxConnectionIdSize)) {
+ if (connidIn.size() > kMaxConnectionIdSize) {
// We can't throw a transport error here because of the dependency. This is
// sad because this will cause an internal error downstream.
throw std::runtime_error("ConnectionId invalid size");
@@ -51,7 +49,7 @@ ConnectionId::ConnectionId(folly::io::Cursor& cursor, size_t len) {
connidLen = 0;
return;
}
- if (len < kMinConnectionIdSize || len > kMaxConnectionIdSize) {
+ if (len > kMaxConnectionIdSize) {
// We can't throw a transport error here because of the dependency. This is
// sad because this will cause an internal error downstream.
throw std::runtime_error("ConnectionId invalid size");

MODIFY
quic/codec/QuicConnectionId.h
quic/codec/QuicConnectionId.h
@@ -21,7 +21,9 @@ namespace quic {
constexpr uint8_t kStatelessResetTokenLength = 16;
using StatelessResetToken = std::array<uint8_t, kStatelessResetTokenLength>;
-constexpr size_t kMinConnectionIdSize = 4;
+// min connId size for one chosen by 'mvfst' as a peer
+constexpr size_t kMinSelfConnectionIdSize = 4;
+// max size of a connId as specified in the draft
constexpr size_t kMaxConnectionIdSize = 20;
// Minimum required length (in bytes) for the destination connection-id

MODIFY
quic/codec/test/QuicConnectionIdTest.cpp
quic/codec/test/QuicConnectionIdTest.cpp
@@ -59,10 +59,10 @@ TEST(ConnectionIdTest, ConnIdSize) {
}
EXPECT_THROW(ConnectionId{testconnid}, std::runtime_error);
testconnid.clear();
- for (size_t i = 0; i < kMinConnectionIdSize - 1; ++i) {
+ for (size_t i = 0; i < kMinSelfConnectionIdSize - 1; ++i) {
testconnid.push_back(0);
}
- EXPECT_THROW(ConnectionId{testconnid}, std::runtime_error);
+ EXPECT_NO_THROW(ConnectionId{testconnid});
}
struct ConnectionIdLengthParams {

MODIFY
quic/server/QuicServerWorker.cpp
quic/server/QuicServerWorker.cpp
@@ -236,7 +236,9 @@ void QuicServerWorker::handleNetworkData(
return;
}
- if (parsedLongHeader->invariant.dstConnId.size() < kMinConnectionIdSize) {
+ if (!isUsingClientConnId &&
+ parsedLongHeader->invariant.dstConnId.size() <
+ kMinSelfConnectionIdSize) {
// drop packet if connId is present but is not valid.
VLOG(3) << "Dropping packet due to invalid connectionId";
QUIC_STATS(

MODIFY
quic/server/test/QuicServerTest.cpp
quic/server/test/QuicServerTest.cpp
@@ -592,7 +592,10 @@ TEST_F(QuicServerWorkerTest, ConnectionIdTooShort) {
PacketNum num = 1;
QuicVersion version = QuicVersion::MVFST;
LongHeader header(LongHeader::Types::Initial, connId, connId, num, version);
- EXPECT_CALL(*transportInfoCb_, onPacketDropped(_));
+ EXPECT_CALL(*transportInfoCb_, onPacketDropped(_)).Times(0);
+ EXPECT_CALL(*transportInfoCb_, onPacketProcessed()).Times(1);
+ EXPECT_CALL(*transportInfoCb_, onPacketSent()).Times(1);
+ EXPECT_CALL(*transportInfoCb_, onWrite(_)).Times(1);
RegularQuicPacketBuilder builder(
kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */);

remove constraints of 4-bytes minimum len for connection-id

Summary:
New QUIC draft no longer has this limitation for connection-id, and allows
connid of len 0 -> 20.

This diff removes the constraints. I still kept the requirement for
*server-chosen* conn-id

Reviewed By: mjoras, lnicco

Differential Revision: D19507366

fbshipit-source-id: 4c73f45617f40b29d47d2d86b7598f6c95588d0a