lib/mvfst
1
0
Fork 0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-11-10 21:22:20 +03:00
Code Activity

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

Browse Source 
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
This commit is contained in:
Udip Pant
2020-01-23 21:57:43 -08:00
committed by Facebook Github Bot
parent e279022ce5
commit 21a7efb2e3
8 changed files with 26 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
quic/client/QuicClientTransport.cpp
View File

@@ -39,8 +39,8 @@ QuicClientTransport::QuicClientTransport(
DCHECK(handshakeFactory); DCHECK(handshakeFactory);
// TODO(T53612743) Only enforce that the initial destination connection id // TODO(T53612743) Only enforce that the initial destination connection id
// is at least kMinInitialDestinationConnIdLength. // is at least kMinInitialDestinationConnIdLength.
// All subsequent connection ids should be in between // All subsequent destination connection ids should be in between
// [kMinConnectionIdSize, kMaxConnectionIdSize] // [kMinSelfConnectionIdSize, kMaxConnectionIdSize]
DCHECK( DCHECK(
connectionIdSize == 0 || connectionIdSize == 0 ||
(connectionIdSize >= kMinInitialDestinationConnIdLength && (connectionIdSize >= kMinInitialDestinationConnIdLength &&

2
quic/codec/Decode.cpp
View File

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

18
quic/codec/DefaultConnectionIdAlgo.cpp
View File

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

6
quic/codec/QuicConnectionId.cpp
View File

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

4
quic/codec/QuicConnectionId.h
View File

@@ -21,7 +21,9 @@ namespace quic {
constexpr uint8_t kStatelessResetTokenLength = 16; constexpr uint8_t kStatelessResetTokenLength = 16;
using StatelessResetToken = std::array<uint8_t, kStatelessResetTokenLength>; 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; constexpr size_t kMaxConnectionIdSize = 20;
// Minimum required length (in bytes) for the destination connection-id // Minimum required length (in bytes) for the destination connection-id

4
quic/codec/test/QuicConnectionIdTest.cpp
View File

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

4
quic/server/QuicServerWorker.cpp
View File

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

5
quic/server/test/QuicServerTest.cpp
View File

@@ -592,7 +592,10 @@ TEST_F(QuicServerWorkerTest, ConnectionIdTooShort) {
PacketNum num = 1; PacketNum num = 1;
QuicVersion version = QuicVersion::MVFST; QuicVersion version = QuicVersion::MVFST;
LongHeader header(LongHeader::Types::Initial, connId, connId, num, version); 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( RegularQuicPacketBuilder builder(
kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */); kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */);