diff --git a/tools/inspector_protocol/encoding/encoding.cc b/tools/inspector_protocol/encoding/encoding.cc index 9a869bbbee29b4..1513767a85592b 100644 --- a/tools/inspector_protocol/encoding/encoding.cc +++ b/tools/inspector_protocol/encoding/encoding.cc @@ -185,11 +185,10 @@ namespace internals { // |type| is the major type as specified in RFC 7049 Section 2.1. // |value| is the payload (e.g. for MajorType::UNSIGNED) or is the size // (e.g. for BYTE_STRING). -// If successful, returns the number of bytes read. Otherwise returns -1. -// TODO(johannes): change return type to size_t and use 0 for error. -int8_t ReadTokenStart(span bytes, MajorType* type, uint64_t* value) { +// If successful, returns the number of bytes read. Otherwise returns 0. +size_t ReadTokenStart(span bytes, MajorType* type, uint64_t* value) { if (bytes.empty()) - return -1; + return 0; uint8_t initial_byte = bytes[0]; *type = MajorType((initial_byte & kMajorTypeMask) >> kMajorTypeBitShift); @@ -203,32 +202,32 @@ int8_t ReadTokenStart(span bytes, MajorType* type, uint64_t* value) { if (additional_information == kAdditionalInformation1Byte) { // Values 24-255 are encoded with one initial byte, followed by the value. if (bytes.size() < 2) - return -1; + return 0; *value = ReadBytesMostSignificantByteFirst(bytes.subspan(1)); return 2; } if (additional_information == kAdditionalInformation2Bytes) { // Values 256-65535: 1 initial byte + 2 bytes payload. if (bytes.size() < 1 + sizeof(uint16_t)) - return -1; + return 0; *value = ReadBytesMostSignificantByteFirst(bytes.subspan(1)); return 3; } if (additional_information == kAdditionalInformation4Bytes) { // 32 bit uint: 1 initial byte + 4 bytes payload. if (bytes.size() < 1 + sizeof(uint32_t)) - return -1; + return 0; *value = ReadBytesMostSignificantByteFirst(bytes.subspan(1)); return 5; } if (additional_information == kAdditionalInformation8Bytes) { // 64 bit uint: 1 initial byte + 8 bytes payload. if (bytes.size() < 1 + sizeof(uint64_t)) - return -1; + return 0; *value = ReadBytesMostSignificantByteFirst(bytes.subspan(1)); return 9; } - return -1; + return 0; } // Writes the start of a token with |type|. The |value| may indicate the size, @@ -770,10 +769,10 @@ void CBORTokenizer::ReadNextToken(bool enter_envelope) { SetToken(CBORTokenTag::NULL_VALUE, 1); return; case kExpectedConversionToBase64Tag: { // BINARY - const int8_t bytes_read = internals::ReadTokenStart( + const size_t bytes_read = internals::ReadTokenStart( bytes_.subspan(status_.pos + 1), &token_start_type_, &token_start_internal_value_); - if (bytes_read < 0 || token_start_type_ != MajorType::BYTE_STRING || + if (!bytes_read || token_start_type_ != MajorType::BYTE_STRING || token_start_internal_value_ > kMaxValidLength) { SetError(Error::CBOR_INVALID_BINARY); return; @@ -823,22 +822,21 @@ void CBORTokenizer::ReadNextToken(bool enter_envelope) { return; } default: { - const int8_t token_start_length = internals::ReadTokenStart( + const size_t bytes_read = internals::ReadTokenStart( bytes_.subspan(status_.pos), &token_start_type_, &token_start_internal_value_); - const bool success = token_start_length >= 0; switch (token_start_type_) { case MajorType::UNSIGNED: // INT32. // INT32 is a signed int32 (int32 makes sense for the // inspector_protocol, it's not a CBOR limitation), so we check // against the signed max, so that the allowable values are // 0, 1, 2, ... 2^31 - 1. - if (!success || std::numeric_limits::max() < - token_start_internal_value_) { + if (!bytes_read || std::numeric_limits::max() < + token_start_internal_value_) { SetError(Error::CBOR_INVALID_INT32); return; } - SetToken(CBORTokenTag::INT32, token_start_length); + SetToken(CBORTokenTag::INT32, bytes_read); return; case MajorType::NEGATIVE: { // INT32. // INT32 is a signed int32 (int32 makes sense for the @@ -851,21 +849,20 @@ void CBORTokenizer::ReadNextToken(bool enter_envelope) { // We check the the payload in token_start_internal_value_ against // that range (2^31-1 is also known as // std::numeric_limits::max()). - if (!success || token_start_internal_value_ > - std::numeric_limits::max()) { + if (!bytes_read || token_start_internal_value_ > + std::numeric_limits::max()) { SetError(Error::CBOR_INVALID_INT32); return; } - SetToken(CBORTokenTag::INT32, token_start_length); + SetToken(CBORTokenTag::INT32, bytes_read); return; } case MajorType::STRING: { // STRING8. - if (!success || token_start_internal_value_ > kMaxValidLength) { + if (!bytes_read || token_start_internal_value_ > kMaxValidLength) { SetError(Error::CBOR_INVALID_STRING8); return; } - uint64_t token_byte_length = - token_start_internal_value_ + token_start_length; + uint64_t token_byte_length = token_start_internal_value_ + bytes_read; if (token_byte_length > remaining_bytes) { SetError(Error::CBOR_INVALID_STRING8); return; @@ -877,13 +874,12 @@ void CBORTokenizer::ReadNextToken(bool enter_envelope) { case MajorType::BYTE_STRING: { // STRING16. // Length must be divisible by 2 since UTF16 is 2 bytes per // character, hence the &1 check. - if (!success || token_start_internal_value_ > kMaxValidLength || + if (!bytes_read || token_start_internal_value_ > kMaxValidLength || token_start_internal_value_ & 1) { SetError(Error::CBOR_INVALID_STRING16); return; } - uint64_t token_byte_length = - token_start_internal_value_ + token_start_length; + uint64_t token_byte_length = token_start_internal_value_ + bytes_read; if (token_byte_length > remaining_bytes) { SetError(Error::CBOR_INVALID_STRING16); return; diff --git a/tools/inspector_protocol/encoding/encoding.h b/tools/inspector_protocol/encoding/encoding.h index 90916d42b36dae..08596e9e1e43f0 100644 --- a/tools/inspector_protocol/encoding/encoding.h +++ b/tools/inspector_protocol/encoding/encoding.h @@ -427,7 +427,7 @@ Status AppendString8EntryToCBORMap(span string8_key, std::string* cbor); namespace internals { // Exposed only for writing tests. -int8_t ReadTokenStart(span bytes, +size_t ReadTokenStart(span bytes, cbor::MajorType* type, uint64_t* value); diff --git a/tools/inspector_protocol/lib/encoding_cpp.template b/tools/inspector_protocol/lib/encoding_cpp.template index d24e9286a12f79..70bf9091a7dd6a 100644 --- a/tools/inspector_protocol/lib/encoding_cpp.template +++ b/tools/inspector_protocol/lib/encoding_cpp.template @@ -192,11 +192,10 @@ namespace internals { // |type| is the major type as specified in RFC 7049 Section 2.1. // |value| is the payload (e.g. for MajorType::UNSIGNED) or is the size // (e.g. for BYTE_STRING). -// If successful, returns the number of bytes read. Otherwise returns -1. -// TODO(johannes): change return type to size_t and use 0 for error. -int8_t ReadTokenStart(span bytes, MajorType* type, uint64_t* value) { +// If successful, returns the number of bytes read. Otherwise returns 0. +size_t ReadTokenStart(span bytes, MajorType* type, uint64_t* value) { if (bytes.empty()) - return -1; + return 0; uint8_t initial_byte = bytes[0]; *type = MajorType((initial_byte & kMajorTypeMask) >> kMajorTypeBitShift); @@ -210,32 +209,32 @@ int8_t ReadTokenStart(span bytes, MajorType* type, uint64_t* value) { if (additional_information == kAdditionalInformation1Byte) { // Values 24-255 are encoded with one initial byte, followed by the value. if (bytes.size() < 2) - return -1; + return 0; *value = ReadBytesMostSignificantByteFirst(bytes.subspan(1)); return 2; } if (additional_information == kAdditionalInformation2Bytes) { // Values 256-65535: 1 initial byte + 2 bytes payload. if (bytes.size() < 1 + sizeof(uint16_t)) - return -1; + return 0; *value = ReadBytesMostSignificantByteFirst(bytes.subspan(1)); return 3; } if (additional_information == kAdditionalInformation4Bytes) { // 32 bit uint: 1 initial byte + 4 bytes payload. if (bytes.size() < 1 + sizeof(uint32_t)) - return -1; + return 0; *value = ReadBytesMostSignificantByteFirst(bytes.subspan(1)); return 5; } if (additional_information == kAdditionalInformation8Bytes) { // 64 bit uint: 1 initial byte + 8 bytes payload. if (bytes.size() < 1 + sizeof(uint64_t)) - return -1; + return 0; *value = ReadBytesMostSignificantByteFirst(bytes.subspan(1)); return 9; } - return -1; + return 0; } // Writes the start of a token with |type|. The |value| may indicate the size, @@ -777,10 +776,10 @@ void CBORTokenizer::ReadNextToken(bool enter_envelope) { SetToken(CBORTokenTag::NULL_VALUE, 1); return; case kExpectedConversionToBase64Tag: { // BINARY - const int8_t bytes_read = internals::ReadTokenStart( + const size_t bytes_read = internals::ReadTokenStart( bytes_.subspan(status_.pos + 1), &token_start_type_, &token_start_internal_value_); - if (bytes_read < 0 || token_start_type_ != MajorType::BYTE_STRING || + if (!bytes_read || token_start_type_ != MajorType::BYTE_STRING || token_start_internal_value_ > kMaxValidLength) { SetError(Error::CBOR_INVALID_BINARY); return; @@ -830,47 +829,47 @@ void CBORTokenizer::ReadNextToken(bool enter_envelope) { return; } default: { - const int8_t token_start_length = internals::ReadTokenStart( + const size_t bytes_read = internals::ReadTokenStart( bytes_.subspan(status_.pos), &token_start_type_, &token_start_internal_value_); - const bool success = token_start_length >= 0; switch (token_start_type_) { case MajorType::UNSIGNED: // INT32. // INT32 is a signed int32 (int32 makes sense for the // inspector_protocol, it's not a CBOR limitation), so we check // against the signed max, so that the allowable values are // 0, 1, 2, ... 2^31 - 1. - if (!success || std::numeric_limits::max() < - token_start_internal_value_) { + if (!bytes_read || std::numeric_limits::max() < + token_start_internal_value_) { SetError(Error::CBOR_INVALID_INT32); return; } - SetToken(CBORTokenTag::INT32, token_start_length); + SetToken(CBORTokenTag::INT32, bytes_read); return; case MajorType::NEGATIVE: { // INT32. // INT32 is a signed int32 (int32 makes sense for the // inspector_protocol, it's not a CBOR limitation); in CBOR, the // negative values for INT32 are represented as NEGATIVE, that is, -1 // INT32 is represented as 1 << 5 | 0 (major type 1, additional info - // value 0). The minimal allowed INT32 value in our protocol is - // std::numeric_limits::min(). We check for it by directly - // checking the payload against the maximal allowed signed (!) int32 - // value. - if (!success || token_start_internal_value_ > - std::numeric_limits::max()) { + // value 0). + // The represented allowed values range is -1 to -2^31. + // They are mapped into the encoded range of 0 to 2^31-1. + // We check the the payload in token_start_internal_value_ against + // that range (2^31-1 is also known as + // std::numeric_limits::max()). + if (!bytes_read || token_start_internal_value_ > + std::numeric_limits::max()) { SetError(Error::CBOR_INVALID_INT32); return; } - SetToken(CBORTokenTag::INT32, token_start_length); + SetToken(CBORTokenTag::INT32, bytes_read); return; } case MajorType::STRING: { // STRING8. - if (!success || token_start_internal_value_ > kMaxValidLength) { + if (!bytes_read || token_start_internal_value_ > kMaxValidLength) { SetError(Error::CBOR_INVALID_STRING8); return; } - uint64_t token_byte_length = - token_start_internal_value_ + token_start_length; + uint64_t token_byte_length = token_start_internal_value_ + bytes_read; if (token_byte_length > remaining_bytes) { SetError(Error::CBOR_INVALID_STRING8); return; @@ -882,13 +881,12 @@ void CBORTokenizer::ReadNextToken(bool enter_envelope) { case MajorType::BYTE_STRING: { // STRING16. // Length must be divisible by 2 since UTF16 is 2 bytes per // character, hence the &1 check. - if (!success || token_start_internal_value_ > kMaxValidLength || + if (!bytes_read || token_start_internal_value_ > kMaxValidLength || token_start_internal_value_ & 1) { SetError(Error::CBOR_INVALID_STRING16); return; } - uint64_t token_byte_length = - token_start_internal_value_ + token_start_length; + uint64_t token_byte_length = token_start_internal_value_ + bytes_read; if (token_byte_length > remaining_bytes) { SetError(Error::CBOR_INVALID_STRING16); return; diff --git a/tools/inspector_protocol/lib/encoding_h.template b/tools/inspector_protocol/lib/encoding_h.template index f1a52a1958a14d..406c4b87ff8aa5 100644 --- a/tools/inspector_protocol/lib/encoding_h.template +++ b/tools/inspector_protocol/lib/encoding_h.template @@ -435,7 +435,7 @@ Status AppendString8EntryToCBORMap(span string8_key, std::string* cbor); namespace internals { // Exposed only for writing tests. -int8_t ReadTokenStart(span bytes, +size_t ReadTokenStart(span bytes, cbor::MajorType* type, uint64_t* value);