Move Quic packet header encoding out of builder constructor

Summary:
Currently the packet builder contructor will encode the packet
builder. This is fine when the builder creates its own output buffer. If later
on we decides not to use this builder, or it fails to build packet, the buffer
will be thrown away. But once the builder uses a buffer provided by caller, and
will be reused, we can no longer just throw it away if we decide not to use
this builder. So we have to delay the header encoding until we know we will use
the builder.

This is still not enough to solve the case where we want to use this builder,
it builds, then it fails . For that, we will need to retreat the tail position
of the IOBuf.

Reviewed By: mjoras

Differential Revision: D21000658

fbshipit-source-id: 4d758b3e260463b17c870618ba68bd4b898a7d4c

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) {

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) {

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);

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();
 }

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) {
+      bodyAppender_(body_.get(), kAppenderGrowthSize) {}
-  writeHeaderBytes(largestAckedPacketNum);
-}
 
 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()) {
+      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();
-}
 
 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

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<PacketNumEncodingResult> 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<folly::IOBuf> header_;
   std::unique_ptr<folly::IOBuf> 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);
   }

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();

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;

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);

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());

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<MockAead>();

quic/codec/test/TypesTest.cpp

@@ -26,6 +26,7 @@ std::pair<uint8_t, Buf> 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<ParsedLongHeaderResult, TransportErrorCode> 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<uint8_t>();

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();

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());

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;

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,

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(