gfe-relnote: Fix appending padding in QuicPacketCreator::CreateAndSerializeStreamFrame. Protected by QUIC_VERSION_99

PiperOrigin-RevId: 251343662
Change-Id: I6d05c8d613818e4fec7d1ba51c4c61f75e47a17f

quic/core/quic_packet_creator.cc

diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc
index 3daf302..dcc3417 100644
--- a/quic/core/quic_packet_creator.cc
+++ b/quic/core/quic_packet_creator.cc
@@ -436,14 +436,25 @@
   QUIC_BUG_IF(iov_offset == write_length && !fin)
       << "Creating a stream frame with no data or fin.";
   const size_t remaining_data_size = write_length - iov_offset;
-  const size_t min_frame_size = QuicFramer::GetMinStreamFrameSize(
+  size_t min_frame_size = QuicFramer::GetMinStreamFrameSize(
       framer_->transport_version(), id, stream_offset,
       /* last_frame_in_packet= */ true, remaining_data_size);
-  const size_t available_size =
+  size_t available_size =
       max_plaintext_size_ - writer.length() - min_frame_size;
-  const size_t bytes_consumed =
-      std::min<size_t>(available_size, remaining_data_size);
-  const size_t plaintext_bytes_written = min_frame_size + bytes_consumed;
+  size_t bytes_consumed = std::min<size_t>(available_size, remaining_data_size);
+  size_t plaintext_bytes_written = min_frame_size + bytes_consumed;
+  bool needs_padding = false;
+  if (plaintext_bytes_written < MinPlaintextPacketSize(framer_->version())) {
+    needs_padding = true;
+    // Recalculate sizes with the stream frame not being marked as the last
+    // frame in the packet.
+    min_frame_size = QuicFramer::GetMinStreamFrameSize(
+        framer_->transport_version(), id, stream_offset,
+        /* last_frame_in_packet= */ false, remaining_data_size);
+    available_size = max_plaintext_size_ - writer.length() - min_frame_size;
+    bytes_consumed = std::min<size_t>(available_size, remaining_data_size);
+    plaintext_bytes_written = min_frame_size + bytes_consumed;
+  }
 
   const bool set_fin = fin && (bytes_consumed == remaining_data_size);
   QuicStreamFrame frame(id, set_fin, stream_offset, bytes_consumed);
@@ -454,17 +465,17 @@
 
   // TODO(ianswett): AppendTypeByte and AppendStreamFrame could be optimized
   // into one method that takes a QuicStreamFrame, if warranted.
-  if (!framer_->AppendTypeByte(QuicFrame(frame),
-                               /* no stream frame length */ true, &writer)) {
+  bool omit_frame_length = !needs_padding;
+  if (!framer_->AppendTypeByte(QuicFrame(frame), omit_frame_length, &writer)) {
     QUIC_BUG << "AppendTypeByte failed";
     return;
   }
-  if (!framer_->AppendStreamFrame(frame, /* no stream frame length */ true,
-                                  &writer)) {
+  if (!framer_->AppendStreamFrame(frame, omit_frame_length, &writer)) {
     QUIC_BUG << "AppendStreamFrame failed";
     return;
   }
-  if (plaintext_bytes_written < MinPlaintextPacketSize(framer_->version()) &&
+  if (needs_padding &&
+      plaintext_bytes_written < MinPlaintextPacketSize(framer_->version()) &&
       !writer.WritePaddingBytes(MinPlaintextPacketSize(framer_->version()) -
                                 plaintext_bytes_written)) {
     QUIC_BUG << "Unable to add padding bytes";

quic/core/quic_packet_creator_test.cc

diff --git a/quic/core/quic_packet_creator_test.cc b/quic/core/quic_packet_creator_test.cc
index ba72d25..eab8670 100644
--- a/quic/core/quic_packet_creator_test.cc
+++ b/quic/core/quic_packet_creator_test.cc
@@ -1396,6 +1396,47 @@
   EXPECT_FALSE(creator_.HasPendingFrames());
 }
 
+TEST_P(QuicPacketCreatorTest, SerializeStreamFrameWithPadding) {
+  // Regression test to check that CreateAndSerializeStreamFrame uses a
+  // correctly formatted stream frame header when appending padding.
+
+  if (!GetParam().version_serialization) {
+    creator_.StopSendingVersion();
+  }
+  EXPECT_FALSE(creator_.HasPendingFrames());
+
+  // Send one byte of stream data.
+  MakeIOVector("a", &iov_);
+  producer_.SaveStreamData(
+      QuicUtils::GetHeadersStreamId(client_framer_.transport_version()), &iov_,
+      1u, 0u, iov_.iov_len);
+  EXPECT_CALL(delegate_, OnSerializedPacket(_))
+      .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket));
+  size_t num_bytes_consumed;
+  creator_.CreateAndSerializeStreamFrame(
+      QuicUtils::GetHeadersStreamId(client_framer_.transport_version()),
+      iov_.iov_len, 0, 0, true, NOT_RETRANSMISSION, &num_bytes_consumed);
+  EXPECT_EQ(1u, num_bytes_consumed);
+
+  // Check that a packet is created.
+  ASSERT_TRUE(serialized_packet_.encrypted_buffer);
+  ASSERT_FALSE(serialized_packet_.retransmittable_frames.empty());
+  {
+    InSequence s;
+    EXPECT_CALL(framer_visitor_, OnPacket());
+    EXPECT_CALL(framer_visitor_, OnUnauthenticatedPublicHeader(_));
+    EXPECT_CALL(framer_visitor_, OnUnauthenticatedHeader(_));
+    EXPECT_CALL(framer_visitor_, OnDecryptedPacket(_));
+    EXPECT_CALL(framer_visitor_, OnPacketHeader(_));
+    EXPECT_CALL(framer_visitor_, OnStreamFrame(_));
+    if (client_framer_.version().HasHeaderProtection()) {
+      EXPECT_CALL(framer_visitor_, OnPaddingFrame(_));
+    }
+    EXPECT_CALL(framer_visitor_, OnPacketComplete());
+  }
+  ProcessPacket(serialized_packet_);
+}
+
 TEST_P(QuicPacketCreatorTest, AddUnencryptedStreamDataClosesConnection) {
   // EXPECT_QUIC_BUG tests are expensive so only run one instance of them.
   if (!IsDefaultTestConfiguration()) {