Refactor QUIC version parsing This CL fixes an oversight where QuicFramer::ParsePublicHeader was not parsing the version when parsing packets from versions <= 43. This was not an issue because there are only two clients of that method: 1) QuicFramer::ProcessIetfPacketHeader and that is not called for versions <= 43. 2) QuicDispatcher::ProcessPacket and that was parsing the version after the fact (this CL now removes that when the quic_use_parse_public_header flag is true which corresponds to when the dispatcher uses ParsePublicHeader. This CL also adds a test to prevent this from regressing. Since this CL only refactors code and does not change functionality, it is not flag protected. gfe-relnote: refactor version parsing, not flag protected PiperOrigin-RevId: 261792952 Change-Id: If5a1333590a41e8a03fd53c6f8cea70fa47e8fd4
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc index 1782e4a..c944862 100644 --- a/quic/core/quic_dispatcher.cc +++ b/quic/core/quic_dispatcher.cc
@@ -253,7 +253,9 @@ QUIC_DLOG(ERROR) << detailed_error; return; } - packet_info.version = ParseQuicVersionLabel(packet_info.version_label); + if (!GetQuicReloadableFlag(quic_use_parse_public_header)) { + packet_info.version = ParseQuicVersionLabel(packet_info.version_label); + } if (packet_info.destination_connection_id.length() != expected_server_connection_id_length_ && !should_update_expected_server_connection_id_length_ &&
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc index 426f0de..ea9d9f5 100644 --- a/quic/core/quic_framer.cc +++ b/quic/core/quic_framer.cc
@@ -6396,6 +6396,7 @@ PacketHeaderFormat* format, bool* version_present, QuicVersionLabel* version_label, + ParsedQuicVersion* parsed_version, QuicConnectionId* destination_connection_id, std::string* detailed_error) { *format = GOOGLE_QUIC_PACKET; @@ -6409,9 +6410,12 @@ *detailed_error = "Unable to read ConnectionId."; return QUIC_INVALID_PACKET_HEADER; } - if (*version_present && !ProcessVersionLabel(reader, version_label)) { - *detailed_error = "Unable to read protocol version."; - return QUIC_INVALID_PACKET_HEADER; + if (*version_present) { + if (!ProcessVersionLabel(reader, version_label)) { + *detailed_error = "Unable to read protocol version."; + return QUIC_INVALID_PACKET_HEADER; + } + *parsed_version = ParseQuicVersionLabel(*version_label); } return QUIC_NO_ERROR; } @@ -6542,7 +6546,7 @@ if (!ietf_format) { return ParsePublicHeaderGoogleQuic( reader, first_byte, format, version_present, version_label, - destination_connection_id, detailed_error); + parsed_version, destination_connection_id, detailed_error); } *format = GetIetfPacketHeaderFormat(*first_byte);
diff --git a/quic/core/quic_framer.h b/quic/core/quic_framer.h index 33b9b37..be5f128 100644 --- a/quic/core/quic_framer.h +++ b/quic/core/quic_framer.h
@@ -870,6 +870,7 @@ PacketHeaderFormat* format, bool* version_present, QuicVersionLabel* version_label, + ParsedQuicVersion* parsed_version, QuicConnectionId* destination_connection_id, std::string* detailed_error);
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc index a853829..99864ad 100644 --- a/quic/core/quic_framer_test.cc +++ b/quic/core/quic_framer_test.cc
@@ -20,6 +20,7 @@ #include "net/third_party/quiche/src/quic/core/quic_packets.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/core/quic_utils.h" +#include "net/third_party/quiche/src/quic/core/quic_versions.h" #include "net/third_party/quiche/src/quic/platform/api/quic_arraysize.h" #include "net/third_party/quiche/src/quic/platform/api/quic_expect_bug.h" #include "net/third_party/quiche/src/quic/platform/api/quic_flags.h" @@ -1096,6 +1097,112 @@ EXPECT_EQ(FramerTestConnectionIdPlusOne(), source_connection_id); } +TEST_P(QuicFramerTest, ParsePublicHeader) { + const unsigned char type_byte = + framer_.transport_version() == QUIC_VERSION_44 ? 0xFD : 0xE3; + // clang-format off + unsigned char packet[] = { + // public flags (version included, 8-byte connection ID, + // 4-byte packet number) + 0x29, + // connection_id + 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10, + // version + QUIC_VERSION_BYTES, + // packet number + 0x12, 0x34, 0x56, 0x78, + // padding frame + 0x00, + }; + unsigned char packet44[] = { + // public flags (long header with packet type HANDSHAKE and + // 4-byte packet number) + type_byte, + // version + QUIC_VERSION_BYTES, + // connection ID lengths + 0x50, + // destination connection ID + 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10, + // long header packet length + 0x05, + // packet number + 0x12, 0x34, 0x56, 0x78, + // padding frame + 0x00, + }; + unsigned char packet99[] = { + // public flags (long header with packet type HANDSHAKE and + // 4-byte packet number) + 0xE3, + // version + QUIC_VERSION_BYTES, + // destination connection ID length + 0x08, + // destination connection ID + 0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10, + // source connection ID length + 0x00, + // long header packet length + 0x05, + // packet number + 0x12, 0x34, 0x56, 0x78, + // padding frame + 0x00, + }; + // clang-format on + unsigned char* p = packet; + size_t p_length = QUIC_ARRAYSIZE(packet); + if (framer_.transport_version() == QUIC_VERSION_99) { + p = packet99; + p_length = QUIC_ARRAYSIZE(packet99); + } else if (framer_.transport_version() >= QUIC_VERSION_44) { + p = packet44; + p_length = QUIC_ARRAYSIZE(packet44); + } + + uint8_t first_byte = 0x33; + PacketHeaderFormat format = GOOGLE_QUIC_PACKET; + bool version_present = false, has_length_prefix = false; + QuicVersionLabel version_label = 0; + ParsedQuicVersion parsed_version = UnsupportedQuicVersion(); + QuicConnectionId destination_connection_id = EmptyQuicConnectionId(), + source_connection_id = EmptyQuicConnectionId(); + QuicLongHeaderType long_packet_type = INVALID_PACKET_TYPE; + QuicVariableLengthIntegerLength retry_token_length_length = + VARIABLE_LENGTH_INTEGER_LENGTH_4; + QuicStringPiece retry_token; + std::string detailed_error = "foobar"; + + QuicDataReader reader(AsChars(p), p_length); + const QuicErrorCode parse_error = QuicFramer::ParsePublicHeader( + &reader, kQuicDefaultConnectionIdLength, + /*ietf_format=*/ + VersionHasIetfInvariantHeader(framer_.transport_version()), &first_byte, + &format, &version_present, &has_length_prefix, &version_label, + &parsed_version, &destination_connection_id, &source_connection_id, + &long_packet_type, &retry_token_length_length, &retry_token, + &detailed_error); + EXPECT_EQ(QUIC_NO_ERROR, parse_error); + EXPECT_EQ("", detailed_error); + EXPECT_EQ(p[0], first_byte); + EXPECT_TRUE(version_present); + EXPECT_EQ(framer_.version().HasLengthPrefixedConnectionIds(), + has_length_prefix); + EXPECT_EQ(CreateQuicVersionLabel(framer_.version()), version_label); + EXPECT_EQ(framer_.version(), parsed_version); + EXPECT_EQ(FramerTestConnectionId(), destination_connection_id); + EXPECT_EQ(EmptyQuicConnectionId(), source_connection_id); + EXPECT_EQ(VARIABLE_LENGTH_INTEGER_LENGTH_0, retry_token_length_length); + EXPECT_EQ(QuicStringPiece(), retry_token); + if (VersionHasIetfInvariantHeader(framer_.transport_version())) { + EXPECT_EQ(IETF_QUIC_LONG_HEADER_PACKET, format); + EXPECT_EQ(HANDSHAKE, long_packet_type); + } else { + EXPECT_EQ(GOOGLE_QUIC_PACKET, format); + } +} + TEST_P(QuicFramerTest, ClientConnectionIdFromShortHeaderToClient) { if (!framer_.version().SupportsClientConnectionIds()) { return;