Deprecate quic_prober_uses_length_prefixed_connection_ids
PiperOrigin-RevId: 337975193
Change-Id: I5c365092604eaf42055c1f6b6e52153aaaeb1716
diff --git a/quic/core/quic_dispatcher_test.cc b/quic/core/quic_dispatcher_test.cc
index 259c734..bfeb0d4 100644
--- a/quic/core/quic_dispatcher_test.cc
+++ b/quic/core/quic_dispatcher_test.cc
@@ -1206,37 +1206,7 @@
"Please add new RejectDeprecatedVersion tests above this assert "
"when deprecating versions");
-TEST_P(QuicDispatcherTestOneVersion, VersionNegotiationProbeOld) {
- SetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids, false);
- QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
- CreateTimeWaitListManager();
- char packet[1200];
- char destination_connection_id_bytes[] = {0x56, 0x4e, 0x20, 0x70,
- 0x6c, 0x7a, 0x20, 0x21};
- EXPECT_TRUE(QuicFramer::WriteClientVersionNegotiationProbePacket(
- packet, sizeof(packet), destination_connection_id_bytes,
- sizeof(destination_connection_id_bytes)));
- QuicEncryptedPacket encrypted(packet, sizeof(packet), false);
- std::unique_ptr<QuicReceivedPacket> received_packet(
- ConstructReceivedPacket(encrypted, mock_helper_.GetClock()->Now()));
- QuicConnectionId client_connection_id = EmptyQuicConnectionId();
- QuicConnectionId server_connection_id(
- destination_connection_id_bytes, sizeof(destination_connection_id_bytes));
- bool ietf_quic = true;
- bool use_length_prefix =
- GetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids);
- EXPECT_CALL(
- *time_wait_list_manager_,
- SendVersionNegotiationPacket(server_connection_id, client_connection_id,
- ietf_quic, use_length_prefix, _, _, _, _))
- .Times(1);
- EXPECT_CALL(*dispatcher_, CreateQuicSession(_, _, _, _, _)).Times(0);
-
- dispatcher_->ProcessPacket(server_address_, client_address, *received_packet);
-}
-
TEST_P(QuicDispatcherTestOneVersion, VersionNegotiationProbe) {
- SetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids, true);
QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
CreateTimeWaitListManager();
char packet[1200];
@@ -1251,13 +1221,10 @@
QuicConnectionId client_connection_id = EmptyQuicConnectionId();
QuicConnectionId server_connection_id(
destination_connection_id_bytes, sizeof(destination_connection_id_bytes));
- bool ietf_quic = true;
- bool use_length_prefix =
- GetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids);
- EXPECT_CALL(
- *time_wait_list_manager_,
- SendVersionNegotiationPacket(server_connection_id, client_connection_id,
- ietf_quic, use_length_prefix, _, _, _, _))
+ EXPECT_CALL(*time_wait_list_manager_,
+ SendVersionNegotiationPacket(
+ server_connection_id, client_connection_id,
+ /*ietf_quic=*/true, /*use_length_prefix=*/true, _, _, _, _))
.Times(1);
EXPECT_CALL(*dispatcher_, CreateQuicSession(_, _, _, _, _)).Times(0);
@@ -1288,53 +1255,7 @@
std::vector<std::unique_ptr<QuicEncryptedPacket>> packets_;
};
-TEST_P(QuicDispatcherTestOneVersion, VersionNegotiationProbeEndToEndOld) {
- SetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids, false);
-
- SavingWriter* saving_writer = new SavingWriter();
- // dispatcher_ takes ownership of saving_writer.
- QuicDispatcherPeer::UseWriter(dispatcher_.get(), saving_writer);
-
- QuicTimeWaitListManager* time_wait_list_manager = new QuicTimeWaitListManager(
- saving_writer, dispatcher_.get(), mock_helper_.GetClock(),
- &mock_alarm_factory_);
- // dispatcher_ takes ownership of time_wait_list_manager.
- QuicDispatcherPeer::SetTimeWaitListManager(dispatcher_.get(),
- time_wait_list_manager);
- char packet[1200] = {};
- char destination_connection_id_bytes[] = {0x56, 0x4e, 0x20, 0x70,
- 0x6c, 0x7a, 0x20, 0x21};
- EXPECT_TRUE(QuicFramer::WriteClientVersionNegotiationProbePacket(
- packet, sizeof(packet), destination_connection_id_bytes,
- sizeof(destination_connection_id_bytes)));
- QuicEncryptedPacket encrypted(packet, sizeof(packet), false);
- std::unique_ptr<QuicReceivedPacket> received_packet(
- ConstructReceivedPacket(encrypted, mock_helper_.GetClock()->Now()));
- EXPECT_CALL(*dispatcher_, CreateQuicSession(_, _, _, _, _)).Times(0);
-
- QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1);
- dispatcher_->ProcessPacket(server_address_, client_address, *received_packet);
- ASSERT_EQ(1u, saving_writer->packets()->size());
-
- char source_connection_id_bytes[255] = {};
- uint8_t source_connection_id_length = sizeof(source_connection_id_bytes);
- std::string detailed_error = "foobar";
- EXPECT_TRUE(QuicFramer::ParseServerVersionNegotiationProbeResponse(
- (*(saving_writer->packets()))[0]->data(),
- (*(saving_writer->packets()))[0]->length(), source_connection_id_bytes,
- &source_connection_id_length, &detailed_error));
- EXPECT_EQ("", detailed_error);
-
- // The source connection ID of the probe response should match the
- // destination connection ID of the probe request.
- quiche::test::CompareCharArraysWithHexError(
- "parsed probe", source_connection_id_bytes, source_connection_id_length,
- destination_connection_id_bytes, sizeof(destination_connection_id_bytes));
-}
-
TEST_P(QuicDispatcherTestOneVersion, VersionNegotiationProbeEndToEnd) {
- SetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids, true);
-
SavingWriter* saving_writer = new SavingWriter();
// dispatcher_ takes ownership of saving_writer.
QuicDispatcherPeer::UseWriter(dispatcher_.get(), saving_writer);
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index a5cc8bd..884f7eb 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -6676,9 +6676,6 @@
QUIC_BUG << "Invalid connection_id_length";
return false;
}
- const bool use_length_prefix =
- GetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids);
- const uint8_t last_version_byte = use_length_prefix ? 0xda : 0xba;
// clang-format off
const unsigned char packet_start_bytes[] = {
// IETF long header with fixed bit set, type initial, all-0 encrypted bits.
@@ -6686,7 +6683,7 @@
// Version, part of the IETF space reserved for negotiation.
// This intentionally differs from QuicVersionReservedForNegotiation()
// to allow differentiating them over the wire.
- 0xca, 0xba, 0xda, last_version_byte,
+ 0xca, 0xba, 0xda, 0xda,
};
// clang-format on
static_assert(sizeof(packet_start_bytes) == 5, "bad packet_start_bytes size");
@@ -6699,8 +6696,8 @@
QuicConnectionId destination_connection_id(destination_connection_id_bytes,
destination_connection_id_length);
if (!AppendIetfConnectionIds(
- /*version_flag=*/true, use_length_prefix, destination_connection_id,
- EmptyQuicConnectionId(), &writer)) {
+ /*version_flag=*/true, /*use_length_prefix=*/true,
+ destination_connection_id, EmptyQuicConnectionId(), &writer)) {
QUIC_BUG << "Failed to write connection IDs";
return false;
}
@@ -6784,40 +6781,15 @@
*detailed_error = "Packet is not a version negotiation packet";
return false;
}
- const bool use_length_prefix =
- GetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids);
+
QuicConnectionId destination_connection_id, source_connection_id;
- if (use_length_prefix) {
- if (!reader.ReadLengthPrefixedConnectionId(&destination_connection_id)) {
- *detailed_error = "Failed to read destination connection ID";
- return false;
- }
- if (!reader.ReadLengthPrefixedConnectionId(&source_connection_id)) {
- *detailed_error = "Failed to read source connection ID";
- return false;
- }
- } else {
- uint8_t expected_server_connection_id_length = 0,
- destination_connection_id_length = 0,
- source_connection_id_length = 0;
- if (!ProcessAndValidateIetfConnectionIdLength(
- &reader, UnsupportedQuicVersion(), Perspective::IS_CLIENT,
- /*should_update_expected_server_connection_id_length=*/true,
- &expected_server_connection_id_length,
- &destination_connection_id_length, &source_connection_id_length,
- detailed_error)) {
- return false;
- }
- if (!reader.ReadConnectionId(&destination_connection_id,
- destination_connection_id_length)) {
- *detailed_error = "Failed to read destination connection ID";
- return false;
- }
- if (!reader.ReadConnectionId(&source_connection_id,
- source_connection_id_length)) {
- *detailed_error = "Failed to read source connection ID";
- return false;
- }
+ if (!reader.ReadLengthPrefixedConnectionId(&destination_connection_id)) {
+ *detailed_error = "Failed to read destination connection ID";
+ return false;
+ }
+ if (!reader.ReadLengthPrefixedConnectionId(&source_connection_id)) {
+ *detailed_error = "Failed to read source connection ID";
+ return false;
}
if (destination_connection_id.length() != 0) {
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index d9baee2..f0f2de0 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -14003,106 +14003,7 @@
CheckFramingBoundaries(packet, QUIC_INVALID_PACKET_HEADER);
}
-TEST_P(QuicFramerTest, WriteClientVersionNegotiationProbePacketOld) {
- SetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids, false);
- // clang-format off
- static const char expected_packet[1200] = {
- // IETF long header with fixed bit set, type initial, all-0 encrypted bits.
- 0xc0,
- // Version, part of the IETF space reserved for negotiation.
- 0xca, 0xba, 0xda, 0xba,
- // Destination connection ID length 8, source connection ID length 0.
- 0x50,
- // 8-byte destination connection ID.
- 0x56, 0x4e, 0x20, 0x70, 0x6c, 0x7a, 0x20, 0x21,
- // 8 bytes of zeroes followed by 8 bytes of ones to ensure that this does
- // not parse with any known version.
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
- // 2 bytes of zeroes to pad to 16 byte boundary.
- 0x00, 0x00,
- // A polite greeting in case a human sees this in tcpdump.
- 0x54, 0x68, 0x69, 0x73, 0x20, 0x70, 0x61, 0x63,
- 0x6b, 0x65, 0x74, 0x20, 0x6f, 0x6e, 0x6c, 0x79,
- 0x20, 0x65, 0x78, 0x69, 0x73, 0x74, 0x73, 0x20,
- 0x74, 0x6f, 0x20, 0x74, 0x72, 0x69, 0x67, 0x67,
- 0x65, 0x72, 0x20, 0x49, 0x45, 0x54, 0x46, 0x20,
- 0x51, 0x55, 0x49, 0x43, 0x20, 0x76, 0x65, 0x72,
- 0x73, 0x69, 0x6f, 0x6e, 0x20, 0x6e, 0x65, 0x67,
- 0x6f, 0x74, 0x69, 0x61, 0x74, 0x69, 0x6f, 0x6e,
- 0x2e, 0x20, 0x50, 0x6c, 0x65, 0x61, 0x73, 0x65,
- 0x20, 0x72, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x64,
- 0x20, 0x77, 0x69, 0x74, 0x68, 0x20, 0x61, 0x20,
- 0x56, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x20,
- 0x4e, 0x65, 0x67, 0x6f, 0x74, 0x69, 0x61, 0x74,
- 0x69, 0x6f, 0x6e, 0x20, 0x70, 0x61, 0x63, 0x6b,
- 0x65, 0x74, 0x20, 0x69, 0x6e, 0x64, 0x69, 0x63,
- 0x61, 0x74, 0x69, 0x6e, 0x67, 0x20, 0x77, 0x68,
- 0x61, 0x74, 0x20, 0x76, 0x65, 0x72, 0x73, 0x69,
- 0x6f, 0x6e, 0x73, 0x20, 0x79, 0x6f, 0x75, 0x20,
- 0x73, 0x75, 0x70, 0x70, 0x6f, 0x72, 0x74, 0x2e,
- 0x20, 0x54, 0x68, 0x61, 0x6e, 0x6b, 0x20, 0x79,
- 0x6f, 0x75, 0x20, 0x61, 0x6e, 0x64, 0x20, 0x68,
- 0x61, 0x76, 0x65, 0x20, 0x61, 0x20, 0x6e, 0x69,
- 0x63, 0x65, 0x20, 0x64, 0x61, 0x79, 0x2e, 0x00,
- };
- // clang-format on
- char packet[1200];
- char destination_connection_id_bytes[] = {0x56, 0x4e, 0x20, 0x70,
- 0x6c, 0x7a, 0x20, 0x21};
- EXPECT_TRUE(QuicFramer::WriteClientVersionNegotiationProbePacket(
- packet, sizeof(packet), destination_connection_id_bytes,
- sizeof(destination_connection_id_bytes)));
- quiche::test::CompareCharArraysWithHexError("constructed packet", packet,
- sizeof(packet), expected_packet,
- sizeof(expected_packet));
- QuicEncryptedPacket encrypted(reinterpret_cast<const char*>(packet),
- sizeof(packet), false);
- // Make sure we fail to parse this packet for the version under test.
- if (!framer_.version().HasIetfInvariantHeader()) {
- // We can only parse the connection ID with an IETF parser.
- EXPECT_FALSE(framer_.ProcessPacket(encrypted));
- return;
- }
- EXPECT_TRUE(framer_.ProcessPacket(encrypted));
- ASSERT_TRUE(visitor_.header_.get());
- QuicConnectionId probe_payload_connection_id(
- reinterpret_cast<const char*>(destination_connection_id_bytes),
- sizeof(destination_connection_id_bytes));
- EXPECT_EQ(probe_payload_connection_id,
- visitor_.header_.get()->destination_connection_id);
-
- PacketHeaderFormat format = GOOGLE_QUIC_PACKET;
- QuicLongHeaderType long_packet_type = INVALID_PACKET_TYPE;
- bool version_present = false, has_length_prefix = false;
- QuicVersionLabel version_label = 0;
- ParsedQuicVersion parsed_version = QuicVersionReservedForNegotiation();
- QuicConnectionId destination_connection_id = TestConnectionId(0x33);
- QuicConnectionId source_connection_id = TestConnectionId(0x34);
- bool retry_token_present = true;
- absl::string_view retry_token;
- std::string detailed_error = "foobar";
-
- QuicErrorCode parse_result = QuicFramer::ParsePublicHeaderDispatcher(
- encrypted, kQuicDefaultConnectionIdLength, &format, &long_packet_type,
- &version_present, &has_length_prefix, &version_label, &parsed_version,
- &destination_connection_id, &source_connection_id, &retry_token_present,
- &retry_token, &detailed_error);
- EXPECT_THAT(parse_result, IsQuicNoError());
- EXPECT_EQ(IETF_QUIC_LONG_HEADER_PACKET, format);
- EXPECT_TRUE(version_present);
- EXPECT_FALSE(has_length_prefix);
- EXPECT_EQ(0xcabadaba, version_label);
- EXPECT_EQ(QUIC_VERSION_UNSUPPORTED, parsed_version.transport_version);
- EXPECT_EQ(probe_payload_connection_id, destination_connection_id);
- EXPECT_EQ(EmptyQuicConnectionId(), source_connection_id);
- EXPECT_FALSE(retry_token_present);
- EXPECT_EQ(absl::string_view(), retry_token);
- EXPECT_EQ("", detailed_error);
-}
-
TEST_P(QuicFramerTest, WriteClientVersionNegotiationProbePacket) {
- SetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids, true);
// clang-format off
static const char expected_packet[1200] = {
// IETF long header with fixed bit set, type initial, all-0 encrypted bits.
@@ -14328,39 +14229,7 @@
EXPECT_EQ("", detailed_error);
}
-TEST_P(QuicFramerTest, ParseServerVersionNegotiationProbeResponseOld) {
- SetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids, false);
- // clang-format off
- const char packet[] = {
- // IETF long header with fixed bit set, type initial, all-0 encrypted bits.
- 0xc0,
- // Version of 0, indicating version negotiation.
- 0x00, 0x00, 0x00, 0x00,
- // Destination connection ID length 0, source connection ID length 8.
- 0x05,
- // 8-byte source connection ID.
- 0x56, 0x4e, 0x20, 0x70, 0x6c, 0x7a, 0x20, 0x21,
- // A few supported versions.
- 0xaa, 0xaa, 0xaa, 0xaa,
- QUIC_VERSION_BYTES,
- };
- // clang-format on
- char probe_payload_bytes[] = {0x56, 0x4e, 0x20, 0x70, 0x6c, 0x7a, 0x20, 0x21};
- char parsed_probe_payload_bytes[255] = {};
- uint8_t parsed_probe_payload_length = sizeof(parsed_probe_payload_bytes);
- std::string parse_detailed_error = "";
- EXPECT_TRUE(QuicFramer::ParseServerVersionNegotiationProbeResponse(
- reinterpret_cast<const char*>(packet), sizeof(packet),
- reinterpret_cast<char*>(parsed_probe_payload_bytes),
- &parsed_probe_payload_length, &parse_detailed_error));
- EXPECT_EQ("", parse_detailed_error);
- quiche::test::CompareCharArraysWithHexError(
- "parsed probe", parsed_probe_payload_bytes, parsed_probe_payload_length,
- probe_payload_bytes, sizeof(probe_payload_bytes));
-}
-
TEST_P(QuicFramerTest, ParseServerVersionNegotiationProbeResponse) {
- SetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids, true);
// clang-format off
const char packet[] = {
// IETF long header with fixed bit set, type initial, all-0 encrypted bits.
@@ -14391,7 +14260,6 @@
}
TEST_P(QuicFramerTest, ParseClientVersionNegotiationProbePacket) {
- SetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids, true);
char packet[1200];
char input_destination_connection_id_bytes[] = {0x56, 0x4e, 0x20, 0x70,
0x6c, 0x7a, 0x20, 0x21};
@@ -14413,52 +14281,6 @@
}
TEST_P(QuicFramerTest, WriteServerVersionNegotiationProbeResponse) {
- SetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids, true);
- char packet[1200];
- size_t packet_length = sizeof(packet);
- char input_source_connection_id_bytes[] = {0x56, 0x4e, 0x20, 0x70,
- 0x6c, 0x7a, 0x20, 0x21};
- ASSERT_TRUE(WriteServerVersionNegotiationProbeResponse(
- packet, &packet_length, input_source_connection_id_bytes,
- sizeof(input_source_connection_id_bytes)));
- char parsed_source_connection_id_bytes[255] = {0};
- uint8_t parsed_source_connection_id_length =
- sizeof(parsed_source_connection_id_bytes);
- std::string detailed_error;
- ASSERT_TRUE(QuicFramer::ParseServerVersionNegotiationProbeResponse(
- packet, packet_length, parsed_source_connection_id_bytes,
- &parsed_source_connection_id_length, &detailed_error))
- << detailed_error;
- quiche::test::CompareCharArraysWithHexError(
- "parsed destination connection ID", parsed_source_connection_id_bytes,
- parsed_source_connection_id_length, input_source_connection_id_bytes,
- sizeof(input_source_connection_id_bytes));
-}
-
-TEST_P(QuicFramerTest, ParseClientVersionNegotiationProbePacketOld) {
- SetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids, false);
- char packet[1200];
- char input_destination_connection_id_bytes[] = {0x56, 0x4e, 0x20, 0x70,
- 0x6c, 0x7a, 0x20, 0x21};
- ASSERT_TRUE(QuicFramer::WriteClientVersionNegotiationProbePacket(
- packet, sizeof(packet), input_destination_connection_id_bytes,
- sizeof(input_destination_connection_id_bytes)));
- char parsed_destination_connection_id_bytes[255] = {0};
- uint8_t parsed_destination_connection_id_length =
- sizeof(parsed_destination_connection_id_bytes);
- ASSERT_TRUE(ParseClientVersionNegotiationProbePacket(
- packet, sizeof(packet), parsed_destination_connection_id_bytes,
- &parsed_destination_connection_id_length));
- quiche::test::CompareCharArraysWithHexError(
- "parsed destination connection ID",
- parsed_destination_connection_id_bytes,
- parsed_destination_connection_id_length,
- input_destination_connection_id_bytes,
- sizeof(input_destination_connection_id_bytes));
-}
-
-TEST_P(QuicFramerTest, WriteServerVersionNegotiationProbeResponseOld) {
- SetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids, false);
char packet[1200];
size_t packet_length = sizeof(packet);
char input_source_connection_id_bytes[] = {0x56, 0x4e, 0x20, 0x70,
diff --git a/quic/test_tools/quic_test_utils.cc b/quic/test_tools/quic_test_utils.cc
index 5321868..9f58985 100644
--- a/quic/test_tools/quic_test_utils.cc
+++ b/quic/test_tools/quic_test_utils.cc
@@ -1531,12 +1531,11 @@
}
QuicConnectionId source_connection_id(source_connection_id_bytes,
source_connection_id_length);
- const bool use_length_prefix =
- GetQuicFlag(FLAGS_quic_prober_uses_length_prefixed_connection_ids);
std::unique_ptr<QuicEncryptedPacket> encrypted_packet =
QuicFramer::BuildVersionNegotiationPacket(
source_connection_id, EmptyQuicConnectionId(),
- /*ietf_quic=*/true, use_length_prefix, ParsedQuicVersionVector{});
+ /*ietf_quic=*/true, /*use_length_prefix=*/true,
+ ParsedQuicVersionVector{});
if (!encrypted_packet) {
QUIC_BUG << "Failed to create version negotiation packet";
return false;
