[DevTools] Roll third_party/inspector_protocol (v8)

This adds a DCHECK into v8-inspector-session-impl.cc, covering what
we previously checked within ConvertCBORToJSON.

Upstream reviews:

"Separate the lightweight check for CBOR messages from ParseCBOR."
https://chromium-review.googlesource.com/c/deps/inspector_protocol/+/2001536

"Remove Exported::writeBinary."
https://chromium-review.googlesource.com/c/deps/inspector_protocol/+/2005797

New Rev: ac6919eb836521a96cc18931f0bf270d8c1b53a1

Change-Id: I52076a8f77b27c24c3afb35c40afbbe94e0ca05c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2002935Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Johannes Henkel <johannes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65854}

src/inspector/v8-inspector-session-impl.cc

@@ -28,6 +28,7 @@ namespace {
 using v8_crdtp::span;
 using v8_crdtp::SpanFrom;
 using v8_crdtp::Status;
+using v8_crdtp::cbor::CheckCBORMessage;
 using v8_crdtp::json::ConvertCBORToJSON;
 using v8_crdtp::json::ConvertJSONToCBOR;
 
@@ -167,6 +168,7 @@ protocol::DictionaryValue* V8InspectorSessionImpl::agentState(
 std::unique_ptr<StringBuffer> V8InspectorSessionImpl::serializeForFrontend(
     std::unique_ptr<protocol::Serializable> message) {
   std::vector<uint8_t> cbor = std::move(*message).TakeSerialized();
+  DCHECK(CheckCBORMessage(SpanFrom(cbor)).ok());
   if (use_binary_protocol_)
     return std::unique_ptr<StringBuffer>(
         new BinaryStringBuffer(std::move(cbor)));

third_party/inspector_protocol/BUILD.gn

@@ -55,6 +55,8 @@ v8_source_set("crdtp_test") {
     "crdtp/serializer_traits_test.cc",
     "crdtp/span_test.cc",
     "crdtp/status_test.cc",
+    "crdtp/status_test_support.cc",
+    "crdtp/status_test_support.h",
   ]
   configs = [ ":crdtp_config" ]
   deps = [

third_party/inspector_protocol/README.v8

@@ -2,7 +2,7 @@ Name: inspector protocol
 Short Name: inspector_protocol
 URL: https://chromium.googlesource.com/deps/inspector_protocol/
 Version: 0
-Revision: 32a87e9a751db8f2903532134a7c8fc6932620ba
+Revision: ac6919eb836521a96cc18931f0bf270d8c1b53a1
 License: BSD
 License File: LICENSE
 Security Critical: no

third_party/inspector_protocol/crdtp/cbor.cc

@@ -203,6 +203,20 @@ bool IsCBORMessage(span<uint8_t> msg) {
          msg[1] == InitialByteFor32BitLengthByteString();
 }
 
+Status CheckCBORMessage(span<uint8_t> msg) {
+  if (msg.empty())
+    return Status(Error::CBOR_NO_INPUT, 0);
+  if (msg[0] != InitialByteForEnvelope())
+    return Status(Error::CBOR_INVALID_START_BYTE, 0);
+  if (msg.size() < 6 || msg[1] != InitialByteFor32BitLengthByteString())
+    return Status(Error::CBOR_INVALID_ENVELOPE, 1);
+  if (msg[2] == 0 && msg[3] == 0 && msg[4] == 0 && msg[5] == 0)
+    return Status(Error::CBOR_INVALID_ENVELOPE, 1);
+  if (msg.size() < 7 || msg[6] != EncodeIndefiniteLengthMapStart())
+    return Status(Error::CBOR_MAP_START_EXPECTED, 6);
+  return Status();
+}
+
 // =============================================================================
 // Encoding invidiual CBOR items
 // =============================================================================
@@ -820,19 +834,12 @@ bool ParseEnvelope(int32_t stack_depth,
         return false;
       break;  // Continue to check pos_past_envelope below.
     case CBORTokenTag::ARRAY_START:
-      if (stack_depth == 0) {  // Not allowed at the top level.
-        out->HandleError(
-            Status{Error::CBOR_MAP_START_EXPECTED, tokenizer->Status().pos});
-        return false;
-      }
       if (!ParseArray(stack_depth + 1, tokenizer, out))
         return false;
       break;  // Continue to check pos_past_envelope below.
     default:
-      out->HandleError(Status{
-          stack_depth == 0 ? Error::CBOR_MAP_START_EXPECTED
-                           : Error::CBOR_MAP_OR_ARRAY_EXPECTED_IN_ENVELOPE,
-          tokenizer->Status().pos});
+      out->HandleError(Status{Error::CBOR_MAP_OR_ARRAY_EXPECTED_IN_ENVELOPE,
+                              tokenizer->Status().pos});
       return false;
   }
   // The contents of the envelope parsed OK, now check that we're at
@@ -977,19 +984,12 @@ void ParseCBOR(span<uint8_t> bytes, ParserHandler* out) {
     out->HandleError(Status{Error::CBOR_NO_INPUT, 0});
     return;
   }
-  if (bytes[0] != kInitialByteForEnvelope) {
-    out->HandleError(Status{Error::CBOR_INVALID_START_BYTE, 0});
-    return;
-  }
   CBORTokenizer tokenizer(bytes);
   if (tokenizer.TokenTag() == CBORTokenTag::ERROR_VALUE) {
     out->HandleError(tokenizer.Status());
     return;
   }
-  // We checked for the envelope start byte above, so the tokenizer
-  // must agree here, since it's not an error.
-  assert(tokenizer.TokenTag() == CBORTokenTag::ENVELOPE);
-  if (!ParseEnvelope(/*stack_depth=*/0, &tokenizer, out))
+  if (!ParseValue(/*stack_depth=*/0, &tokenizer, out))
     return;
   if (tokenizer.TokenTag() == CBORTokenTag::DONE)
     return;

third_party/inspector_protocol/crdtp/cbor.h

@@ -53,6 +53,15 @@ uint8_t InitialByteFor32BitLengthByteString();
 // Checks whether |msg| is a cbor message.
 bool IsCBORMessage(span<uint8_t> msg);
 
+// Performs a leightweight check of |msg|.
+// Disallows:
+// - Empty message
+// - Not starting with the two bytes 0xd8, 0x5a
+// - Empty envelope (all length bytes are 0)
+// - Not starting with a map after the envelope stanza
+// DevTools messages should pass this check.
+Status CheckCBORMessage(span<uint8_t> msg);
+
 // =============================================================================
 // Encoding individual CBOR items
 // =============================================================================

third_party/inspector_protocol/crdtp/json_test.cc

@@ -18,6 +18,7 @@
 #include "parser_handler.h"
 #include "span.h"
 #include "status.h"
+#include "status_test_support.h"
 #include "test_platform.h"
 
 namespace v8_crdtp {
@@ -180,8 +181,7 @@ TEST(JsonStdStringWriterTest, HandlesErrors) {
   writer->HandleMapBegin();
   WriteUTF8AsUTF16(writer.get(), "msg1");
   writer->HandleError(Status{Error::JSON_PARSER_VALUE_EXPECTED, 42});
-  EXPECT_EQ(Error::JSON_PARSER_VALUE_EXPECTED, status.error);
-  EXPECT_EQ(42u, status.pos);
+  EXPECT_THAT(status, StatusIs(Error::JSON_PARSER_VALUE_EXPECTED, 42u));
   EXPECT_EQ("", out);
 }
 
@@ -401,8 +401,8 @@ TEST_F(JsonParserTest, UnprocessedInputRemainsError) {
   size_t junk_idx = json.find("junk");
   EXPECT_NE(junk_idx, std::string::npos);
   ParseJSON(SpanFrom(json), &log_);
-  EXPECT_EQ(Error::JSON_PARSER_UNPROCESSED_INPUT_REMAINS, log_.status().error);
-  EXPECT_EQ(junk_idx, log_.status().pos);
+  EXPECT_THAT(log_.status(),
+              StatusIs(Error::JSON_PARSER_UNPROCESSED_INPUT_REMAINS, junk_idx));
   EXPECT_EQ("", log_.str());
 }
 
@@ -442,38 +442,36 @@ TEST_F(JsonParserTest, StackLimitExceededError_AtLimit) {
   ParseJSON(span<uint8_t>(reinterpret_cast<const uint8_t*>(json_limit.data()),
                           json_limit.size()),
             &log_);
-  EXPECT_TRUE(log_.status().ok());
+  EXPECT_THAT(log_.status(), StatusIsOk());
 }
 
 TEST_F(JsonParserTest, StackLimitExceededError_AboveLimit) {
   // Now with kStackLimit + 1 (301) - it exceeds in the innermost instance.
   std::string exceeded = MakeNestedJson(301);
   ParseJSON(SpanFrom(exceeded), &log_);
-  EXPECT_EQ(Error::JSON_PARSER_STACK_LIMIT_EXCEEDED, log_.status().error);
-  EXPECT_EQ(strlen("{\"foo\":") * 301, log_.status().pos);
+  EXPECT_THAT(log_.status(), StatusIs(Error::JSON_PARSER_STACK_LIMIT_EXCEEDED,
+                                      strlen("{\"foo\":") * 301));
 }
 
 TEST_F(JsonParserTest, StackLimitExceededError_WayAboveLimit) {
   // Now way past the limit. Still, the point of exceeding is 301.
   std::string far_out = MakeNestedJson(320);
   ParseJSON(SpanFrom(far_out), &log_);
-  EXPECT_EQ(Error::JSON_PARSER_STACK_LIMIT_EXCEEDED, log_.status().error);
-  EXPECT_EQ(strlen("{\"foo\":") * 301, log_.status().pos);
+  EXPECT_THAT(log_.status(), StatusIs(Error::JSON_PARSER_STACK_LIMIT_EXCEEDED,
+                                      strlen("{\"foo\":") * 301));
 }
 
 TEST_F(JsonParserTest, NoInputError) {
   std::string json = "";
   ParseJSON(SpanFrom(json), &log_);
-  EXPECT_EQ(Error::JSON_PARSER_NO_INPUT, log_.status().error);
-  EXPECT_EQ(0u, log_.status().pos);
+  EXPECT_THAT(log_.status(), StatusIs(Error::JSON_PARSER_NO_INPUT, 0u));
   EXPECT_EQ("", log_.str());
 }
 
 TEST_F(JsonParserTest, InvalidTokenError) {
   std::string json = "|";
   ParseJSON(SpanFrom(json), &log_);
-  EXPECT_EQ(Error::JSON_PARSER_INVALID_TOKEN, log_.status().error);
-  EXPECT_EQ(0u, log_.status().pos);
+  EXPECT_THAT(log_.status(), StatusIs(Error::JSON_PARSER_INVALID_TOKEN, 0u));
   EXPECT_EQ("", log_.str());
 }
 
@@ -481,8 +479,7 @@ TEST_F(JsonParserTest, InvalidNumberError) {
   // Mantissa exceeds max (the constant used here is int64_t max).
   std::string json = "1E9223372036854775807";
   ParseJSON(SpanFrom(json), &log_);
-  EXPECT_EQ(Error::JSON_PARSER_INVALID_NUMBER, log_.status().error);
-  EXPECT_EQ(0u, log_.status().pos);
+  EXPECT_THAT(log_.status(), StatusIs(Error::JSON_PARSER_INVALID_NUMBER, 0u));
   EXPECT_EQ("", log_.str());
 }
 
@@ -490,25 +487,23 @@ TEST_F(JsonParserTest, InvalidStringError) {
   // \x22 is an unsupported escape sequence
   std::string json = "\"foo\\x22\"";
   ParseJSON(SpanFrom(json), &log_);
-  EXPECT_EQ(Error::JSON_PARSER_INVALID_STRING, log_.status().error);
-  EXPECT_EQ(0u, log_.status().pos);
+  EXPECT_THAT(log_.status(), StatusIs(Error::JSON_PARSER_INVALID_STRING, 0u));
   EXPECT_EQ("", log_.str());
 }
 
 TEST_F(JsonParserTest, UnexpectedArrayEndError) {
   std::string json = "[1,2,]";
   ParseJSON(SpanFrom(json), &log_);
-  EXPECT_EQ(Error::JSON_PARSER_UNEXPECTED_ARRAY_END, log_.status().error);
-  EXPECT_EQ(5u, log_.status().pos);
+  EXPECT_THAT(log_.status(),
+              StatusIs(Error::JSON_PARSER_UNEXPECTED_ARRAY_END, 5u));
   EXPECT_EQ("", log_.str());
 }
 
 TEST_F(JsonParserTest, CommaOrArrayEndExpectedError) {
   std::string json = "[1,2 2";
   ParseJSON(SpanFrom(json), &log_);
-  EXPECT_EQ(Error::JSON_PARSER_COMMA_OR_ARRAY_END_EXPECTED,
-            log_.status().error);
-  EXPECT_EQ(5u, log_.status().pos);
+  EXPECT_THAT(log_.status(),
+              StatusIs(Error::JSON_PARSER_COMMA_OR_ARRAY_END_EXPECTED, 5u));
   EXPECT_EQ("", log_.str());
 }
 
@@ -516,24 +511,23 @@ TEST_F(JsonParserTest, StringLiteralExpectedError) {
   // There's an error because the key bar, a string, is not terminated.
   std::string json = "{\"foo\": 3.1415, \"bar: 31415e-4}";
   ParseJSON(SpanFrom(json), &log_);
-  EXPECT_EQ(Error::JSON_PARSER_STRING_LITERAL_EXPECTED, log_.status().error);
-  EXPECT_EQ(16u, log_.status().pos);
+  EXPECT_THAT(log_.status(),
+              StatusIs(Error::JSON_PARSER_STRING_LITERAL_EXPECTED, 16u));
   EXPECT_EQ("", log_.str());
 }
 
 TEST_F(JsonParserTest, ColonExpectedError) {
   std::string json = "{\"foo\", 42}";
   ParseJSON(SpanFrom(json), &log_);
-  EXPECT_EQ(Error::JSON_PARSER_COLON_EXPECTED, log_.status().error);
-  EXPECT_EQ(6u, log_.status().pos);
+  EXPECT_THAT(log_.status(), StatusIs(Error::JSON_PARSER_COLON_EXPECTED, 6u));
   EXPECT_EQ("", log_.str());
 }
 
 TEST_F(JsonParserTest, UnexpectedMapEndError) {
   std::string json = "{\"foo\": 42, }";
   ParseJSON(SpanFrom(json), &log_);
-  EXPECT_EQ(Error::JSON_PARSER_UNEXPECTED_MAP_END, log_.status().error);
-  EXPECT_EQ(12u, log_.status().pos);
+  EXPECT_THAT(log_.status(),
+              StatusIs(Error::JSON_PARSER_UNEXPECTED_MAP_END, 12u));
   EXPECT_EQ("", log_.str());
 }
 
@@ -541,16 +535,15 @@ TEST_F(JsonParserTest, CommaOrMapEndExpectedError) {
   // The second separator should be a comma.
   std::string json = "{\"foo\": 3.1415: \"bar\": 0}";
   ParseJSON(SpanFrom(json), &log_);
-  EXPECT_EQ(Error::JSON_PARSER_COMMA_OR_MAP_END_EXPECTED, log_.status().error);
-  EXPECT_EQ(14u, log_.status().pos);
+  EXPECT_THAT(log_.status(),
+              StatusIs(Error::JSON_PARSER_COMMA_OR_MAP_END_EXPECTED, 14u));
   EXPECT_EQ("", log_.str());
 }
 
 TEST_F(JsonParserTest, ValueExpectedError) {
   std::string json = "}";
   ParseJSON(SpanFrom(json), &log_);
-  EXPECT_EQ(Error::JSON_PARSER_VALUE_EXPECTED, log_.status().error);
-  EXPECT_EQ(0u, log_.status().pos);
+  EXPECT_THAT(log_.status(), StatusIs(Error::JSON_PARSER_VALUE_EXPECTED, 0u));
   EXPECT_EQ("", log_.str());
 }
 
@@ -561,21 +554,29 @@ using ContainerTestTypes = ::testing::Types<std::vector<uint8_t>, std::string>;
 TYPED_TEST_SUITE(ConvertJSONToCBORTest, ContainerTestTypes);
 
 TYPED_TEST(ConvertJSONToCBORTest, RoundTripValidJson) {
-  std::string json_in = "{\"msg\":\"Hello, world.\",\"lst\":[1,2,3]}";
-  TypeParam json(json_in.begin(), json_in.end());
-  std::vector<uint8_t> cbor;
-  {
-    Status status = ConvertJSONToCBOR(SpanFrom(json), &cbor);
-    EXPECT_EQ(Error::OK, status.error);
-    EXPECT_EQ(Status::npos(), status.pos);
-  }
-  TypeParam roundtrip_json;
-  {
-    Status status = ConvertCBORToJSON(SpanFrom(cbor), &roundtrip_json);
-    EXPECT_EQ(Error::OK, status.error);
-    EXPECT_EQ(Status::npos(), status.pos);
+  for (const std::string& json_in : {
+           "{\"msg\":\"Hello, world.\",\"lst\":[1,2,3]}",
+           "3.1415",
+           "false",
+           "true",
+           "\"Hello, world.\"",
+           "[1,2,3]",
+           "[]",
+       }) {
+    SCOPED_TRACE(json_in);
+    TypeParam json(json_in.begin(), json_in.end());
+    std::vector<uint8_t> cbor;
+    {
+      Status status = ConvertJSONToCBOR(SpanFrom(json), &cbor);
+      EXPECT_THAT(status, StatusIsOk());
+    }
+    TypeParam roundtrip_json;
+    {
+      Status status = ConvertCBORToJSON(SpanFrom(cbor), &roundtrip_json);
+      EXPECT_THAT(status, StatusIsOk());
+    }
+    EXPECT_EQ(json, roundtrip_json);
   }
-  EXPECT_EQ(json, roundtrip_json);
 }
 
 TYPED_TEST(ConvertJSONToCBORTest, RoundTripValidJson16) {
@@ -587,14 +588,12 @@ TYPED_TEST(ConvertJSONToCBORTest, RoundTripValidJson16) {
   {
     Status status =
         ConvertJSONToCBOR(span<uint16_t>(json16.data(), json16.size()), &cbor);
-    EXPECT_EQ(Error::OK, status.error);
-    EXPECT_EQ(Status::npos(), status.pos);
+    EXPECT_THAT(status, StatusIsOk());
   }
   TypeParam roundtrip_json;
   {
     Status status = ConvertCBORToJSON(SpanFrom(cbor), &roundtrip_json);
-    EXPECT_EQ(Error::OK, status.error);
-    EXPECT_EQ(Status::npos(), status.pos);
+    EXPECT_THAT(status, StatusIsOk());
   }
   std::string json = "{\"msg\":\"Hello, \\ud83c\\udf0e.\",\"lst\":[1,2,3]}";
   TypeParam expected_json(json.begin(), json.end());

third_party/inspector_protocol/crdtp/status_test.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "status.h"
+#include "status_test_support.h"
 #include "test_platform.h"
 
 namespace v8_crdtp {
@@ -18,4 +19,11 @@ TEST(StatusTest, StatusToASCIIString) {
   Status cbor_error(Error::CBOR_TRAILING_JUNK, 21);
   EXPECT_EQ("CBOR: trailing junk at position 21", cbor_error.ToASCIIString());
 }
+
+TEST(StatusTest, StatusTestSupport) {
+  Status ok_status;
+  EXPECT_THAT(ok_status, StatusIsOk());
+  Status json_error(Error::JSON_PARSER_COLON_EXPECTED, 42);
+  EXPECT_THAT(json_error, StatusIs(Error::JSON_PARSER_COLON_EXPECTED, 42));
+}
 }  // namespace v8_crdtp

third_party/inspector_protocol/crdtp/status_test_support.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "status_test_support.h"
+
+namespace v8_crdtp {
+void PrintTo(const Status& status, std::ostream* os) {
+  *os << status.ToASCIIString() << " (error: 0x" << std::hex
+      << static_cast<int>(status.error) << ", "
+      << "pos: " << std::dec << status.pos << ")";
+}
+
+namespace {
+class StatusIsMatcher : public testing::MatcherInterface<Status> {
+ public:
+  explicit StatusIsMatcher(Status status) : expected_(status) {}
+
+  bool MatchAndExplain(Status status,
+                       testing::MatchResultListener* listener) const override {
+    return status.error == expected_.error && status.pos == expected_.pos;
+  }
+
+  void DescribeTo(std::ostream* os) const override {
+    *os << "equals to ";
+    PrintTo(expected_, os);
+  }
+
+ private:
+  Status expected_;
+};
+
+class StatusIsOkMatcher : public testing::MatcherInterface<Status> {
+  bool MatchAndExplain(Status status,
+                       testing::MatchResultListener* listener) const override {
+    return status.ok();
+  }
+
+  void DescribeTo(std::ostream* os) const override { *os << "is ok"; }
+};
+}  // namespace
+
+testing::Matcher<Status> StatusIsOk() {
+  return MakeMatcher(new StatusIsOkMatcher());
+}
+
+testing::Matcher<Status> StatusIs(Error error, size_t pos) {
+  return MakeMatcher(new StatusIsMatcher(Status(error, pos)));
+}
+}  // namespace v8_crdtp

third_party/inspector_protocol/crdtp/status_test_support.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_CRDTP_STATUS_TEST_SUPPORT_H_
+#define V8_CRDTP_STATUS_TEST_SUPPORT_H_
+
+#include <ostream>
+#include "status.h"
+#include "test_platform.h"
+
+namespace v8_crdtp {
+// Supports gtest, to conveniently match Status objects and
+// get useful error messages when tests fail.
+// Typically used with EXPECT_THAT, e.g.
+//
+// EXPECT_THAT(status, StatusIs(Error::JSON_PARSER_COLON_EXPECTED, 42));
+//
+// EXPECT_THAT(status, StatusIsOk());
+
+// Prints a |status|, including its generated error message, error code, and
+// position. This is used by gtest for pretty printing actual vs. expected.
+void PrintTo(const Status& status, std::ostream* os);
+
+// Matches any status with |status.ok()|.
+testing::Matcher<Status> StatusIsOk();
+
+// Matches any status with |error| and |pos|.
+testing::Matcher<Status> StatusIs(Error error, size_t pos);
+}  // namespace v8_crdtp
+
+#endif  // V8_CRDTP_STATUS_TEST_SUPPORT_H_

third_party/inspector_protocol/roll.py

@@ -38,6 +38,8 @@ FILES_TO_SYNC = [
     'crdtp/status.cc',
     'crdtp/status.h',
     'crdtp/status_test.cc',
+    'crdtp/status_test_support.cc',
+    'crdtp/status_test_support.h',
     'inspector_protocol.gni',
     'inspector_protocol.gypi',
     'lib/*',

third_party/inspector_protocol/templates/Exported_h.template

@@ -22,9 +22,6 @@ class {{config.exported.export_macro}} Exported {
 public:
     virtual {{config.exported.string_out}} toJSONString() const = 0;
 
-    V8_DEPRECATE_SOON("Use AppendSerialized instead.")
-    virtual void writeBinary(std::vector<uint8_t>* out) const = 0;
-
     virtual void AppendSerialized(std::vector<uint8_t>* out) const = 0;
 
     virtual ~Exported() { }

third_party/inspector_protocol/templates/TypeBuilder_cpp.template

@@ -120,11 +120,6 @@ std::unique_ptr<{{type.id}}> {{type.id}}::clone() const
     return {{config.exported.to_string_out % "json"}};
 }
 
-void {{type.id}}::writeBinary(std::vector<uint8_t>* out) const
-{
-    AppendSerialized(out);
-}
-
 // static
 std::unique_ptr<API::{{type.id}}> API::{{type.id}}::fromBinary(const uint8_t* data, size_t length)
 {

third_party/inspector_protocol/templates/TypeBuilder_h.template

@@ -105,7 +105,6 @@ public:
     std::unique_ptr<{{type.id}}> clone() const;
     {% if protocol.is_exported(domain.domain, type.id) %}
     {{config.exported.string_out}} toJSONString() const override;
-    void writeBinary(std::vector<uint8_t>* out) const override;
     {% endif %}
 
     template<int STATE>