gfe-relnote: Fix appending padding in QuicPacketCreator::CreateAndSerializeStreamFrame. Protected by QUIC_VERSION_99
PiperOrigin-RevId: 251343662
Change-Id: I6d05c8d613818e4fec7d1ba51c4c61f75e47a17f
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";
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()) {
