Interpret varint in MAX_STREAMS_FRAME as count and not as stream-id
Summary: post draft-17 it is interpreted as count not the max stream id
MODIFY
quic/api/test/QuicPacketSchedulerTest.cpp
quic/api/test/QuicPacketSchedulerTest.cpp
@@ -362,7 +362,7 @@ TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) {
// Create few frames
ConnectionCloseFrame connCloseFrame(
TransportErrorCode::FRAME_ENCODING_ERROR, "The sun is in the sky.");
- MaxStreamsFrame maxStreamIdFrame(0x1024, true);
+ MaxStreamsFrame maxStreamFrame(999, true);
PingFrame pingFrame;
IntervalSet<PacketNum> ackBlocks;
ackBlocks.insert(10, 100);
@@ -371,7 +371,7 @@ TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) {
// Write those framses with a regular builder
writeFrame(connCloseFrame, regularBuilder);
- writeFrame(maxStreamIdFrame, regularBuilder);
+ writeFrame(maxStreamFrame, regularBuilder);
writeFrame(pingFrame, regularBuilder);
writeAckFrame(ackMeta, regularBuilder);

MODIFY
quic/codec/Decode.cpp
quic/codec/Decode.cpp
@@ -376,27 +376,25 @@ MaxStreamDataFrame decodeMaxStreamDataFrame(folly::io::Cursor& cursor) {
}
MaxStreamsFrame decodeBiDiMaxStreamsFrame(folly::io::Cursor& cursor) {
- auto streamId = decodeQuicInteger(cursor);
- if (UNLIKELY(!streamId || !isBidirectionalStream(streamId->first))) {
+ auto streamCount = decodeQuicInteger(cursor);
+ if (UNLIKELY(!streamCount)) {
throw QuicTransportException(
"Invalid Bi-directional streamId",
quic::TransportErrorCode::FRAME_ENCODING_ERROR,
quic::FrameType::MAX_STREAMS_BIDI);
}
- return MaxStreamsFrame(
- folly::to<StreamId>(streamId->first), true /* isBidirectional*/);
+ return MaxStreamsFrame(streamCount->first, true /* isBidirectional*/);
}
MaxStreamsFrame decodeUniMaxStreamsFrame(folly::io::Cursor& cursor) {
- auto streamId = decodeQuicInteger(cursor);
- if (UNLIKELY(!streamId || !isUnidirectionalStream(streamId->first))) {
+ auto streamCount = decodeQuicInteger(cursor);
+ if (UNLIKELY(!streamCount)) {
throw QuicTransportException(
"Invalid Uni-directional streamId",
quic::TransportErrorCode::FRAME_ENCODING_ERROR,
quic::FrameType::MAX_STREAMS_UNI);
}
- return MaxStreamsFrame(
- folly::to<StreamId>(streamId->first), false /* isBidirectional */);
+ return MaxStreamsFrame(streamCount->first, false /* isUnidirectional */);
}
DataBlockedFrame decodeDataBlockedFrame(folly::io::Cursor& cursor) {

MODIFY
quic/codec/QuicWriteCodec.cpp
quic/codec/QuicWriteCodec.cpp
@@ -474,11 +474,11 @@ size_t writeFrame(QuicWriteFrame&& frame, PacketBuilderInterface& builder) {
? FrameType::MAX_STREAMS_BIDI
: FrameType::MAX_STREAMS_UNI;
QuicInteger packetType(static_cast<FrameTypeType>(frameType));
- QuicInteger streamId(maxStreamsFrame.maxStreams);
- auto maxStreamsFrameSize = packetType.getSize() + streamId.getSize();
+ QuicInteger streamCount(maxStreamsFrame.maxStreams);
+ auto maxStreamsFrameSize = packetType.getSize() + streamCount.getSize();
if (packetSpaceCheck(spaceLeft, maxStreamsFrameSize)) {
builder.write(packetType);
- builder.write(streamId);
+ builder.write(streamCount);
builder.appendFrame(std::move(maxStreamsFrame));
return maxStreamsFrameSize;
}

MODIFY
quic/codec/test/QuicPacketRebuilderTest.cpp
quic/codec/test/QuicPacketRebuilderTest.cpp
@@ -63,7 +63,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildPacket) {
TransportErrorCode::FRAME_ENCODING_ERROR,
"The sun is in the sky.",
FrameType::ACK);
- MaxStreamsFrame maxStreamIdFrame(0x1024, true);
+ MaxStreamsFrame maxStreamsFrame(4321, true);
PingFrame pingFrame;
IntervalSet<PacketNum> ackBlocks;
ackBlocks.insert(10, 100);
@@ -83,7 +83,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildPacket) {
// Write them with a regular builder
writeFrame(connCloseFrame, regularBuilder1);
- writeFrame(maxStreamIdFrame, regularBuilder1);
+ writeFrame(maxStreamsFrame, regularBuilder1);
writeFrame(pingFrame, regularBuilder1);
writeAckFrame(ackMeta, regularBuilder1);
writeStreamFrame(streamMeta, regularBuilder1);
@@ -125,7 +125,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildPacket) {
EXPECT_EQ(FrameType::ACK, closeFrame.closingFrameType);
},
[](const MaxStreamsFrame& maxStreamFrame) {
- EXPECT_EQ(0x1024, maxStreamFrame.maxStreams);
+ EXPECT_EQ(4321, maxStreamFrame.maxStreams);
},
[](const PingFrame& ping) { EXPECT_EQ(PingFrame(), ping); },
[](const WriteAckFrame& ack) {

MODIFY
quic/codec/test/QuicWriteCodecTest.cpp
quic/codec/test/QuicWriteCodecTest.cpp
@@ -945,50 +945,56 @@ TEST_F(QuicWriteCodecTest, NoSpaceForMaxData) {
}
TEST_F(QuicWriteCodecTest, WriteMaxStreamId) {
+ for (uint64_t i = 0; i < 100; i++) {
MockQuicPacketBuilder pktBuilder;
setupCommonExpects(pktBuilder);
- StreamId maxStream = 0x1234;
+ uint64_t maxStream = i;
bool isBidirectional = true;
- MaxStreamsFrame maxStreamIdFrame(maxStream, isBidirectional);
- auto bytesWritten = writeFrame(maxStreamIdFrame, pktBuilder);
+ MaxStreamsFrame maxStreamsFrame(maxStream, isBidirectional);
+ auto bytesWritten = writeFrame(maxStreamsFrame, pktBuilder);
auto builtOut = std::move(pktBuilder).buildPacket();
auto regularPacket = builtOut.first;
- // 1 byte for the type and 2 bytes for the stream id.
- EXPECT_EQ(1 + 2, bytesWritten);
+ auto streamCountSize = i < 64 ? 1 : 2;
+ // 1 byte for the type and up to 2 bytes for the stream count.
+ EXPECT_EQ(1 + streamCountSize, bytesWritten);
auto resultMaxStreamIdFrame =
boost::get<MaxStreamsFrame>(regularPacket.frames[0]);
- EXPECT_EQ(0x1234, resultMaxStreamIdFrame.maxStreams);
+ EXPECT_EQ(i, resultMaxStreamIdFrame.maxStreams);
auto wireBuf = std::move(builtOut.second);
folly::io::Cursor cursor(wireBuf.get());
- auto wireStreamIdFrame = boost::get<MaxStreamsFrame>(parseQuicFrame(cursor));
- EXPECT_EQ(0x1234, wireStreamIdFrame.maxStreams);
+ auto wireStreamsFrame = boost::get<MaxStreamsFrame>(parseQuicFrame(cursor));
+ EXPECT_EQ(i, wireStreamsFrame.maxStreams);
EXPECT_TRUE(cursor.isAtEnd());
}
+}
TEST_F(QuicWriteCodecTest, WriteUniMaxStreamId) {
+ for (uint64_t i = 0; i < 100; i++) {
MockQuicPacketBuilder pktBuilder;
setupCommonExpects(pktBuilder);
- StreamId maxStream = 0x1233;
+ uint64_t maxStream = i;
bool isBidirectional = false;
- MaxStreamsFrame maxStreamIdFrame(maxStream, isBidirectional);
- auto bytesWritten = writeFrame(maxStreamIdFrame, pktBuilder);
+ MaxStreamsFrame maxStreamsFrame(maxStream, isBidirectional);
+ auto bytesWritten = writeFrame(maxStreamsFrame, pktBuilder);
auto builtOut = std::move(pktBuilder).buildPacket();
auto regularPacket = builtOut.first;
- // 1 byte for the type and 2 bytes for the stream id.
- EXPECT_EQ(1 + 2, bytesWritten);
+ auto streamCountSize = i < 64 ? 1 : 2;
+ // 1 byte for the type and up to 2 bytes for the stream count.
+ EXPECT_EQ(1 + streamCountSize, bytesWritten);
auto resultMaxStreamIdFrame =
boost::get<MaxStreamsFrame>(regularPacket.frames[0]);
- EXPECT_EQ(0x1233, resultMaxStreamIdFrame.maxStreams);
+ EXPECT_EQ(i, resultMaxStreamIdFrame.maxStreams);
auto wireBuf = std::move(builtOut.second);
folly::io::Cursor cursor(wireBuf.get());
- auto wireStreamIdFrame = boost::get<MaxStreamsFrame>(parseQuicFrame(cursor));
- EXPECT_EQ(0x1233, wireStreamIdFrame.maxStreams);
+ auto wireStreamsFrame = boost::get<MaxStreamsFrame>(parseQuicFrame(cursor));
+ EXPECT_EQ(i, wireStreamsFrame.maxStreams);
EXPECT_TRUE(cursor.isAtEnd());
}
+}
TEST_F(QuicWriteCodecTest, NoSpaceForMaxStreamId) {
MockQuicPacketBuilder pktBuilder;

Interpret varint in MAX_STREAMS_FRAME as count and not as stream-id

Summary: post draft-17 it is interpreted as count not the max stream id

Reviewed By: mjoras

Differential Revision: D15523021

fbshipit-source-id: 779a1c5269eb8dbcdcd169303caa7a65e467339c