Deprecate --gfe2_reloadable_flag_quic_pass_path_response_to_validator. PiperOrigin-RevId: 436769765
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc index b1b9b4f..2a0603d 100644 --- a/quic/core/http/end_to_end_test.cc +++ b/quic/core/http/end_to_end_test.cc
@@ -5332,8 +5332,7 @@ TEST_P(EndToEndPacketReorderingTest, ReorderedPathChallenge) { ASSERT_TRUE(Initialize()); - if (!version_.HasIetfQuicFrames() || - !client_->client()->session()->connection()->use_path_validator()) { + if (!version_.HasIetfQuicFrames()) { return; } client_.reset(EndToEndTest::CreateQuicClient(nullptr)); @@ -5393,8 +5392,7 @@ TEST_P(EndToEndPacketReorderingTest, PathValidationFailure) { ASSERT_TRUE(Initialize()); - if (!version_.HasIetfQuicFrames() || - !client_->client()->session()->connection()->use_path_validator()) { + if (!version_.HasIetfQuicFrames()) { return; }
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index 99441b6..5dfff49 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -625,7 +625,7 @@ QUIC_RELOADABLE_FLAG_COUNT( quic_remove_connection_migration_connection_option); } - if (framer_.version().HasIetfQuicFrames() && use_path_validator_ && + if (framer_.version().HasIetfQuicFrames() && count_bytes_on_alternative_path_separately_ && GetQuicReloadableFlag(quic_server_reverse_validate_new_path3) && (remove_connection_migration_connection_option || @@ -1751,19 +1751,8 @@ debug_visitor_->OnPathResponseFrame(frame); } MaybeUpdateAckTimeout(); - if (use_path_validator_) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_pass_path_response_to_validator, 1, 4); path_validator_.OnPathResponse( frame.data_buffer, last_received_packet_info_.destination_address); - } else { - if (!transmitted_connectivity_probe_payload_ || - *transmitted_connectivity_probe_payload_ != frame.data_buffer) { - // Is not for the probe we sent, ignore it. - return true; - } - // Have received the matching PATH RESPONSE, saved payload no longer valid. - transmitted_connectivity_probe_payload_ = nullptr; - } return connected_; } @@ -2248,8 +2237,6 @@ QUICHE_DCHECK(version().HasIetfInvariantHeader()); QUICHE_DCHECK_EQ(perspective_, Perspective::IS_CLIENT); - if (use_path_validator_) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_pass_path_response_to_validator, 4, 4); if (!IsDefaultPath(last_received_packet_info_.destination_address, last_received_packet_info_.source_address)) { // This packet is received on a probing path. Do not close connection. @@ -2265,12 +2252,6 @@ } return; } - } else if (!visitor_->ValidateStatelessReset( - last_received_packet_info_.destination_address, - last_received_packet_info_.source_address)) { - // This packet is received on a probing path. Do not close connection. - return; - } const std::string error_details = "Received stateless reset."; QUIC_CODE_COUNT(quic_tear_down_local_connection_on_stateless_reset); @@ -3537,8 +3518,7 @@ // The write failed, but the writer is not blocked, so return true. return true; } - if (use_path_validator_ && !send_on_current_path) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_pass_path_response_to_validator, 2, 4); + if (!send_on_current_path) { // Only handle MSG_TOO_BIG as error on current path. return true; } @@ -4602,10 +4582,8 @@ // Cancel the alarms so they don't trigger any action now that the // connection is closed. CancelAllAlarms(); - if (use_path_validator_) { - QUIC_RELOADABLE_FLAG_COUNT_N(quic_pass_path_response_to_validator, 3, 4); CancelPathValidation(); - } + peer_issued_cid_manager_.reset(); self_issued_cid_manager_.reset(); } @@ -4997,16 +4975,12 @@ } else { // IETF QUIC path challenge. // Send a path probe request using IETF QUIC PATH_CHALLENGE frame. - transmitted_connectivity_probe_payload_ = - std::make_unique<QuicPathFrameBuffer>(); - random_generator_->RandBytes(transmitted_connectivity_probe_payload_.get(), + QuicPathFrameBuffer transmitted_connectivity_probe_payload; + random_generator_->RandBytes(&transmitted_connectivity_probe_payload, sizeof(QuicPathFrameBuffer)); probing_packet = packet_creator_.SerializePathChallengeConnectivityProbingPacket( - *transmitted_connectivity_probe_payload_); - if (!probing_packet) { - transmitted_connectivity_probe_payload_ = nullptr; - } + transmitted_connectivity_probe_payload); } QUICHE_DCHECK_EQ(IsRetransmittable(*probing_packet), NO_RETRANSMITTABLE_DATA); return WritePacketUsingWriter(std::move(probing_packet), probing_writer, @@ -6465,7 +6439,6 @@ void QuicConnection::ValidatePath( std::unique_ptr<QuicPathValidationContext> context, std::unique_ptr<QuicPathValidator::ResultDelegate> result_delegate) { - QUICHE_DCHECK(use_path_validator_); if (!connection_migration_use_new_cid_ && perspective_ == Perspective::IS_CLIENT && !IsDefaultPath(context->self_address(), context->peer_address())) { @@ -6582,17 +6555,14 @@ } bool QuicConnection::HasPendingPathValidation() const { - QUICHE_DCHECK(use_path_validator_); return path_validator_.HasPendingPathValidation(); } QuicPathValidationContext* QuicConnection::GetPathValidationContext() const { - QUICHE_DCHECK(use_path_validator_); return path_validator_.GetContext(); } void QuicConnection::CancelPathValidation() { - QUICHE_DCHECK(use_path_validator_); path_validator_.CancelPathValidation(); }
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index ebeed1c..0f7cac3 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -141,12 +141,6 @@ // bandwidth. Returns true if data was sent, false otherwise. virtual bool SendProbingData() = 0; - // Called when stateless reset packet is received. Returns true if the - // connection needs to be closed. - virtual bool ValidateStatelessReset( - const quic::QuicSocketAddress& self_address, - const quic::QuicSocketAddress& peer_address) = 0; - // Called when the connection experiences a change in congestion window. virtual void OnCongestionWindowChange(QuicTime now) = 0; @@ -1153,8 +1147,6 @@ // Can only be set if this is a client connection. void EnableLegacyVersionEncapsulation(const std::string& server_name); - bool use_path_validator() const { return use_path_validator_; } - // If now is close to idle timeout, returns true and sends a connectivity // probing packet to test the connection for liveness. Otherwise, returns // false. @@ -2145,12 +2137,6 @@ // Time this connection can release packets into the future. QuicTime::Delta release_time_into_future_; - // Payload of most recently transmitted IETF QUIC connectivity - // probe packet (the PATH_CHALLENGE payload). This implementation transmits - // only one PATH_CHALLENGE per connectivity probe, so only one - // QuicPathFrameBuffer is needed. - std::unique_ptr<QuicPathFrameBuffer> transmitted_connectivity_probe_payload_; - // Payloads that were received in the most recent probe. This needs to be a // Deque because the peer might no be using this implementation, and others // might send a packet with more than one PATH_CHALLENGE, so all need to be @@ -2209,9 +2195,6 @@ size_t anti_amplification_factor_ = GetQuicFlag(FLAGS_quic_anti_amplification_factor); - bool use_path_validator_ = - GetQuicReloadableFlag(quic_pass_path_response_to_validator); - // True if AckFrequencyFrame is supported. bool can_receive_ack_frequency_frame_ = false;
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index 86da486..2716e00 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -2499,7 +2499,6 @@ EXPECT_EQ(kPeerAddress, connection_.peer_address()); EXPECT_EQ(kPeerAddress, connection_.effective_peer_address()); if (GetParam().version.HasIetfQuicFrames() && - connection_.use_path_validator() && GetQuicReloadableFlag(quic_count_bytes_on_alternative_path_seperately)) { QuicByteCount bytes_sent = QuicConnectionPeer::BytesSentOnAlternativePath(&connection_); @@ -6955,9 +6954,6 @@ kTestStatelessResetToken)); std::unique_ptr<QuicReceivedPacket> received( ConstructReceivedPacket(*packet, QuicTime::Zero())); - if (!connection_.use_path_validator()) { - EXPECT_CALL(visitor_, ValidateStatelessReset(_, _)).WillOnce(Return(true)); - } EXPECT_CALL(visitor_, OnConnectionClosed(_, ConnectionCloseSource::FROM_PEER)) .WillOnce(Invoke(this, &QuicConnectionTest::SaveConnectionCloseFrame)); connection_.ProcessUdpPacket(kSelfAddress, kPeerAddress, *received); @@ -8894,8 +8890,7 @@ } TEST_P(QuicConnectionTest, ClientResponseToPathChallengeOnAlternativeSocket) { - if (!VersionHasIetfQuicFrames(connection_.version().transport_version) || - !connection_.use_path_validator()) { + if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) { return; } PathProbeTestInit(Perspective::IS_CLIENT); @@ -11891,8 +11886,7 @@ } TEST_P(QuicConnectionTest, PathValidationOnNewSocketSuccess) { - if (!VersionHasIetfQuicFrames(connection_.version().transport_version) || - !connection_.use_path_validator()) { + if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) { return; } PathProbeTestInit(Perspective::IS_CLIENT); @@ -11925,8 +11919,7 @@ } TEST_P(QuicConnectionTest, NewPathValidationCancelsPreviousOne) { - if (!VersionHasIetfQuicFrames(connection_.version().transport_version) || - !connection_.use_path_validator()) { + if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) { return; } PathProbeTestInit(Perspective::IS_CLIENT); @@ -11984,8 +11977,7 @@ // Regression test for b/182571515. TEST_P(QuicConnectionTest, PathValidationRetry) { - if (!VersionHasIetfQuicFrames(connection_.version().transport_version) || - !connection_.use_path_validator()) { + if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) { return; } PathProbeTestInit(Perspective::IS_CLIENT); @@ -12017,8 +12009,7 @@ } TEST_P(QuicConnectionTest, PathValidationReceivesStatelessReset) { - if (!VersionHasIetfQuicFrames(connection_.version().transport_version) || - !connection_.use_path_validator()) { + if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) { return; } PathProbeTestInit(Perspective::IS_CLIENT); @@ -12104,8 +12095,7 @@ // Tests that PATH_CHALLENGE is dropped if it is sent via the default writer // and the writer is blocked. TEST_P(QuicConnectionTest, SendPathChallengeUsingBlockedDefaultSocket) { - if (!VersionHasIetfQuicFrames(connection_.version().transport_version) || - !connection_.use_path_validator()) { + if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) { return; } PathProbeTestInit(Perspective::IS_SERVER); @@ -12178,8 +12168,7 @@ // Tests that write error on the alternate socket should be ignored. TEST_P(QuicConnectionTest, SendPathChallengeFailOnNewSocket) { - if (!VersionHasIetfQuicFrames(connection_.version().transport_version) || - !connection_.use_path_validator()) { + if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) { return; } PathProbeTestInit(Perspective::IS_CLIENT); @@ -12210,8 +12199,7 @@ // Tests that write error while sending PATH_CHALLANGE from the default socket // should close the connection. TEST_P(QuicConnectionTest, SendPathChallengeFailOnDefaultPath) { - if (!VersionHasIetfQuicFrames(connection_.version().transport_version) || - !connection_.use_path_validator()) { + if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) { return; } PathProbeTestInit(Perspective::IS_CLIENT); @@ -12245,8 +12233,7 @@ } TEST_P(QuicConnectionTest, SendPathChallengeFailOnAlternativePeerAddress) { - if (!VersionHasIetfQuicFrames(connection_.version().transport_version) || - !connection_.use_path_validator()) { + if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) { return; } PathProbeTestInit(Perspective::IS_CLIENT); @@ -12277,8 +12264,7 @@ TEST_P(QuicConnectionTest, SendPathChallengeFailPacketTooBigOnAlternativePeerAddress) { - if (!VersionHasIetfQuicFrames(connection_.version().transport_version) || - !connection_.use_path_validator()) { + if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) { return; } PathProbeTestInit(Perspective::IS_CLIENT); @@ -13471,8 +13457,7 @@ } TEST_P(QuicConnectionTest, MigrateToNewPathDuringProbing) { - if (!VersionHasIetfQuicFrames(connection_.version().transport_version) || - !connection_.use_path_validator()) { + if (!VersionHasIetfQuicFrames(connection_.version().transport_version)) { return; } PathProbeTestInit(Perspective::IS_CLIENT); @@ -14473,7 +14458,7 @@ TEST_P( QuicConnectionTest, ReplacePeerIssuedConnectionIdOnBothPathsTriggeredByNewConnectionIdFrame) { - if (!version().HasIetfQuicFrames() || !connection_.use_path_validator() || + if (!version().HasIetfQuicFrames() || !connection_.count_bytes_on_alternative_path_separately()) { return; }
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h index 90def64..cb521fc 100644 --- a/quic/core/quic_flags_list.h +++ b/quic/core/quic_flags_list.h
@@ -73,8 +73,6 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_ignore_max_push_id, true) // If true, include stream information in idle timeout connection close detail. QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_add_stream_info_to_idle_close_detail, true) -// If true, pass the received PATH_RESPONSE payload to path validator to move forward the path validation. -QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_pass_path_response_to_validator, true) // If true, quic server will send ENABLE_CONNECT_PROTOCOL setting and and endpoint will validate required request/response headers and extended CONNECT mechanism and update code counts of valid/invalid headers. QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_verify_request_headers_2, true) // If true, record addresses that server has sent reset to recently, and do not send reset if the address lives in the set.
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h index b8f175e..dd1fa42 100644 --- a/quic/core/quic_session.h +++ b/quic/core/quic_session.h
@@ -139,11 +139,6 @@ bool is_connectivity_probe) override; void OnCanWrite() override; bool SendProbingData() override; - bool ValidateStatelessReset( - const quic::QuicSocketAddress& /*self_address*/, - const quic::QuicSocketAddress& /*peer_address*/) override { - return true; - } void OnCongestionWindowChange(QuicTime /*now*/) override {} void OnConnectionMigration(AddressChangeType /*type*/) override {} // Adds a connection level WINDOW_UPDATE frame.
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h index 82262eb..ddbc9b9 100644 --- a/quic/test_tools/quic_test_utils.h +++ b/quic/test_tools/quic_test_utils.h
@@ -454,9 +454,6 @@ MOCK_METHOD(void, OnWriteBlocked, (), (override)); MOCK_METHOD(void, OnCanWrite, (), (override)); MOCK_METHOD(bool, SendProbingData, (), (override)); - MOCK_METHOD(bool, ValidateStatelessReset, - (const quic::QuicSocketAddress&, const quic::QuicSocketAddress&), - (override)); MOCK_METHOD(void, OnCongestionWindowChange, (QuicTime now), (override)); MOCK_METHOD(void, OnConnectionMigration, (AddressChangeType type), (override));
diff --git a/quic/test_tools/simulator/quic_endpoint.h b/quic/test_tools/simulator/quic_endpoint.h index 7fc4d87..136410f 100644 --- a/quic/test_tools/simulator/quic_endpoint.h +++ b/quic/test_tools/simulator/quic_endpoint.h
@@ -51,11 +51,6 @@ void OnCryptoFrame(const QuicCryptoFrame& frame) override; void OnCanWrite() override; bool SendProbingData() override; - bool ValidateStatelessReset( - const quic::QuicSocketAddress& /*self_address*/, - const quic::QuicSocketAddress& /*peer_address*/) override { - return true; - } bool WillingAndAbleToWrite() const override; bool ShouldKeepConnectionAlive() const override;
diff --git a/quic/tools/quic_client_base.cc b/quic/tools/quic_client_base.cc index e11e25b..e26f08c 100644 --- a/quic/tools/quic_client_base.cc +++ b/quic/tools/quic_client_base.cc
@@ -289,8 +289,7 @@ bool QuicClientBase::ValidateAndMigrateSocket(const QuicIpAddress& new_host) { QUICHE_DCHECK(VersionHasIetfQuicFrames( - session_->connection()->version().transport_version) && - session_->connection()->use_path_validator()); + session_->connection()->version().transport_version)); if (!connected()) { return false; }