Remove `QuicConnection::GetEffectivePeerAddressFromCurrentPacket`, it was only used by the deprecated UDP proxy. The change is mechanically created by: - Remove the `GetEffectivePeerAddressFromCurrentPacket` function. - Remove tests using it. - Change all callers of `GetEffectivePeerAddressFromCurrentPacket` to use `last_received_packet_info_.source_address` directly. PiperOrigin-RevId: 977915771
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index 6f2240c..fe84062 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -1251,12 +1251,6 @@ visitor_->OnPacketDecrypted(level); } -QuicSocketAddress QuicConnection::GetEffectivePeerAddressFromCurrentPacket() - const { - // By default, the connection is not proxied, and the effective peer address - // is the packet's source address, i.e. the direct peer address. - return last_received_packet_info_.source_address; -} bool QuicConnection::OnPacketHeader(const QuicPacketHeader& header) { if (spin_bit_enabled_ && header.form == IETF_QUIC_SHORT_HEADER_PACKET) { @@ -1349,7 +1343,7 @@ // TODO(fayang): only change peer addresses in application data packet // number space. UpdatePeerAddress(last_received_packet_info_.source_address); - default_path_.peer_address = GetEffectivePeerAddressFromCurrentPacket(); + default_path_.peer_address = last_received_packet_info_.source_address; } } } else { @@ -1367,10 +1361,10 @@ current_effective_peer_migration_type_ = QuicUtils::DetermineAddressChangeType( default_path_.peer_address, - GetEffectivePeerAddressFromCurrentPacket()); + last_received_packet_info_.source_address); if (version().IsIetfQuic()) { - auto effective_peer_address = GetEffectivePeerAddressFromCurrentPacket(); + auto effective_peer_address = last_received_packet_info_.source_address; // Since server does not send new connection ID to client before handshake // completion and source connection ID is omitted in short header packet, // the server_connection_id on PathState on the server side does not @@ -1409,7 +1403,7 @@ QUIC_DLOG_IF(INFO, current_effective_peer_migration_type_ != NO_CHANGE) << ENDPOINT << "Effective peer's ip:port changed from " << default_path_.peer_address.ToString() << " to " - << GetEffectivePeerAddressFromCurrentPacket().ToString() + << last_received_packet_info_.source_address.ToString() << ", active_effective_peer_migration_type is " << active_effective_peer_migration_type_; } @@ -1830,7 +1824,7 @@ const QuicSocketAddress effective_peer_address_to_respond = perspective_ == Perspective::IS_CLIENT ? effective_peer_address() - : GetEffectivePeerAddressFromCurrentPacket(); + : last_received_packet_info_.source_address; const QuicSocketAddress direct_peer_address_to_respond = perspective_ == Perspective::IS_CLIENT ? direct_peer_address_ @@ -2393,7 +2387,7 @@ token, *default_path_.stateless_reset_token); } if (IsAlternativePath(last_received_packet_info_.destination_address, - GetEffectivePeerAddressFromCurrentPacket())) { + last_received_packet_info_.source_address)) { QUIC_RELOADABLE_FLAG_COUNT_N(quic_check_alternate_reset_token, 2, 2); return alternative_path_.stateless_reset_token.has_value() && QuicUtils::AreStatelessResetTokensEqual( @@ -2416,7 +2410,7 @@ 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())) { + last_received_packet_info_.source_address)) { QUIC_BUG_IF(quic_bug_12714_18, alternative_path_.validated) << "STATELESS_RESET received on alternate path after it's " "validated."; @@ -2880,7 +2874,7 @@ if (!default_path_.peer_address.IsInitialized()) { const QuicSocketAddress effective_peer_addr = - GetEffectivePeerAddressFromCurrentPacket(); + last_received_packet_info_.source_address; // The default path peer_address must be initialized at the beginning of the // first packet processed(here). If effective_peer_addr is uninitialized, @@ -3201,8 +3195,7 @@ if (perspective_ == Perspective::IS_SERVER && last_received_packet_info_.actual_destination_address.IsInitialized() && !IsHandshakeConfirmed() && - GetEffectivePeerAddressFromCurrentPacket() != - default_path_.peer_address) { + last_received_packet_info_.source_address != default_path_.peer_address) { // Our client implementation has an optimization to spray packets from // different sockets to the server's preferred address before handshake // gets confirmed. In this case, do not kick off client address migration @@ -5511,13 +5504,13 @@ QUIC_DLOG(INFO) << ENDPOINT << "Effective peer's ip:port changed from " << default_path_.peer_address.ToString() << " to " - << GetEffectivePeerAddressFromCurrentPacket().ToString() + << last_received_packet_info_.source_address.ToString() << ", address change type is " << type << ", migrating connection without validating new client address."; highest_packet_sent_before_effective_peer_migration_ = sent_packet_manager_.GetLargestSentPacket(); - default_path_.peer_address = GetEffectivePeerAddressFromCurrentPacket(); + default_path_.peer_address = last_received_packet_info_.source_address; active_effective_peer_migration_type_ = type; OnConnectionMigration(); @@ -5548,7 +5541,7 @@ // is validated or not and which path the incoming packet is on. const QuicSocketAddress current_effective_peer_address = - GetEffectivePeerAddressFromCurrentPacket(); + last_received_packet_info_.source_address; QUIC_DLOG(INFO) << ENDPOINT << "Effective peer's ip:port changed from " << default_path_.peer_address.ToString() << " to " << current_effective_peer_address.ToString() @@ -5783,7 +5776,7 @@ return connected_; } QuicSocketAddress current_effective_peer_address = - GetEffectivePeerAddressFromCurrentPacket(); + last_received_packet_info_.source_address; if (IsDefaultPath(last_received_packet_info_.destination_address, last_received_packet_info_.source_address)) { return connected_; @@ -5852,7 +5845,7 @@ QUIC_LOG_EVERY_N_SEC(INFO, 60) << ENDPOINT << "Effective peer's ip:port changed from " << default_path_.peer_address.ToString() << " to " - << GetEffectivePeerAddressFromCurrentPacket().ToString() + << last_received_packet_info_.source_address.ToString() << " before handshake confirmed, " "current_effective_peer_migration_type_: " << current_effective_peer_migration_type_; @@ -5864,7 +5857,7 @@ absl::StrFormat( "Peer address changed from %s to %s before handshake is confirmed.", default_path_.peer_address.ToString(), - GetEffectivePeerAddressFromCurrentPacket().ToString()), + last_received_packet_info_.source_address.ToString()), ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); return; } @@ -7326,14 +7319,14 @@ QuicByteCount received_packet_size) { if (!version().IsIetfQuic() || perspective_ != Perspective::IS_SERVER || !IsAlternativePath(last_received_packet_info_.destination_address, - GetEffectivePeerAddressFromCurrentPacket()) || + last_received_packet_info_.source_address) || last_received_packet_info_.received_bytes_counted) { return; } // Only update bytes received if this probing frame is received on the most // recent alternative path. QUICHE_DCHECK(!IsDefaultPath(last_received_packet_info_.destination_address, - GetEffectivePeerAddressFromCurrentPacket())); + last_received_packet_info_.source_address)); if (!alternative_path_.validated) { alternative_path_.bytes_received_before_address_validation += received_packet_size; @@ -7402,7 +7395,7 @@ bool QuicConnection::IsReceivedPeerAddressValidated() const { QuicSocketAddress current_effective_peer_address = - GetEffectivePeerAddressFromCurrentPacket(); + last_received_packet_info_.source_address; QUICHE_DCHECK(current_effective_peer_address.IsInitialized()); return (alternative_path_.peer_address.host() == current_effective_peer_address.host() &&
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h index 80f4104..1b1cc1a 100644 --- a/quiche/quic/core/quic_connection.h +++ b/quiche/quic/core/quic_connection.h
@@ -1553,24 +1553,6 @@ // Called when a effective peer address migration is validated. virtual void OnEffectivePeerMigrationValidated(bool is_migration_linkable); - // Get the effective peer address from the packet being processed. For proxied - // connections, effective peer address is the address of the endpoint behind - // the proxy. For non-proxied connections, effective peer address is the same - // as peer address. - // - // Notes for implementations in subclasses: - // - If the connection is not proxied, the overridden method should use the - // base implementation: - // - // return QuicConnection::GetEffectivePeerAddressFromCurrentPacket(); - // - // - If the connection is proxied, the overridden method may return either of - // the following: - // a) The address of the endpoint behind the proxy. The address is used to - // drive effective peer migration. - // b) An uninitialized address, meaning the effective peer address does not - // change. - virtual QuicSocketAddress GetEffectivePeerAddressFromCurrentPacket() const; AddressChangeType active_effective_peer_migration_type() const { return active_effective_peer_migration_type_;
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index bfea0bc..9afc888 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -515,10 +515,6 @@ void set_notifier(SimpleSessionNotifier* notifier) { notifier_ = notifier; } - void ReturnEffectivePeerAddressForNextPacket(const QuicSocketAddress& addr) { - next_effective_peer_addr_ = std::make_unique<QuicSocketAddress>(addr); - } - void SendOrQueuePacket(SerializedPacket packet) override { QuicConnection::SendOrQueuePacket(std::move(packet)); self_address_on_default_path_while_sending_packet_ = self_address(); @@ -533,14 +529,6 @@ using QuicConnection::active_effective_peer_migration_type; using QuicConnection::set_defer_send_in_response_to_packets; - protected: - QuicSocketAddress GetEffectivePeerAddressFromCurrentPacket() const override { - if (next_effective_peer_addr_) { - return *std::move(next_effective_peer_addr_); - } - return QuicConnection::GetEffectivePeerAddressFromCurrentPacket(); - } - private: TestPacketWriter* writer() { return absl::down_cast<TestPacketWriter*>(QuicConnection::writer()); @@ -550,8 +538,6 @@ SimpleSessionNotifier* notifier_; - std::unique_ptr<QuicSocketAddress> next_effective_peer_addr_; - QuicSocketAddress self_address_on_default_path_while_sending_packet_; uint32_t num_unlinkable_client_migration_ = 0; @@ -2096,128 +2082,6 @@ EXPECT_EQ(2u, writer_->packets_write_attempts()); } -TEST_P(QuicConnectionTest, EffectivePeerAddressChangeAtServer) { - if (GetQuicFlag(quic_enforce_strict_amplification_factor)) { - return; - } - set_perspective(Perspective::IS_SERVER); - QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false); - EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective()); - if (version().IsIetfQuic()) { - QuicConnectionPeer::SetAddressValidated(&connection_); - } - connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); - // Discard INITIAL key. - connection_.RemoveEncrypter(ENCRYPTION_INITIAL); - connection_.NeuterUnencryptedPackets(); - EXPECT_CALL(visitor_, GetHandshakeState()) - .WillRepeatedly(Return(HANDSHAKE_CONFIRMED)); - - // Clear direct_peer_address. - QuicConnectionPeer::SetDirectPeerAddress(&connection_, QuicSocketAddress()); - // Clear effective_peer_address, it is different from direct_peer_address for - // this test. - QuicConnectionPeer::SetEffectivePeerAddress(&connection_, - QuicSocketAddress()); - const QuicSocketAddress kEffectivePeerAddress = - QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/43210); - connection_.ReturnEffectivePeerAddressForNextPacket(kEffectivePeerAddress); - - if (VersionIsIetfQuic(connection_.transport_version())) { - EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber()); - } else { - EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber()); - } - ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress, kPeerAddress, - ENCRYPTION_FORWARD_SECURE); - EXPECT_EQ(kPeerAddress, connection_.peer_address()); - EXPECT_EQ(kEffectivePeerAddress, connection_.effective_peer_address()); - - // Process another packet with the same direct peer address and different - // effective peer address on server side will start connection migration. - const QuicSocketAddress kNewEffectivePeerAddress = - QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/54321); - connection_.ReturnEffectivePeerAddressForNextPacket(kNewEffectivePeerAddress); - EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(1); - ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress, kPeerAddress, - ENCRYPTION_FORWARD_SECURE); - EXPECT_EQ(kPeerAddress, connection_.peer_address()); - EXPECT_EQ(kNewEffectivePeerAddress, connection_.effective_peer_address()); - EXPECT_EQ(kPeerAddress, writer_->last_write_peer_address()); - if (GetParam().version.IsIetfQuic()) { - EXPECT_EQ(NO_CHANGE, connection_.active_effective_peer_migration_type()); - EXPECT_EQ(1u, connection_.GetStats().num_validated_peer_migration); - EXPECT_EQ(1u, connection_.num_linkable_client_migration()); - } - - // Process another packet with a different direct peer address and the same - // effective peer address on server side will not start connection migration. - const QuicSocketAddress kNewPeerAddress = - QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/23456); - connection_.ReturnEffectivePeerAddressForNextPacket(kNewEffectivePeerAddress); - EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(0); - - if (!GetParam().version.IsIetfQuic()) { - // ack_frame is used to complete the migration started by the last packet, - // we need to make sure a new migration does not start after the previous - // one is completed. - QuicAckFrame ack_frame = InitAckFrame(1); - EXPECT_CALL(*send_algorithm_, OnCongestionEvent(_, _, _, _, _, _, _)); - ProcessFramePacketWithAddresses(QuicFrame(&ack_frame), kSelfAddress, - kNewPeerAddress, ENCRYPTION_FORWARD_SECURE); - EXPECT_EQ(kNewPeerAddress, connection_.peer_address()); - EXPECT_EQ(kNewEffectivePeerAddress, connection_.effective_peer_address()); - EXPECT_EQ(NO_CHANGE, connection_.active_effective_peer_migration_type()); - } - - // Process another packet with different direct peer address and different - // effective peer address on server side will start connection migration. - const QuicSocketAddress kNewerEffectivePeerAddress = - QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/65432); - const QuicSocketAddress kFinalPeerAddress = - QuicSocketAddress(QuicIpAddress::Loopback6(), /*port=*/34567); - connection_.ReturnEffectivePeerAddressForNextPacket( - kNewerEffectivePeerAddress); - EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(1); - ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress, - kFinalPeerAddress, ENCRYPTION_FORWARD_SECURE); - EXPECT_EQ(kFinalPeerAddress, connection_.peer_address()); - EXPECT_EQ(kNewerEffectivePeerAddress, connection_.effective_peer_address()); - if (GetParam().version.IsIetfQuic()) { - EXPECT_EQ(NO_CHANGE, connection_.active_effective_peer_migration_type()); - EXPECT_EQ(send_algorithm_, - connection_.sent_packet_manager().GetSendAlgorithm()); - EXPECT_EQ(2u, connection_.GetStats().num_validated_peer_migration); - } - - // While the previous migration is ongoing, process another packet with the - // same direct peer address and different effective peer address on server - // side will start a new connection migration. - const QuicSocketAddress kNewestEffectivePeerAddress = - QuicSocketAddress(QuicIpAddress::Loopback4(), /*port=*/65430); - connection_.ReturnEffectivePeerAddressForNextPacket( - kNewestEffectivePeerAddress); - EXPECT_CALL(visitor_, OnConnectionMigration(IPV6_TO_IPV4_CHANGE)).Times(1); - if (!GetParam().version.IsIetfQuic()) { - EXPECT_CALL(*send_algorithm_, OnConnectionMigration()).Times(1); - } - ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress, - kFinalPeerAddress, ENCRYPTION_FORWARD_SECURE); - EXPECT_EQ(kFinalPeerAddress, connection_.peer_address()); - EXPECT_EQ(kNewestEffectivePeerAddress, connection_.effective_peer_address()); - EXPECT_EQ(IPV6_TO_IPV4_CHANGE, - connection_.active_effective_peer_migration_type()); - if (GetParam().version.IsIetfQuic()) { - EXPECT_NE(send_algorithm_, - connection_.sent_packet_manager().GetSendAlgorithm()); - EXPECT_EQ(kFinalPeerAddress, writer_->last_write_peer_address()); - EXPECT_FALSE(writer_->path_challenge_frames().empty()); - EXPECT_EQ(0u, connection_.GetStats() - .num_peer_migration_while_validating_default_path); - EXPECT_TRUE(connection_.HasPendingPathValidation()); - } -} - // Regression test for b/200020764. TEST_P(QuicConnectionTest, ConnectionMigrationWithPendingPaddingBytes) { // TODO(haoyuewang) Move these test setup code to a common member function.