Automated g4 rollback of changelist 437128224. *** Reason for rollback *** Roll forward the original CL/436769765 as the chromium merge conflict is resolved by disabling gQUIC migration and deprecating the relevant tests, tracked in b/227344861 *** Original change description *** Automated g4 rollback of changelist 436769765. *** Reason for rollback *** Blocking merge because there are several tests with this flag disabled. *** Original change description *** Deprecate --gfe2_reloadable_flag_quic_pass_path_response_to_validator. *** *** PiperOrigin-RevId: 440999512
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc index e0bfbb9..fdf0f5b 100644 --- a/quiche/quic/core/http/end_to_end_test.cc +++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -5231,8 +5231,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)); @@ -5292,8 +5291,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/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index c76ade9..42341f3 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -629,7 +629,7 @@ QUIC_RELOADABLE_FLAG_COUNT( quic_remove_connection_migration_connection_option_v2); } - 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 || @@ -1757,19 +1757,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; - } + path_validator_.OnPathResponse( + frame.data_buffer, last_received_packet_info_.destination_address); return connected_; } @@ -2255,27 +2244,19 @@ 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. - if (IsAlternativePath(last_received_packet_info_.destination_address, - GetEffectivePeerAddressFromCurrentPacket())) { - QUIC_BUG_IF(quic_bug_12714_18, alternative_path_.validated) - << "STATELESS_RESET received on alternate path after it's " - "validated."; - path_validator_.CancelPathValidation(); - } else { - QUIC_BUG(quic_bug_10511_17) - << "Received Stateless Reset on unknown socket."; - } - return; - } - } else if (!visitor_->ValidateStatelessReset( - last_received_packet_info_.destination_address, - last_received_packet_info_.source_address)) { + 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. + if (IsAlternativePath(last_received_packet_info_.destination_address, + GetEffectivePeerAddressFromCurrentPacket())) { + QUIC_BUG_IF(quic_bug_12714_18, alternative_path_.validated) + << "STATELESS_RESET received on alternate path after it's " + "validated."; + path_validator_.CancelPathValidation(); + } else { + QUIC_BUG(quic_bug_10511_17) + << "Received Stateless Reset on unknown socket."; + } return; } @@ -3544,8 +3525,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; } @@ -4632,10 +4612,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(); - } + CancelPathValidation(); + peer_issued_cid_manager_.reset(); self_issued_cid_manager_.reset(); } @@ -5068,16 +5046,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, @@ -6538,7 +6512,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())) { @@ -6655,17 +6628,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/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h index 23b78be..7577a57 100644 --- a/quiche/quic/core/quic_connection.h +++ b/quiche/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/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index cce10e7..50ae2f1 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -2497,7 +2497,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_); @@ -6951,9 +6950,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); @@ -8925,8 +8921,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); @@ -11922,8 +11917,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); @@ -11956,8 +11950,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); @@ -12015,8 +12008,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); @@ -12048,8 +12040,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); @@ -12135,8 +12126,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); @@ -12209,8 +12199,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); @@ -12241,8 +12230,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); @@ -12276,8 +12264,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); @@ -12308,8 +12295,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); @@ -13499,8 +13485,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); @@ -14501,7 +14486,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/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h index 609734e..edc9ee5 100644 --- a/quiche/quic/core/quic_flags_list.h +++ b/quiche/quic/core/quic_flags_list.h
@@ -73,8 +73,6 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_add_stream_info_to_idle_close_detail, true) // If true, limit the size of HPACK encoder dynamic table to 16 kB. Only affects gQUIC; QPACK encoder dynamic table size used in IETF QUIC is already bounded. QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_limit_encoder_dynamic_table_size, 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/quiche/quic/core/quic_session.h b/quiche/quic/core/quic_session.h index a5aae44..3051cf6 100644 --- a/quiche/quic/core/quic_session.h +++ b/quiche/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/quiche/quic/test_tools/quic_test_utils.h b/quiche/quic/test_tools/quic_test_utils.h index 80a88d1..a26349b 100644 --- a/quiche/quic/test_tools/quic_test_utils.h +++ b/quiche/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/quiche/quic/test_tools/simulator/quic_endpoint.h b/quiche/quic/test_tools/simulator/quic_endpoint.h index e29d594..6ce7b57 100644 --- a/quiche/quic/test_tools/simulator/quic_endpoint.h +++ b/quiche/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/quiche/quic/tools/quic_client_base.cc b/quiche/quic/tools/quic_client_base.cc index 8cabb60..63c604d 100644 --- a/quiche/quic/tools/quic_client_base.cc +++ b/quiche/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; }