Internal change
PiperOrigin-RevId: 390263074
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index e559110..071ee65 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -5297,14 +5297,17 @@
<< "EffectivePeerMigration started without address change.";
return;
}
- if (packet_creator_.HasPendingFrames()) {
- QUIC_BUG(bug_731_2)
- << "Starts effective peer migration with pending frame types: "
- << packet_creator_.GetPendingFramesInfo() << ". Address change type is "
- << AddressChangeTypeToString(type)
- << ". Current frame type: " << framer_.current_received_frame_type()
- << ". Previous frame type: "
- << framer_.previously_received_frame_type();
+ // There could be pending NEW_TOKEN_FRAME triggered by non-probing
+ // PATH_RESPONSE_FRAME in the same packet.
+ if (GetQuicReloadableFlag(
+ quic_flush_pending_frame_before_updating_default_path) &&
+ packet_creator_.HasPendingFrames()) {
+ QUICHE_RELOADABLE_FLAG_COUNT(
+ quic_flush_pending_frame_before_updating_default_path);
+ packet_creator_.FlushCurrentPacket();
+ if (!connected_) {
+ return;
+ }
}
// Action items:
@@ -5361,17 +5364,7 @@
std::move(alternative_path_.rtt_stats).value());
}
}
-
// Update to the new peer address.
- if (packet_creator_.HasPendingFrames()) {
- QUIC_BUG(bug_731_1)
- << "Starts effective peer migration with pending frame types: "
- << packet_creator_.GetPendingFramesInfo() << ". Address change type is "
- << AddressChangeTypeToString(type)
- << ". Current frame type: " << framer_.current_received_frame_type()
- << ". Previous frame type: "
- << framer_.previously_received_frame_type();
- }
UpdatePeerAddress(last_received_packet_info_.source_address);
// Update the default path.
if (IsAlternativePath(last_received_packet_info_.destination_address,
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 8466ab0..e991fbf 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -2079,6 +2079,65 @@
}
}
+// Regression test for b/196208556.
+TEST_P(QuicConnectionTest,
+ ReversePathValidationResponseReceivedFromUnexpectedPeerAddress) {
+ set_perspective(Perspective::IS_SERVER);
+ if (!GetQuicReloadableFlag(
+ quic_flush_pending_frame_before_updating_default_path) ||
+ !connection_.connection_migration_use_new_cid()) {
+ return;
+ }
+ QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false);
+ connection_.CreateConnectionIdManager();
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
+ QuicConnectionPeer::SetPeerAddress(&connection_, kPeerAddress);
+ QuicConnectionPeer::SetEffectivePeerAddress(&connection_, kPeerAddress);
+ QuicConnectionPeer::SetAddressValidated(&connection_);
+ EXPECT_EQ(kPeerAddress, connection_.peer_address());
+ EXPECT_EQ(kPeerAddress, connection_.effective_peer_address());
+
+ // Sends new server CID to client.
+ QuicConnectionId new_cid;
+ EXPECT_CALL(visitor_, OnServerConnectionIdIssued(_))
+ .WillOnce(Invoke([&](const QuicConnectionId& cid) { new_cid = cid; }));
+ EXPECT_CALL(visitor_, SendNewConnectionId(_));
+ connection_.OnHandshakeComplete();
+ EXPECT_CALL(visitor_, GetHandshakeState())
+ .WillRepeatedly(Return(HANDSHAKE_CONFIRMED));
+
+ // Process a non-probing packet to migrate to path 2 and kick off reverse path
+ // validation.
+ EXPECT_CALL(visitor_, OnConnectionMigration(IPV6_TO_IPV4_CHANGE)).Times(1);
+ const QuicSocketAddress kPeerAddress2 =
+ QuicSocketAddress(QuicIpAddress::Loopback4(), /*port=*/23456);
+ peer_creator_.SetServerConnectionId(new_cid);
+ ProcessFramesPacketWithAddresses({QuicFrame(QuicPingFrame())}, kSelfAddress,
+ kPeerAddress2, ENCRYPTION_FORWARD_SECURE);
+ EXPECT_FALSE(writer_->path_challenge_frames().empty());
+ QuicPathFrameBuffer reverse_path_challenge_payload =
+ writer_->path_challenge_frames().front().data_buffer;
+
+ // Receiveds a packet from path 3 with PATH_RESPONSE frame intended to
+ // validate path 2 and a non-probing frame.
+ {
+ QuicConnection::ScopedPacketFlusher flusher(&connection_);
+ const QuicSocketAddress kPeerAddress3 =
+ QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/56789);
+ auto ack_frame = InitAckFrame(1);
+ EXPECT_CALL(visitor_, OnConnectionMigration(IPV4_TO_IPV6_CHANGE)).Times(1);
+ EXPECT_CALL(visitor_, MaybeSendAddressToken()).WillOnce(Invoke([this]() {
+ connection_.SendControlFrame(
+ QuicFrame(new QuicNewTokenFrame(1, "new_token")));
+ }));
+ ProcessFramesPacketWithAddresses({QuicFrame(new QuicPathResponseFrame(
+ 0, reverse_path_challenge_payload)),
+ QuicFrame(&ack_frame)},
+ kSelfAddress, kPeerAddress3,
+ ENCRYPTION_FORWARD_SECURE);
+ }
+}
+
TEST_P(QuicConnectionTest, ReversePathValidationFailureAtServer) {
set_perspective(Perspective::IS_SERVER);
if (!connection_.connection_migration_use_new_cid()) {
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index 258f70c..0f4fd18 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -75,6 +75,8 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_enable_server_on_wire_ping, true)
// If true, fix a bug in TlsServerHandshaker where the ticket decrypt callback is cleared without being cancelled first.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_tls_fix_ticket_decrypt, true)
+// If true, flush any pending frame before default path is about to be updated.
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_flush_pending_frame_before_updating_default_path, true)
// If true, ignore peer_max_ack_delay during handshake.
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_ignore_peer_max_ack_delay_during_handshake, true)
// If true, include stream information in idle timeout connection close detail.
