Pass PATH_CHALLENGE payload to QuicPacketCreator::SendPathChallenge() instead of getting it populated by this interface.
This changes the behavior in the old implementation of IETF connection migration. But it is not used in production.
PiperOrigin-RevId: 342098354
Change-Id: I8b719738184c49907e313d53bee0281546ca0fae
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 845e078..6e6ded6 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -4432,9 +4432,11 @@
// 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(),
+ sizeof(QuicPathFrameBuffer));
probing_packet =
packet_creator_.SerializePathChallengeConnectivityProbingPacket(
- transmitted_connectivity_probe_payload_.get());
+ *transmitted_connectivity_probe_payload_);
if (!probing_packet) {
transmitted_connectivity_probe_payload_ = nullptr;
}
@@ -5437,7 +5439,7 @@
return sent_packet_manager_.GetRetransmissionTime();
}
-void QuicConnection::SendPathChallenge(QuicPathFrameBuffer* data_buffer,
+void QuicConnection::SendPathChallenge(const QuicPathFrameBuffer& data_buffer,
const QuicSocketAddress& self_address,
const QuicSocketAddress& peer_address,
QuicPacketWriter* writer) {
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 35d15ff..b19796a 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -1115,7 +1115,7 @@
// PATH_CHALLENGE will be written in an individual packet and it will be
// dropped if write fails. |data_buffer| will be populated with the payload
// for future validation.
- void SendPathChallenge(QuicPathFrameBuffer* data_buffer,
+ void SendPathChallenge(const QuicPathFrameBuffer& data_buffer,
const QuicSocketAddress& self_address,
const QuicSocketAddress& peer_address,
QuicPacketWriter* writer);
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 1d55c1a..5f8af1c 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -1176,7 +1176,7 @@
{0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
return QuicPacketCreatorPeer::
SerializePathChallengeConnectivityProbingPacket(&peer_creator_,
- &payload);
+ payload);
}
return QuicPacketCreatorPeer::SerializeConnectivityProbingPacket(
&peer_creator_);
@@ -6505,9 +6505,10 @@
.Times(1);
if (connection_.send_path_response() &&
VersionHasIetfQuicFrames(GetParam().version.transport_version)) {
- QuicPathFrameBuffer payload;
+ QuicPathFrameBuffer payload{
+ {0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
QuicConnection::ScopedPacketFlusher flusher(&connection_);
- connection_.SendPathChallenge(&payload, connection_.self_address(),
+ connection_.SendPathChallenge(payload, connection_.self_address(),
connection_.peer_address(), writer_.get());
} else {
connection_.SendConnectivityProbingPacket(writer_.get(),
@@ -11342,7 +11343,7 @@
const QuicSocketAddress kNewSourceAddress(QuicIpAddress::Any6(), 12345);
EXPECT_NE(kNewSourceAddress, connection_.self_address());
TestPacketWriter new_writer(version(), &clock_, Perspective::IS_CLIENT);
- QuicPathFrameBuffer payload;
+ QuicPathFrameBuffer payload{{0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _))
.WillOnce(Invoke([&]() {
EXPECT_EQ(1u, new_writer.packets_write_attempts());
@@ -11355,7 +11356,7 @@
EXPECT_EQ(kNewSourceAddress.host(),
new_writer.last_write_source_address());
}));
- connection_.SendPathChallenge(&payload, kNewSourceAddress,
+ connection_.SendPathChallenge(payload, kNewSourceAddress,
connection_.peer_address(), &new_writer);
EXPECT_EQ(0u, writer_->packets_write_attempts());
}
@@ -11370,7 +11371,7 @@
EXPECT_NE(kNewSourceAddress, connection_.self_address());
TestPacketWriter new_writer(version(), &clock_, Perspective::IS_CLIENT);
new_writer.BlockOnNextWrite();
- QuicPathFrameBuffer payload;
+ QuicPathFrameBuffer payload{{0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
EXPECT_CALL(visitor_, OnWriteBlocked()).Times(0);
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _))
.WillOnce(Invoke([&]() {
@@ -11386,7 +11387,7 @@
EXPECT_EQ(kNewSourceAddress.host(),
new_writer.last_write_source_address());
}));
- connection_.SendPathChallenge(&payload, kNewSourceAddress,
+ connection_.SendPathChallenge(payload, kNewSourceAddress,
connection_.peer_address(), &new_writer);
EXPECT_EQ(0u, writer_->packets_write_attempts());
@@ -11408,7 +11409,7 @@
}
const QuicSocketAddress kNewPeerAddress(QuicIpAddress::Any6(), 12345);
writer_->BlockOnNextWrite();
- QuicPathFrameBuffer payload;
+ QuicPathFrameBuffer payload{{0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
// 1st time is after writer returns WRITE_STATUS_BLOCKED. 2nd time is in
// ShouldGeneratePacket();
EXPECT_CALL(visitor_, OnWriteBlocked()).Times(2u);
@@ -11423,14 +11424,14 @@
EXPECT_EQ(1u, writer_->padding_frames().size());
EXPECT_EQ(kNewPeerAddress, writer_->last_write_peer_address());
}));
- connection_.SendPathChallenge(&payload, connection_.self_address(),
+ connection_.SendPathChallenge(payload, connection_.self_address(),
kNewPeerAddress, writer_.get());
EXPECT_EQ(1u, writer_->packets_write_attempts());
memset(payload.data(), 0, sizeof(payload));
// Try again with the new socket blocked from the beginning. The 2nd
// PATH_CHALLENGE shouldn't be serialized, but be dropped.
- connection_.SendPathChallenge(&payload, connection_.self_address(),
+ connection_.SendPathChallenge(payload, connection_.self_address(),
kNewPeerAddress, writer_.get());
// No more write attempt should be made.
EXPECT_EQ(1u, writer_->packets_write_attempts());
@@ -11453,11 +11454,11 @@
EXPECT_NE(kNewSourceAddress, connection_.self_address());
TestPacketWriter new_writer(version(), &clock_, Perspective::IS_CLIENT);
new_writer.SetShouldWriteFail();
- QuicPathFrameBuffer payload;
+ QuicPathFrameBuffer payload{{0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
EXPECT_CALL(visitor_, OnConnectionClosed(_, ConnectionCloseSource::FROM_SELF))
.Times(0);
- connection_.SendPathChallenge(&payload, kNewSourceAddress,
+ connection_.SendPathChallenge(payload, kNewSourceAddress,
connection_.peer_address(), &new_writer);
// Regardless of the write error, the PATH_CHALLENGE should still be
// treated as sent.
@@ -11481,7 +11482,7 @@
}
PathProbeTestInit(Perspective::IS_CLIENT);
writer_->SetShouldWriteFail();
- QuicPathFrameBuffer payload;
+ QuicPathFrameBuffer payload{{0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
EXPECT_CALL(visitor_, OnConnectionClosed(_, ConnectionCloseSource::FROM_SELF))
.WillOnce(
Invoke([](QuicConnectionCloseFrame frame, ConnectionCloseSource) {
@@ -11492,7 +11493,7 @@
// Add a flusher to force flush, otherwise the frames will remain in the
// packet creator.
QuicConnection::ScopedPacketFlusher flusher(&connection_);
- connection_.SendPathChallenge(&payload, connection_.self_address(),
+ connection_.SendPathChallenge(payload, connection_.self_address(),
connection_.peer_address(), writer_.get());
}
EXPECT_EQ(1u, writer_->packets_write_attempts());
diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc
index 3db37ed..087b427 100644
--- a/quic/core/quic_packet_creator.cc
+++ b/quic/core/quic_packet_creator.cc
@@ -875,7 +875,7 @@
std::unique_ptr<SerializedPacket>
QuicPacketCreator::SerializePathChallengeConnectivityProbingPacket(
- QuicPathFrameBuffer* payload) {
+ const QuicPathFrameBuffer& payload) {
QUIC_BUG_IF(!VersionHasIetfQuicFrames(framer_->transport_version()))
<< "Must be version 99 to serialize path challenge connectivity probe, "
"is version "
@@ -888,9 +888,9 @@
QUIC_DVLOG(2) << ENDPOINT << "Serializing path challenge packet " << header;
std::unique_ptr<char[]> buffer(new char[kMaxOutgoingPacketSize]);
- size_t length = BuildPaddedPathChallengePacket(
- header, buffer.get(), max_plaintext_size_, payload, random_,
- packet_.encryption_level);
+ size_t length =
+ BuildPaddedPathChallengePacket(header, buffer.get(), max_plaintext_size_,
+ payload, packet_.encryption_level);
DCHECK(length);
DCHECK_EQ(packet_.encryption_level, ENCRYPTION_FORWARD_SECURE);
@@ -960,15 +960,13 @@
const QuicPacketHeader& header,
char* buffer,
size_t packet_length,
- QuicPathFrameBuffer* payload,
- QuicRandom* randomizer,
+ const QuicPathFrameBuffer& payload,
EncryptionLevel level) {
DCHECK(VersionHasIetfQuicFrames(framer_->transport_version()));
QuicFrames frames;
// Write a PATH_CHALLENGE frame, which has a random 8-byte payload
- randomizer->RandBytes(payload->data(), payload->size());
- QuicPathChallengeFrame path_challenge_frame(0, *payload);
+ QuicPathChallengeFrame path_challenge_frame(0, payload);
frames.push_back(QuicFrame(&path_challenge_frame));
if (debug_delegate_ != nullptr) {
@@ -2008,10 +2006,10 @@
packet_.encryption_level = level;
}
-void QuicPacketCreator::AddPathChallengeFrame(QuicPathFrameBuffer* payload) {
+void QuicPacketCreator::AddPathChallengeFrame(
+ const QuicPathFrameBuffer& payload) {
// Write a PATH_CHALLENGE frame, which has a random 8-byte payload.
- random_->RandBytes(payload->data(), payload->size());
- auto path_challenge_frame = new QuicPathChallengeFrame(0, *payload);
+ auto path_challenge_frame = new QuicPathChallengeFrame(0, payload);
QuicFrame frame(path_challenge_frame);
if (AddPaddedFrameWithRetry(frame)) {
return;
diff --git a/quic/core/quic_packet_creator.h b/quic/core/quic_packet_creator.h
index 54da59c..f1ace67 100644
--- a/quic/core/quic_packet_creator.h
+++ b/quic/core/quic_packet_creator.h
@@ -239,7 +239,8 @@
// SerializePathChallengeConnectivityProbingPacket will pad the packet to be
// MTU bytes long.
std::unique_ptr<SerializedPacket>
- SerializePathChallengeConnectivityProbingPacket(QuicPathFrameBuffer* payload);
+ SerializePathChallengeConnectivityProbingPacket(
+ const QuicPathFrameBuffer& payload);
// If |is_padded| is true then SerializePathResponseConnectivityProbingPacket
// will pad the packet to be MTU bytes long, else it will not pad the packet.
@@ -255,7 +256,7 @@
// Add PATH_CHALLENGE to current packet, flush before or afterwards if needed.
// This is a best effort adding. It may fail becasue of delegate state, but
// it's okay because of path validation retry mechanism.
- void AddPathChallengeFrame(QuicPathFrameBuffer* payload);
+ void AddPathChallengeFrame(const QuicPathFrameBuffer& payload);
// Returns a dummy packet that is valid but contains no useful information.
static SerializedPacket NoPacket();
@@ -432,8 +433,7 @@
size_t BuildPaddedPathChallengePacket(const QuicPacketHeader& header,
char* buffer,
size_t packet_length,
- QuicPathFrameBuffer* payload,
- QuicRandom* randomizer,
+ const QuicPathFrameBuffer& payload,
EncryptionLevel level);
// Serialize a probing response packet that uses IETF QUIC's PATH RESPONSE
diff --git a/quic/core/quic_packet_creator_test.cc b/quic/core/quic_packet_creator_test.cc
index d76f198..b33389d 100644
--- a/quic/core/quic_packet_creator_test.cc
+++ b/quic/core/quic_packet_creator_test.cc
@@ -592,7 +592,9 @@
header.reset_flag = false;
header.version_flag = false;
header.packet_number = kPacketNumber;
+ MockRandom randomizer;
QuicPathFrameBuffer payload;
+ randomizer.RandBytes(payload.data(), payload.size());
// clang-format off
unsigned char packet[] = {
@@ -614,10 +616,9 @@
// clang-format on
std::unique_ptr<char[]> buffer(new char[kMaxOutgoingPacketSize]);
- MockRandom randomizer;
size_t length = creator_.BuildPaddedPathChallengePacket(
- header, buffer.get(), ABSL_ARRAYSIZE(packet), &payload, &randomizer,
+ header, buffer.get(), ABSL_ARRAYSIZE(packet), payload,
ENCRYPTION_INITIAL);
EXPECT_EQ(length, ABSL_ARRAYSIZE(packet));
@@ -921,7 +922,7 @@
QuicPathFrameBuffer payload = {
{0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
encrypted =
- creator_.SerializePathChallengeConnectivityProbingPacket(&payload);
+ creator_.SerializePathChallengeConnectivityProbingPacket(payload);
} else {
encrypted = creator_.SerializeConnectivityProbingPacket();
}
@@ -956,7 +957,7 @@
creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE);
std::unique_ptr<SerializedPacket> encrypted(
- creator_.SerializePathChallengeConnectivityProbingPacket(&payload));
+ creator_.SerializePathChallengeConnectivityProbingPacket(payload));
{
InSequence s;
EXPECT_CALL(framer_visitor_, OnPacket());
@@ -3413,7 +3414,7 @@
QuicPathFrameBuffer payload = {
{0xde, 0xad, 0xbe, 0xef, 0xba, 0xdc, 0x0f, 0xfe}};
probing_packet =
- creator_.SerializePathChallengeConnectivityProbingPacket(&payload);
+ creator_.SerializePathChallengeConnectivityProbingPacket(payload);
} else {
probing_packet = creator_.SerializeConnectivityProbingPacket();
}
