In QuicDispatcher::CleanUpSession, Only serialize close packets if connection is self closed, i.e. not closed by peer.
Protected by FLAGS_quic_reloadable_flag_quic_dispatcher_only_serialize_close_if_closed_by_self.
PiperOrigin-RevId: 704698836
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h
index 9596fe3..b5047ca 100755
--- a/quiche/common/quiche_feature_flags_list.h
+++ b/quiche/common/quiche_feature_flags_list.h
@@ -31,6 +31,7 @@
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_disable_version_q046, false, true, "If true, disable QUIC version Q046.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_disable_version_rfcv1, false, false, "If true, disable QUIC version h3 (RFCv1).")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_discard_initial_packet_with_key_dropped, false, true, "If true, discard INITIAL packet if the key has been dropped.")
+QUICHE_FLAG(bool, quiche_reloadable_flag_quic_dispatcher_only_serialize_close_if_closed_by_self, false, false, "If true, QuicDispatcher::CleanUpSession only serializes a connection close if the connection is closed by self, did not complete handshake and does not have termination packets.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_ecn_in_first_ack, false, true, "When true, reports ECN in counts in the ACK of the a client initial that goes in the buffered packet store.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_enable_disable_resumption, true, true, "If true, disable resumption when receiving NRES connection option.")
QUICHE_FLAG(bool, quiche_reloadable_flag_quic_enable_mtu_discovery_at_server, false, false, "If true, QUIC will default enable MTU discovery at server, with a target of 1450 bytes.")
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc
index a2dd544..a679f71 100644
--- a/quiche/quic/core/http/end_to_end_test.cc
+++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -5228,6 +5228,10 @@
// Regression test for b/116200989.
TEST_P(EndToEndTest,
SendStatelessResetIfServerConnectionClosedLocallyDuringHandshake) {
+ SetQuicFlag(quic_allow_chlo_buffering, true);
+ SetQuicFlag(quic_dispatcher_max_ack_sent_per_connection, 1);
+ // Make the client hello to span 2 packets.
+ client_extra_copts_.push_back(kCHP1);
connect_to_server_on_initialize_ = false;
ASSERT_TRUE(Initialize());
@@ -5246,17 +5250,26 @@
return;
}
// Note: this writer will only used by the server connection, not the time
- // wait list.
+ // wait list. We start failing the write after the first packet, which is the
+ // ACK of the first CHLO packet sent by the dispatcher.
QuicDispatcherPeer::UseWriter(
dispatcher,
- // This cause the first server-sent packet, a.k.a REJ, to fail.
- new BadPacketWriter(/*packet_causing_write_error=*/0, EPERM));
+ // This cause the all server-sent packets to fail except the first one.
+ new BadPacketWriter(/*packet_causing_write_error=*/1, EPERM));
server_thread_->Resume();
client_.reset(CreateQuicClient(client_writer_));
EXPECT_EQ("", client_->SendSynchronousRequest("/foo"));
- EXPECT_THAT(client_->connection_error(),
- IsError(QUIC_HANDSHAKE_FAILED_SYNTHETIC_CONNECTION_CLOSE));
+
+ if (GetQuicReloadableFlag(
+ quic_dispatcher_only_serialize_close_if_closed_by_self)) {
+ EXPECT_THAT(client_->connection_error(),
+ IsError(QUIC_HANDSHAKE_FAILED_SYNTHETIC_CONNECTION_CLOSE));
+ } else {
+ EXPECT_THAT(client_->connection_error(),
+ testing::AnyOf(IsError(QUIC_NETWORK_IDLE_TIMEOUT),
+ IsError(QUIC_HANDSHAKE_TIMEOUT)));
+ }
}
// Regression test for b/116200989.
diff --git a/quiche/quic/core/quic_dispatcher.cc b/quiche/quic/core/quic_dispatcher.cc
index 40f8b11..cf9ce47 100644
--- a/quiche/quic/core/quic_dispatcher.cc
+++ b/quiche/quic/core/quic_dispatcher.cc
@@ -783,7 +783,7 @@
QuicConnection* connection,
QuicErrorCode /*error*/,
const std::string& /*error_details*/,
- ConnectionCloseSource /*source*/) {
+ ConnectionCloseSource source) {
write_blocked_list_.Remove(*connection);
QuicTimeWaitListManager::TimeWaitAction action =
QuicTimeWaitListManager::SEND_STATELESS_RESET;
@@ -792,26 +792,59 @@
termination_packets = connection->ConsumeTerminationPackets();
action = QuicTimeWaitListManager::SEND_CONNECTION_CLOSE_PACKETS;
} else {
- if (!connection->IsHandshakeComplete()) {
- // TODO(fayang): Do not serialize connection close packet if the
- // connection is closed by the client.
- QUIC_CODE_COUNT(quic_v44_add_to_time_wait_list_with_handshake_failed);
- // This serializes a connection close termination packet and adds the
- // connection to the time wait list.
- // TODO(b/359200165): Fix |last_sent_packet_number|.
- StatelessConnectionTerminator terminator(
- server_connection_id,
- connection->GetOriginalDestinationConnectionId(),
- connection->version(), /*last_sent_packet_number=*/QuicPacketNumber(),
- helper_.get(), time_wait_list_manager_.get());
- terminator.CloseConnection(
- QUIC_HANDSHAKE_FAILED_SYNTHETIC_CONNECTION_CLOSE,
- "Connection is closed by server before handshake confirmed",
- /*ietf_quic=*/true, connection->GetActiveServerConnectionIds());
- return;
+ if (GetQuicReloadableFlag(
+ quic_dispatcher_only_serialize_close_if_closed_by_self)) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(
+ quic_dispatcher_only_serialize_close_if_closed_by_self, 1, 2);
+ if (!connection->IsHandshakeComplete() &&
+ source == ConnectionCloseSource::FROM_SELF) {
+ // This counter used to be called
+ // `quic_v44_add_to_time_wait_list_with_handshake_failed`.
+ QUIC_CODE_COUNT(quic_add_to_time_wait_list_with_handshake_failed);
+ if (connection->sent_packet_manager()
+ .GetLargestSentPacket()
+ .IsInitialized()) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(
+ quic_dispatcher_only_serialize_close_if_closed_by_self, 2, 2);
+ }
+ // This serializes a connection close termination packet and adds the
+ // connection to the time wait list.
+ StatelessConnectionTerminator terminator(
+ server_connection_id,
+ connection->GetOriginalDestinationConnectionId(),
+ connection->version(),
+ connection->sent_packet_manager().GetLargestSentPacket(),
+ helper_.get(), time_wait_list_manager_.get());
+ terminator.CloseConnection(
+ QUIC_HANDSHAKE_FAILED_SYNTHETIC_CONNECTION_CLOSE,
+ "Connection is closed by server before handshake confirmed",
+ /*ietf_quic=*/true, connection->GetActiveServerConnectionIds());
+ return;
+ }
+ } else {
+ if (!connection->IsHandshakeComplete()) {
+ // TODO(fayang): Do not serialize connection close packet if the
+ // connection is closed by the client.
+ QUIC_CODE_COUNT(quic_v44_add_to_time_wait_list_with_handshake_failed);
+ // This serializes a connection close termination packet and adds the
+ // connection to the time wait list.
+ // TODO(b/359200165): Fix |last_sent_packet_number|.
+ StatelessConnectionTerminator terminator(
+ server_connection_id,
+ connection->GetOriginalDestinationConnectionId(),
+ connection->version(),
+ /*last_sent_packet_number=*/QuicPacketNumber(), helper_.get(),
+ time_wait_list_manager_.get());
+ terminator.CloseConnection(
+ QUIC_HANDSHAKE_FAILED_SYNTHETIC_CONNECTION_CLOSE,
+ "Connection is closed by server before handshake confirmed",
+ /*ietf_quic=*/true, connection->GetActiveServerConnectionIds());
+ return;
+ }
}
QUIC_CODE_COUNT(quic_v44_add_to_time_wait_list_with_stateless_reset);
}
+
time_wait_list_manager_->AddConnectionIdToTimeWait(
action,
TimeWaitConnectionInfo(