Let QuicConnection::UpdatePacketContent() return connection state.
This allows callers to correctly handle errors occurred.
Protected by quic_reloadable_flag_quic_update_packet_content_returns_connected.
PiperOrigin-RevId: 353360764
Change-Id: Ie1e9874398311cf3f3f6873bac5dc53d6b1a94d8
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 5090991..bf8cd2d 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -1262,7 +1262,9 @@
// Since a stream frame was received, this is not a connectivity probe.
// A probe only contains a PING and full padding.
- UpdatePacketContent(STREAM_FRAME);
+ if (!UpdatePacketContent(STREAM_FRAME)) {
+ return false;
+ }
if (debug_visitor_ != nullptr) {
debug_visitor_->OnStreamFrame(frame);
@@ -1300,7 +1302,9 @@
// Since a CRYPTO frame was received, this is not a connectivity probe.
// A probe only contains a PING and full padding.
- UpdatePacketContent(CRYPTO_FRAME);
+ if (!UpdatePacketContent(CRYPTO_FRAME)) {
+ return false;
+ }
if (debug_visitor_ != nullptr) {
debug_visitor_->OnCryptoFrame(frame);
@@ -1325,7 +1329,9 @@
// Since an ack frame was received, this is not a connectivity probe.
// A probe only contains a PING and full padding.
- UpdatePacketContent(ACK_FRAME);
+ if (!UpdatePacketContent(ACK_FRAME)) {
+ return false;
+ }
QUIC_DVLOG(1) << ENDPOINT
<< "OnAckFrameStart, largest_acked: " << largest_acked;
@@ -1459,7 +1465,9 @@
// Since a stop waiting frame was received, this is not a connectivity probe.
// A probe only contains a PING and full padding.
- UpdatePacketContent(STOP_WAITING_FRAME);
+ if (!UpdatePacketContent(STOP_WAITING_FRAME)) {
+ return false;
+ }
if (no_stop_waiting_frames_) {
return true;
@@ -1492,7 +1500,9 @@
QUIC_BUG_IF(!connected_)
<< "Processing PADDING frame when connection is closed. Last frame: "
<< most_recent_frame_type_;
- UpdatePacketContent(PADDING_FRAME);
+ if (!UpdatePacketContent(PADDING_FRAME)) {
+ return false;
+ }
if (debug_visitor_ != nullptr) {
debug_visitor_->OnPaddingFrame(frame);
@@ -1504,7 +1514,9 @@
QUIC_BUG_IF(!connected_)
<< "Processing PING frame when connection is closed. Last frame: "
<< most_recent_frame_type_;
- UpdatePacketContent(PING_FRAME);
+ if (!UpdatePacketContent(PING_FRAME)) {
+ return false;
+ }
if (debug_visitor_ != nullptr) {
QuicTime::Delta ping_received_delay = QuicTime::Delta::Zero();
@@ -1549,7 +1561,9 @@
// Since a reset stream frame was received, this is not a connectivity probe.
// A probe only contains a PING and full padding.
- UpdatePacketContent(RST_STREAM_FRAME);
+ if (!UpdatePacketContent(RST_STREAM_FRAME)) {
+ return false;
+ }
if (debug_visitor_ != nullptr) {
debug_visitor_->OnRstStreamFrame(frame);
@@ -1570,7 +1584,9 @@
// Since a reset stream frame was received, this is not a connectivity probe.
// A probe only contains a PING and full padding.
- UpdatePacketContent(STOP_SENDING_FRAME);
+ if (!UpdatePacketContent(STOP_SENDING_FRAME)) {
+ return false;
+ }
if (debug_visitor_ != nullptr) {
debug_visitor_->OnStopSendingFrame(frame);
@@ -1594,7 +1610,9 @@
// Only respond to the 1st PATH_CHALLENGE.
return true;
}
- UpdatePacketContent(PATH_CHALLENGE_FRAME);
+ if (!UpdatePacketContent(PATH_CHALLENGE_FRAME)) {
+ return false;
+ }
if (debug_visitor_ != nullptr) {
debug_visitor_->OnPathChallengeFrame(frame);
}
@@ -1630,7 +1648,9 @@
QUIC_BUG_IF(!connected_) << "Processing PATH_RESPONSE frame when connection "
"is closed. Last frame: "
<< most_recent_frame_type_;
- UpdatePacketContent(PATH_RESPONSE_FRAME);
+ if (!UpdatePacketContent(PATH_RESPONSE_FRAME)) {
+ return false;
+ }
if (debug_visitor_ != nullptr) {
debug_visitor_->OnPathResponseFrame(frame);
}
@@ -1658,7 +1678,9 @@
// Since a connection close frame was received, this is not a connectivity
// probe. A probe only contains a PING and full padding.
- UpdatePacketContent(CONNECTION_CLOSE_FRAME);
+ if (!UpdatePacketContent(CONNECTION_CLOSE_FRAME)) {
+ return false;
+ }
if (debug_visitor_ != nullptr) {
debug_visitor_->OnConnectionCloseFrame(frame);
@@ -1703,7 +1725,10 @@
QUIC_BUG_IF(!connected_)
<< "Processing MAX_STREAMS frame when connection is closed. Last frame: "
<< most_recent_frame_type_;
- UpdatePacketContent(MAX_STREAMS_FRAME);
+ if (!UpdatePacketContent(MAX_STREAMS_FRAME)) {
+ return false;
+ }
+
if (debug_visitor_ != nullptr) {
debug_visitor_->OnMaxStreamsFrame(frame);
}
@@ -1715,7 +1740,10 @@
QUIC_BUG_IF(!connected_) << "Processing STREAMS_BLOCKED frame when "
"connection is closed. Last frame: "
<< most_recent_frame_type_;
- UpdatePacketContent(STREAMS_BLOCKED_FRAME);
+ if (!UpdatePacketContent(STREAMS_BLOCKED_FRAME)) {
+ return false;
+ }
+
if (debug_visitor_ != nullptr) {
debug_visitor_->OnStreamsBlockedFrame(frame);
}
@@ -1729,7 +1757,9 @@
// Since a go away frame was received, this is not a connectivity probe.
// A probe only contains a PING and full padding.
- UpdatePacketContent(GOAWAY_FRAME);
+ if (!UpdatePacketContent(GOAWAY_FRAME)) {
+ return false;
+ }
if (debug_visitor_ != nullptr) {
debug_visitor_->OnGoAwayFrame(frame);
@@ -1750,7 +1780,9 @@
// Since a window update frame was received, this is not a connectivity probe.
// A probe only contains a PING and full padding.
- UpdatePacketContent(WINDOW_UPDATE_FRAME);
+ if (!UpdatePacketContent(WINDOW_UPDATE_FRAME)) {
+ return false;
+ }
if (debug_visitor_ != nullptr) {
debug_visitor_->OnWindowUpdateFrame(
@@ -1767,7 +1799,10 @@
QUIC_BUG_IF(!connected_) << "Processing NEW_CONNECTION_ID frame when "
"connection is closed. Last frame: "
<< most_recent_frame_type_;
- UpdatePacketContent(NEW_CONNECTION_ID_FRAME);
+ if (!UpdatePacketContent(NEW_CONNECTION_ID_FRAME)) {
+ return false;
+ }
+
if (debug_visitor_ != nullptr) {
debug_visitor_->OnNewConnectionIdFrame(frame);
}
@@ -1779,7 +1814,10 @@
QUIC_BUG_IF(!connected_) << "Processing RETIRE_CONNECTION_ID frame when "
"connection is closed. Last frame: "
<< most_recent_frame_type_;
- UpdatePacketContent(RETIRE_CONNECTION_ID_FRAME);
+ if (!UpdatePacketContent(RETIRE_CONNECTION_ID_FRAME)) {
+ return false;
+ }
+
if (debug_visitor_ != nullptr) {
debug_visitor_->OnRetireConnectionIdFrame(frame);
}
@@ -1790,7 +1828,10 @@
QUIC_BUG_IF(!connected_)
<< "Processing NEW_TOKEN frame when connection is closed. Last frame: "
<< most_recent_frame_type_;
- UpdatePacketContent(NEW_TOKEN_FRAME);
+ if (!UpdatePacketContent(NEW_TOKEN_FRAME)) {
+ return false;
+ }
+
if (debug_visitor_ != nullptr) {
debug_visitor_->OnNewTokenFrame(frame);
}
@@ -1815,7 +1856,9 @@
// Since a message frame was received, this is not a connectivity probe.
// A probe only contains a PING and full padding.
- UpdatePacketContent(MESSAGE_FRAME);
+ if (!UpdatePacketContent(MESSAGE_FRAME)) {
+ return false;
+ }
if (debug_visitor_ != nullptr) {
debug_visitor_->OnMessageFrame(frame);
@@ -1846,7 +1889,9 @@
// Since a handshake done frame was received, this is not a connectivity
// probe. A probe only contains a PING and full padding.
- UpdatePacketContent(HANDSHAKE_DONE_FRAME);
+ if (!UpdatePacketContent(HANDSHAKE_DONE_FRAME)) {
+ return false;
+ }
if (debug_visitor_ != nullptr) {
debug_visitor_->OnHandshakeDoneFrame(frame);
@@ -1863,7 +1908,10 @@
if (debug_visitor_ != nullptr) {
debug_visitor_->OnAckFrequencyFrame(frame);
}
- UpdatePacketContent(ACK_FREQUENCY_FRAME);
+ if (!UpdatePacketContent(ACK_FREQUENCY_FRAME)) {
+ return false;
+ }
+
if (!can_receive_ack_frequency_frame_) {
QUIC_LOG_EVERY_N_SEC(ERROR, 120) << "Get unexpected AckFrequencyFrame.";
return false;
@@ -1888,7 +1936,9 @@
// Since a blocked frame was received, this is not a connectivity probe.
// A probe only contains a PING and full padding.
- UpdatePacketContent(BLOCKED_FRAME);
+ if (!UpdatePacketContent(BLOCKED_FRAME)) {
+ return false;
+ }
if (debug_visitor_ != nullptr) {
debug_visitor_->OnBlockedFrame(frame);
@@ -4777,19 +4827,22 @@
sent_packet_manager_.OnApplicationLimited();
}
-void QuicConnection::UpdatePacketContent(QuicFrameType type) {
+bool QuicConnection::UpdatePacketContent(QuicFrameType type) {
+ if (update_packet_content_returns_connected_) {
+ QUIC_RELOADABLE_FLAG_COUNT(quic_update_packet_content_returns_connected);
+ }
most_recent_frame_type_ = type;
if (version().HasIetfQuicFrames()) {
if (!QuicUtils::IsProbingFrame(type)) {
MaybeStartIetfPeerMigration();
- return;
+ return !update_packet_content_returns_connected_ || connected_;
}
QuicSocketAddress current_effective_peer_address =
GetEffectivePeerAddressFromCurrentPacket();
if (!count_bytes_on_alternative_path_seperately_ ||
IsDefaultPath(last_packet_destination_address_,
last_packet_source_address_)) {
- return;
+ return !update_packet_content_returns_connected_ || connected_;
}
QUIC_CODE_COUNT_N(quic_count_bytes_on_alternative_path_seperately, 3, 5);
if (type == PATH_CHALLENGE_FRAME &&
@@ -4804,7 +4857,7 @@
last_packet_destination_address_, current_effective_peer_address);
}
MaybeUpdateBytesReceivedFromAlternativeAddress(last_size_);
- return;
+ return !update_packet_content_returns_connected_ || connected_;
}
// Packet content is tracked to identify connectivity probe in non-IETF
// version, where a connectivity probe is defined as
@@ -4815,13 +4868,13 @@
// We have already learned the current packet is not a connectivity
// probing packet. Peer migration should have already been started earlier
// if needed.
- return;
+ return !update_packet_content_returns_connected_ || connected_;
}
if (type == PING_FRAME) {
if (current_packet_content_ == NO_FRAMES_RECEIVED) {
current_packet_content_ = FIRST_FRAME_IS_PING;
- return;
+ return !update_packet_content_returns_connected_ || connected_;
}
}
@@ -4851,7 +4904,7 @@
<< last_packet_destination_address_
<< ", self_address_:" << self_address_;
}
- return;
+ return !update_packet_content_returns_connected_ || connected_;
}
current_packet_content_ = NOT_PADDED_PING;
@@ -4865,6 +4918,7 @@
}
}
current_effective_peer_migration_type_ = NO_CHANGE;
+ return !update_packet_content_returns_connected_ || connected_;
}
void QuicConnection::MaybeStartIetfPeerMigration() {
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 45211d2..b250810 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -54,6 +54,7 @@
#include "quic/core/uber_received_packet_manager.h"
#include "quic/platform/api/quic_containers.h"
#include "quic/platform/api/quic_export.h"
+#include "quic/platform/api/quic_flags.h"
#include "quic/platform/api/quic_socket_address.h"
#include "common/platform/api/quiche_str_cat.h"
@@ -1450,7 +1451,8 @@
// starts effective peer migration if current packet is confirmed not a
// connectivity probe and |current_effective_peer_migration_type_| indicates
// effective peer address change.
- void UpdatePacketContent(QuicFrameType type);
+ // Returns true if connection is still alive.
+ ABSL_MUST_USE_RESULT bool UpdatePacketContent(QuicFrameType type);
// Called when last received ack frame has been processed.
// |send_stop_waiting| indicates whether a stop waiting needs to be sent.
@@ -2049,6 +2051,9 @@
bool count_bytes_on_alternative_path_seperately_ =
GetQuicReloadableFlag(quic_count_bytes_on_alternative_path_seperately);
+
+ bool update_packet_content_returns_connected_ =
+ GetQuicReloadableFlag(quic_update_packet_content_returns_connected);
};
} // namespace quic
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 29ad505..5643114 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -13265,6 +13265,7 @@
QUIC_INVALID_0RTT_PACKET_NUMBER_OUT_OF_ORDER);
}
+// Regression test for b/177312785
TEST_P(QuicConnectionTest, PeerMigrateBeforeHandshakeConfirm) {
if (!VersionHasIetfQuicFrames(version().transport_version) ||
!GetQuicReloadableFlag(quic_start_peer_migration_earlier)) {
@@ -13294,12 +13295,22 @@
// Process another packet with a different peer address on server side will
// close connection.
+ QuicAckFrame frame = InitAckFrame(1);
EXPECT_CALL(visitor_, BeforeConnectionCloseSent());
EXPECT_CALL(visitor_,
OnConnectionClosed(_, ConnectionCloseSource::FROM_SELF));
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0u);
- ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
- kNewPeerAddress, ENCRYPTION_INITIAL);
+ if (!GetQuicReloadableFlag(quic_update_packet_content_returns_connected)) {
+ EXPECT_CALL(*send_algorithm_, OnCongestionEvent(_, _, _, _, _));
+ EXPECT_QUIC_BUG(
+ ProcessFramePacketWithAddresses(QuicFrame(&frame), kSelfAddress,
+ kNewPeerAddress, ENCRYPTION_INITIAL),
+ "");
+ } else {
+ EXPECT_CALL(*send_algorithm_, OnCongestionEvent(_, _, _, _, _)).Times(0);
+ ProcessFramePacketWithAddresses(QuicFrame(&frame), kSelfAddress,
+ kNewPeerAddress, ENCRYPTION_INITIAL);
+ }
EXPECT_FALSE(connection_.connected());
}
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h
index aa3b43d..73ae73e 100644
--- a/quic/core/quic_flags_list.h
+++ b/quic/core/quic_flags_list.h
@@ -67,6 +67,7 @@
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_tls_use_early_select_cert, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_tls_use_per_handshaker_proof_source, false)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_unified_iw_options, false)
+QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_update_packet_content_returns_connected, false)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_use_circular_deque_for_unacked_packets_v2, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_use_encryption_level_context, true)
QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_use_write_or_buffer_data_at_level, false)
