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(); }
diff --git a/quic/test_tools/quic_packet_creator_peer.cc b/quic/test_tools/quic_packet_creator_peer.cc index f94620b..1686c03 100644 --- a/quic/test_tools/quic_packet_creator_peer.cc +++ b/quic/test_tools/quic_packet_creator_peer.cc
@@ -131,7 +131,7 @@ std::unique_ptr<SerializedPacket> QuicPacketCreatorPeer::SerializePathChallengeConnectivityProbingPacket( QuicPacketCreator* creator, - QuicPathFrameBuffer* payload) { + const QuicPathFrameBuffer& payload) { return creator->SerializePathChallengeConnectivityProbingPacket(payload); }
diff --git a/quic/test_tools/quic_packet_creator_peer.h b/quic/test_tools/quic_packet_creator_peer.h index 2dc9413..3d5b1cb 100644 --- a/quic/test_tools/quic_packet_creator_peer.h +++ b/quic/test_tools/quic_packet_creator_peer.h
@@ -53,8 +53,9 @@ static std::unique_ptr<SerializedPacket> SerializeConnectivityProbingPacket( QuicPacketCreator* creator); static std::unique_ptr<SerializedPacket> - SerializePathChallengeConnectivityProbingPacket(QuicPacketCreator* creator, - QuicPathFrameBuffer* payload); + SerializePathChallengeConnectivityProbingPacket( + QuicPacketCreator* creator, + const QuicPathFrameBuffer& payload); static EncryptionLevel GetEncryptionLevel(QuicPacketCreator* creator); static QuicFramer* framer(QuicPacketCreator* creator);