Fix C++ defensive coding advice from the QUICv2 CL (cl/424961568).
There is much more to be done, but that could get out of control fast. Let me know if you want to abandon this, push more into this CL, or commit as-is.
PiperOrigin-RevId: 425625946
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index 1afbbdd..627f701 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -215,30 +215,22 @@
}
}
-bool GetLongHeaderType(uint8_t type, const ParsedQuicVersion& version,
- QuicLongHeaderType* long_header_type) {
+QuicLongHeaderType GetLongHeaderType(uint8_t type,
+ const ParsedQuicVersion& version) {
QUICHE_DCHECK((type & FLAGS_LONG_HEADER));
switch ((type & 0x30) >> 4) {
case 0:
- *long_header_type = version.UsesV2PacketTypes() ? RETRY : INITIAL;
- break;
+ return version.UsesV2PacketTypes() ? RETRY : INITIAL;
case 1:
- *long_header_type =
- version.UsesV2PacketTypes() ? INITIAL : ZERO_RTT_PROTECTED;
- break;
+ return version.UsesV2PacketTypes() ? INITIAL : ZERO_RTT_PROTECTED;
case 2:
- *long_header_type =
- version.UsesV2PacketTypes() ? ZERO_RTT_PROTECTED : HANDSHAKE;
- break;
+ return version.UsesV2PacketTypes() ? ZERO_RTT_PROTECTED : HANDSHAKE;
case 3:
- *long_header_type = version.UsesV2PacketTypes() ? HANDSHAKE : RETRY;
- break;
+ return version.UsesV2PacketTypes() ? HANDSHAKE : RETRY;
default:
QUIC_BUG(quic_bug_10850_4) << "Unreachable statement";
- *long_header_type = INVALID_PACKET_TYPE;
- return false;
+ return INVALID_PACKET_TYPE;
}
- return true;
}
QuicPacketNumberLength GetLongHeaderPacketNumberLength(uint8_t type) {
@@ -2627,22 +2619,27 @@
set_detailed_error("Fixed bit is 0 in long header.");
return false;
}
- if (!GetLongHeaderType(type, header->version,
- &header->long_packet_type)) {
- set_detailed_error("Illegal long header type value.");
- return false;
- }
- if (header->long_packet_type == RETRY) {
- if (!version().SupportsRetry()) {
- set_detailed_error("RETRY not supported in this version.");
+ header->long_packet_type = GetLongHeaderType(type, header->version);
+ switch (header->long_packet_type) {
+ case INVALID_PACKET_TYPE:
+ set_detailed_error("Illegal long header type value.");
return false;
- }
- if (perspective_ == Perspective::IS_SERVER) {
- set_detailed_error("Client-initiated RETRY is invalid.");
- return false;
- }
- } else if (!header->version.HasHeaderProtection()) {
- header->packet_number_length = GetLongHeaderPacketNumberLength(type);
+ case RETRY:
+ if (!version().SupportsRetry()) {
+ set_detailed_error("RETRY not supported in this version.");
+ return false;
+ }
+ if (perspective_ == Perspective::IS_SERVER) {
+ set_detailed_error("Client-initiated RETRY is invalid.");
+ return false;
+ }
+ break;
+ default:
+ if (!header->version.HasHeaderProtection()) {
+ header->packet_number_length =
+ GetLongHeaderPacketNumberLength(type);
+ }
+ break;
}
}
}
@@ -4597,7 +4594,8 @@
QuicLongHeaderType header_type;
if (IsLongHeader(type_byte)) {
bitmask = 0x0f;
- if (!GetLongHeaderType(type_byte, version_, &header_type)) {
+ header_type = GetLongHeaderType(type_byte, version_);
+ if (header_type == INVALID_PACKET_TYPE) {
return false;
}
}
@@ -6894,15 +6892,15 @@
}
inline bool ParseLongHeaderConnectionIds(
- QuicDataReader* reader, bool has_length_prefix,
- QuicVersionLabel version_label, QuicConnectionId* destination_connection_id,
- QuicConnectionId* source_connection_id, std::string* detailed_error) {
+ QuicDataReader& reader, bool has_length_prefix,
+ QuicVersionLabel version_label, QuicConnectionId& destination_connection_id,
+ QuicConnectionId& source_connection_id, std::string& detailed_error) {
if (has_length_prefix) {
- if (!reader->ReadLengthPrefixedConnectionId(destination_connection_id)) {
- *detailed_error = "Unable to read destination connection ID.";
+ if (!reader.ReadLengthPrefixedConnectionId(&destination_connection_id)) {
+ detailed_error = "Unable to read destination connection ID.";
return false;
}
- if (!reader->ReadLengthPrefixedConnectionId(source_connection_id)) {
+ if (!reader.ReadLengthPrefixedConnectionId(&source_connection_id)) {
if (version_label == kProxVersionLabel) {
// The "PROX" version does not follow the length-prefixed invariants,
// and can therefore attempt to read a payload byte and interpret it
@@ -6911,14 +6909,14 @@
// parsing as successful.
return true;
}
- *detailed_error = "Unable to read source connection ID.";
+ detailed_error = "Unable to read source connection ID.";
return false;
}
} else {
// Parse connection ID lengths.
uint8_t connection_id_lengths_byte;
- if (!reader->ReadUInt8(&connection_id_lengths_byte)) {
- *detailed_error = "Unable to read connection ID lengths.";
+ if (!reader.ReadUInt8(&connection_id_lengths_byte)) {
+ detailed_error = "Unable to read connection ID lengths.";
return false;
}
uint8_t destination_connection_id_length =
@@ -6933,16 +6931,16 @@
}
// Read destination connection ID.
- if (!reader->ReadConnectionId(destination_connection_id,
- destination_connection_id_length)) {
- *detailed_error = "Unable to read destination connection ID.";
+ if (!reader.ReadConnectionId(&destination_connection_id,
+ destination_connection_id_length)) {
+ detailed_error = "Unable to read destination connection ID.";
return false;
}
// Read source connection ID.
- if (!reader->ReadConnectionId(source_connection_id,
- source_connection_id_length)) {
- *detailed_error = "Unable to read source connection ID.";
+ if (!reader.ReadConnectionId(&source_connection_id,
+ source_connection_id_length)) {
+ detailed_error = "Unable to read source connection ID.";
return false;
}
}
@@ -7015,9 +7013,9 @@
*reader, *parsed_version, *version_label, *first_byte);
// Parse connection IDs.
- if (!ParseLongHeaderConnectionIds(reader, *has_length_prefix, *version_label,
- destination_connection_id,
- source_connection_id, detailed_error)) {
+ if (!ParseLongHeaderConnectionIds(*reader, *has_length_prefix, *version_label,
+ *destination_connection_id,
+ *source_connection_id, *detailed_error)) {
return QUIC_INVALID_PACKET_HEADER;
}
@@ -7027,14 +7025,20 @@
}
// Parse long packet type.
- if (!GetLongHeaderType(*first_byte, *parsed_version, long_packet_type)) {
- *detailed_error = "Unable to parse long packet type.";
- return QUIC_INVALID_PACKET_HEADER;
- }
+ *long_packet_type = GetLongHeaderType(*first_byte, *parsed_version);
- if (!parsed_version->SupportsRetry() || *long_packet_type != INITIAL) {
- // Retry token is only present on initial packets for some versions.
- return QUIC_NO_ERROR;
+ switch (*long_packet_type) {
+ case INVALID_PACKET_TYPE:
+ *detailed_error = "Unable to parse long packet type.";
+ return QUIC_INVALID_PACKET_HEADER;
+ case INITIAL:
+ if (!parsed_version->SupportsRetry()) {
+ // Retry token is only present on initial packets for some versions.
+ return QUIC_NO_ERROR;
+ }
+ break;
+ default:
+ return QUIC_NO_ERROR;
}
*retry_token_length_length = reader->PeekVarInt62Length();
