diff --git a/quic/api/QuicPacketScheduler.cpp b/quic/api/QuicPacketScheduler.cpp index b1cc5f762..d36d5b520 100644 --- a/quic/api/QuicPacketScheduler.cpp +++ b/quic/api/QuicPacketScheduler.cpp @@ -117,6 +117,7 @@ FrameScheduler::FrameScheduler(std::string name) : name_(std::move(name)) {} SchedulingResult FrameScheduler::scheduleFramesForPacket( PacketBuilderInterface&& builder, uint32_t writableBytes) { + builder.encodePacketHeader(); // We need to keep track of writable bytes after writing header. writableBytes = writableBytes > builder.getHeaderBytes() ? writableBytes - builder.getHeaderBytes() @@ -545,6 +546,7 @@ SchedulingResult CloningScheduler::scheduleFramesForPacket( conn_.udpSendPacketLen, builder.getPacketHeader(), getAckState(conn_, builderPnSpace).largestAckedByPeer); + regularBuilder.encodePacketHeader(); PacketRebuilder rebuilder(regularBuilder, conn_); // We shouldn't clone Handshake packet. if (iter->isHandshake) { diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp index 854b6e60e..adad3b89d 100644 --- a/quic/api/QuicTransportFunctions.cpp +++ b/quic/api/QuicTransportFunctions.cpp @@ -176,6 +176,7 @@ DataPathResult iobufChainBasedBuildScheduleEncrypt( connection.udpSendPacketLen, std::move(header), getAckState(connection, pnSpace).largestAckedByPeer); + // It's the scheduler's job to invoke encode header pktBuilder.setCipherOverhead(cipherOverhead); auto result = scheduler.scheduleFramesForPacket(std::move(pktBuilder), writableBytes); @@ -790,6 +791,7 @@ void writeCloseCommon( connection.udpSendPacketLen, std::move(header), getAckState(connection, pnSpace).largestAckedByPeer); + packetBuilder.encodePacketHeader(); packetBuilder.setCipherOverhead(aead.getCipherOverhead()); size_t written = 0; if (!closeDetails) { diff --git a/quic/api/test/QuicPacketSchedulerTest.cpp b/quic/api/test/QuicPacketSchedulerTest.cpp index 6515ffb79..5390d06d3 100644 --- a/quic/api/test/QuicPacketSchedulerTest.cpp +++ b/quic/api/test/QuicPacketSchedulerTest.cpp @@ -82,26 +82,6 @@ class QuicPacketSchedulerTest : public Test { QuicVersion version{QuicVersion::MVFST}; }; -TEST_F(QuicPacketSchedulerTest, NoopScheduler) { - QuicConnectionStateBase conn(QuicNodeType::Client); - FrameScheduler scheduler("frame"); - EXPECT_FALSE(scheduler.hasData()); - - LongHeader header( - LongHeader::Types::Initial, - getTestConnectionId(1), - getTestConnectionId(), - 0x1356, - version); - RegularQuicPacketBuilder builder( - conn.udpSendPacketLen, - std::move(header), - conn.ackStates.initialAckState.largestAckedByPeer); - - auto builtPacket = std::move(builder).buildPacket(); - EXPECT_TRUE(builtPacket.packet.frames.empty()); -} - TEST_F(QuicPacketSchedulerTest, CryptoPaddingInitialPacket) { QuicClientConnectionState conn( FizzClientQuicHandshakeContext::Builder().build()); @@ -330,6 +310,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoSchedulerOnlySingleLossFits) { conn.udpSendPacketLen, std::move(longHeader), conn.ackStates.handshakeAckState.largestAckedByPeer); + builder.encodePacketHeader(); PacketBuilderWrapper builderWrapper(builder, 13); CryptoStreamScheduler scheduler( conn, *getCryptoStream(*conn.cryptoState, EncryptionLevel::Handshake)); @@ -394,6 +375,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerExists) { conn.udpSendPacketLen, std::move(shortHeader), conn.ackStates.appDataAckState.largestAckedByPeer); + builder.encodePacketHeader(); auto originalSpace = builder.remainingSpaceInPkt(); conn.streamManager->queueWindowUpdate(stream->id); scheduler.writeWindowUpdates(builder); @@ -415,6 +397,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameNoSpace) { conn.udpSendPacketLen, std::move(shortHeader), conn.ackStates.appDataAckState.largestAckedByPeer); + builder.encodePacketHeader(); PacketBuilderWrapper builderWrapper(builder, 2); auto originalSpace = builder.remainingSpaceInPkt(); conn.streamManager->queueWindowUpdate(stream->id); @@ -436,6 +419,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerStreamNotExists) { conn.udpSendPacketLen, std::move(shortHeader), conn.ackStates.appDataAckState.largestAckedByPeer); + builder.encodePacketHeader(); auto originalSpace = builder.remainingSpaceInPkt(); conn.streamManager->queueWindowUpdate(nonExistentStream); scheduler.writeWindowUpdates(builder); @@ -470,75 +454,6 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerTest) { EXPECT_EQ(packetNum, *result.packetEvent); } -TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) { - QuicClientConnectionState conn( - FizzClientQuicHandshakeContext::Builder().build()); - FrameScheduler noopScheduler("frame"); - ASSERT_FALSE(noopScheduler.hasData()); - CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0); - EXPECT_FALSE(cloningScheduler.hasData()); - auto packetNum = addOutstandingPacket(conn); - // There needs to have retransmittable frame for the rebuilder to work - conn.outstandingPackets.back().packet.frames.push_back( - MaxDataFrame(conn.flowControlState.advertisedMaxOffset)); - EXPECT_TRUE(cloningScheduler.hasData()); - - ASSERT_FALSE(noopScheduler.hasData()); - ShortHeader header( - ProtectionType::KeyPhaseOne, - conn.clientConnectionId.value_or(getTestConnectionId()), - getNextPacketNum(conn, PacketNumberSpace::AppData)); - RegularQuicPacketBuilder regularBuilder( - conn.udpSendPacketLen, - std::move(header), - conn.ackStates.appDataAckState.largestAckedByPeer); - - // Create few frames - ConnectionCloseFrame connCloseFrame( - QuicErrorCode(TransportErrorCode::FRAME_ENCODING_ERROR), - "The sun is in the sky."); - MaxStreamsFrame maxStreamFrame(999, true); - PingFrame pingFrame; - AckBlocks ackBlocks; - ackBlocks.insert(10, 100); - ackBlocks.insert(200, 1000); - AckFrameMetaData ackMeta(ackBlocks, 0us, kDefaultAckDelayExponent); - - // Write those framses with a regular builder - writeFrame(connCloseFrame, regularBuilder); - writeFrame(QuicSimpleFrame(maxStreamFrame), regularBuilder); - writeFrame(QuicSimpleFrame(pingFrame), regularBuilder); - writeAckFrame(ackMeta, regularBuilder); - - auto result = cloningScheduler.scheduleFramesForPacket( - std::move(regularBuilder), kDefaultUDPSendPacketLen); - EXPECT_TRUE(result.packetEvent.has_value() && result.packet.has_value()); - EXPECT_EQ(packetNum, *result.packetEvent); - // written packet (result.packet) should not have any frame in the builder - auto& writtenPacket = *result.packet; - auto shortHeader = writtenPacket.packet.header.asShort(); - CHECK(shortHeader); - EXPECT_EQ(ProtectionType::KeyPhaseOne, shortHeader->getProtectionType()); - EXPECT_EQ( - conn.ackStates.appDataAckState.nextPacketNum, - shortHeader->getPacketSequenceNum()); - - // Test that the only frame that's written is maxdataframe - EXPECT_GE(writtenPacket.packet.frames.size(), 1); - auto& writtenFrame = writtenPacket.packet.frames.at(0); - auto maxDataFrame = writtenFrame.asMaxDataFrame(); - CHECK(maxDataFrame); - for (auto& frame : writtenPacket.packet.frames) { - bool present = false; - /* the next four frames should not be written */ - present |= frame.asConnectionCloseFrame() ? true : false; - present |= frame.asQuicSimpleFrame() ? true : false; - present |= frame.asQuicSimpleFrame() ? true : false; - present |= frame.asWriteAckFrame() ? true : false; - ASSERT_FALSE(present); - } -} - TEST_F(QuicPacketSchedulerTest, DoNotCloneProcessedClonedPacket) { QuicClientConnectionState conn( FizzClientQuicHandshakeContext::Builder().build()); @@ -819,6 +734,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerAllFit) { conn.udpSendPacketLen, std::move(shortHeader), conn.ackStates.appDataAckState.largestAckedByPeer); + builder.encodePacketHeader(); auto stream1 = conn.streamManager->createNextBidirectionalStream().value()->id; auto stream2 = @@ -853,10 +769,11 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobin) { ProtectionType::KeyPhaseZero, connId, getNextPacketNum(conn, PacketNumberSpace::AppData)); - RegularQuicPacketBuilder builder1( + RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(shortHeader1), conn.ackStates.appDataAckState.largestAckedByPeer); + builder.encodePacketHeader(); auto stream1 = conn.streamManager->createNextBidirectionalStream().value()->id; auto stream2 = @@ -882,7 +799,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobin) { false); // Force the wraparound initially. conn.schedulingState.nextScheduledStream = stream3 + 8; - scheduler.writeStreams(builder1); + scheduler.writeStreams(builder); EXPECT_EQ(conn.schedulingState.nextScheduledStream, 4); // Should write frames for stream2, stream3, followed by stream1 again. @@ -986,10 +903,11 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobinControl) { ProtectionType::KeyPhaseZero, connId, getNextPacketNum(conn, PacketNumberSpace::AppData)); - RegularQuicPacketBuilder builder1( + RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(shortHeader1), conn.ackStates.appDataAckState.largestAckedByPeer); + builder.encodePacketHeader(); auto stream1 = conn.streamManager->createNextBidirectionalStream().value()->id; auto stream2 = @@ -1025,7 +943,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobinControl) { false); // Force the wraparound initially. conn.schedulingState.nextScheduledStream = stream4 + 8; - scheduler.writeStreams(builder1); + scheduler.writeStreams(builder); EXPECT_EQ(conn.schedulingState.nextScheduledStream, stream3); EXPECT_EQ(conn.schedulingState.nextScheduledControlStream, stream2); @@ -1071,6 +989,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerOneStream) { conn.udpSendPacketLen, std::move(shortHeader), conn.ackStates.appDataAckState.largestAckedByPeer); + builder.encodePacketHeader(); auto stream1 = conn.streamManager->createNextBidirectionalStream().value(); writeDataToQuicStream(*stream1, folly::IOBuf::copyBuffer("some data"), false); scheduler.writeStreams(builder); diff --git a/quic/api/test/QuicTransportFunctionsTest.cpp b/quic/api/test/QuicTransportFunctionsTest.cpp index 0c8c54dcb..f616528bc 100644 --- a/quic/api/test/QuicTransportFunctionsTest.cpp +++ b/quic/api/test/QuicTransportFunctionsTest.cpp @@ -121,6 +121,7 @@ auto buildEmptyPacket( conn.udpSendPacketLen, std::move(*header), getAckState(conn, pnSpace).largestAckedByPeer); + builder.encodePacketHeader(); DCHECK(builder.canBuildPacket()); return std::move(builder).buildPacket(); } diff --git a/quic/codec/QuicPacketBuilder.cpp b/quic/codec/QuicPacketBuilder.cpp index d2bc0e6d0..5d8e41b19 100644 --- a/quic/codec/QuicPacketBuilder.cpp +++ b/quic/codec/QuicPacketBuilder.cpp @@ -118,13 +118,12 @@ RegularQuicPacketBuilder::RegularQuicPacketBuilder( PacketHeader header, PacketNum largestAckedPacketNum) : remainingBytes_(remainingBytes), + largestAckedPacketNum_(largestAckedPacketNum), packet_(std::move(header)), header_(folly::IOBuf::create(kLongHeaderHeaderSize)), body_(folly::IOBuf::create(kAppenderGrowthSize)), headerAppender_(header_.get(), kLongHeaderHeaderSize), - bodyAppender_(body_.get(), kAppenderGrowthSize) { - writeHeaderBytes(largestAckedPacketNum); -} + bodyAppender_(body_.get(), kAppenderGrowthSize) {} uint32_t RegularQuicPacketBuilder::getHeaderBytes() const { bool isLongHeader = packet_.header.getHeaderForm() == HeaderForm::Long; @@ -202,6 +201,7 @@ void RegularQuicPacketBuilder::appendFrame(QuicWriteFrame frame) { } RegularQuicPacketBuilder::Packet RegularQuicPacketBuilder::buildPacket() && { + CHECK(packetNumberEncoding_.hasValue()); // at this point everything should been set in the packet_ LongHeader* longHeader = packet_.header.asLong(); size_t minBodySize = kMaxPacketNumEncodingSize - @@ -228,17 +228,6 @@ RegularQuicPacketBuilder::Packet RegularQuicPacketBuilder::buildPacket() && { return Packet(std::move(packet_), std::move(header_), std::move(body_)); } -void RegularQuicPacketBuilder::writeHeaderBytes( - PacketNum largestAckedPacketNum) { - if (packet_.header.getHeaderForm() == HeaderForm::Long) { - LongHeader& longHeader = *packet_.header.asLong(); - encodeLongHeader(longHeader, largestAckedPacketNum); - } else { - ShortHeader& shortHeader = *packet_.header.asShort(); - encodeShortHeader(shortHeader, largestAckedPacketNum); - } -} - void RegularQuicPacketBuilder::encodeLongHeader( const LongHeader& longHeader, PacketNum largestAckedPacketNum) { @@ -265,7 +254,7 @@ void RegularQuicPacketBuilder::push(const uint8_t* data, size_t len) { } bool RegularQuicPacketBuilder::canBuildPacket() const noexcept { - return remainingBytes_ != 0; + return remainingBytes_ != 0 && packetNumberEncoding_.hasValue(); } const PacketHeader& RegularQuicPacketBuilder::getPacketHeader() const { @@ -370,35 +359,9 @@ InplaceQuicPacketBuilder::InplaceQuicPacketBuilder( iobuf_(bufAccessor_.obtain()), bufWriter_(*iobuf_, remainingBytes), remainingBytes_(remainingBytes), + largestAckedPacketNum_(largestAckedPacketNum), packet_(std::move(header)), - headerStart_(iobuf_->tail()) { - if (packet_.header.getHeaderForm() == HeaderForm::Long) { - LongHeader& longHeader = *packet_.header.asLong(); - packetNumberEncoding_ = encodeLongHeaderHelper( - longHeader, bufWriter_, remainingBytes, largestAckedPacketNum); - if (longHeader.getHeaderType() != LongHeader::Types::Retry) { - // Remember the position to write packet number and packet length. - packetLenOffset_ = iobuf_->length(); - // With this builder, we will have to always use kMaxPacketLenSize to - // write packet length. - packetNumOffset_ = packetLenOffset_ + kMaxPacketLenSize; - // Inside BufWriter, we already countde the packet len and packet number - // bytes as written. Note that remainingBytes_ also already counted them. - bufWriter_.append(packetNumberEncoding_->length + kMaxPacketLenSize); - } - } else { - ShortHeader& shortHeader = *packet_.header.asShort(); - packetNumberEncoding_ = encodeShortHeaderHelper( - shortHeader, bufWriter_, remainingBytes_, largestAckedPacketNum); - if (packetNumberEncoding_) { - appendBytes( - bufWriter_, - packetNumberEncoding_->result, - packetNumberEncoding_->length); - } - } - bodyStart_ = iobuf_->writableTail(); -} + headerStart_(iobuf_->tail()) {} uint32_t InplaceQuicPacketBuilder::remainingSpaceInPkt() const { return remainingBytes_; @@ -466,6 +429,7 @@ const PacketHeader& InplaceQuicPacketBuilder::getPacketHeader() const { } PacketBuilderInterface::Packet InplaceQuicPacketBuilder::buildPacket() && { + CHECK(packetNumberEncoding_.hasValue()); LongHeader* longHeader = packet_.header.asLong(); size_t minBodySize = kMaxPacketNumEncodingSize - packetNumberEncoding_->length + sizeof(Sample); @@ -527,7 +491,7 @@ void InplaceQuicPacketBuilder::push(const uint8_t* data, size_t len) { } bool InplaceQuicPacketBuilder::canBuildPacket() const noexcept { - return remainingBytes_ != 0; + return remainingBytes_ != 0 && packetNumberEncoding_.hasValue(); } uint32_t InplaceQuicPacketBuilder::getHeaderBytes() const { @@ -564,4 +528,45 @@ InplaceQuicPacketBuilder::~InplaceQuicPacketBuilder() { releaseOutputBufferInternal(); } +void RegularQuicPacketBuilder::encodePacketHeader() { + CHECK(!packetNumberEncoding_.hasValue()); + if (packet_.header.getHeaderForm() == HeaderForm::Long) { + LongHeader& longHeader = *packet_.header.asLong(); + encodeLongHeader(longHeader, largestAckedPacketNum_); + } else { + ShortHeader& shortHeader = *packet_.header.asShort(); + encodeShortHeader(shortHeader, largestAckedPacketNum_); + } +} + +void InplaceQuicPacketBuilder::encodePacketHeader() { + CHECK(!packetNumberEncoding_.hasValue()); + if (packet_.header.getHeaderForm() == HeaderForm::Long) { + LongHeader& longHeader = *packet_.header.asLong(); + packetNumberEncoding_ = encodeLongHeaderHelper( + longHeader, bufWriter_, remainingBytes_, largestAckedPacketNum_); + if (longHeader.getHeaderType() != LongHeader::Types::Retry) { + // Remember the position to write packet number and packet length. + packetLenOffset_ = iobuf_->length(); + // With this builder, we will have to always use kMaxPacketLenSize to + // write packet length. + packetNumOffset_ = packetLenOffset_ + kMaxPacketLenSize; + // Inside BufWriter, we already countde the packet len and packet number + // bytes as written. Note that remainingBytes_ also already counted them. + bufWriter_.append(packetNumberEncoding_->length + kMaxPacketLenSize); + } + } else { + ShortHeader& shortHeader = *packet_.header.asShort(); + packetNumberEncoding_ = encodeShortHeaderHelper( + shortHeader, bufWriter_, remainingBytes_, largestAckedPacketNum_); + if (packetNumberEncoding_) { + appendBytes( + bufWriter_, + packetNumberEncoding_->result, + packetNumberEncoding_->length); + } + } + bodyStart_ = iobuf_->writableTail(); +} + } // namespace quic diff --git a/quic/codec/QuicPacketBuilder.h b/quic/codec/QuicPacketBuilder.h index 8356c184e..221dfc208 100644 --- a/quic/codec/QuicPacketBuilder.h +++ b/quic/codec/QuicPacketBuilder.h @@ -60,6 +60,8 @@ class PacketBuilderInterface { FOLLY_NODISCARD virtual uint32_t remainingSpaceInPkt() const = 0; + virtual void encodePacketHeader() = 0; + // Functions to write bytes to the packet virtual void writeBE(uint8_t data) = 0; virtual void writeBE(uint16_t data) = 0; @@ -123,6 +125,8 @@ class InplaceQuicPacketBuilder final : public PacketBuilderInterface { // PacketBuilderInterface FOLLY_NODISCARD uint32_t remainingSpaceInPkt() const override; + void encodePacketHeader() override; + void writeBE(uint8_t data) override; void writeBE(uint16_t data) override; void writeBE(uint64_t data) override; @@ -161,6 +165,7 @@ class InplaceQuicPacketBuilder final : public PacketBuilderInterface { Buf iobuf_; BufWriter bufWriter_; uint32_t remainingBytes_; + PacketNum largestAckedPacketNum_; RegularQuicWritePacket packet_; uint32_t cipherOverhead_{0}; folly::Optional packetNumberEncoding_; @@ -192,6 +197,8 @@ class RegularQuicPacketBuilder final : public PacketBuilderInterface { FOLLY_NODISCARD uint32_t getHeaderBytes() const override; + void encodePacketHeader() override; + // PacketBuilderInterface FOLLY_NODISCARD uint32_t remainingSpaceInPkt() const override; @@ -228,7 +235,6 @@ class RegularQuicPacketBuilder final : public PacketBuilderInterface { void releaseOutputBuffer() && override; private: - void writeHeaderBytes(PacketNum largestAckedPacketNum); void encodeLongHeader( const LongHeader& longHeader, PacketNum largestAckedPacketNum); @@ -238,6 +244,7 @@ class RegularQuicPacketBuilder final : public PacketBuilderInterface { private: uint32_t remainingBytes_; + PacketNum largestAckedPacketNum_; RegularQuicWritePacket packet_; std::unique_ptr header_; std::unique_ptr body_; @@ -312,6 +319,11 @@ class PacketBuilderWrapper : public PacketBuilderInterface { : 0; } + void encodePacketHeader() override { + CHECK(false) + << "We only support wrapping builder that has already encoded header"; + } + void write(const QuicInteger& quicInteger) override { builder.write(quicInteger); } diff --git a/quic/codec/test/Mocks.h b/quic/codec/test/Mocks.h index 4a759c354..911207f73 100644 --- a/quic/codec/test/Mocks.h +++ b/quic/codec/test/Mocks.h @@ -78,6 +78,7 @@ class MockQuicPacketBuilder : public PacketBuilderInterface { GMOCK_METHOD0_(, const, , getHeaderBytes, uint32_t()); GMOCK_METHOD0_(, const, , hasFramesPending, bool()); MOCK_METHOD0(releaseOutputBufferMock, void()); + MOCK_METHOD0(encodePacketHeader, void()); void releaseOutputBuffer() && override { releaseOutputBufferMock(); diff --git a/quic/codec/test/QuicHeaderCodecTest.cpp b/quic/codec/test/QuicHeaderCodecTest.cpp index 37ace54ef..f7e2f7ad8 100644 --- a/quic/codec/test/QuicHeaderCodecTest.cpp +++ b/quic/codec/test/QuicHeaderCodecTest.cpp @@ -52,6 +52,7 @@ TEST_F(QuicHeaderCodecTest, ShortHeaderTest) { ShortHeader( ProtectionType::KeyPhaseZero, getTestConnectionId(), packetNum), 0 /* largestAcked */); + builder.encodePacketHeader(); auto packet = std::move(builder).buildPacket(); auto result = parseHeader(*packet.header); auto& header = result->parsedHeader; diff --git a/quic/codec/test/QuicPacketBuilderTest.cpp b/quic/codec/test/QuicPacketBuilderTest.cpp index 5dac4e6ad..34c65a272 100644 --- a/quic/codec/test/QuicPacketBuilderTest.cpp +++ b/quic/codec/test/QuicPacketBuilderTest.cpp @@ -243,6 +243,7 @@ TEST_P(QuicPacketBuilderTest, ShortHeaderRegularPacket) { ShortHeader(ProtectionType::KeyPhaseZero, connId, pktNum), largestAckedPacketNum, 2000); + builder->encodePacketHeader(); // write out at least one frame writeFrame(PaddingFrame(), *builder); @@ -288,6 +289,7 @@ TEST_P(QuicPacketBuilderTest, ShortHeaderWithNoFrames) { ShortHeader(ProtectionType::KeyPhaseZero, connId, pktNum), 0 /*largestAckedPacketNum*/, kDefaultUDPSendPacketLen); + builder->encodePacketHeader(); EXPECT_TRUE(builder->canBuildPacket()); auto builtOut = std::move(*builder).buildPacket(); @@ -319,6 +321,7 @@ TEST_P(QuicPacketBuilderTest, TestPaddingAccountsForCipherOverhead) { ShortHeader(ProtectionType::KeyPhaseZero, connId, pktNum), largestAckedPacketNum, kDefaultUDPSendPacketLen); + builder->encodePacketHeader(); builder->setCipherOverhead(cipherOverhead); EXPECT_TRUE(builder->canBuildPacket()); writeFrame(PaddingFrame(), *builder); @@ -345,6 +348,7 @@ TEST_P(QuicPacketBuilderTest, TestPaddingRespectsRemainingBytes) { ShortHeader(ProtectionType::KeyPhaseZero, connId, pktNum), largestAckedPacketNum, 2000); + builder->encodePacketHeader(); EXPECT_TRUE(builder->canBuildPacket()); writeFrame(PaddingFrame(), *builder); auto builtOut = std::move(*builder).buildPacket(); @@ -385,6 +389,7 @@ TEST_P(QuicPacketBuilderTest, LongHeaderBytesCounting) { std::move(header), largestAcked, kDefaultUDPSendPacketLen); + builder->encodePacketHeader(); auto expectedWrittenHeaderFieldLen = sizeof(uint8_t) + sizeof(QuicVersionType) + sizeof(uint8_t) + clientCid.size() + sizeof(uint8_t) + serverCid.size(); @@ -407,6 +412,7 @@ TEST_P(QuicPacketBuilderTest, ShortHeaderBytesCounting) { ShortHeader(ProtectionType::KeyPhaseZero, cid, pktNum), largestAcked, 2000); + builder->encodePacketHeader(); auto headerBytes = builder->getHeaderBytes(); writeFrame(PaddingFrame(), *builder); EXPECT_EQ( @@ -435,6 +441,7 @@ TEST_P(QuicPacketBuilderTest, InplaceBuilderReleaseBufferInBuild) { 1000, ShortHeader(ProtectionType::KeyPhaseZero, getTestConnectionId(), 0), 0); + builder->encodePacketHeader(); EXPECT_FALSE(bufAccessor.ownsBuffer()); writeFrame(PaddingFrame(), *builder); std::move(*builder).buildPacket(); @@ -449,6 +456,7 @@ TEST_P(QuicPacketBuilderTest, BuildTwoInplaces) { 1000, ShortHeader(ProtectionType::KeyPhaseZero, getTestConnectionId(), 0), 0); + builder1->encodePacketHeader(); auto headerBytes = builder1->getHeaderBytes(); for (size_t i = 0; i < 20; i++) { writeFrame(PaddingFrame(), *builder1); @@ -465,6 +473,7 @@ TEST_P(QuicPacketBuilderTest, BuildTwoInplaces) { 1000, ShortHeader(ProtectionType::KeyPhaseZero, getTestConnectionId(), 0), 0); + builder2->encodePacketHeader(); EXPECT_EQ(headerBytes, builder2->getHeaderBytes()); for (size_t i = 0; i < 40; i++) { writeFrame(PaddingFrame(), *builder2); diff --git a/quic/codec/test/QuicPacketRebuilderTest.cpp b/quic/codec/test/QuicPacketRebuilderTest.cpp index faa435dac..2447703d8 100644 --- a/quic/codec/test/QuicPacketRebuilderTest.cpp +++ b/quic/codec/test/QuicPacketRebuilderTest.cpp @@ -38,6 +38,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildEmpty) { kDefaultUDPSendPacketLen, ShortHeader(ProtectionType::KeyPhaseZero, getTestConnectionId(), 0), 0 /* largestAcked */); + regularBuilder.encodePacketHeader(); QuicConnectionStateBase conn(QuicNodeType::Client); PacketRebuilder rebuilder(regularBuilder, conn); auto packet = std::move(regularBuilder).buildPacket(); @@ -51,7 +52,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildPacket) { ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); RegularQuicPacketBuilder regularBuilder1( kDefaultUDPSendPacketLen, std::move(shortHeader1), 0 /* largestAcked */); - + regularBuilder1.encodePacketHeader(); // Get a bunch frames ConnectionCloseFrame connCloseFrame( QuicErrorCode(TransportErrorCode::FRAME_ENCODING_ERROR), @@ -109,6 +110,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildPacket) { ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); RegularQuicPacketBuilder regularBuilder2( kDefaultUDPSendPacketLen, std::move(shortHeader2), 0 /* largestAcked */); + regularBuilder2.encodePacketHeader(); PacketRebuilder rebuilder(regularBuilder2, conn); auto outstanding = makeDummyOutstandingPacket(packet1.packet, 1000); EXPECT_TRUE(rebuilder.rebuildFromPacket(outstanding).has_value()); @@ -198,6 +200,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildAfterResetStream) { ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); RegularQuicPacketBuilder regularBuilder1( kDefaultUDPSendPacketLen, std::move(shortHeader1), 0 /* largestAcked */); + regularBuilder1.encodePacketHeader(); QuicServerConnectionState conn; conn.streamManager->setMaxLocalBidirectionalStreams(10); auto stream = conn.streamManager->createNextBidirectionalStream().value(); @@ -221,6 +224,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildAfterResetStream) { ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); RegularQuicPacketBuilder regularBuilder2( kDefaultUDPSendPacketLen, std::move(shortHeader2), 0 /* largestAcked */); + regularBuilder2.encodePacketHeader(); PacketRebuilder rebuilder(regularBuilder2, conn); auto outstanding = makeDummyOutstandingPacket(packet1.packet, 1000); EXPECT_FALSE(rebuilder.rebuildFromPacket(outstanding).has_value()); @@ -231,6 +235,7 @@ TEST_F(QuicPacketRebuilderTest, FinOnlyStreamRebuild) { ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); RegularQuicPacketBuilder regularBuilder1( kDefaultUDPSendPacketLen, std::move(shortHeader1), 0 /* largestAcked */); + regularBuilder1.encodePacketHeader(); QuicServerConnectionState conn; conn.streamManager->setMaxLocalBidirectionalStreams(10); auto stream = conn.streamManager->createNextBidirectionalStream().value(); @@ -249,6 +254,7 @@ TEST_F(QuicPacketRebuilderTest, FinOnlyStreamRebuild) { ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); RegularQuicPacketBuilder regularBuilder2( kDefaultUDPSendPacketLen, std::move(shortHeader2), 0 /* largestAcked */); + regularBuilder2.encodePacketHeader(); PacketRebuilder rebuilder(regularBuilder2, conn); auto outstanding = makeDummyOutstandingPacket(packet1.packet, 2000); EXPECT_TRUE(rebuilder.rebuildFromPacket(outstanding).has_value()); @@ -271,7 +277,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildDataStreamAndEmptyCryptoStream) { ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); RegularQuicPacketBuilder regularBuilder1( kDefaultUDPSendPacketLen, std::move(shortHeader1), 0 /* largestAcked */); - + regularBuilder1.encodePacketHeader(); // Get a bunch frames QuicServerConnectionState conn; conn.streamManager->setMaxLocalBidirectionalStreams(10); @@ -308,6 +314,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildDataStreamAndEmptyCryptoStream) { ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); RegularQuicPacketBuilder regularBuilder2( kDefaultUDPSendPacketLen, std::move(shortHeader2), 0 /* largestAcked */); + regularBuilder2.encodePacketHeader(); PacketRebuilder rebuilder(regularBuilder2, conn); auto outstanding = makeDummyOutstandingPacket(packet1.packet, 1000); EXPECT_TRUE(rebuilder.rebuildFromPacket(outstanding).has_value()); @@ -332,7 +339,7 @@ TEST_F(QuicPacketRebuilderTest, CannotRebuildEmptyCryptoStream) { ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); RegularQuicPacketBuilder regularBuilder1( kDefaultUDPSendPacketLen, std::move(shortHeader1), 0 /* largestAcked */); - + regularBuilder1.encodePacketHeader(); // Get a bunch frames QuicServerConnectionState conn; uint64_t cryptoOffset = 0; @@ -350,6 +357,7 @@ TEST_F(QuicPacketRebuilderTest, CannotRebuildEmptyCryptoStream) { ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); RegularQuicPacketBuilder regularBuilder2( kDefaultUDPSendPacketLen, std::move(shortHeader2), 0 /* largestAcked */); + regularBuilder2.encodePacketHeader(); PacketRebuilder rebuilder(regularBuilder2, conn); auto outstanding = makeDummyOutstandingPacket(packet1.packet, 1000); EXPECT_FALSE(rebuilder.rebuildFromPacket(outstanding).has_value()); @@ -360,7 +368,7 @@ TEST_F(QuicPacketRebuilderTest, CannotRebuild) { ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); RegularQuicPacketBuilder regularBuilder1( kDefaultUDPSendPacketLen, std::move(shortHeader1), 0 /* largestAcked */); - + regularBuilder1.encodePacketHeader(); // Get a bunch frames ConnectionCloseFrame connCloseFrame( QuicErrorCode(TransportErrorCode::FRAME_ENCODING_ERROR), @@ -409,6 +417,7 @@ TEST_F(QuicPacketRebuilderTest, CannotRebuild) { 2, std::move(shortHeader2), 0 /* largestAcked */); + regularBuilder2.encodePacketHeader(); PacketRebuilder rebuilder(regularBuilder2, conn); auto outstanding = makeDummyOutstandingPacket(packet1.packet, 1000); EXPECT_FALSE(rebuilder.rebuildFromPacket(outstanding).has_value()); @@ -419,6 +428,7 @@ TEST_F(QuicPacketRebuilderTest, CloneCounter) { ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); RegularQuicPacketBuilder regularBuilder( kDefaultUDPSendPacketLen, std::move(shortHeader1), 0 /* largestAcked */); + regularBuilder.encodePacketHeader(); PingFrame pingFrame; writeFrame(QuicSimpleFrame(pingFrame), regularBuilder); auto packet = std::move(regularBuilder).buildPacket(); @@ -428,6 +438,7 @@ TEST_F(QuicPacketRebuilderTest, CloneCounter) { ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); RegularQuicPacketBuilder regularBuilder2( kDefaultUDPSendPacketLen, std::move(shortHeader2), 0 /* largestAcked */); + regularBuilder2.encodePacketHeader(); PacketRebuilder rebuilder(regularBuilder2, conn); rebuilder.rebuildFromPacket(outstandingPacket); EXPECT_TRUE(outstandingPacket.associatedEvent.has_value()); diff --git a/quic/codec/test/QuicReadCodecTest.cpp b/quic/codec/test/QuicReadCodecTest.cpp index 586c00399..ea0e42b0c 100644 --- a/quic/codec/test/QuicReadCodecTest.cpp +++ b/quic/codec/test/QuicReadCodecTest.cpp @@ -143,6 +143,7 @@ TEST_F(QuicReadCodecTest, LongHeaderPacketLenMismatch) { RegularQuicPacketBuilder builder( kDefaultUDPSendPacketLen, std::move(headerIn), 0 /* largestAcked */); + builder.encodePacketHeader(); builder.setCipherOverhead(0); writeCryptoFrame(0, folly::IOBuf::copyBuffer("CHLO"), builder); auto packet = packetToBuf(std::move(builder).buildPacket()); @@ -222,6 +223,7 @@ TEST_F(QuicReadCodecTest, StreamWithShortHeaderOnlyHeader) { ShortHeader header(ProtectionType::KeyPhaseZero, connId, packetNum); RegularQuicPacketBuilder builder( kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); auto packetBuf = packetToBuf(std::move(builder).buildPacket()); auto aead = std::make_unique(); diff --git a/quic/codec/test/TypesTest.cpp b/quic/codec/test/TypesTest.cpp index 4b06402b0..772adec0a 100644 --- a/quic/codec/test/TypesTest.cpp +++ b/quic/codec/test/TypesTest.cpp @@ -26,6 +26,7 @@ std::pair encodeShortHeader(const ShortHeader& header) { ShortHeader headerCopy = header; RegularQuicPacketBuilder builder( kDefaultUDPSendPacketLen, std::move(headerCopy), 0 /* largestAcked */); + builder.encodePacketHeader(); auto packet = std::move(builder).buildPacket(); Buf out; folly::io::Cursor cursor(packet.header.get()); @@ -66,6 +67,7 @@ folly::Expected makeLongHeader( packetType == LongHeader::Types::Retry ? std::move(headerRetry) : std::move(headerRegular), 0 /* largestAcked */); + builder.encodePacketHeader(); auto packet = packetToBuf(std::move(builder).buildPacket()); folly::io::Cursor cursor(packet.get()); uint8_t initialByte = cursor.readBE(); diff --git a/quic/common/test/TestUtils.cpp b/quic/common/test/TestUtils.cpp index 3b06726d2..079a7ac7f 100644 --- a/quic/common/test/TestUtils.cpp +++ b/quic/common/test/TestUtils.cpp @@ -120,6 +120,7 @@ RegularQuicPacketBuilder::Packet createAckPacket( dstConn.udpSendPacketLen, std::move(*header), getAckState(dstConn, pnSpace).largestAckedByPeer); + builder.encodePacketHeader(); if (aead) { builder.setCipherOverhead(aead->getCipherOverhead()); } @@ -328,6 +329,7 @@ RegularQuicPacketBuilder::Packet createStreamPacket( builder.reset(new RegularQuicPacketBuilder( packetSizeLimit, std::move(header), largestAcked)); } + builder->encodePacketHeader(); builder->setCipherOverhead(cipherOverhead); writeStreamFrameHeader( *builder, @@ -362,6 +364,7 @@ RegularQuicPacketBuilder::Packet createInitialCryptoPacket( if (!builder) { builder = &fallbackBuilder; } + builder->encodePacketHeader(); builder->setCipherOverhead(aead.getCipherOverhead()); writeCryptoFrame(offset, data.clone(), *builder); return std::move(*builder).buildPacket(); @@ -403,6 +406,7 @@ RegularQuicPacketBuilder::Packet createCryptoPacket( } RegularQuicPacketBuilder builder( packetSizeLimit, std::move(*header), largestAcked); + builder.encodePacketHeader(); builder.setCipherOverhead(aead.getCipherOverhead()); writeCryptoFrame(offset, data.clone(), builder); return std::move(builder).buildPacket(); diff --git a/quic/fizz/client/test/QuicClientTransportTest.cpp b/quic/fizz/client/test/QuicClientTransportTest.cpp index 381c83679..bf2be9561 100644 --- a/quic/fizz/client/test/QuicClientTransportTest.cpp +++ b/quic/fizz/client/test/QuicClientTransportTest.cpp @@ -3127,6 +3127,7 @@ TEST_F(QuicClientTransportAfterStartTest, RecvNewConnectionIdValid) { ShortHeader header(ProtectionType::KeyPhaseZero, *conn.clientConnectionId, 1); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); auto token = StatelessResetToken{1, 9, 2, 0}; @@ -3153,6 +3154,7 @@ TEST_F( ShortHeader header(ProtectionType::KeyPhaseZero, *conn.clientConnectionId, 1); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); NewConnectionIdFrame newConnId( 1, 0, ConnectionId({2, 4, 2, 3}), StatelessResetToken()); @@ -3173,6 +3175,7 @@ TEST_F(QuicClientTransportAfterStartTest, RecvNewConnectionIdInvalidRetire) { ShortHeader header(ProtectionType::KeyPhaseZero, *conn.clientConnectionId, 1); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); NewConnectionIdFrame newConnId( 1, 3, ConnectionId({2, 4, 2, 3}), StatelessResetToken()); @@ -3196,6 +3199,7 @@ TEST_F(QuicClientTransportAfterStartTest, RecvNewConnectionIdUsing0LenCid) { ShortHeader header(ProtectionType::KeyPhaseZero, *conn.clientConnectionId, 1); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); NewConnectionIdFrame newConnId( 1, 0, ConnectionId({2, 4, 2, 3}), StatelessResetToken()); @@ -3226,6 +3230,7 @@ TEST_F( ShortHeader header(ProtectionType::KeyPhaseZero, *conn.clientConnectionId, 1); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); NewConnectionIdFrame newConnId(1, 0, connId2, StatelessResetToken()); writeSimpleFrame(QuicSimpleFrame(newConnId), builder); @@ -3250,6 +3255,7 @@ TEST_F( ShortHeader header(ProtectionType::KeyPhaseZero, *conn.clientConnectionId, 1); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); NewConnectionIdFrame newConnId(2, 0, connId2, StatelessResetToken()); writeSimpleFrame(QuicSimpleFrame(newConnId), builder); @@ -3377,6 +3383,7 @@ TEST_F(QuicClientTransportAfterStartTest, RecvPathChallengeNoAvailablePeerIds) { ShortHeader header(ProtectionType::KeyPhaseZero, *conn.clientConnectionId, 1); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); PathChallengeFrame pathChallenge(123); ASSERT_TRUE(builder.canBuildPacket()); writeSimpleFrame(QuicSimpleFrame(pathChallenge), builder); @@ -3403,6 +3410,7 @@ TEST_F(QuicClientTransportAfterStartTest, RecvPathChallengeAvailablePeerId) { ShortHeader header(ProtectionType::KeyPhaseZero, *conn.clientConnectionId, 1); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); PathChallengeFrame pathChallenge(123); ASSERT_TRUE(builder.canBuildPacket()); writeSimpleFrame(QuicSimpleFrame(pathChallenge), builder); @@ -4048,6 +4056,7 @@ TEST_F( client->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); writeFrame(rstFrame, builder); auto packet = packetToBuf(std::move(builder).buildPacket()); @@ -4068,6 +4077,7 @@ TEST_F( client->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); writeFrame(rstFrame, builder); auto packet = packetToBuf(std::move(builder).buildPacket()); deliverData(packet->coalesce()); @@ -4081,6 +4091,7 @@ TEST_F( client->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder2.encodePacketHeader(); writeFrame(rstFrame, builder2); auto data = folly::IOBuf::copyBuffer("hello"); @@ -4133,6 +4144,7 @@ TEST_F(QuicClientTransportAfterStartTest, ReceiveRstStreamAfterEom) { ProtectionType::KeyPhaseZero, *originalConnId, appDataPacketNum++); RegularQuicPacketBuilder builder( client->getConn().udpSendPacketLen, std::move(header), 0); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); writeFrame(rstFrame, builder); auto packet2 = packetToBuf(std::move(builder).buildPacket()); @@ -4542,6 +4554,7 @@ TEST_F(QuicClientTransportVersionAndRetryTest, UnencryptedAckData) { version); RegularQuicPacketBuilder builder( kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); DCHECK(builder.canBuildPacket()); AckFrameMetaData ackData(acks, 0us, 0); writeAckFrame(ackData, builder); @@ -4563,6 +4576,7 @@ TEST_F(QuicClientTransportVersionAndRetryTest, UnencryptedPing) { version); RegularQuicPacketBuilder builder( kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); DCHECK(builder.canBuildPacket()); writeFrame(QuicWriteFrame(PingFrame()), builder); auto packet = packetToBufCleartext( @@ -4590,6 +4604,7 @@ Buf getHandshakePacketWithFrame( kDefaultUDPSendPacketLen, std::move(header), packetNum / 2 /* largestAcked */); + builder.encodePacketHeader(); builder.setCipherOverhead(serverWriteCipher.getCipherOverhead()); writeFrame(std::move(frame), builder); return packetToBufCleartext( @@ -4905,6 +4920,7 @@ TEST_F(QuicClientTransportAfterStartTest, ReceiveConnectionClose) { ProtectionType::KeyPhaseZero, *originalConnId, appDataPacketNum++); RegularQuicPacketBuilder builder( client->getConn().udpSendPacketLen, std::move(header), 0); + builder.encodePacketHeader(); ConnectionCloseFrame connClose( QuicErrorCode(TransportErrorCode::NO_ERROR), "Stand clear of the closing doors, please"); @@ -4931,6 +4947,7 @@ TEST_F(QuicClientTransportAfterStartTest, ReceiveApplicationClose) { ProtectionType::KeyPhaseZero, *originalConnId, appDataPacketNum++); RegularQuicPacketBuilder builder( client->getConn().udpSendPacketLen, std::move(header), 0); + builder.encodePacketHeader(); ConnectionCloseFrame appClose( QuicErrorCode(GenericApplicationErrorCode::UNKNOWN), "Stand clear of the closing doors, please"); @@ -4969,6 +4986,7 @@ TEST_F(QuicClientTransportAfterStartTest, ReceiveApplicationCloseNoError) { ProtectionType::KeyPhaseZero, *originalConnId, appDataPacketNum++); RegularQuicPacketBuilder builder( client->getConn().udpSendPacketLen, std::move(header), 0); + builder.encodePacketHeader(); ConnectionCloseFrame appClose( QuicErrorCode(GenericApplicationErrorCode::NO_ERROR), "No Error"); writeFrame(std::move(appClose), builder); @@ -5129,6 +5147,7 @@ TEST_F(QuicClientTransportAfterStartTest, PingIsRetransmittable) { client->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); writeFrame(QuicSimpleFrame(pingFrame), builder); auto packet = packetToBuf(std::move(builder).buildPacket()); deliverData(packet->coalesce()); diff --git a/quic/loss/test/QuicLossFunctionsTest.cpp b/quic/loss/test/QuicLossFunctionsTest.cpp index 48a2ce588..2f4f69767 100644 --- a/quic/loss/test/QuicLossFunctionsTest.cpp +++ b/quic/loss/test/QuicLossFunctionsTest.cpp @@ -173,6 +173,7 @@ PacketNum QuicLossFunctionsTest::sendPacket( conn.udpSendPacketLen, std::move(*header), getAckState(conn, packetNumberSpace).largestAckedByPeer); + builder.encodePacketHeader(); EXPECT_TRUE(builder.canBuildPacket()); auto packet = std::move(builder).buildPacket(); uint32_t encodedSize = 0; diff --git a/quic/server/test/QuicServerTest.cpp b/quic/server/test/QuicServerTest.cpp index e77c327ad..373fd754d 100644 --- a/quic/server/test/QuicServerTest.cpp +++ b/quic/server/test/QuicServerTest.cpp @@ -627,6 +627,7 @@ TEST_F(QuicServerWorkerTest, ZeroLengthConnectionId) { RegularQuicPacketBuilder builder( kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); auto packet = packetToBuf(std::move(builder).buildPacket()); worker_->handleNetworkData(kClientAddr, std::move(packet), Clock::now()); eventbase_.loop(); @@ -641,6 +642,7 @@ TEST_F(QuicServerWorkerTest, ClientInitialCounting) { LongHeader::Types::Initial, srcConnId, destConnId, num, version); RegularQuicPacketBuilder initialBuilder( kDefaultUDPSendPacketLen, std::move(initialHeader), 0); + initialBuilder.encodePacketHeader(); auto initialPacket = packetToBuf(std::move(initialBuilder).buildPacket()); EXPECT_CALL(*transportInfoCb_, onClientInitialReceived()).Times(1); worker_->handleNetworkData( @@ -653,6 +655,7 @@ TEST_F(QuicServerWorkerTest, ClientInitialCounting) { LongHeader::Types::Initial, srcConnId, destConnId, bignum, version); RegularQuicPacketBuilder initialBuilderBigNum( kDefaultUDPSendPacketLen, std::move(initialHeaderBigNum), 0); + initialBuilderBigNum.encodePacketHeader(); auto initialPacketBigNum = packetToBuf(std::move(initialBuilderBigNum).buildPacket()); EXPECT_CALL(*transportInfoCb_, onClientInitialReceived()).Times(1); @@ -664,6 +667,7 @@ TEST_F(QuicServerWorkerTest, ClientInitialCounting) { LongHeader::Types::Handshake, srcConnId, destConnId, num, version); RegularQuicPacketBuilder handshakeBuilder( kDefaultUDPSendPacketLen, std::move(handshakeHeader), 0); + handshakeBuilder.encodePacketHeader(); auto handshakePacket = packetToBuf(std::move(handshakeBuilder).buildPacket()); EXPECT_CALL(*transportInfoCb_, onClientInitialReceived()).Times(0); worker_->handleNetworkData( @@ -684,6 +688,7 @@ TEST_F(QuicServerWorkerTest, ConnectionIdTooShort) { RegularQuicPacketBuilder builder( kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); auto packet = packetToBuf(std::move(builder).buildPacket()); worker_->handleNetworkData(kClientAddr, std::move(packet), Clock::now()); eventbase_.loop(); @@ -703,6 +708,7 @@ TEST_F(QuicServerWorkerTest, FailToParseConnectionId) { LongHeader::Types::Initial, srcConnId, dstConnId, num, version); RegularQuicPacketBuilder builder( kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); while (builder.remainingSpaceInPkt() > 0) { writeFrame(PaddingFrame(), builder); } @@ -744,6 +750,7 @@ TEST_F(QuicServerWorkerTest, ConnectionIdTooShortDispatch) { RegularQuicPacketBuilder builder( kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); while (builder.remainingSpaceInPkt() > 0) { writeFrame(PaddingFrame(), builder); } @@ -770,6 +777,7 @@ TEST_F(QuicServerWorkerTest, ConnectionIdTooLargeDispatch) { RegularQuicPacketBuilder builder( kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); while (builder.remainingSpaceInPkt() > 0) { writeFrame(PaddingFrame(), builder); } @@ -814,6 +822,7 @@ TEST_F(QuicServerWorkerTest, PacketAfterShutdown) { RegularQuicPacketBuilder builder( kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); auto packet = packetToBuf(std::move(builder).buildPacket()); worker_->handleNetworkData(kClientAddr, std::move(packet), Clock::now()); eventbase_.terminateLoopSoon(); @@ -880,6 +889,7 @@ auto createInitialStream( pktHeaderType == LongHeader::Types::Retry ? std::move(headerRetry) : std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); auto streamData = data.clone(); auto dataLen = writeStreamFrameHeader( builder, diff --git a/quic/server/test/QuicServerTransportTest.cpp b/quic/server/test/QuicServerTransportTest.cpp index 8d57d186d..4331d375e 100644 --- a/quic/server/test/QuicServerTransportTest.cpp +++ b/quic/server/test/QuicServerTransportTest.cpp @@ -707,6 +707,7 @@ TEST_F(QuicServerTransportTest, TestReadMultipleStreams) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); auto buf1 = IOBuf::copyBuffer("Aloha"); @@ -1001,6 +1002,7 @@ TEST_F(QuicServerTransportTest, ReceivePacketAfterLocalError) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); // Deliver a reset to non existent stream to trigger a local conn error @@ -1023,6 +1025,7 @@ TEST_F(QuicServerTransportTest, ReceivePacketAfterLocalError) { server->getConn().udpSendPacketLen, std::move(header2), 0 /* largestAcked */); + builder2.encodePacketHeader(); RstStreamFrame rstFrame2(streamId, GenericApplicationErrorCode::UNKNOWN, 0); writeFrame(std::move(rstFrame2), builder2); auto packet2 = std::move(builder2).buildPacket(); @@ -1046,6 +1049,7 @@ TEST_F(QuicServerTransportTest, ReceiveCloseAfterLocalError) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); // Deliver a reset to non existent stream to trigger a local conn error @@ -1072,6 +1076,7 @@ TEST_F(QuicServerTransportTest, ReceiveCloseAfterLocalError) { server->getConn().udpSendPacketLen, std::move(header2), 0 /* largestAcked */); + builder2.encodePacketHeader(); std::string errMsg = "Mind the gap"; ConnectionCloseFrame connClose( QuicErrorCode(TransportErrorCode::NO_ERROR), errMsg); @@ -1112,6 +1117,7 @@ TEST_F(QuicServerTransportTest, NoDataExceptCloseProcessedAfterClosing) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); auto buf = folly::IOBuf::copyBuffer("hello"); @@ -1281,6 +1287,7 @@ TEST_F(QuicServerTransportTest, RecvRstStreamFrameNonexistClientStream) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); RstStreamFrame rstFrame(streamId, GenericApplicationErrorCode::UNKNOWN, 0); @@ -1306,6 +1313,7 @@ TEST_F(QuicServerTransportTest, ReceiveRstStreamNonExistentAndOtherFrame) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); writeFrame(rstFrame, builder); auto packet = packetToBuf(std::move(builder).buildPacket()); deliverData(std::move(packet)); @@ -1321,6 +1329,7 @@ TEST_F(QuicServerTransportTest, ReceiveRstStreamNonExistentAndOtherFrame) { server->getConn().udpSendPacketLen, std::move(header2), 0 /* largestAcked */); + builder2.encodePacketHeader(); writeFrame(rstFrame, builder2); auto data = folly::IOBuf::copyBuffer("hello"); @@ -1351,6 +1360,7 @@ TEST_F(QuicServerTransportTest, RecvRstStreamFrameNonexistServerStream) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); StreamId streamId = 0x01; @@ -1392,6 +1402,7 @@ TEST_F(QuicServerTransportTest, RecvRstStreamFrame) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); RstStreamFrame rstFrame( streamId, @@ -1449,7 +1460,7 @@ TEST_F(QuicServerTransportTest, RecvStopSendingFrame) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); - + builder.encodePacketHeader(); StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); ASSERT_TRUE(builder.canBuildPacket()); @@ -1494,6 +1505,7 @@ TEST_F(QuicServerTransportTest, RecvStopSendingFrameAfterCloseStream) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); @@ -1538,6 +1550,7 @@ TEST_F(QuicServerTransportTest, RecvInvalidMaxStreamData) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); MaxStreamDataFrame maxStreamDataFrame(streamId, 100); ASSERT_TRUE(builder.canBuildPacket()); @@ -1578,6 +1591,7 @@ TEST_F(QuicServerTransportTest, RecvStopSendingFrameAfterHalfCloseRemote) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); @@ -1605,6 +1619,7 @@ TEST_F(QuicServerTransportTest, RecvStopSendingBeforeStream) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); @@ -1663,6 +1678,7 @@ TEST_F(QuicServerTransportTest, RecvStopSendingFrameAfterReset) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); StopSendingFrame stopSendingFrame1( streamId1, GenericApplicationErrorCode::UNKNOWN); @@ -1692,6 +1708,7 @@ TEST_F(QuicServerTransportTest, StopSendingLoss) { server->getConn().udpSendPacketLen, std::move(header), server->getConn().ackStates.appDataAckState.largestAckedByPeer); + builder.encodePacketHeader(); StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); ASSERT_TRUE(builder.canBuildPacket()); @@ -1722,6 +1739,7 @@ TEST_F(QuicServerTransportTest, StopSendingLossAfterStreamClosed) { server->getConn().udpSendPacketLen, std::move(header), server->getConn().ackStates.appDataAckState.largestAckedByPeer); + builder.encodePacketHeader(); StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); ASSERT_TRUE(builder.canBuildPacket()); @@ -1810,6 +1828,7 @@ TEST_F(QuicServerTransportTest, RecvPathChallenge) { ProtectionType::KeyPhaseZero, *conn.serverConnectionId, 10); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); PathChallengeFrame pathChallenge(123); ASSERT_TRUE(builder.canBuildPacket()); writeSimpleFrame(QuicSimpleFrame(pathChallenge), builder); @@ -1860,6 +1879,7 @@ TEST_F(QuicServerTransportTest, ReceiveConnectionClose) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); std::string errMsg = "Stand clear of the closing doors, please"; ConnectionCloseFrame connClose( QuicErrorCode(TransportErrorCode::NO_ERROR), errMsg); @@ -1897,6 +1917,7 @@ TEST_F(QuicServerTransportTest, ReceiveApplicationClose) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); std::string errMsg = "Stand clear of the closing doors, please"; ConnectionCloseFrame appClose( @@ -1937,6 +1958,7 @@ TEST_F(QuicServerTransportTest, ReceiveConnectionCloseTwice) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); std::string errMsg = "Mind the gap"; ConnectionCloseFrame connClose( QuicErrorCode(TransportErrorCode::NO_ERROR), errMsg); @@ -2168,6 +2190,7 @@ TEST_P( server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); writeSimpleFrame(PathChallengeFrame(123), builder); @@ -2295,6 +2318,7 @@ TEST_P(QuicServerTransportAllowMigrationTest, MigrateToUnvalidatedPeer) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); writeSimpleFrame( @@ -2359,6 +2383,7 @@ TEST_P(QuicServerTransportAllowMigrationTest, ResetPathRttPathResponse) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); writeSimpleFrame( @@ -2425,6 +2450,7 @@ TEST_P(QuicServerTransportAllowMigrationTest, IgnoreInvalidPathResponse) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); writeSimpleFrame( @@ -2481,6 +2507,7 @@ TEST_P( server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); writeSimpleFrame( PathResponseFrame(server->getConn().outstandingPathValidation->pathData), @@ -3124,6 +3151,7 @@ TEST_F(QuicServerTransportTest, PingIsRetransmittable) { server->getConn().udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); writeFrame(QuicSimpleFrame(pingFrame), builder); auto packet = std::move(builder).buildPacket(); deliverData(packetToBuf(packet)); @@ -3139,6 +3167,7 @@ TEST_F(QuicServerTransportTest, RecvNewConnectionIdValid) { ShortHeader header(ProtectionType::KeyPhaseZero, *conn.clientConnectionId, 1); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); NewConnectionIdFrame newConnId( 1, 0, ConnectionId({2, 4, 2, 3}), StatelessResetToken{9, 8, 7, 6}); @@ -3161,6 +3190,7 @@ TEST_F(QuicServerTransportTest, RecvNewConnectionIdTooManyReceivedIds) { ShortHeader header(ProtectionType::KeyPhaseZero, *conn.clientConnectionId, 1); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); NewConnectionIdFrame newConnId( 1, 0, ConnectionId({2, 4, 2, 3}), StatelessResetToken()); @@ -3180,6 +3210,7 @@ TEST_F(QuicServerTransportTest, RecvNewConnectionIdInvalidRetire) { ShortHeader header(ProtectionType::KeyPhaseZero, *conn.clientConnectionId, 1); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); NewConnectionIdFrame newConnId( 1, 3, ConnectionId({2, 4, 2, 3}), StatelessResetToken()); @@ -3201,6 +3232,7 @@ TEST_F(QuicServerTransportTest, RecvNewConnectionIdNoopValidDuplicate) { ShortHeader header(ProtectionType::KeyPhaseZero, *conn.clientConnectionId, 1); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); NewConnectionIdFrame newConnId(1, 0, connId2, StatelessResetToken()); writeSimpleFrame(QuicSimpleFrame(newConnId), builder); @@ -3221,6 +3253,7 @@ TEST_F(QuicServerTransportTest, RecvNewConnectionIdExceptionInvalidDuplicate) { ShortHeader header(ProtectionType::KeyPhaseZero, *conn.clientConnectionId, 1); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); ASSERT_TRUE(builder.canBuildPacket()); NewConnectionIdFrame newConnId(2, 0, connId2, StatelessResetToken()); writeSimpleFrame(QuicSimpleFrame(newConnId), builder); @@ -3295,6 +3328,7 @@ TEST_F(QuicUnencryptedServerTransportTest, TestUnencryptedAck) { QuicVersion::MVFST); RegularQuicPacketBuilder builder( kDefaultUDPSendPacketLen, std::move(header), 0 /* largestAcked */); + builder.encodePacketHeader(); DCHECK(builder.canBuildPacket()); AckFrameMetaData ackData(acks, 0us, 0); writeAckFrame(ackData, builder); @@ -3726,6 +3760,7 @@ Buf getHandshakePacketWithFrame( kDefaultUDPSendPacketLen, std::move(header), clientPacketNum / 2 /* largestAcked */); + builder.encodePacketHeader(); builder.setCipherOverhead(clientWriteCipher.getCipherOverhead()); writeFrame(std::move(frame), builder); return packetToBufCleartext(