Skip to content

V
V8
Commit 495cc46b authored by Andrey Kosyakov's avatar Andrey Kosyakov Committed by V8 LUCI CQ
Browse files
Options

Revert "Roll inspector_protocol to 87e75896dcfcafda7869b0c9714db9b6cdc4c765" 

This reverts commit dec192fd.

Reason for revert: broke gcc builds because of [[nodiscard]]

Original change's description:
> Roll inspector_protocol to 87e75896dcfcafda7869b0c9714db9b6cdc4c765
>
> This lets us accept spec-compliant CBOR tag for message envelopes.
>
> This also includes a change in v8-inspector-session-impl.cc that
> relaxes an envelope check to allow spec-compliant envelopes.
>
> Change-Id: Id77c1e0fc4b62d78e8580f81ef38d50e3eb54a1d
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3662540
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#80761}

Change-Id: Iaa0cc65510c9af6391a2c7d0ef7baf903335a328
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3669468
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80763}
parent 66d3d281
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 127 additions and 290 deletions
src/inspector/v8-inspector-session-impl.cc
View file @ 495cc46b
......@@ -34,10 +34,8 @@ using v8_crdtp::json::ConvertCBORToJSON;
using v8_crdtp::json::ConvertJSONToCBOR;
bool IsCBORMessage(StringView msg) {
if (!msg.is8Bit() || msg.length() < 3) return false;
const uint8_t* bytes = msg.characters8();
return bytes[0] == 0xd8 &&
(bytes[1] == 0x5a || (bytes[1] == 0x18 && bytes[2] == 0x5a));
return msg.is8Bit() && msg.length() >= 2 && msg.characters8()[0] == 0xd8 &&
msg.characters8()[1] == 0x5a;
}
Status ConvertToCBOR(StringView state, std::vector<uint8_t>* cbor) {
......
third_party/inspector_protocol/README.v8
View file @ 495cc46b
......@@ -2,7 +2,7 @@ Name: inspector protocol
Short Name: inspector_protocol
URL: https://chromium.googlesource.com/deps/inspector_protocol/
Version: 0
Revision: 87e75896dcfcafda7869b0c9714db9b6cdc4c765
Revision: 817313aa48ebb9a53cba1bd88bbe6a1c5048060c
License: BSD
License File: LICENSE
Security Critical: no
......
third_party/inspector_protocol/crdtp/cbor.h
View file @ 495cc46b
......@@ -43,6 +43,13 @@ namespace cbor {
// Detecting CBOR content
// =============================================================================
// The first byte for an envelope, which we use for wrapping dictionaries
// and arrays; and the byte that indicates a byte string with 32 bit length.
// These two bytes start an envelope, and thereby also any CBOR message
// produced or consumed by this protocol. See also |EnvelopeEncoder| below.
uint8_t InitialByteForEnvelope();
uint8_t InitialByteFor32BitLengthByteString();
// Checks whether |msg| is a cbor message.
bool IsCBORMessage(span<uint8_t> msg);
......@@ -122,29 +129,6 @@ class EnvelopeEncoder {
size_t byte_size_pos_ = 0;
};
class EnvelopeHeader {
public:
EnvelopeHeader() = default;
~EnvelopeHeader() = default;
// Parse envelope. Implies that `in` accomodates the entire size of envelope.
static StatusOr<EnvelopeHeader> Parse(span<uint8_t> in);
// Parse envelope, but allow `in` to only include the beginning of the
// envelope.
static StatusOr<EnvelopeHeader> ParseFromFragment(span<uint8_t> in);
size_t header_size() const { return header_size_; }
size_t content_size() const { return content_size_; }
size_t outer_size() const { return header_size_ + content_size_; }
private:
EnvelopeHeader(size_t header_size, size_t content_size)
: header_size_(header_size), content_size_(content_size) {}
size_t header_size_ = 0;
size_t content_size_ = 0;
};
// =============================================================================
// cbor::NewCBOREncoder - for encoding from a streaming parser
// =============================================================================
......@@ -272,22 +256,17 @@ class CBORTokenizer {
// enclosing envelope (the header, basically).
span<uint8_t> GetEnvelopeContents() const;
// To be called only if ::TokenTag() == CBORTokenTag::ENVELOPE.
// Returns the envelope header.
const EnvelopeHeader& GetEnvelopeHeader() const;
private:
void ReadNextToken();
void ReadNextToken(bool enter_envelope);
void SetToken(CBORTokenTag token, size_t token_byte_length);
void SetError(Error error);
const span<uint8_t> bytes_;
span<uint8_t> bytes_;
CBORTokenTag token_tag_;
struct Status status_;
size_t token_byte_length_ = 0;
size_t token_byte_length_;
MajorType token_start_type_;
uint64_t token_start_internal_value_;
EnvelopeHeader envelope_header_;
};
// =============================================================================
......
third_party/inspector_protocol/crdtp/cbor_test.cc
View file @ 495cc46b
......@@ -21,7 +21,6 @@
#include "test_platform.h"
using testing::ElementsAreArray;
using testing::Eq;
namespace v8_crdtp {
namespace cbor {
......@@ -73,7 +72,7 @@ TEST(CheckCBORMessage, ValidCBORButNotValidMessage) {
TEST(CheckCBORMessage, EmptyMessage) {
std::vector<uint8_t> empty;
Status status = CheckCBORMessage(SpanFrom(empty));
EXPECT_THAT(status, StatusIs(Error::CBOR_UNEXPECTED_EOF_IN_ENVELOPE, 0));
EXPECT_THAT(status, StatusIs(Error::CBOR_NO_INPUT, 0));
}
TEST(CheckCBORMessage, InvalidStartByte) {
......@@ -87,25 +86,25 @@ TEST(CheckCBORMessage, InvalidStartByte) {
TEST(CheckCBORMessage, InvalidEnvelopes) {
std::vector<uint8_t> bytes = {0xd8, 0x5a};
EXPECT_THAT(CheckCBORMessage(SpanFrom(bytes)),
StatusIs(Error::CBOR_UNEXPECTED_EOF_IN_ENVELOPE, 2));
StatusIs(Error::CBOR_INVALID_ENVELOPE, 1));
bytes = {0xd8, 0x5a, 0};
EXPECT_THAT(CheckCBORMessage(SpanFrom(bytes)),
StatusIs(Error::CBOR_UNEXPECTED_EOF_IN_ENVELOPE, 3));
StatusIs(Error::CBOR_INVALID_ENVELOPE, 1));
bytes = {0xd8, 0x5a, 0, 0};
EXPECT_THAT(CheckCBORMessage(SpanFrom(bytes)),
StatusIs(Error::CBOR_UNEXPECTED_EOF_IN_ENVELOPE, 4));
StatusIs(Error::CBOR_INVALID_ENVELOPE, 1));
bytes = {0xd8, 0x5a, 0, 0, 0};
EXPECT_THAT(CheckCBORMessage(SpanFrom(bytes)),
StatusIs(Error::CBOR_UNEXPECTED_EOF_IN_ENVELOPE, 5));
StatusIs(Error::CBOR_INVALID_ENVELOPE, 1));
bytes = {0xd8, 0x5a, 0, 0, 0, 0};
EXPECT_THAT(CheckCBORMessage(SpanFrom(bytes)),
StatusIs(Error::CBOR_MAP_OR_ARRAY_EXPECTED_IN_ENVELOPE, 6));
StatusIs(Error::CBOR_INVALID_ENVELOPE, 1));
}
TEST(CheckCBORMessage, MapStartExpected) {
std::vector<uint8_t> bytes = {0xd8, 0x5a, 0, 0, 0, 1};
EXPECT_THAT(CheckCBORMessage(SpanFrom(bytes)),
StatusIs(Error::CBOR_ENVELOPE_CONTENTS_LENGTH_MISMATCH, 6));
StatusIs(Error::CBOR_MAP_START_EXPECTED, 6));
}
// =============================================================================
......@@ -872,8 +871,12 @@ TEST(ParseCBORTest, ParseCBORHelloWorld) {
TEST(ParseCBORTest, UTF8IsSupportedInKeys) {
const uint8_t kPayloadLen = 11;
std::vector<uint8_t> bytes = {0xd8, 0x5a, // envelope
0, 0, 0, kPayloadLen};
std::vector<uint8_t> bytes = {cbor::InitialByteForEnvelope(),
cbor::InitialByteFor32BitLengthByteString(),
0,
0,
0,
kPayloadLen};
bytes.push_back(cbor::EncodeIndefiniteLengthMapStart());
// Two UTF16 chars.
EncodeString8(SpanFrom("🌎"), &bytes);
......@@ -898,7 +901,7 @@ TEST(ParseCBORTest, NoInputError) {
std::unique_ptr<ParserHandler> json_writer =
json::NewJSONEncoder(&out, &status);
ParseCBOR(span<uint8_t>(in.data(), in.size()), json_writer.get());
EXPECT_THAT(status, StatusIs(Error::CBOR_UNEXPECTED_EOF_IN_ENVELOPE, 0u));
EXPECT_THAT(status, StatusIs(Error::CBOR_NO_INPUT, 0u));
EXPECT_EQ("", out);
}
......@@ -951,39 +954,6 @@ TEST(ParseCBORTest, UnexpectedEofInMapError) {
EXPECT_EQ("", out);
}
TEST(ParseCBORTest, EnvelopeEncodingLegacy) {
constexpr uint8_t kPayloadLen = 8;
std::vector<uint8_t> bytes = {0xd8, 0x5a, 0, 0, 0, kPayloadLen}; // envelope
bytes.push_back(cbor::EncodeIndefiniteLengthMapStart());
EncodeString8(SpanFrom("foo"), &bytes);
EncodeInt32(42, &bytes);
bytes.emplace_back(EncodeStop());
std::string out;
Status status;
std::unique_ptr<ParserHandler> json_writer =
json::NewJSONEncoder(&out, &status);
ParseCBOR(span<uint8_t>(bytes.data(), bytes.size()), json_writer.get());
EXPECT_THAT(status, StatusIsOk());
EXPECT_EQ(out, "{\"foo\":42}");
}
TEST(ParseCBORTest, EnvelopeEncodingBySpec) {
constexpr uint8_t kPayloadLen = 8;
std::vector<uint8_t> bytes = {0xd8, 0x18, 0x5a, 0,
0, 0, kPayloadLen}; // envelope
bytes.push_back(cbor::EncodeIndefiniteLengthMapStart());
EncodeString8(SpanFrom("foo"), &bytes);
EncodeInt32(42, &bytes);
bytes.emplace_back(EncodeStop());
std::string out;
Status status;
std::unique_ptr<ParserHandler> json_writer =
json::NewJSONEncoder(&out, &status);
ParseCBOR(span<uint8_t>(bytes.data(), bytes.size()), json_writer.get());
EXPECT_THAT(status, StatusIsOk());
EXPECT_EQ(out, "{\"foo\":42}");
}
TEST(ParseCBORTest, NoEmptyEnvelopesAllowed) {
std::vector<uint8_t> bytes = {0xd8, 0x5a, 0, 0, 0, 0}; // envelope
std::string out;
......@@ -1001,7 +971,7 @@ TEST(ParseCBORTest, OnlyMapsAndArraysSupportedInsideEnvelopes) {
// is an envelope that contains just a number (1). We don't
// allow numbers to be contained in an envelope though, only
// maps and arrays.
constexpr uint8_t kPayloadLen = 8;
constexpr uint8_t kPayloadLen = 1;
std::vector<uint8_t> bytes = {0xd8,
0x5a,
0,
......@@ -1277,69 +1247,6 @@ TEST(ParseCBORTest, EnvelopeContentsLengthMismatch) {
EXPECT_EQ("", out);
}
// =============================================================================
// cbor::EnvelopeHeader - for parsing envelope headers
// =============================================================================
// Note most of converage for this is historically on a higher level of
// ParseCBOR(). This provides just a few essnetial scenarios for now.
template <typename T>
class EnvelopeHeaderTest : public ::testing::Test {};
TEST(EnvelopeHeaderTest, EnvelopeStartLegacy) {
std::vector<uint8_t> bytes = {0xd8, // Tag start
0x5a, // Byte string, 4 bytes length
0, 0, 0, 2, // Length
0xbf, 0xff}; // map start / map end
auto result = EnvelopeHeader::Parse(SpanFrom(bytes));
ASSERT_THAT(result.status(), StatusIsOk());
EXPECT_THAT((*result).header_size(), Eq(6u));
EXPECT_THAT((*result).content_size(), Eq(2u));
EXPECT_THAT((*result).outer_size(), Eq(8u));
}
TEST(EnvelopeHeaderTest, EnvelopeStartSpecCompliant) {
std::vector<uint8_t> bytes = {0xd8, // Tag start
0x18, // Tag type (CBOR)
0x5a, // Byte string, 4 bytes length
0, 0, 0, 2, // Length
0xbf, 0xff}; // map start / map end
auto result = EnvelopeHeader::Parse(SpanFrom(bytes));
ASSERT_THAT(result.status(), StatusIsOk());
EXPECT_THAT((*result).header_size(), Eq(7u));
EXPECT_THAT((*result).content_size(), Eq(2u));
EXPECT_THAT((*result).outer_size(), Eq(9u));
}
TEST(EnvelopeHeaderTest, EnvelopeStartShortLen) {
std::vector<uint8_t> bytes = {0xd8, // Tag start
0x18, // Tag type (CBOR)
0x58, // Byte string, 1 byte length
2, // Length
0xbf, 0xff}; // map start / map end
auto result = EnvelopeHeader::Parse(SpanFrom(bytes));
ASSERT_THAT(result.status(), StatusIsOk());
EXPECT_THAT((*result).header_size(), Eq(4u));
EXPECT_THAT((*result).content_size(), Eq(2u));
EXPECT_THAT((*result).outer_size(), Eq(6u));
}
TEST(EnvelopeHeaderTest, ParseFragment) {
std::vector<uint8_t> bytes = {0xd8, // Tag start
0x18, // Tag type (CBOR)
0x5a, // Byte string, 4 bytes length
0, 0, 0, 20, 0xbf}; // map start
auto result = EnvelopeHeader::ParseFromFragment(SpanFrom(bytes));
ASSERT_THAT(result.status(), StatusIsOk());
EXPECT_THAT((*result).header_size(), Eq(7u));
EXPECT_THAT((*result).content_size(), Eq(20u));
EXPECT_THAT((*result).outer_size(), Eq(27u));
result = EnvelopeHeader::Parse(SpanFrom(bytes));
ASSERT_THAT(result.status(),
StatusIs(Error::CBOR_ENVELOPE_CONTENTS_LENGTH_MISMATCH, 8));
}
// =============================================================================
// cbor::AppendString8EntryToMap - for limited in-place editing of messages
// =============================================================================
......@@ -1424,7 +1331,7 @@ TEST(AppendString8EntryToMapTest, InvalidEnvelope_Error) {
0xd8, 0x7a, 0, 0, 0, 2, EncodeIndefiniteLengthMapStart(), EncodeStop()};
Status status =
AppendString8EntryToCBORMap(SpanFrom("key"), SpanFrom("value"), &msg);
EXPECT_THAT(status, StatusIs(Error::CBOR_INVALID_ENVELOPE, 1u));
EXPECT_THAT(status, StatusIs(Error::CBOR_INVALID_ENVELOPE, 0u));
}
{ // Invalid envelope size example.
std::vector<uint8_t> msg = {
......@@ -1432,8 +1339,7 @@ TEST(AppendString8EntryToMapTest, InvalidEnvelope_Error) {
};
Status status =
AppendString8EntryToCBORMap(SpanFrom("key"), SpanFrom("value"), &msg);
EXPECT_THAT(status,
StatusIs(Error::CBOR_ENVELOPE_CONTENTS_LENGTH_MISMATCH, 8u));
EXPECT_THAT(status, StatusIs(Error::CBOR_INVALID_ENVELOPE, 0u));
}
{ // Invalid envelope size example.
std::vector<uint8_t> msg = {
......@@ -1441,7 +1347,7 @@ TEST(AppendString8EntryToMapTest, InvalidEnvelope_Error) {
};
Status status =
AppendString8EntryToCBORMap(SpanFrom("key"), SpanFrom("value"), &msg);
EXPECT_THAT(status, StatusIs(Error::CBOR_INVALID_ENVELOPE, 0));
EXPECT_THAT(status, StatusIs(Error::CBOR_INVALID_ENVELOPE, 0u));
}
}
} // namespace cbor
......
third_party/inspector_protocol/crdtp/dispatch.cc
View file @ 495cc46b
......@@ -78,17 +78,13 @@ DispatchResponse DispatchResponse::ServerError(std::string message) {
return result;
}
// static
DispatchResponse DispatchResponse::SessionNotFound(std::string message) {
DispatchResponse result;
result.code_ = DispatchCode::SESSION_NOT_FOUND;
result.message_ = std::move(message);
return result;
}
// =============================================================================
// Dispatchable - a shallow parser for CBOR encoded DevTools messages
// =============================================================================
namespace {
constexpr size_t kEncodedEnvelopeHeaderSize = 1 + 1 + sizeof(uint32_t);
} // namespace
Dispatchable::Dispatchable(span<uint8_t> serialized) : serialized_(serialized) {
Status s = cbor::CheckCBORMessage(serialized);
if (!s.ok()) {
......@@ -109,8 +105,9 @@ Dispatchable::Dispatchable(span<uint8_t> serialized) : serialized_(serialized) {
// expect to see after we're done parsing the envelope contents.
// This way we can compare and produce an error if the contents
// didn't fit exactly into the envelope length.
const size_t pos_past_envelope =
tokenizer.Status().pos + tokenizer.GetEnvelopeHeader().outer_size();
const size_t pos_past_envelope = tokenizer.Status().pos +
kEncodedEnvelopeHeaderSize +
tokenizer.GetEnvelopeContents().size();
tokenizer.EnterEnvelope();
if (tokenizer.TokenTag() == cbor::CBORTokenTag::ERROR_VALUE) {
status_ = tokenizer.Status();
......
third_party/inspector_protocol/crdtp/dispatch.h
View file @ 495cc46b
......@@ -38,7 +38,6 @@ enum class DispatchCode {
INVALID_PARAMS = -32602,
INTERNAL_ERROR = -32603,
SERVER_ERROR = -32000,
SESSION_NOT_FOUND = SERVER_ERROR - 1,
};
// Information returned by command handlers. Usually returned after command
......@@ -77,10 +76,6 @@ class DispatchResponse {
// Used for application level errors, e.g. within protocol agents.
static DispatchResponse ServerError(std::string message);
// Indicate that session with the id specified in the protocol message
// was not found (e.g. because it has already been detached).
static DispatchResponse SessionNotFound(std::string message);
private:
DispatchResponse() = default;
DispatchCode code_;
......
third_party/inspector_protocol/crdtp/dispatch_test.cc
View file @ 495cc46b
......@@ -27,13 +27,6 @@ TEST(DispatchResponseTest, ServerError) {
EXPECT_EQ("Oops!", error.Message());
}
TEST(DispatchResponseTest, SessionNotFound) {
DispatchResponse error = DispatchResponse::SessionNotFound("OMG!");
EXPECT_FALSE(error.IsSuccess());
EXPECT_EQ(DispatchCode::SESSION_NOT_FOUND, error.Code());
EXPECT_EQ("OMG!", error.Message());
}
TEST(DispatchResponseTest, InternalError) {
DispatchResponse error = DispatchResponse::InternalError();
EXPECT_FALSE(error.IsSuccess());
......
third_party/inspector_protocol/crdtp/status.cc
View file @ 495cc46b
......@@ -58,16 +58,16 @@ std::string Status::Message() const {
return "CBOR: invalid binary";
case Error::CBOR_UNSUPPORTED_VALUE:
return "CBOR: unsupported value";
case Error::CBOR_UNEXPECTED_EOF_IN_ENVELOPE:
return "CBOR: unexpected EOF reading envelope";
case Error::CBOR_NO_INPUT:
return "CBOR: no input";
case Error::CBOR_INVALID_START_BYTE:
return "CBOR: invalid start byte";
case Error::CBOR_UNEXPECTED_EOF_EXPECTED_VALUE:
return "CBOR: unexpected EOF expected value";
return "CBOR: unexpected eof expected value";
case Error::CBOR_UNEXPECTED_EOF_IN_ARRAY:
return "CBOR: unexpected EOF in array";
return "CBOR: unexpected eof in array";
case Error::CBOR_UNEXPECTED_EOF_IN_MAP:
return "CBOR: unexpected EOF in map";
return "CBOR: unexpected eof in map";
case Error::CBOR_INVALID_MAP_KEY:
return "CBOR: invalid map key";
case Error::CBOR_DUPLICATE_MAP_KEY:
......
third_party/inspector_protocol/crdtp/status.h
View file @ 495cc46b
......@@ -46,7 +46,7 @@ enum class Error {
CBOR_INVALID_STRING16 = 0x14,
CBOR_INVALID_BINARY = 0x15,
CBOR_UNSUPPORTED_VALUE = 0x16,
CBOR_UNEXPECTED_EOF_IN_ENVELOPE = 0x17,
CBOR_NO_INPUT = 0x17,
CBOR_INVALID_START_BYTE = 0x18,
CBOR_UNEXPECTED_EOF_EXPECTED_VALUE = 0x19,
CBOR_UNEXPECTED_EOF_IN_ARRAY = 0x1a,
......@@ -90,7 +90,7 @@ struct Status {
Error error = Error::OK;
size_t pos = npos();
[[nodiscard]] Status(Error error, size_t pos) : error(error), pos(pos) {}
Status(Error error, size_t pos) : error(error), pos(pos) {}
Status() = default;
bool IsMessageError() const {
......
third_party/inspector_protocol/lib/Values_cpp.template
View file @ 495cc46b
......@@ -31,6 +31,7 @@ using {{config.crdtp.namespace}}::cbor::EncodeStop;
using {{config.crdtp.namespace}}::cbor::EncodeString8;
using {{config.crdtp.namespace}}::cbor::EncodeTrue;
using {{config.crdtp.namespace}}::cbor::EnvelopeEncoder;
using {{config.crdtp.namespace}}::cbor::InitialByteForEnvelope;
} // namespace cbor
// Uses the parsing events received from driver of |ParserHandler|
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment