Add LengthPrefixedConnectionId support to QuicDataReader and Writer

This is currently only used in v99 NEW_CONNECTION_ID frames but will soon be used to parse the new connection ID invariants

gfe-relnote: n/a, protected by disabled v99 flag
PiperOrigin-RevId: 258684056
Change-Id: I666d150b9392e195a073272d2c5e79bd970d5862
diff --git a/quic/core/quic_data_reader.cc b/quic/core/quic_data_reader.cc
index a919018..e2871d7 100644
--- a/quic/core/quic_data_reader.cc
+++ b/quic/core/quic_data_reader.cc
@@ -165,6 +165,18 @@
   return ok;
 }
 
+bool QuicDataReader::ReadLengthPrefixedConnectionId(
+    QuicConnectionId* connection_id) {
+  uint8_t connection_id_length;
+  if (!ReadUInt8(&connection_id_length)) {
+    return false;
+  }
+  if (connection_id_length > kQuicMaxConnectionIdLength) {
+    return false;
+  }
+  return ReadConnectionId(connection_id, connection_id_length);
+}
+
 bool QuicDataReader::ReadTag(uint32_t* tag) {
   return ReadBytes(tag, sizeof(*tag));
 }
diff --git a/quic/core/quic_data_reader.h b/quic/core/quic_data_reader.h
index a03b927..72e2d13 100644
--- a/quic/core/quic_data_reader.h
+++ b/quic/core/quic_data_reader.h
@@ -83,6 +83,11 @@
   // Returns true on success, false otherwise.
   bool ReadConnectionId(QuicConnectionId* connection_id, uint8_t length);
 
+  // Reads 8-bit connection ID length followed by connection ID of that length.
+  // Forwards the internal iterator on success.
+  // Returns true on success, false otherwise.
+  bool ReadLengthPrefixedConnectionId(QuicConnectionId* connection_id);
+
   // Reads tag represented as 32-bit unsigned integer into given output
   // parameter. Tags are in big endian on the wire (e.g., CHLO is
   // 'C','H','L','O') and are read in byte order, so tags in memory are in big
diff --git a/quic/core/quic_data_writer.cc b/quic/core/quic_data_writer.cc
index 42f1e4e..e01eb6c 100644
--- a/quic/core/quic_data_writer.cc
+++ b/quic/core/quic_data_writer.cc
@@ -181,6 +181,11 @@
   return WriteBytes(connection_id.data(), connection_id.length());
 }
 
+bool QuicDataWriter::WriteLengthPrefixedConnectionId(
+    QuicConnectionId connection_id) {
+  return WriteUInt8(connection_id.length()) && WriteConnectionId(connection_id);
+}
+
 bool QuicDataWriter::WriteTag(uint32_t tag) {
   return WriteBytes(&tag, sizeof(tag));
 }
diff --git a/quic/core/quic_data_writer.h b/quic/core/quic_data_writer.h
index d2d2b6b..c43d0ff 100644
--- a/quic/core/quic_data_writer.h
+++ b/quic/core/quic_data_writer.h
@@ -108,6 +108,9 @@
   // Write connection ID to the payload.
   bool WriteConnectionId(QuicConnectionId connection_id);
 
+  // Write 8-bit length followed by connection ID to the payload.
+  bool WriteLengthPrefixedConnectionId(QuicConnectionId connection_id);
+
   // Write tag as a 32-bit unsigned integer to the payload. As tags are already
   // converted to big endian (e.g., CHLO is 'C','H','L','O') in memory by TAG or
   // MakeQuicTag and tags are written in byte order, so tags on the wire are
diff --git a/quic/core/quic_data_writer_test.cc b/quic/core/quic_data_writer_test.cc
index 07f0313..727c813 100644
--- a/quic/core/quic_data_writer_test.cc
+++ b/quic/core/quic_data_writer_test.cc
@@ -5,6 +5,7 @@
 #include "net/third_party/quiche/src/quic/core/quic_data_writer.h"
 
 #include <cstdint>
+#include <cstring>
 
 #include "net/third_party/quiche/src/quic/core/quic_connection_id.h"
 #include "net/third_party/quiche/src/quic/core/quic_data_reader.h"
@@ -26,6 +27,12 @@
 struct TestParams {
   explicit TestParams(Endianness endianness) : endianness(endianness) {}
 
+  friend std::ostream& operator<<(std::ostream& os, const TestParams& p) {
+    os << "{ " << (p.endianness == NETWORK_BYTE_ORDER ? "network" : "host")
+       << " byte order }";
+    return os;
+  }
+
   Endianness endianness;
 };
 
@@ -270,6 +277,48 @@
   EXPECT_EQ(connection_id, read_connection_id);
 }
 
+TEST_P(QuicDataWriterTest, LengthPrefixedConnectionId) {
+  QuicConnectionId connection_id =
+      TestConnectionId(UINT64_C(0x0011223344556677));
+  char length_prefixed_connection_id[] = {
+      0x08, 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
+  };
+  EXPECT_EQ(QUIC_ARRAYSIZE(length_prefixed_connection_id),
+            kConnectionIdLengthSize + connection_id.length());
+  char buffer[kConnectionIdLengthSize + kQuicMaxConnectionIdLength] = {};
+  QuicDataWriter writer(QUIC_ARRAYSIZE(buffer), buffer);
+  EXPECT_TRUE(writer.WriteLengthPrefixedConnectionId(connection_id));
+  test::CompareCharArraysWithHexError(
+      "WriteLengthPrefixedConnectionId", buffer, writer.length(),
+      length_prefixed_connection_id,
+      QUIC_ARRAYSIZE(length_prefixed_connection_id));
+
+  // Verify that writing length then connection ID produces the same output.
+  memset(buffer, 0, QUIC_ARRAYSIZE(buffer));
+  QuicDataWriter writer2(QUIC_ARRAYSIZE(buffer), buffer);
+  EXPECT_TRUE(writer2.WriteUInt8(connection_id.length()));
+  EXPECT_TRUE(writer2.WriteConnectionId(connection_id));
+  test::CompareCharArraysWithHexError(
+      "Write length then ConnectionId", buffer, writer2.length(),
+      length_prefixed_connection_id,
+      QUIC_ARRAYSIZE(length_prefixed_connection_id));
+
+  QuicConnectionId read_connection_id;
+  QuicDataReader reader(buffer, QUIC_ARRAYSIZE(buffer));
+  EXPECT_TRUE(reader.ReadLengthPrefixedConnectionId(&read_connection_id));
+  EXPECT_EQ(connection_id, read_connection_id);
+
+  // Verify that reading length then connection ID produces the same output.
+  uint8_t read_connection_id_length2 = 33;
+  QuicConnectionId read_connection_id2;
+  QuicDataReader reader2(buffer, QUIC_ARRAYSIZE(buffer));
+  ASSERT_TRUE(reader2.ReadUInt8(&read_connection_id_length2));
+  EXPECT_EQ(connection_id.length(), read_connection_id_length2);
+  EXPECT_TRUE(reader2.ReadConnectionId(&read_connection_id2,
+                                       read_connection_id_length2));
+  EXPECT_EQ(connection_id, read_connection_id2);
+}
+
 TEST_P(QuicDataWriterTest, EmptyConnectionIds) {
   QuicConnectionId empty_connection_id = EmptyQuicConnectionId();
   char buffer[2];
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index 1280f2e..5c9887d 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -6053,12 +6053,7 @@
     set_detailed_error("Can not write New Connection ID retire_prior_to");
     return false;
   }
-  if (!writer->WriteUInt8(frame.connection_id.length())) {
-    set_detailed_error(
-        "Can not write New Connection ID frame connection ID Length");
-    return false;
-  }
-  if (!writer->WriteConnectionId(frame.connection_id)) {
+  if (!writer->WriteLengthPrefixedConnectionId(frame.connection_id)) {
     set_detailed_error("Can not write New Connection ID frame connection ID");
     return false;
   }
@@ -6089,30 +6084,23 @@
     set_detailed_error("Retire_prior_to > sequence_number.");
     return false;
   }
-  uint8_t connection_id_length;
-  if (!reader->ReadUInt8(&connection_id_length)) {
-    set_detailed_error(
-        "Unable to read new connection ID frame connection id length.");
+
+  if (!reader->ReadLengthPrefixedConnectionId(&frame->connection_id)) {
+    set_detailed_error("Unable to read new connection ID frame connection id.");
     return false;
   }
 
-  if (connection_id_length > kQuicMaxConnectionIdLength) {
+  if (frame->connection_id.length() > kQuicMaxConnectionIdLength) {
     set_detailed_error("New connection ID length too high.");
     return false;
   }
 
-  if (connection_id_length != kQuicDefaultConnectionIdLength &&
-      !QuicUtils::VariableLengthConnectionIdAllowedForVersion(
-          transport_version())) {
+  if (!QuicUtils::IsConnectionIdValidForVersion(frame->connection_id,
+                                                transport_version())) {
     set_detailed_error("Invalid new connection ID length for version.");
     return false;
   }
 
-  if (!reader->ReadConnectionId(&frame->connection_id, connection_id_length)) {
-    set_detailed_error("Unable to read new connection ID frame connection id.");
-    return false;
-  }
-
   if (!reader->ReadBytes(&frame->stateless_reset_token,
                          sizeof(frame->stateless_reset_token))) {
     set_detailed_error("Can not read new connection ID frame reset token.");
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index 690e911..d73c412 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -16,6 +16,7 @@
 #include "net/third_party/quiche/src/quic/core/crypto/quic_decrypter.h"
 #include "net/third_party/quiche/src/quic/core/crypto/quic_encrypter.h"
 #include "net/third_party/quiche/src/quic/core/quic_connection_id.h"
+#include "net/third_party/quiche/src/quic/core/quic_error_codes.h"
 #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"
@@ -11293,7 +11294,7 @@
        {kVarInt62OneByte + 0x11}},
       {"Unable to read new connection ID frame retire_prior_to.",
        {kVarInt62OneByte + 0x09}},
-      {"Unable to read new connection ID frame connection id length.",
+      {"Unable to read new connection ID frame connection id.",
        {0x08}},  // connection ID length
       {"Unable to read new connection ID frame connection id.",
        {0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x11}},
@@ -11352,7 +11353,7 @@
        {kVarInt62OneByte + 0x11}},
       {"Unable to read new connection ID frame retire_prior_to.",
        {kVarInt62OneByte + 0x0a}},
-      {"Unable to read new connection ID frame connection id length.",
+      {"Unable to read new connection ID frame connection id.",
        {0x09}},  // connection ID length
       {"Unable to read new connection ID frame connection id.",
        {0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10, 0x42}},
@@ -11413,7 +11414,7 @@
        {kVarInt62OneByte + 0x11}},
       {"Unable to read new connection ID frame retire_prior_to.",
        {kVarInt62OneByte + 0x0b}},
-      {"Unable to read new connection ID frame connection id length.",
+      {"Unable to read new connection ID frame connection id.",
        {0x13}},  // connection ID length
       {"Unable to read new connection ID frame connection id.",
        {0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10,
@@ -11429,7 +11430,8 @@
       AssemblePacketFromFragments(packet99));
   EXPECT_FALSE(framer_.ProcessPacket(*encrypted));
   EXPECT_EQ(QUIC_INVALID_NEW_CONNECTION_ID_DATA, framer_.error());
-  EXPECT_EQ("New connection ID length too high.", framer_.detailed_error());
+  EXPECT_EQ("Unable to read new connection ID frame connection id.",
+            framer_.detailed_error());
 }
 
 // Verifies that parsing a NEW_CONNECTION_ID frame with an invalid