Refactor some QUIC version code
Our VersionSupportsFeatureFoo functions currently take multiple forms (QuicUtils::VersionSupportsFeatureFoo(), VersionHasFeatureFoo(QuicTransportVersion) and ParsedQuicVersion::HasFeatureFoo()). This CL starts consolidating these on ParsedQuicVersion in an effort to clean this up.
This CL also adds a few DCHECKs to validate the assumption that we only use valid versions outside of the early parsing code in the dispatcher.
Note that VersionAllowsVariableLengthConnectionIds now only allows known versions so callers that deal with unknown versions need to check ParsedQuicVersion::IsKnown before calling VersionAllowsVariableLengthConnectionIds.
gfe-relnote: refactor, no behavior change, not flag-protected
PiperOrigin-RevId: 289536281
Change-Id: Iafa1ab9c228d9a8fe0ab4c998437526a1b7191dd
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index 352cd37..15ef0f9 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -779,8 +779,7 @@
}
TEST_P(EndToEndTest, SimpleRequestResponseZeroConnectionID) {
- if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- GetParam().negotiated_version.transport_version)) {
+ if (!GetParam().negotiated_version.AllowsVariableLengthConnectionIds()) {
ASSERT_TRUE(Initialize());
return;
}
@@ -802,8 +801,7 @@
}
TEST_P(EndToEndTestWithTls, ZeroConnectionID) {
- if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- GetParam().negotiated_version.transport_version)) {
+ if (!GetParam().negotiated_version.AllowsVariableLengthConnectionIds()) {
ASSERT_TRUE(Initialize());
return;
}
@@ -819,8 +817,7 @@
}
TEST_P(EndToEndTestWithTls, BadConnectionIdLength) {
- if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- GetParam().negotiated_version.transport_version)) {
+ if (!GetParam().negotiated_version.AllowsVariableLengthConnectionIds()) {
ASSERT_TRUE(Initialize());
return;
}
@@ -838,8 +835,7 @@
// Tests a very long (16-byte) initial destination connection ID to make
// sure the dispatcher properly replaces it with an 8-byte one.
TEST_P(EndToEndTestWithTls, LongBadConnectionIdLength) {
- if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- GetParam().negotiated_version.transport_version)) {
+ if (!GetParam().negotiated_version.AllowsVariableLengthConnectionIds()) {
ASSERT_TRUE(Initialize());
return;
}
@@ -890,8 +886,7 @@
}
TEST_P(EndToEndTestWithTls, ForcedVersionNegotiationAndBadConnectionIdLength) {
- if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- GetParam().negotiated_version.transport_version)) {
+ if (!GetParam().negotiated_version.AllowsVariableLengthConnectionIds()) {
ASSERT_TRUE(Initialize());
return;
}
@@ -913,8 +908,7 @@
// connection ID.
TEST_P(EndToEndTestWithTls, ForcedVersNegoAndClientCIDAndLongCID) {
if (!GetParam().negotiated_version.SupportsClientConnectionIds() ||
- !QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- GetParam().negotiated_version.transport_version)) {
+ !GetParam().negotiated_version.AllowsVariableLengthConnectionIds()) {
ASSERT_TRUE(Initialize());
return;
}
@@ -939,8 +933,7 @@
}
TEST_P(EndToEndTest, MixGoodAndBadConnectionIdLengths) {
- if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- GetParam().negotiated_version.transport_version)) {
+ if (!GetParam().negotiated_version.AllowsVariableLengthConnectionIds()) {
ASSERT_TRUE(Initialize());
return;
}
@@ -1096,8 +1089,7 @@
}
TEST_P(EndToEndTest, MultipleRequestResponseZeroConnectionID) {
- if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- GetParam().negotiated_version.transport_version)) {
+ if (!GetParam().negotiated_version.AllowsVariableLengthConnectionIds()) {
ASSERT_TRUE(Initialize());
return;
}
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index a36ea2a..4df2699 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -196,8 +196,8 @@
Perspective perspective) {
return perspective == Perspective::IS_CLIENT &&
header.form == IETF_QUIC_LONG_HEADER_PACKET &&
- QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- header.version.transport_version) &&
+ header.version.IsKnown() &&
+ header.version.AllowsVariableLengthConnectionIds() &&
(header.long_packet_type == INITIAL ||
header.long_packet_type == RETRY);
}
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc
index c628993..f1fbb32 100644
--- a/quic/core/quic_dispatcher.cc
+++ b/quic/core/quic_dispatcher.cc
@@ -286,8 +286,8 @@
if (packet_info.destination_connection_id.length() !=
expected_server_connection_id_length_ &&
!should_update_expected_server_connection_id_length_ &&
- !QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- packet_info.version.transport_version)) {
+ packet_info.version.IsKnown() &&
+ !packet_info.version.AllowsVariableLengthConnectionIds()) {
SetLastError(QUIC_INVALID_PACKET_HEADER);
QUIC_DLOG(ERROR) << "Invalid Connection Id Length";
return;
@@ -330,8 +330,7 @@
if (server_connection_id.length() == expected_server_connection_id_length_) {
return server_connection_id;
}
- DCHECK(QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- version.transport_version));
+ DCHECK(version.AllowsVariableLengthConnectionIds());
QuicConnectionId new_connection_id =
GenerateNewServerConnectionId(version, server_connection_id);
@@ -372,8 +371,7 @@
server_connection_id.length() < expected_server_connection_id_length_ &&
!allow_short_initial_server_connection_ids_) {
DCHECK(packet_info.version_flag);
- DCHECK(QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- packet_info.version.transport_version));
+ DCHECK(packet_info.version.AllowsVariableLengthConnectionIds());
QUIC_DLOG(INFO) << "Packet with short destination connection ID "
<< server_connection_id << " expected "
<< static_cast<int>(expected_server_connection_id_length_);
diff --git a/quic/core/quic_dispatcher_test.cc b/quic/core/quic_dispatcher_test.cc
index 83bb756..1462f7e 100644
--- a/quic/core/quic_dispatcher_test.cc
+++ b/quic/core/quic_dispatcher_test.cc
@@ -744,8 +744,7 @@
// Makes sure nine-byte connection IDs are replaced by 8-byte ones.
TEST_F(QuicDispatcherTest, LongConnectionIdLengthReplaced) {
- if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- CurrentSupportedVersions()[0].transport_version)) {
+ if (!CurrentSupportedVersions()[0].AllowsVariableLengthConnectionIds()) {
// When variable length connection IDs are not supported, the connection
// fails. See StrayPacketTruncatedConnectionId.
return;
@@ -777,8 +776,7 @@
// Makes sure zero-byte connection IDs are replaced by 8-byte ones.
TEST_F(QuicDispatcherTest, InvalidShortConnectionIdLengthReplaced) {
- if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- CurrentSupportedVersions()[0].transport_version)) {
+ if (!CurrentSupportedVersions()[0].AllowsVariableLengthConnectionIds()) {
// When variable length connection IDs are not supported, the connection
// fails. See StrayPacketTruncatedConnectionId.
return;
@@ -816,8 +814,7 @@
// Makes sure TestConnectionId(1) creates a new connection and
// TestConnectionIdNineBytesLong(2) gets replaced.
TEST_F(QuicDispatcherTest, MixGoodAndBadConnectionIdLengthPackets) {
- if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- CurrentSupportedVersions()[0].transport_version)) {
+ if (!CurrentSupportedVersions()[0].AllowsVariableLengthConnectionIds()) {
return;
}
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index 11d3aa6..8c2f86a 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -423,7 +423,7 @@
current_received_frame_type_(0) {
DCHECK(!supported_versions.empty());
version_ = supported_versions_[0];
- DCHECK(version_.transport_version != QUIC_VERSION_UNSUPPORTED ||
+ DCHECK(version_.IsKnown() ||
!GetQuicRestartFlag(quic_fix_handling_of_bad_prox_packet))
<< ParsedQuicVersionVectorToString(supported_versions_);
}
@@ -2528,10 +2528,7 @@
if (!should_update_expected_server_connection_id_length &&
(dcil != *destination_connection_id_length ||
scil != *source_connection_id_length) &&
- !QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- version.transport_version)) {
- // TODO(dschinazi): use the framer's version once the
- // OnProtocolVersionMismatch call is moved to before this is run.
+ version.IsKnown() && !version.AllowsVariableLengthConnectionIds()) {
QUIC_DVLOG(1) << "dcil: " << static_cast<uint32_t>(dcil)
<< ", scil: " << static_cast<uint32_t>(scil);
*detailed_error = "Invalid ConnectionId length.";
@@ -2626,7 +2623,7 @@
}
return true;
}
- if (!header->version.HasHeaderProtection()) {
+ if (header->version.IsKnown() && !header->version.HasHeaderProtection()) {
header->packet_number_length =
GetLongHeaderPacketNumberLength(header->type_byte);
}
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index 17779bd..b794cb1 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -12900,8 +12900,7 @@
}
TEST_P(QuicFramerTest, PacketHeaderWithVariableLengthConnectionId) {
- if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- framer_.transport_version())) {
+ if (!framer_.version().AllowsVariableLengthConnectionIds()) {
return;
}
SetDecrypterLevel(ENCRYPTION_FORWARD_SECURE);
@@ -13645,8 +13644,7 @@
}
const bool parse_success =
framer_.ProcessPacket(QuicEncryptedPacket(AsChars(p), p_length, false));
- if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- framer_.transport_version())) {
+ if (!framer_.version().AllowsVariableLengthConnectionIds()) {
EXPECT_FALSE(parse_success);
EXPECT_THAT(framer_.error(), IsError(QUIC_INVALID_PACKET_HEADER));
EXPECT_EQ("Invalid ConnectionId length.", framer_.detailed_error());
@@ -13711,8 +13709,7 @@
}
const bool parse_success =
framer_.ProcessPacket(QuicEncryptedPacket(AsChars(p), p_length, false));
- if (!QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- framer_.transport_version())) {
+ if (!framer_.version().AllowsVariableLengthConnectionIds()) {
EXPECT_FALSE(parse_success);
EXPECT_THAT(framer_.error(), IsError(QUIC_INVALID_PACKET_HEADER));
EXPECT_EQ("Invalid ConnectionId length.", framer_.detailed_error());
diff --git a/quic/core/quic_utils.cc b/quic/core/quic_utils.cc
index e37d019..733ee39 100644
--- a/quic/core/quic_utils.cc
+++ b/quic/core/quic_utils.cc
@@ -517,18 +517,9 @@
}
// static
-bool QuicUtils::VariableLengthConnectionIdAllowedForVersion(
- QuicTransportVersion version) {
- // We allow variable length connection IDs for unsupported versions to
- // ensure that IETF version negotiation works when other implementations
- // trigger version negotiation with custom connection ID lengths.
- return version > QUIC_VERSION_46 || version == QUIC_VERSION_UNSUPPORTED;
-}
-
-// static
QuicConnectionId QuicUtils::CreateZeroConnectionId(
QuicTransportVersion version) {
- if (!VariableLengthConnectionIdAllowedForVersion(version)) {
+ if (!VersionAllowsVariableLengthConnectionIds(version)) {
char connection_id_bytes[8] = {0, 0, 0, 0, 0, 0, 0, 0};
return QuicConnectionId(static_cast<char*>(connection_id_bytes),
QUICHE_ARRAYSIZE(connection_id_bytes));
@@ -558,7 +549,7 @@
const uint8_t connection_id_length8 =
static_cast<uint8_t>(connection_id_length);
// Versions that do not support variable lengths only support length 8.
- if (!VariableLengthConnectionIdAllowedForVersion(transport_version)) {
+ if (!VersionAllowsVariableLengthConnectionIds(transport_version)) {
return connection_id_length8 == kQuicDefaultConnectionIdLength;
}
// Versions that do support variable length but do not have length-prefixed
diff --git a/quic/core/quic_utils.h b/quic/core/quic_utils.h
index 590d377..a0e0452 100644
--- a/quic/core/quic_utils.h
+++ b/quic/core/quic_utils.h
@@ -186,10 +186,6 @@
static QuicConnectionId CreateRandomConnectionId(uint8_t connection_id_length,
QuicRandom* random);
- // Returns true if the QUIC version allows variable length connection IDs.
- static bool VariableLengthConnectionIdAllowedForVersion(
- QuicTransportVersion version);
-
// Returns true if the connection ID length is valid for this QUIC version.
static bool IsConnectionIdLengthValidForVersion(
size_t connection_id_length,
diff --git a/quic/core/quic_utils_test.cc b/quic/core/quic_utils_test.cc
index 69be371..aac0fd8 100644
--- a/quic/core/quic_utils_test.cc
+++ b/quic/core/quic_utils_test.cc
@@ -247,8 +247,7 @@
}
TEST_F(QuicUtilsTest, VariableLengthConnectionId) {
- EXPECT_FALSE(
- QuicUtils::VariableLengthConnectionIdAllowedForVersion(QUIC_VERSION_43));
+ EXPECT_FALSE(VersionAllowsVariableLengthConnectionIds(QUIC_VERSION_43));
EXPECT_TRUE(QuicUtils::IsConnectionIdValidForVersion(
QuicUtils::CreateZeroConnectionId(QUIC_VERSION_43), QUIC_VERSION_43));
EXPECT_TRUE(QuicUtils::IsConnectionIdValidForVersion(
diff --git a/quic/core/quic_versions.cc b/quic/core/quic_versions.cc
index 536797f..6daac15 100644
--- a/quic/core/quic_versions.cc
+++ b/quic/core/quic_versions.cc
@@ -49,52 +49,106 @@
}
bool ParsedQuicVersion::KnowsWhichDecrypterToUse() const {
+ DCHECK(IsKnown());
return transport_version > QUIC_VERSION_46 ||
handshake_protocol == PROTOCOL_TLS1_3;
}
bool ParsedQuicVersion::UsesInitialObfuscators() const {
+ DCHECK(IsKnown());
return transport_version > QUIC_VERSION_49 ||
handshake_protocol == PROTOCOL_TLS1_3;
}
bool ParsedQuicVersion::AllowsLowFlowControlLimits() const {
+ DCHECK(IsKnown());
return transport_version == QUIC_VERSION_99 &&
handshake_protocol == PROTOCOL_TLS1_3;
}
bool ParsedQuicVersion::HasHeaderProtection() const {
+ DCHECK(IsKnown());
return transport_version > QUIC_VERSION_49;
}
bool ParsedQuicVersion::SupportsRetry() const {
+ DCHECK(IsKnown());
return transport_version > QUIC_VERSION_46;
}
bool ParsedQuicVersion::SendsVariableLengthPacketNumberInLongHeader() const {
+ DCHECK(IsKnown());
return transport_version > QUIC_VERSION_46;
}
+bool ParsedQuicVersion::AllowsVariableLengthConnectionIds() const {
+ DCHECK(IsKnown());
+ return VersionAllowsVariableLengthConnectionIds(transport_version);
+}
+
bool ParsedQuicVersion::SupportsClientConnectionIds() const {
+ DCHECK(IsKnown() ||
+ !GetQuicRestartFlag(quic_fix_handling_of_bad_prox_packet));
return transport_version > QUIC_VERSION_48;
}
bool ParsedQuicVersion::HasLengthPrefixedConnectionIds() const {
+ DCHECK(IsKnown() ||
+ !GetQuicRestartFlag(quic_fix_handling_of_bad_prox_packet));
return VersionHasLengthPrefixedConnectionIds(transport_version);
}
bool ParsedQuicVersion::SupportsAntiAmplificationLimit() const {
+ DCHECK(IsKnown());
return transport_version == QUIC_VERSION_99 &&
handshake_protocol == PROTOCOL_TLS1_3;
}
bool ParsedQuicVersion::CanSendCoalescedPackets() const {
+ DCHECK(IsKnown());
return QuicVersionHasLongHeaderLengths(transport_version) &&
handshake_protocol == PROTOCOL_TLS1_3;
}
+bool ParsedQuicVersion::SupportsGoogleAltSvcFormat() const {
+ DCHECK(IsKnown());
+ return VersionSupportsGoogleAltSvcFormat(transport_version);
+}
+
+bool ParsedQuicVersion::HasIetfInvariantHeader() const {
+ DCHECK(IsKnown());
+ return VersionHasIetfInvariantHeader(transport_version);
+}
+
+bool ParsedQuicVersion::SupportsMessageFrames() const {
+ DCHECK(IsKnown());
+ return VersionSupportsMessageFrames(transport_version);
+}
+
+bool ParsedQuicVersion::UsesHttp3() const {
+ DCHECK(IsKnown());
+ return VersionUsesHttp3(transport_version);
+}
+
+bool ParsedQuicVersion::HasLongHeaderLengths() const {
+ DCHECK(IsKnown());
+ return QuicVersionHasLongHeaderLengths(transport_version);
+}
+
+bool ParsedQuicVersion::UsesCryptoFrames() const {
+ DCHECK(IsKnown());
+ return QuicVersionUsesCryptoFrames(transport_version);
+}
+
+bool ParsedQuicVersion::HasIetfQuicFrames() const {
+ DCHECK(IsKnown());
+ return VersionHasIetfQuicFrames(transport_version);
+}
+
bool VersionHasLengthPrefixedConnectionIds(
QuicTransportVersion transport_version) {
+ DCHECK(transport_version != QUIC_VERSION_UNSUPPORTED ||
+ !GetQuicRestartFlag(quic_fix_handling_of_bad_prox_packet));
return transport_version > QUIC_VERSION_48;
}
@@ -432,6 +486,12 @@
return transport_version <= QUIC_VERSION_46;
}
+bool VersionAllowsVariableLengthConnectionIds(
+ QuicTransportVersion transport_version) {
+ DCHECK_NE(transport_version, QUIC_VERSION_UNSUPPORTED);
+ return transport_version > QUIC_VERSION_46;
+}
+
bool QuicVersionLabelUses4BitConnectionIdLength(
QuicVersionLabel version_label) {
// As we deprecate old versions, we still need the ability to send valid
diff --git a/quic/core/quic_versions.h b/quic/core/quic_versions.h
index 66553cd..6adf748 100644
--- a/quic/core/quic_versions.h
+++ b/quic/core/quic_versions.h
@@ -154,11 +154,14 @@
constexpr ParsedQuicVersion(HandshakeProtocol handshake_protocol,
QuicTransportVersion transport_version)
: handshake_protocol(handshake_protocol),
- transport_version(transport_version) {}
+ transport_version(transport_version) {
+ DCHECK(ParsedQuicVersionIsValid(handshake_protocol, transport_version))
+ << QuicVersionToString(transport_version) << " "
+ << HandshakeProtocolToString(handshake_protocol);
+ }
constexpr ParsedQuicVersion(const ParsedQuicVersion& other)
- : handshake_protocol(other.handshake_protocol),
- transport_version(other.transport_version) {}
+ : ParsedQuicVersion(other.handshake_protocol, other.transport_version) {}
ParsedQuicVersion& operator=(const ParsedQuicVersion& other) {
if (this != &other) {
@@ -204,6 +207,10 @@
// header.
bool SendsVariableLengthPacketNumberInLongHeader() const;
+ // Returns whether this version allows server connection ID lengths
+ // that are not 64 bits.
+ bool AllowsVariableLengthConnectionIds() const;
+
// Returns whether this version supports client connection ID.
bool SupportsClientConnectionIds() const;
@@ -219,6 +226,39 @@
// Returns true if this version can send coalesced packets.
bool CanSendCoalescedPackets() const;
+
+ // Returns true if this version supports the old Google-style Alt-Svc
+ // advertisement format.
+ bool SupportsGoogleAltSvcFormat() const;
+
+ // Returns true if |transport_version| uses IETF invariant headers.
+ bool HasIetfInvariantHeader() const;
+
+ // Returns true if |transport_version| supports MESSAGE frames.
+ bool SupportsMessageFrames() const;
+
+ // If true, HTTP/3 instead of gQUIC will be used at the HTTP layer.
+ // Notable changes are:
+ // * Headers stream no longer exists.
+ // * PRIORITY, HEADERS are moved from headers stream to HTTP/3 control stream.
+ // * PUSH_PROMISE is moved to request stream.
+ // * Unidirectional streams will have their first byte as a stream type.
+ // * HEADERS frames are compressed using QPACK.
+ // * DATA frame has frame headers.
+ // * GOAWAY is moved to HTTP layer.
+ bool UsesHttp3() const;
+
+ // Returns whether the transport_version supports the variable length integer
+ // length field as defined by IETF QUIC draft-13 and later.
+ bool HasLongHeaderLengths() const;
+
+ // Returns whether |transport_version| uses CRYPTO frames for the handshake
+ // instead of stream 1.
+ bool UsesCryptoFrames() const;
+
+ // Returns whether |transport_version| makes use of IETF QUIC
+ // frames or not.
+ bool HasIetfQuicFrames() const;
};
QUIC_EXPORT_PRIVATE ParsedQuicVersion UnsupportedQuicVersion();
@@ -433,6 +473,11 @@
QUIC_EXPORT_PRIVATE bool VersionSupportsGoogleAltSvcFormat(
QuicTransportVersion transport_version);
+// Returns whether this version allows server connection ID lengths that are
+// not 64 bits.
+QUIC_EXPORT_PRIVATE bool VersionAllowsVariableLengthConnectionIds(
+ QuicTransportVersion transport_version);
+
// Returns whether this version label supports long header 4-bit encoded
// connection ID lengths as described in draft-ietf-quic-invariants-05 and
// draft-ietf-quic-transport-21.
