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(