Allow minimum size QUIC packets. Protected by FLAGS_quic_restart_flag_quic_allow_smaller_packets. PiperOrigin-RevId: 496688436
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h index 653029c..2b404a7 100644 --- a/quiche/quic/core/quic_flags_list.h +++ b/quiche/quic/core/quic_flags_list.h
@@ -63,6 +63,8 @@ QUIC_FLAG(quic_reloadable_flag_quic_remove_connection_migration_connection_option_v2, false) // If true, include stream information in idle timeout connection close detail. QUIC_FLAG(quic_reloadable_flag_quic_add_stream_info_to_idle_close_detail, true) +// If true, lowers the minimum packet size to that in the spec. +QUIC_FLAG(quic_restart_flag_quic_allow_smaller_packets, false) // If true, quic server will send ENABLE_CONNECT_PROTOCOL setting and and endpoint will validate required request/response headers and extended CONNECT mechanism and update code counts of valid/invalid headers. QUIC_FLAG(quic_reloadable_flag_quic_verify_request_headers_2, true) // If true, reject or send error response code upon receiving invalid request or response headers. This flag depends on --gfe2_reloadable_flag_quic_verify_request_headers_2.
diff --git a/quiche/quic/core/quic_packet_creator.cc b/quiche/quic/core/quic_packet_creator.cc index ad4e785..cfcd3e2 100644 --- a/quiche/quic/core/quic_packet_creator.cc +++ b/quiche/quic/core/quic_packet_creator.cc
@@ -19,6 +19,7 @@ #include "absl/types/optional.h" #include "quiche/quic/core/crypto/crypto_protocol.h" #include "quiche/quic/core/frames/quic_frame.h" +#include "quiche/quic/core/frames/quic_padding_frame.h" #include "quiche/quic/core/frames/quic_path_challenge_frame.h" #include "quiche/quic/core/frames/quic_stream_frame.h" #include "quiche/quic/core/quic_chaos_protector.h" @@ -167,8 +168,10 @@ max_packet_length_ = length; max_plaintext_size_ = framer_->GetMaxPlaintextSize(max_packet_length_); - QUIC_BUG_IF(quic_bug_12398_2, max_plaintext_size_ - PacketHeaderSize() < - MinPlaintextPacketSize(framer_->version())) + QUIC_BUG_IF( + quic_bug_12398_2, + max_plaintext_size_ - PacketHeaderSize() < + MinPlaintextPacketSize(framer_->version(), GetPacketNumberLength())) << ENDPOINT << "Attempted to set max packet length too small"; } @@ -197,7 +200,8 @@ return; } if (framer_->GetMaxPlaintextSize(length) < - PacketHeaderSize() + MinPlaintextPacketSize(framer_->version())) { + PacketHeaderSize() + + MinPlaintextPacketSize(framer_->version(), GetPacketNumberLength())) { // Please note: this would not guarantee to fit next packet if the size of // packet header increases (e.g., encryption level changes). QUIC_DLOG(INFO) << ENDPOINT << length @@ -611,16 +615,20 @@ 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())) { + const size_t min_plaintext_size = + MinPlaintextPacketSize(framer_->version(), GetPacketNumberLength()); + if (plaintext_bytes_written < min_plaintext_size) { 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; + if (!GetQuicRestartFlag(quic_allow_smaller_packets)) { + // 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); @@ -634,6 +642,15 @@ // TODO(ianswett): AppendTypeByte and AppendStreamFrame could be optimized // into one method that takes a QuicStreamFrame, if warranted. + if (needs_padding && GetQuicRestartFlag(quic_allow_smaller_packets)) { + QUIC_RESTART_FLAG_COUNT_N(quic_allow_smaller_packets, 2, 5); + if (!writer.WritePaddingBytes(min_plaintext_size - + plaintext_bytes_written)) { + QUIC_BUG(quic_bug_10752_12) << ENDPOINT << "Unable to add padding bytes"; + return; + } + needs_padding = false; + } bool omit_frame_length = !needs_padding; if (!framer_->AppendTypeByte(QuicFrame(frame), omit_frame_length, &writer)) { QUIC_BUG(quic_bug_10752_10) << ENDPOINT << "AppendTypeByte failed"; @@ -643,10 +660,8 @@ QUIC_BUG(quic_bug_10752_11) << ENDPOINT << "AppendStreamFrame failed"; return; } - if (needs_padding && - plaintext_bytes_written < MinPlaintextPacketSize(framer_->version()) && - !writer.WritePaddingBytes(MinPlaintextPacketSize(framer_->version()) - - plaintext_bytes_written)) { + if (needs_padding && plaintext_bytes_written < min_plaintext_size && + !writer.WritePaddingBytes(min_plaintext_size - plaintext_bytes_written)) { QUIC_BUG(quic_bug_10752_12) << ENDPOINT << "Unable to add padding bytes"; return; } @@ -737,6 +752,18 @@ std::min(max_plaintext_size_, PacketSize() + ExpansionOnNewFrame()); } +size_t QuicPacketCreator::BytesFreeForPadding() const { + size_t consumed = PacketSize(); + + if (!GetQuicRestartFlag(quic_allow_smaller_packets)) { + consumed += ExpansionOnNewFrame(); + } else { + // The next frame (which is PADDING) will be prepended to the packet. + QUIC_RESTART_FLAG_COUNT_N(quic_allow_smaller_packets, 5, 5); + } + return max_plaintext_size_ - std::min(max_plaintext_size_, consumed); +} + size_t QuicPacketCreator::PacketSize() const { return queued_frames_.empty() ? PacketHeaderSize() : packet_size_; } @@ -1680,7 +1707,8 @@ // Calculate frame bytes and bytes free with this frame added. const size_t frame_bytes = PacketSize() - PacketHeaderSize() + ExpansionOnNewFrame() + serialized_frame_length; - if (frame_bytes >= MinPlaintextPacketSize(framer_->version())) { + if (frame_bytes >= + MinPlaintextPacketSize(framer_->version(), GetPacketNumberLength())) { // No extra bytes is needed. return serialized_frame_length; } @@ -1694,7 +1722,8 @@ // padding + expansion. const size_t extra_bytes_needed = std::max( 1 + ExpansionOnNewFrameWithLastFrame(frame, framer_->transport_version()), - MinPlaintextPacketSize(framer_->version()) - frame_bytes); + MinPlaintextPacketSize(framer_->version(), GetPacketNumberLength()) - + frame_bytes); if (bytes_free < extra_bytes_needed) { // This frame does not fit. return 0; @@ -1814,13 +1843,24 @@ return; } const size_t frame_bytes = PacketSize() - PacketHeaderSize(); - if (frame_bytes >= MinPlaintextPacketSize(framer_->version())) { + if (frame_bytes >= + MinPlaintextPacketSize(framer_->version(), GetPacketNumberLength())) { return; } - const QuicByteCount min_header_protection_padding = - std::max(1 + ExpansionOnNewFrame(), - MinPlaintextPacketSize(framer_->version()) - frame_bytes) - - ExpansionOnNewFrame(); + QuicByteCount min_header_protection_padding; + if (GetQuicRestartFlag(quic_allow_smaller_packets)) { + QUIC_RESTART_FLAG_COUNT_N(quic_allow_smaller_packets, 4, 5); + min_header_protection_padding = + MinPlaintextPacketSize(framer_->version(), GetPacketNumberLength()) - + frame_bytes; + } else { + min_header_protection_padding = + std::max(1 + ExpansionOnNewFrame(), + MinPlaintextPacketSize(framer_->version(), + GetPacketNumberLength()) - + frame_bytes) - + ExpansionOnNewFrame(); + } // Update pending_padding_bytes_. pending_padding_bytes_ = std::max(pending_padding_bytes_, min_header_protection_padding); @@ -1876,7 +1916,7 @@ void QuicPacketCreator::MaybeAddPadding() { // The current packet should have no padding bytes because padding is only // added when this method is called just before the packet is serialized. - if (BytesFree() == 0) { + if (BytesFreeForPadding() == 0) { // Don't pad full packets. return; } @@ -1903,15 +1943,36 @@ int padding_bytes = -1; if (!needs_full_padding_) { - padding_bytes = std::min<int16_t>(pending_padding_bytes_, BytesFree()); + padding_bytes = + std::min<int16_t>(pending_padding_bytes_, BytesFreeForPadding()); pending_padding_bytes_ -= padding_bytes; } - bool success = AddFrame(QuicFrame(QuicPaddingFrame(padding_bytes)), - packet_.transmission_type); - QUIC_BUG_IF(quic_bug_10752_36, !success) - << ENDPOINT << "Failed to add padding_bytes: " << padding_bytes - << " transmission_type: " << packet_.transmission_type; + if (!queued_frames_.empty() && + GetQuicRestartFlag(quic_allow_smaller_packets)) { + // Insert PADDING before the other frames to avoid adding a length field + // to any trailing STREAM frame. + QUIC_RESTART_FLAG_COUNT_N(quic_allow_smaller_packets, 3, 5); + if (needs_full_padding_) { + padding_bytes = BytesFreeForPadding(); + } + // AddFrame cannot be used here because it adds the frame to the end of the + // packet. + QuicFrame frame{QuicPaddingFrame(padding_bytes)}; + queued_frames_.insert(queued_frames_.begin(), frame); + packet_size_ += padding_bytes; + packet_.nonretransmittable_frames.push_back(frame); + if (packet_.transmission_type == NOT_RETRANSMISSION) { + packet_.bytes_not_retransmitted.emplace( + packet_.bytes_not_retransmitted.value_or(0) + padding_bytes); + } + } else { + bool success = AddFrame(QuicFrame(QuicPaddingFrame(padding_bytes)), + packet_.transmission_type); + QUIC_BUG_IF(quic_bug_10752_36, !success) + << ENDPOINT << "Failed to add padding_bytes: " << padding_bytes + << " transmission_type: " << packet_.transmission_type; + } } bool QuicPacketCreator::IncludeNonceInPublicHeader() const { @@ -2055,7 +2116,8 @@ // static size_t QuicPacketCreator::MinPlaintextPacketSize( - const ParsedQuicVersion& version) { + const ParsedQuicVersion& version, + QuicPacketNumberLength packet_number_length) { if (!version.HasHeaderProtection()) { return 0; } @@ -2077,7 +2139,10 @@ // 1.3 is used, unittests still use NullEncrypter/NullDecrypter (and other // test crypters) which also only use 12 byte tags. // - // TODO(b/234061734): Set this based on the handshake protocol in use. + if (GetQuicRestartFlag(quic_allow_smaller_packets)) { + QUIC_RESTART_FLAG_COUNT_N(quic_allow_smaller_packets, 1, 5); + return (version.UsesTls() ? 4 : 8) - packet_number_length; + } return 7; }
diff --git a/quiche/quic/core/quic_packet_creator.h b/quiche/quic/core/quic_packet_creator.h index 0bcd745..785efb1 100644 --- a/quiche/quic/core/quic_packet_creator.h +++ b/quiche/quic/core/quic_packet_creator.h
@@ -212,6 +212,10 @@ // value than max_packet_size - PacketSize(), in this case. size_t BytesFree() const; + // Since PADDING frames are always prepended, a separate function computes + // available space without considering STREAM frame expansion. + size_t BytesFreeForPadding() const; + // Returns the number of bytes that the packet will expand by if a new frame // is added to the packet. If the last frame was a stream frame, it will // expand slightly when a new frame is added, and this method returns the @@ -428,7 +432,9 @@ } // Returns the minimum size that the plaintext of a packet must be. - static size_t MinPlaintextPacketSize(const ParsedQuicVersion& version); + static size_t MinPlaintextPacketSize( + const ParsedQuicVersion& version, + QuicPacketNumberLength packet_number_length); // Indicates whether packet flusher is currently attached. bool PacketFlusherAttached() const;
diff --git a/quiche/quic/core/quic_packet_creator_test.cc b/quiche/quic/core/quic_packet_creator_test.cc index f71d9e4..6dcd4a5 100644 --- a/quiche/quic/core/quic_packet_creator_test.cc +++ b/quiche/quic/core/quic_packet_creator_test.cc
@@ -279,17 +279,24 @@ ::testing::PrintToStringParamName()); TEST_P(QuicPacketCreatorTest, SerializeFrames) { + ParsedQuicVersion version = client_framer_.version(); for (int i = ENCRYPTION_INITIAL; i < NUM_ENCRYPTION_LEVELS; ++i) { EncryptionLevel level = static_cast<EncryptionLevel>(i); + bool has_ack = false, has_stream = false; creator_.set_encryption_level(level); + size_t payload_len = 0; if (level != ENCRYPTION_ZERO_RTT) { frames_.push_back(QuicFrame(new QuicAckFrame(InitAckFrame(1)))); + has_ack = true; + payload_len += version.UsesTls() ? 12 : 6; } - QuicStreamId stream_id = QuicUtils::GetFirstBidirectionalStreamId( - client_framer_.transport_version(), Perspective::IS_CLIENT); if (level != ENCRYPTION_INITIAL && level != ENCRYPTION_HANDSHAKE) { + QuicStreamId stream_id = QuicUtils::GetFirstBidirectionalStreamId( + client_framer_.transport_version(), Perspective::IS_CLIENT); frames_.push_back(QuicFrame( QuicStreamFrame(stream_id, false, 0u, absl::string_view()))); + has_stream = true; + payload_len += 2; } SerializedPacket serialized = SerializeAllFrames(frames_); EXPECT_EQ(level, serialized.encryption_level); @@ -297,7 +304,13 @@ delete frames_[0].ack_frame; } frames_.clear(); - + ASSERT_GT(payload_len, 0); // Must have a frame! + size_t min_payload = + (version.UsesTls() && GetQuicRestartFlag(quic_allow_smaller_packets)) + ? 3 + : 7; + bool need_padding = + (version.HasHeaderProtection() && (payload_len < min_payload)); { InSequence s; EXPECT_CALL(framer_visitor_, OnPacket()); @@ -305,7 +318,10 @@ EXPECT_CALL(framer_visitor_, OnUnauthenticatedHeader(_)); EXPECT_CALL(framer_visitor_, OnDecryptedPacket(_, _)); EXPECT_CALL(framer_visitor_, OnPacketHeader(_)); - if (level != ENCRYPTION_ZERO_RTT) { + if (need_padding && GetQuicRestartFlag(quic_allow_smaller_packets)) { + EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); + } + if (has_ack) { EXPECT_CALL(framer_visitor_, OnAckFrameStart(_, _)) .WillOnce(Return(true)); EXPECT_CALL(framer_visitor_, @@ -314,12 +330,11 @@ EXPECT_CALL(framer_visitor_, OnAckFrameEnd(QuicPacketNumber(1))) .WillOnce(Return(true)); } - if (level != ENCRYPTION_INITIAL && level != ENCRYPTION_HANDSHAKE) { + if (has_stream) { EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); } - if (client_framer_.version().HasHeaderProtection()) { - EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)) - .Times(testing::AnyNumber()); + if (need_padding && !GetQuicRestartFlag(quic_allow_smaller_packets)) { + EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); } EXPECT_CALL(framer_visitor_, OnPacketComplete()); } @@ -409,8 +424,10 @@ const size_t overhead = GetPacketHeaderOverhead(client_framer_.transport_version()) + GetEncryptionOverhead(); - for (size_t i = overhead + QuicPacketCreator::MinPlaintextPacketSize( - client_framer_.version()); + for (size_t i = overhead + + QuicPacketCreator::MinPlaintextPacketSize( + client_framer_.version(), + QuicPacketCreatorPeer::GetPacketNumberLength(&creator_)); i < overhead + 100; ++i) { SCOPED_TRACE(i); creator_.SetMaxPacketLength(i); @@ -478,7 +495,12 @@ overhead += QuicFramer::GetMinCryptoFrameSize(kOffset, kMaxOutgoingPacketSize); } else { - overhead += GetStreamFrameOverhead(client_framer_.transport_version()); + overhead += + GetQuicRestartFlag(quic_allow_smaller_packets) + ? QuicFramer::GetMinStreamFrameSize( + client_framer_.transport_version(), + GetNthClientInitiatedStreamId(1), kOffset, false, 0) + : GetStreamFrameOverhead(client_framer_.transport_version()); } ASSERT_GT(kMaxOutgoingPacketSize, overhead); size_t capacity = kDefaultMaxPacketSize - overhead; @@ -516,7 +538,7 @@ // (1 byte) and to expand the stream frame (another 2 bytes) the packet // will not be padded. // Padding is skipped when we try to send coalesced packets. - if ((bytes_free < 3 && + if ((!GetQuicRestartFlag(quic_allow_smaller_packets) && bytes_free < 3 && !QuicVersionUsesCryptoFrames(client_framer_.transport_version())) || client_framer_.version().CanSendCoalescedPackets()) { EXPECT_EQ(kDefaultMaxPacketSize - bytes_free, @@ -1495,20 +1517,20 @@ } EXPECT_FALSE(creator_.HasPendingFrames()); - // Send one byte of stream data. - const std::string data("a"); - producer_.SaveStreamData(GetNthClientInitiatedStreamId(0), data); + // Send zero bytes of stream data. This requires padding. EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); size_t num_bytes_consumed; - creator_.CreateAndSerializeStreamFrame( - GetNthClientInitiatedStreamId(0), data.length(), 0, 0, true, - NOT_RETRANSMISSION, &num_bytes_consumed); - EXPECT_EQ(1u, num_bytes_consumed); + creator_.CreateAndSerializeStreamFrame(GetNthClientInitiatedStreamId(0), 0, 0, + 0, true, NOT_RETRANSMISSION, + &num_bytes_consumed); + EXPECT_EQ(0u, num_bytes_consumed); // Check that a packet is created. ASSERT_TRUE(serialized_packet_->encrypted_buffer); ASSERT_FALSE(serialized_packet_->retransmittable_frames.empty()); + ASSERT_EQ(serialized_packet_->packet_number_length, + PACKET_1BYTE_PACKET_NUMBER); { InSequence s; EXPECT_CALL(framer_visitor_, OnPacket()); @@ -1516,9 +1538,16 @@ 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(_)); + if (GetQuicRestartFlag(quic_allow_smaller_packets)) { + EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); + EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); + } else { + EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); + EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); + } + } else { + EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); } EXPECT_CALL(framer_visitor_, OnPacketComplete()); } @@ -1639,13 +1668,14 @@ // 1. QuicStreamId stream_id = QuicUtils::GetFirstBidirectionalStreamId( client_framer_.transport_version(), Perspective::IS_CLIENT); - size_t length = - GetPacketHeaderOverhead(client_framer_.transport_version()) + - GetEncryptionOverhead() + - QuicFramer::GetMinStreamFrameSize( - client_framer_.transport_version(), stream_id, 0, - /*last_frame_in_packet=*/false, kStreamFramePayloadSize + 1) + - kStreamFramePayloadSize + 1; + size_t length = GetPacketHeaderOverhead(client_framer_.transport_version()) + + GetEncryptionOverhead() + + QuicFramer::GetMinStreamFrameSize( + client_framer_.transport_version(), stream_id, 0, + /*last_frame_in_packet=*/ + GetQuicRestartFlag(quic_allow_smaller_packets), + kStreamFramePayloadSize + 1) + + kStreamFramePayloadSize + 1; creator_.SetMaxPacketLength(length); creator_.AddPendingPadding(kMaxNumRandomPaddingBytes); QuicByteCount pending_padding_bytes = creator_.pending_padding_bytes(); @@ -2098,8 +2128,13 @@ EXPECT_CALL(framer_visitor_, OnDecryptedPacket(_, _)); EXPECT_CALL(framer_visitor_, OnPacketHeader(_)) .WillOnce(DoAll(SaveArg<0>(&header), Return(true))); + if (client_framer_.version().HasHeaderProtection() && + GetQuicRestartFlag(quic_allow_smaller_packets)) { + EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); + } EXPECT_CALL(framer_visitor_, OnPingFrame(_)); - if (client_framer_.version().HasHeaderProtection()) { + if (client_framer_.version().HasHeaderProtection() && + !GetQuicRestartFlag(quic_allow_smaller_packets)) { EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); } EXPECT_CALL(framer_visitor_, OnPacketComplete()); @@ -2277,27 +2312,38 @@ EXPECT_CALL(framer_visitor_, OnDecryptedPacket(_, _)); EXPECT_CALL(framer_visitor_, OnPacketHeader(_)); if (i != ENCRYPTION_ZERO_RTT) { + if (GetQuicRestartFlag(quic_allow_smaller_packets) && + i != ENCRYPTION_INITIAL) { + EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)) + .Times(testing::AtMost(1)); + } EXPECT_CALL(framer_visitor_, OnAckFrameStart(_, _)) .WillOnce(Return(true)); EXPECT_CALL(framer_visitor_, OnAckRange(QuicPacketNumber(1), QuicPacketNumber(2))) .WillOnce(Return(true)); EXPECT_CALL(framer_visitor_, OnAckFrameEnd(_)).WillOnce(Return(true)); + if (!GetQuicRestartFlag(quic_allow_smaller_packets)) { + EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)) + .Times(testing::AtMost(1)); + } } if (i == ENCRYPTION_INITIAL) { // Verify padding is added. EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); - } else if (i != ENCRYPTION_ZERO_RTT) { - EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)).Times(testing::AtMost(1)); + } + if (GetQuicRestartFlag(quic_allow_smaller_packets) && + i == ENCRYPTION_ZERO_RTT) { + EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); } if (i != ENCRYPTION_INITIAL && i != ENCRYPTION_HANDSHAKE) { EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); } - if (i == ENCRYPTION_ZERO_RTT) { + if (!GetQuicRestartFlag(quic_allow_smaller_packets) && + i == ENCRYPTION_ZERO_RTT) { EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); } EXPECT_CALL(framer_visitor_, OnPacketComplete()); - server_framer_.ProcessPacket(*packets[i]); } } @@ -2307,7 +2353,9 @@ QuicByteCount previous_max_packet_length = creator_.max_packet_length(); const size_t overhead = GetPacketHeaderOverhead(client_framer_.transport_version()) + - QuicPacketCreator::MinPlaintextPacketSize(client_framer_.version()) + + QuicPacketCreator::MinPlaintextPacketSize( + client_framer_.version(), + QuicPacketCreatorPeer::GetPacketNumberLength(&creator_)) + GetEncryptionOverhead(); // Make sure a length which cannot accommodate header (includes header // protection minimal length) gets rejected. @@ -2382,7 +2430,9 @@ const QuicByteCount previous_max_packet_length = creator_.max_packet_length(); const size_t min_acceptable_packet_size = GetPacketHeaderOverhead(client_framer_.transport_version()) + - QuicPacketCreator::MinPlaintextPacketSize(client_framer_.version()) + + QuicPacketCreator::MinPlaintextPacketSize( + client_framer_.version(), + QuicPacketCreatorPeer::GetPacketNumberLength(&creator_)) + GetEncryptionOverhead(); // Then set the soft max packet length to the lowest allowed value. creator_.SetSoftMaxPacketLength(min_acceptable_packet_size); @@ -2398,6 +2448,88 @@ EXPECT_EQ(creator_.max_packet_length(), previous_max_packet_length); } +TEST_P(QuicPacketCreatorTest, MinPayloadLength) { + ParsedQuicVersion version = client_framer_.version(); + for (QuicPacketNumberLength pn_length : + {PACKET_1BYTE_PACKET_NUMBER, PACKET_2BYTE_PACKET_NUMBER, + PACKET_3BYTE_PACKET_NUMBER, PACKET_4BYTE_PACKET_NUMBER}) { + if (!version.HasHeaderProtection()) { + EXPECT_EQ(creator_.MinPlaintextPacketSize(version, pn_length), 0); + } else if (!GetQuicRestartFlag(quic_allow_smaller_packets)) { + EXPECT_EQ(creator_.MinPlaintextPacketSize(version, pn_length), 7); + } else { + EXPECT_EQ(creator_.MinPlaintextPacketSize(version, pn_length), + (version.UsesTls() ? 4 : 8) - pn_length); + } + } +} + +// A variant of StreamFrameConsumption that tests when expansion of the stream +// frame puts it at or over the max length, but the packet is supposed to be +// padded to max length. +TEST_P(QuicPacketCreatorTest, PadWhenAlmostMaxLength) { + creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE); + // Compute the total overhead for a single frame in packet. + const size_t overhead = + GetPacketHeaderOverhead(client_framer_.transport_version()) + + GetEncryptionOverhead() + + GetStreamFrameOverhead(client_framer_.transport_version()); + size_t capacity = kDefaultMaxPacketSize - overhead; + for (size_t bytes_free = 1; bytes_free <= 2; bytes_free++) { + std::string data(capacity - bytes_free, 'A'); + + QuicFrame frame; + ASSERT_TRUE(creator_.ConsumeDataToFillCurrentPacket( + GetNthClientInitiatedStreamId(1), data, kOffset, false, + /*needs_full_padding=*/true, NOT_RETRANSMISSION, &frame)); + + // BytesFree() returns bytes available for the next frame, which will + // be two bytes smaller since the stream frame would need to be grown. + EXPECT_EQ(2u, creator_.ExpansionOnNewFrame()); + EXPECT_EQ(0u, creator_.BytesFree()); + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); + creator_.FlushCurrentPacket(); + /* Without the fix, the packet is not full-length. */ + if (GetQuicRestartFlag(quic_allow_smaller_packets)) { + EXPECT_EQ(serialized_packet_->encrypted_length, kDefaultMaxPacketSize); + } else { + EXPECT_EQ(serialized_packet_->encrypted_length, + kDefaultMaxPacketSize - bytes_free); + } + DeleteSerializedPacket(); + } +} + +TEST_P(QuicPacketCreatorTest, MorePendingPaddingThanBytesFree) { + creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE); + // Compute the total overhead for a single frame in packet. + const size_t overhead = + GetPacketHeaderOverhead(client_framer_.transport_version()) + + GetEncryptionOverhead() + + GetStreamFrameOverhead(client_framer_.transport_version()); + size_t capacity = kDefaultMaxPacketSize - overhead; + const size_t pending_padding = 10; + std::string data(capacity - pending_padding, 'A'); + QuicFrame frame; + // The stream frame means that BytesFree() will be less than the + // available space, because of the frame length field. + ASSERT_TRUE(creator_.ConsumeDataToFillCurrentPacket( + GetNthClientInitiatedStreamId(1), data, kOffset, false, + /*needs_full_padding=*/false, NOT_RETRANSMISSION, &frame)); + creator_.AddPendingPadding(pending_padding); + EXPECT_EQ(2u, creator_.ExpansionOnNewFrame()); + // BytesFree() does not know about pending_padding because that's added + // when flushed. + EXPECT_EQ(pending_padding - 2u, creator_.BytesFree()); + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); + creator_.FlushCurrentPacket(); + /* Without the fix, the packet is not full-length. */ + EXPECT_EQ(serialized_packet_->encrypted_length, kDefaultMaxPacketSize); + DeleteSerializedPacket(); +} + class MockDelegate : public QuicPacketCreator::DelegateInterface { public: MockDelegate() {} @@ -2603,7 +2735,6 @@ if (num_retransmittable_frames == 0) { ASSERT_TRUE(packet.retransmittable_frames.empty()); } else { - ASSERT_FALSE(packet.retransmittable_frames.empty()); EXPECT_EQ(num_retransmittable_frames, packet.retransmittable_frames.size()); } @@ -2926,7 +3057,8 @@ // frames, so the expected packet length differs slightly. expected_packet_length = 32; } - if (framer_.version().HasHeaderProtection()) { + if (framer_.version().HasHeaderProtection() && + !GetQuicRestartFlag(quic_allow_smaller_packets)) { expected_packet_length = 33; } EXPECT_EQ(expected_packet_length, packets_[0].encrypted_length); @@ -3872,18 +4004,22 @@ return; } delegate_.SetCanWriteAnything(); - + // If the packet number length > 1, we won't get padding. + EXPECT_EQ(QuicPacketCreatorPeer::GetPacketNumberLength(&creator_), + PACKET_1BYTE_PACKET_NUMBER); EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce( Invoke(this, &QuicPacketCreatorMultiplePacketsTest::SavePacket)); + // with no data and no offset, this is a 2B STREAM frame. creator_.ConsumeData(QuicUtils::GetFirstBidirectionalStreamId( framer_.transport_version(), Perspective::IS_CLIENT), - "a", 0, FIN); + "", 0, FIN); creator_.Flush(); ASSERT_FALSE(packets_[0].nonretransmittable_frames.empty()); QuicFrame padding = packets_[0].nonretransmittable_frames[0]; // Verify stream frame expansion is excluded. - EXPECT_EQ(3, padding.padding_frame.num_padding_bytes); + EXPECT_EQ(padding.padding_frame.num_padding_bytes, + GetQuicRestartFlag(quic_allow_smaller_packets) ? 1 : 4); } TEST_F(QuicPacketCreatorMultiplePacketsTest,
diff --git a/quiche/quic/test_tools/quic_test_utils.cc b/quiche/quic/test_tools/quic_test_utils.cc index 8063aa2..62c8f69 100644 --- a/quiche/quic/test_tools/quic_test_utils.cc +++ b/quiche/quic/test_tools/quic_test_utils.cc
@@ -939,8 +939,8 @@ // We need a minimum number of bytes of encrypted payload. This will // guarantee that we have at least that much. (It ignores the overhead of // the stream/crypto framing, so it overpads slightly.) - size_t min_plaintext_size = - QuicPacketCreator::MinPlaintextPacketSize(version); + size_t min_plaintext_size = QuicPacketCreator::MinPlaintextPacketSize( + version, packet_number_length); if (data.length() < min_plaintext_size) { size_t padding_length = min_plaintext_size - data.length(); frames.push_back(QuicFrame(QuicPaddingFrame(padding_length)));