Clear packet_ when QuicPacketCreator::SerializePacket() fails. The new test double-frees without this code. Protected by FLAGS_quic_reloadable_flag_quic_clear_packet_on_serialization_failure. PiperOrigin-RevId: 890646454
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h index fef9d4b..82c94e5 100755 --- a/quiche/common/quiche_feature_flags_list.h +++ b/quiche/common/quiche_feature_flags_list.h
@@ -17,6 +17,7 @@ QUICHE_FLAG(bool, quiche_reloadable_flag_quic_bbr_exit_startup_on_loss_network_param, false, false, "Enables adjustment of BBRv1's exit-startup-on-loss behavior on established connections via NetworkParams::enable_bbr_exit_startup_on_loss.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_block_until_settings_received_copt, true, true, "If enabled and a BSUS connection is received, blocks server connections until SETTINGS frame is received.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_clear_body_manager_along_with_sequencer, false, true, "If true, QuicSpdyStream::StopReading always clears BodyManager along with the SequenceBuffer.") +QUICHE_FLAG(bool, quiche_reloadable_flag_quic_clear_packet_on_serialization_failure, false, false, "If true, clear QuicPacketCreator state when serialization failure.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_client_check_blockage_before_on_can_write, false, false, "If true, quic clients will only call OnCanWrite() upon write events if the writer is unblocked.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_close_connection_on_underflow, true, true, "If true, close QUIC connections if a RESET_STREAM frame is received with a too-small final byte offset value.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_close_on_idle_timeout, false, false, "If true, closes the connection if it has exceeded the idle timeout when deciding whether to open a stream.")
diff --git a/quiche/quic/core/quic_packet_creator.cc b/quiche/quic/core/quic_packet_creator.cc index 137a4e4..1e3f587 100644 --- a/quiche/quic/core/quic_packet_creator.cc +++ b/quiche/quic/core/quic_packet_creator.cc
@@ -2415,6 +2415,12 @@ QUIC_BUG(quic_bug_10752_38) << ENDPOINT2 << error_details; creator_->delegate_->OnUnrecoverableError(QUIC_FAILED_TO_SERIALIZE_PACKET, error_details); + if (GetQuicReloadableFlag(quic_clear_packet_on_serialization_failure)) { + QUIC_RELOADABLE_FLAG_COUNT(quic_clear_packet_on_serialization_failure); + creator_->packet_.retransmittable_frames.clear(); + creator_->packet_.nonretransmittable_frames.clear(); + creator_->ClearPacket(); + } } }
diff --git a/quiche/quic/core/quic_packet_creator_test.cc b/quiche/quic/core/quic_packet_creator_test.cc index f4c3262..1f4035d 100644 --- a/quiche/quic/core/quic_packet_creator_test.cc +++ b/quiche/quic/core/quic_packet_creator_test.cc
@@ -15,7 +15,9 @@ #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "quiche/quic/core/frames/quic_frame.h" +#include "quiche/quic/core/frames/quic_ping_frame.h" #include "quiche/quic/core/frames/quic_stream_frame.h" +#include "quiche/quic/core/quic_coalesced_packet.h" #include "quiche/quic/core/quic_connection_id.h" #include "quiche/quic/core/quic_constants.h" #include "quiche/quic/core/quic_data_writer.h" @@ -23,8 +25,10 @@ #include "quiche/quic/core/quic_packets.h" #include "quiche/quic/core/quic_types.h" #include "quiche/quic/core/quic_utils.h" +#include "quiche/quic/core/quic_versions.h" #include "quiche/quic/platform/api/quic_expect_bug.h" #include "quiche/quic/platform/api/quic_flags.h" +#include "quiche/quic/platform/api/quic_ip_address.h" #include "quiche/quic/platform/api/quic_socket_address.h" #include "quiche/quic/platform/api/quic_test.h" #include "quiche/quic/test_tools/quic_framer_peer.h" @@ -260,6 +264,39 @@ n * 2; } + // Builds a minimal packet at |level| and tries to coalesce it with + // |coalesced|. Returns true if successful. Tries to mimic the responses of + // QuicConnection before the Handshake is confirmed. + bool BuildAndCoalescePacket(QuicCoalescedPacket& coalesced, + EncryptionLevel level) { + creator_.set_encryption_level(level); + creator_.AttachPacketFlusher(); + if (level == ENCRYPTION_INITIAL || level == ENCRYPTION_HANDSHAKE) { + EXPECT_CALL(delegate_, MaybeBundleOpportunistically); + } + EXPECT_CALL(delegate_, ShouldGeneratePacket).WillRepeatedly(Return(true)); + if (level == ENCRYPTION_FORWARD_SECURE || level == ENCRYPTION_ZERO_RTT) { + creator_.AddFrame(QuicFrame(QuicPingFrame()), NOT_RETRANSMISSION); + } else { + std::string data = "crypto data"; + producer_.SaveCryptoData(ENCRYPTION_INITIAL, 0, data); + creator_.ConsumeCryptoData(ENCRYPTION_INITIAL, data.length(), 0); + } + EXPECT_CALL(delegate_, GetSerializedPacketFate(false, level)) + .WillRepeatedly(Return(COALESCE)); + // OnSerializedPacket() will call WritePacket(), which sees the fate is + // COALESCE and puts in QuicConnection::coalesced_packet_. + EXPECT_CALL(delegate_, OnSerializedPacket) + .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); + creator_.Flush(); + + QuicSocketAddress self_address(QuicIpAddress::Loopback4(), 1); + QuicSocketAddress peer_address(QuicIpAddress::Loopback4(), 2); + return coalesced.MaybeCoalescePacket( + *serialized_packet_, self_address, peer_address, &allocator_, + creator_.max_packet_length(), ECN_NOT_ECT, 0); + } + void TestChaosProtection(bool enabled); static constexpr QuicStreamOffset kOffset = 0u; @@ -4491,6 +4528,22 @@ creator_.FlushCurrentPacket(); } +TEST_P(QuicPacketCreatorTest, InitialPacketBufferOverflow) { + if (!VersionIsIetfQuic(creator_.transport_version())) { + return; + } + SetQuicReloadableFlag(quic_clear_packet_on_serialization_failure, true); + creator_.SetMaxPacketLength(kMaxOutgoingPacketSize); + QuicCoalescedPacket coalesced; + EXPECT_TRUE(BuildAndCoalescePacket(coalesced, ENCRYPTION_INITIAL)); + char buffer[kMaxOutgoingPacketSize]; + EXPECT_CALL(framer_visitor_, OnError); + EXPECT_CALL(delegate_, OnUnrecoverableError); + EXPECT_QUIC_BUG(creator_.SerializeCoalescedPacket(coalesced, buffer, + kMaxOutgoingPacketSize - 1), + "Client: Failed to encrypt packet number 1"); +} + } // namespace } // namespace test } // namespace quic