Modifications to QuicPathValidator.
Add effective peer address to PathValidationContext and pass it to SendPathChallenge().
CancelPathValidation() to call validation failure callback.
Add a QUIC_BUG to prevent StartValidatingPath() to be called while there is on-going validation.
PiperOrigin-RevId: 348542080
Change-Id: I5ce4f2a7ea4b621ce6a2c25c1852a86b454c9b58
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 1599d10..a8067cd 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -5380,10 +5380,12 @@
return sent_packet_manager_.GetRetransmissionTime();
}
-bool QuicConnection::SendPathChallenge(const QuicPathFrameBuffer& data_buffer,
- const QuicSocketAddress& self_address,
- const QuicSocketAddress& peer_address,
- QuicPacketWriter* writer) {
+bool QuicConnection::SendPathChallenge(
+ const QuicPathFrameBuffer& data_buffer,
+ const QuicSocketAddress& self_address,
+ const QuicSocketAddress& peer_address,
+ const QuicSocketAddress& /*effective_peer_address*/,
+ QuicPacketWriter* writer) {
if (writer == writer_) {
{
// It's on current path, add the PATH_CHALLENGE the same way as other
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index dd453cb..02f70de 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -1129,6 +1129,7 @@
bool SendPathChallenge(const QuicPathFrameBuffer& data_buffer,
const QuicSocketAddress& self_address,
const QuicSocketAddress& peer_address,
+ const QuicSocketAddress& effective_peer_address,
QuicPacketWriter* writer) override;
// If |writer| is the default writer and |peer_address| is the same as
// peer_address(), return the PTO of this connection. Otherwise, return 3 *
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 5d26ab0..c1ce8b4 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -6395,8 +6395,9 @@
QuicPathFrameBuffer payload{
{0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
QuicConnection::ScopedPacketFlusher flusher(&connection_);
- connection_.SendPathChallenge(payload, connection_.self_address(),
- connection_.peer_address(), writer_.get());
+ connection_.SendPathChallenge(
+ payload, connection_.self_address(), connection_.peer_address(),
+ connection_.effective_peer_address(), writer_.get());
} else {
connection_.SendConnectivityProbingPacket(writer_.get(),
connection_.peer_address());
diff --git a/quic/core/quic_path_validator.cc b/quic/core/quic_path_validator.cc
index fe9e046..0722216 100644
--- a/quic/core/quic_path_validator.cc
+++ b/quic/core/quic_path_validator.cc
@@ -59,16 +59,22 @@
probing_data_.end()) {
result_delegate_->OnPathValidationSuccess(std::move(path_context_));
ResetPathValidation();
+ } else {
+ QUIC_DVLOG(1) << "PATH_RESPONSE with payload " << probing_data.data()
+ << " doesn't match the probing data.";
}
}
void QuicPathValidator::StartPathValidation(
std::unique_ptr<QuicPathValidationContext> context,
std::unique_ptr<ResultDelegate> result_delegate) {
- CancelPathValidation();
DCHECK_NE(nullptr, context);
QUIC_DLOG(INFO) << "Start validating path " << *context
<< " via writer: " << context->WriterToUse();
+ if (path_context_ != nullptr) {
+ QUIC_BUG << "There is an on-going validation on path " << *path_context_;
+ ResetPathValidation();
+ }
path_context_ = std::move(context);
result_delegate_ = std::move(result_delegate);
@@ -87,6 +93,7 @@
return;
}
QUIC_DVLOG(1) << "Cancel validation on path" << *path_context_;
+ result_delegate_->OnPathValidationFailure(std::move(path_context_));
ResetPathValidation();
}
@@ -107,7 +114,6 @@
void QuicPathValidator::OnRetryTimeout() {
++retry_count_;
if (retry_count_ > kMaxRetryTimes) {
- result_delegate_->OnPathValidationFailure(std::move(path_context_));
CancelPathValidation();
return;
}
@@ -118,7 +124,8 @@
void QuicPathValidator::SendPathChallengeAndSetAlarm() {
bool should_continue = send_delegate_->SendPathChallenge(
GeneratePathChallengePayload(), path_context_->self_address(),
- path_context_->peer_address(), path_context_->WriterToUse());
+ path_context_->peer_address(), path_context_->effective_peer_address(),
+ path_context_->WriterToUse());
if (!should_continue) {
// The delegate doesn't want to continue the path validation.
diff --git a/quic/core/quic_path_validator.h b/quic/core/quic_path_validator.h
index b9c9c5d..1463720 100644
--- a/quic/core/quic_path_validator.h
+++ b/quic/core/quic_path_validator.h
@@ -31,7 +31,16 @@
public:
QuicPathValidationContext(const QuicSocketAddress& self_address,
const QuicSocketAddress& peer_address)
- : self_address_(self_address), peer_address_(peer_address) {}
+ : self_address_(self_address),
+ peer_address_(peer_address),
+ effective_peer_address_(peer_address) {}
+
+ QuicPathValidationContext(const QuicSocketAddress& self_address,
+ const QuicSocketAddress& peer_address,
+ const QuicSocketAddress& effective_peer_address)
+ : self_address_(self_address),
+ peer_address_(peer_address),
+ effective_peer_address_(effective_peer_address) {}
virtual ~QuicPathValidationContext() = default;
@@ -39,6 +48,9 @@
const QuicSocketAddress& self_address() const { return self_address_; }
const QuicSocketAddress& peer_address() const { return peer_address_; }
+ const QuicSocketAddress& effective_peer_address() const {
+ return effective_peer_address_;
+ }
private:
QUIC_EXPORT_PRIVATE friend std::ostream& operator<<(
@@ -46,7 +58,11 @@
const QuicPathValidationContext& context);
QuicSocketAddress self_address_;
+ // The address to send PATH_CHALLENGE.
QuicSocketAddress peer_address_;
+ // The actual peer address which is different from |peer_address_| if the peer
+ // is behind a proxy.
+ QuicSocketAddress effective_peer_address_;
};
// Used to validate a path by sending up to 3 PATH_CHALLENGE frames before
@@ -64,10 +80,12 @@
// Send a PATH_CHALLENGE with |data_buffer| as the frame payload using given
// path information. Return false if the delegate doesn't want to continue
// the validation.
- virtual bool SendPathChallenge(const QuicPathFrameBuffer& data_buffer,
- const QuicSocketAddress& self_address,
- const QuicSocketAddress& peer_address,
- QuicPacketWriter* writer) = 0;
+ virtual bool SendPathChallenge(
+ const QuicPathFrameBuffer& data_buffer,
+ const QuicSocketAddress& self_address,
+ const QuicSocketAddress& peer_address,
+ const QuicSocketAddress& effective_peer_address,
+ QuicPacketWriter* writer) = 0;
// Return the time to retry sending PATH_CHALLENGE again based on given peer
// address and writer.
virtual QuicTime GetRetryTimeout(const QuicSocketAddress& peer_address,
diff --git a/quic/core/quic_path_validator_test.cc b/quic/core/quic_path_validator_test.cc
index 5fd7f9f..fab0c7c 100644
--- a/quic/core/quic_path_validator_test.cc
+++ b/quic/core/quic_path_validator_test.cc
@@ -35,6 +35,7 @@
(const QuicPathFrameBuffer&,
const QuicSocketAddress&,
const QuicSocketAddress&,
+ const QuicSocketAddress&,
QuicPacketWriter*),
(override));
@@ -50,8 +51,10 @@
: path_validator_(&alarm_factory_, &arena_, &send_delegate_, &random_),
context_(new MockQuicPathValidationContext(self_address_,
peer_address_,
+ effective_peer_address_,
&writer_)),
- result_delegate_(new MockQuicPathValidationResultDelegate()) {
+ result_delegate_(
+ new testing::StrictMock<MockQuicPathValidationResultDelegate>()) {
clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(1));
ON_CALL(send_delegate_, GetRetryTimeout(_, _))
.WillByDefault(
@@ -68,6 +71,7 @@
QuicPathValidator path_validator_;
QuicSocketAddress self_address_{QuicIpAddress::Any4(), 443};
QuicSocketAddress peer_address_{QuicIpAddress::Loopback4(), 443};
+ QuicSocketAddress effective_peer_address_{QuicIpAddress::Loopback4(), 12345};
MockPacketWriter writer_;
MockQuicPathValidationContext* context_;
MockQuicPathValidationResultDelegate* result_delegate_;
@@ -76,10 +80,11 @@
TEST_F(QuicPathValidatorTest, PathValidationSuccessOnFirstRound) {
QuicPathFrameBuffer challenge_data;
EXPECT_CALL(send_delegate_,
- SendPathChallenge(_, self_address_, peer_address_, &writer_))
+ SendPathChallenge(_, self_address_, peer_address_,
+ effective_peer_address_, &writer_))
.WillOnce(Invoke([&](const QuicPathFrameBuffer& payload,
const QuicSocketAddress&, const QuicSocketAddress&,
- QuicPacketWriter*) {
+ const QuicSocketAddress&, QuicPacketWriter*) {
memcpy(challenge_data.data(), payload.data(), payload.size());
return true;
}));
@@ -99,10 +104,11 @@
TEST_F(QuicPathValidatorTest, RespondWithDifferentSelfAddress) {
QuicPathFrameBuffer challenge_data;
EXPECT_CALL(send_delegate_,
- SendPathChallenge(_, self_address_, peer_address_, &writer_))
+ SendPathChallenge(_, self_address_, peer_address_,
+ effective_peer_address_, &writer_))
.WillOnce(Invoke([&](const QuicPathFrameBuffer payload,
const QuicSocketAddress&, const QuicSocketAddress&,
- QuicPacketWriter*) {
+ const QuicSocketAddress&, QuicPacketWriter*) {
memcpy(challenge_data.data(), payload.data(), payload.size());
return true;
}));
@@ -126,17 +132,18 @@
TEST_F(QuicPathValidatorTest, RespondAfter1stRetry) {
QuicPathFrameBuffer challenge_data;
EXPECT_CALL(send_delegate_,
- SendPathChallenge(_, self_address_, peer_address_, &writer_))
+ SendPathChallenge(_, self_address_, peer_address_,
+ effective_peer_address_, &writer_))
.WillOnce(Invoke([&](const QuicPathFrameBuffer& payload,
const QuicSocketAddress&, const QuicSocketAddress&,
- QuicPacketWriter*) {
+ const QuicSocketAddress&, QuicPacketWriter*) {
// Store up the 1st PATH_CHALLANGE payload.
memcpy(challenge_data.data(), payload.data(), payload.size());
return true;
}))
.WillOnce(Invoke([&](const QuicPathFrameBuffer& payload,
const QuicSocketAddress&, const QuicSocketAddress&,
- QuicPacketWriter*) {
+ const QuicSocketAddress&, QuicPacketWriter*) {
EXPECT_NE(payload, challenge_data);
return true;
}));
@@ -160,16 +167,17 @@
TEST_F(QuicPathValidatorTest, RespondToRetryChallenge) {
QuicPathFrameBuffer challenge_data;
EXPECT_CALL(send_delegate_,
- SendPathChallenge(_, self_address_, peer_address_, &writer_))
+ SendPathChallenge(_, self_address_, peer_address_,
+ effective_peer_address_, &writer_))
.WillOnce(Invoke([&](const QuicPathFrameBuffer& payload,
const QuicSocketAddress&, const QuicSocketAddress&,
- QuicPacketWriter*) {
+ const QuicSocketAddress&, QuicPacketWriter*) {
memcpy(challenge_data.data(), payload.data(), payload.size());
return true;
}))
.WillOnce(Invoke([&](const QuicPathFrameBuffer& payload,
const QuicSocketAddress&, const QuicSocketAddress&,
- QuicPacketWriter*) {
+ const QuicSocketAddress&, QuicPacketWriter*) {
EXPECT_NE(challenge_data, payload);
memcpy(challenge_data.data(), payload.data(), payload.size());
return true;
@@ -193,7 +201,8 @@
TEST_F(QuicPathValidatorTest, ValidationTimeOut) {
EXPECT_CALL(send_delegate_,
- SendPathChallenge(_, self_address_, peer_address_, &writer_))
+ SendPathChallenge(_, self_address_, peer_address_,
+ effective_peer_address_, &writer_))
.Times(3u)
.WillRepeatedly(Return(true));
EXPECT_CALL(send_delegate_, GetRetryTimeout(peer_address_, &writer_))
@@ -221,9 +230,11 @@
TEST_F(QuicPathValidatorTest, SendPathChallengeError) {
EXPECT_CALL(send_delegate_,
- SendPathChallenge(_, self_address_, peer_address_, &writer_))
+ SendPathChallenge(_, self_address_, peer_address_,
+ effective_peer_address_, &writer_))
.WillOnce(Invoke([&](const QuicPathFrameBuffer&, const QuicSocketAddress&,
- const QuicSocketAddress&, QuicPacketWriter*) {
+ const QuicSocketAddress&, const QuicSocketAddress&,
+ QuicPacketWriter*) {
// Abandon this validation in the call stack shouldn't cause crash and
// should cancel the alarm.
path_validator_.CancelPathValidation();
@@ -231,6 +242,7 @@
}));
EXPECT_CALL(send_delegate_, GetRetryTimeout(peer_address_, &writer_))
.Times(0u);
+ EXPECT_CALL(*result_delegate_, OnPathValidationFailure(_));
path_validator_.StartPathValidation(
std::unique_ptr<QuicPathValidationContext>(context_),
std::unique_ptr<MockQuicPathValidationResultDelegate>(result_delegate_));
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h
index 428d695..e78839a 100644
--- a/quic/test_tools/quic_test_utils.h
+++ b/quic/test_tools/quic_test_utils.h
@@ -734,6 +734,7 @@
(const QuicPathFrameBuffer&,
const QuicSocketAddress&,
const QuicSocketAddress&,
+ const QuicSocketAddress&,
QuicPacketWriter*),
(override));
@@ -1658,8 +1659,11 @@
public:
MockQuicPathValidationContext(const QuicSocketAddress& self_address,
const QuicSocketAddress& peer_address,
+ const QuicSocketAddress& effective_peer_address,
QuicPacketWriter* writer)
- : QuicPathValidationContext(self_address, peer_address),
+ : QuicPathValidationContext(self_address,
+ peer_address,
+ effective_peer_address),
writer_(writer) {}
QuicPacketWriter* WriterToUse() override { return writer_; }
