Skip to content

Commit

Permalink
Properly pad cloned Initials.
Browse files Browse the repository at this point in the history
Summary: After changing the way padding frames are stored, we introduced a bug where the cloning path didn't properly clone the initials. Fix this by always filling up the Initials that are cloned.

Reviewed By: kvtsoy

Differential Revision: D45374087

fbshipit-source-id: 0ef9549187c0cd584443b0681bf2a903399b432b
  • Loading branch information
Matt Joras authored and facebook-github-bot committed Apr 28, 2023
1 parent 44435ad commit a5a9edb
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 0 deletions.
8 changes: 8 additions & 0 deletions quic/codec/QuicPacketRebuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,14 @@ folly::Optional<PacketEvent> PacketRebuilder::rebuildFromPacket(
(shouldWriteWindowUpdate && !windowUpdateWritten && !writeSuccess)) {
return folly::none;
}

if (encryptionLevel == EncryptionLevel::Initial) {
// Pad anything else that's left.
while (builder_.remainingSpaceInPkt() > 0) {
writeFrame(PaddingFrame(), builder_);
}
}

return cloneOutstandingPacket(packet);
}

Expand Down
39 changes: 39 additions & 0 deletions quic/codec/test/QuicPacketRebuilderTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,45 @@ TEST_F(QuicPacketRebuilderTest, RebuildEmpty) {
EXPECT_TRUE(packet.body->empty());
}

TEST_F(QuicPacketRebuilderTest, RebuildSmallInitial) {
auto srcConnId = getTestConnectionId(0);
auto dstConnId = getTestConnectionId(1);
QuicVersion version = QuicVersion::MVFST;
PacketNum num = 1;
LongHeader initialHeader1(
LongHeader::Types::Initial, srcConnId, dstConnId, num, version, "");
LongHeader initialHeader2(
LongHeader::Types::Initial, srcConnId, dstConnId, num + 1, version, "");

RegularQuicPacketBuilder regularBuilder1(
kDefaultUDPSendPacketLen, std::move(initialHeader1), 0);
RegularQuicPacketBuilder regularBuilder2(
kDefaultUDPSendPacketLen, std::move(initialHeader2), 0);

PingFrame pingFrame{};
writeFrame(pingFrame, regularBuilder1);
MaxStreamsFrame maxStreamsFrame(4321, true);
writeFrame(QuicSimpleFrame(maxStreamsFrame), regularBuilder1);
regularBuilder1.encodePacketHeader();
QuicConnectionStateBase conn(QuicNodeType::Client);
PacketRebuilder rebuilder(regularBuilder2, conn);
auto packet = std::move(regularBuilder1).buildPacket();
auto outstanding = makeDummyOutstandingPacket(packet.packet, 1000);
EXPECT_FALSE(packet.header->empty());
ASSERT_EQ(packet.packet.frames.size(), 2);
EXPECT_FALSE(packet.body->empty());
regularBuilder2.encodePacketHeader();
ASSERT_TRUE(rebuilder.rebuildFromPacket(outstanding).has_value());
auto rebuilt = std::move(regularBuilder2).buildPacket();
EXPECT_FALSE(rebuilt.header->empty());
ASSERT_EQ(rebuilt.packet.frames.size(), 3);
auto padding = rebuilt.packet.frames.back().asPaddingFrame();
ASSERT_TRUE(padding != nullptr);
EXPECT_GT(padding->numFrames, 1000);
EXPECT_FALSE(rebuilt.body->empty());
EXPECT_GT(rebuilt.body->computeChainDataLength(), 1200);
}

TEST_F(QuicPacketRebuilderTest, RebuildPacket) {
ShortHeader shortHeader1(
ProtectionType::KeyPhaseZero, getTestConnectionId(), 0);
Expand Down

0 comments on commit a5a9edb

Please sign in to comment.