Increase array size in QuicConnectionId.
Some users of this class try to minimize heap allocations, and compliant QUIC clients can pick IDs that trigger a heap allocation.
Also, when we eventually transition to encrypted connection IDs, CID length close to the maximum will become the norm.
This increases the size of QuicConnectionId from 16B to 24B, undoing the performance gain from cl/338718268 for the (current) common case where connection IDs are 8B.
PiperOrigin-RevId: 743590623
diff --git a/quiche/quic/core/quic_connection_id.cc b/quiche/quic/core/quic_connection_id.cc
index 1dcc505..dce13b2 100644
--- a/quiche/quic/core/quic_connection_id.cc
+++ b/quiche/quic/core/quic_connection_id.cc
@@ -6,20 +6,17 @@
#include <cstddef>
#include <cstdint>
+#include <cstdlib>
#include <cstring>
-#include <iomanip>
#include <ostream>
#include <string>
#include "absl/strings/escaping.h"
+#include "absl/strings/string_view.h"
+#include "absl/types/span.h"
#include "openssl/siphash.h"
#include "quiche/quic/core/crypto/quic_random.h"
-#include "quiche/quic/core/quic_types.h"
-#include "quiche/quic/platform/api/quic_bug_tracker.h"
-#include "quiche/quic/platform/api/quic_flag_utils.h"
-#include "quiche/quic/platform/api/quic_flags.h"
-#include "quiche/quic/platform/api/quic_logging.h"
-#include "quiche/common/quiche_endian.h"
+#include "quiche/common/platform/api/quiche_logging.h"
namespace quic {
@@ -55,7 +52,7 @@
static_assert(offsetof(QuicConnectionId, padding_) ==
offsetof(QuicConnectionId, length_),
"bad offset");
- static_assert(sizeof(QuicConnectionId) <= 16, "bad size");
+ static_assert(sizeof(QuicConnectionId) <= 24, "bad size");
}
QuicConnectionId::QuicConnectionId(const char* data, uint8_t length) {
diff --git a/quiche/quic/core/quic_connection_id.h b/quiche/quic/core/quic_connection_id.h
index b7f8b48..84702db 100644
--- a/quiche/quic/core/quic_connection_id.h
+++ b/quiche/quic/core/quic_connection_id.h
@@ -5,12 +5,13 @@
#ifndef QUICHE_QUIC_CORE_QUIC_CONNECTION_ID_H_
#define QUICHE_QUIC_CORE_QUIC_CONNECTION_ID_H_
+#include <cstddef>
#include <cstdint>
+#include <ostream>
#include <string>
-#include <vector>
#include "absl/types/span.h"
-#include "quiche/quic/platform/api/quic_export.h"
+#include "quiche/common/platform/api/quiche_export.h"
namespace quic {
@@ -44,7 +45,7 @@
// Creates a connection ID from network order bytes.
QuicConnectionId(const char* data, uint8_t length);
- QuicConnectionId(const absl::Span<const uint8_t> data);
+ QuicConnectionId(absl::Span<const uint8_t> data);
// Creates a connection ID from another connection ID.
QuicConnectionId(const QuicConnectionId& other);
@@ -107,12 +108,13 @@
// first |length_| bytes of |data_short_|.
// Otherwise it is stored in |data_long_| which is guaranteed to have a size
// equal to |length_|.
- // A value of 11 was chosen because our commonly used connection ID length
- // is 8 and with the length, the class is padded to at least 12 bytes
- // anyway.
+ // A value of 23 was chosen because it can hold 20 byte IDs that are legal
+ // in currently supported versions, expanded to a 24-byte struct to be 8-
+ // byte aligned. Unknown versions that would trigger version negotiation
+ // packets might have IDs beyond 23 bytes, which are stored in |data_long_|.
struct {
uint8_t padding_; // Match length_ field of the other union member.
- char data_short_[11];
+ char data_short_[23];
};
struct {
uint8_t length_; // length of the connection ID, in bytes.
diff --git a/quiche/quic/core/quic_connection_id_test.cc b/quiche/quic/core/quic_connection_id_test.cc
index fdd4c6f..9d663a4 100644
--- a/quiche/quic/core/quic_connection_id_test.cc
+++ b/quiche/quic/core/quic_connection_id_test.cc
@@ -6,10 +6,8 @@
#include <cstdint>
#include <cstring>
-#include <string>
-#include "absl/base/macros.h"
-#include "quiche/quic/core/quic_types.h"
+#include "absl/types/span.h"
#include "quiche/quic/platform/api/quic_test.h"
#include "quiche/quic/test_tools/quic_test_utils.h"
@@ -143,37 +141,43 @@
TEST_F(QuicConnectionIdTest, ChangeLength) {
QuicConnectionId connection_id64_1 = test::TestConnectionId(1);
QuicConnectionId connection_id64_2 = test::TestConnectionId(2);
- QuicConnectionId connection_id136_2 = test::TestConnectionId(2);
- connection_id136_2.set_length(17);
- memset(connection_id136_2.mutable_data() + 8, 0, 9);
- char connection_id136_2_bytes[17] = {0, 0, 0, 0, 0, 0, 0, 2, 0,
- 0, 0, 0, 0, 0, 0, 0, 0};
- QuicConnectionId connection_id136_2b(connection_id136_2_bytes,
- sizeof(connection_id136_2_bytes));
- EXPECT_EQ(connection_id136_2, connection_id136_2b);
+ QuicConnectionId connection_id200_2 = test::TestConnectionId(2);
+ connection_id200_2.set_length(25);
+ memset(connection_id200_2.mutable_data() + 8, 0, 17);
+ char connection_id200_2_bytes[25] = {0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
+ QuicConnectionId connection_id200_2b(connection_id200_2_bytes,
+ sizeof(connection_id200_2_bytes));
+ EXPECT_EQ(connection_id200_2, connection_id200_2b);
QuicConnectionId connection_id = connection_id64_1;
- connection_id.set_length(17);
+ connection_id.set_length(25);
EXPECT_NE(connection_id64_1, connection_id);
// Check resizing big to small.
connection_id.set_length(8);
EXPECT_EQ(connection_id64_1, connection_id);
// Check resizing small to big.
- connection_id.set_length(17);
+ connection_id.set_length(25);
memset(connection_id.mutable_data(), 0, connection_id.length());
memcpy(connection_id.mutable_data(), connection_id64_2.data(),
connection_id64_2.length());
- EXPECT_EQ(connection_id136_2, connection_id);
- EXPECT_EQ(connection_id136_2b, connection_id);
- QuicConnectionId connection_id120(connection_id136_2_bytes, 15);
- connection_id.set_length(15);
- EXPECT_EQ(connection_id120, connection_id);
+ EXPECT_EQ(connection_id200_2, connection_id);
+ EXPECT_EQ(connection_id200_2b, connection_id);
+ QuicConnectionId connection_id192(connection_id200_2_bytes, 24);
+ connection_id.set_length(24);
+ EXPECT_EQ(connection_id192, connection_id);
// Check resizing big to big.
- QuicConnectionId connection_id2 = connection_id120;
- connection_id2.set_length(17);
- connection_id2.mutable_data()[15] = 0;
- connection_id2.mutable_data()[16] = 0;
- EXPECT_EQ(connection_id136_2, connection_id2);
- EXPECT_EQ(connection_id136_2b, connection_id2);
+ QuicConnectionId connection_id2 = connection_id192;
+ connection_id2.set_length(25);
+ connection_id2.mutable_data()[24] = 0;
+ EXPECT_EQ(connection_id200_2, connection_id2);
+ EXPECT_EQ(connection_id200_2b, connection_id2);
+}
+
+TEST_F(QuicConnectionIdTest, MaximumLength) {
+ char bytes[255];
+ memset(bytes, 0xa, sizeof(bytes));
+ QuicConnectionId max_length(bytes, sizeof(bytes));
+ EXPECT_EQ(memcmp(max_length.data(), bytes, sizeof(bytes)), 0);
}
} // namespace
diff --git a/quiche/quic/test_tools/quic_connection_peer.cc b/quiche/quic/test_tools/quic_connection_peer.cc
index f5428d1..f1a720d 100644
--- a/quiche/quic/test_tools/quic_connection_peer.cc
+++ b/quiche/quic/test_tools/quic_connection_peer.cc
@@ -571,7 +571,7 @@
<< " ecn_codepoint passed: " << (info.ecn_codepoint == ECN_NOT_ECT)
<< " sizeof(ReceivedPacketInfo) passed: "
<< (sizeof(size_t) != 8 ||
- sizeof(QuicConnection::ReceivedPacketInfo) == 280);
+ sizeof(QuicConnection::ReceivedPacketInfo) == 304);
return info.destination_address == QuicSocketAddress() &&
info.source_address == QuicSocketAddress() &&
info.receipt_time == QuicTime::Zero() &&
@@ -585,7 +585,7 @@
// have changed. Please add the relevant conditions and update the
// length below.
(sizeof(size_t) != 8 ||
- sizeof(QuicConnection::ReceivedPacketInfo) == 280);
+ sizeof(QuicConnection::ReceivedPacketInfo) == 304);
}
// static
