Commit ce3ce2f6 authored by Johannes Henkel's avatar Johannes Henkel Committed by Commit Bot

[DevTools] Roll inspector_protocol (V8) (strtod)

New revision: 7a44a37f66b58358dd8ab85ccde1998fafa95e53

Upstream review:
https://chromium-review.googlesource.com/c/deps/inspector_protocol/+/1899564

Previously, we used a wrapper library, v8-inspector-protocol-encoding.h
to inject the string<->number conversion into the cbor parser. This
meant that the unittests, in
third_party/inspector_protocol/crdtp/json_test.cc inadvertently did
not run with the same string<->number conversion code. Thus, we missed
that the production implementation would return INF for out of range
double literals, as opposed to rejecting the incoming JSON.
When switching the library to the build dependency on json_platform_v8.cc
as it's in this change, the test immediately failed which made it
trivial to fix the implementation.

Old implementation:
https://chromium-review.googlesource.com/c/v8/v8/+/1913424/6/src/inspector/v8-inspector-protocol-encoding.cc

New implementation (checks std::isfinite):
https://chromium-review.googlesource.com/c/v8/v8/+/1913424/6/third_party/inspector_protocol/crdtp/json_platform_v8.cc

Change-Id: Ia48fe1f4e359eea47d0ede9ceadea1fd635292e0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1913424Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
Commit-Queue: Johannes Henkel <johannes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65219}
parent bd9efe1f
......@@ -143,8 +143,6 @@ v8_source_set("inspector") {
"v8-heap-profiler-agent-impl.h",
"v8-inspector-impl.cc",
"v8-inspector-impl.h",
"v8-inspector-protocol-encoding.cc",
"v8-inspector-protocol-encoding.h",
"v8-inspector-session-impl.cc",
"v8-inspector-session-impl.h",
"v8-profiler-agent-impl.cc",
......
// Copyright 2019 the V8 project 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 "src/inspector/v8-inspector-protocol-encoding.h"
#include <cmath>
#include "../../third_party/inspector_protocol/crdtp/json.h"
#include "src/numbers/conversions.h"
#include "src/utils/vector.h"
namespace v8_inspector {
namespace {
using v8_crdtp::span;
using v8_crdtp::Status;
class Platform : public v8_crdtp::json::Platform {
// Parses |str| into |result|. Returns false iff there are
// leftover characters or parsing errors.
bool StrToD(const char* str, double* result) const override {
*result = v8::internal::StringToDouble(str, v8::internal::NO_FLAGS);
return !std::isnan(*result);
}
// Prints |value| in a format suitable for JSON.
std::unique_ptr<char[]> DToStr(double value) const override {
v8::internal::ScopedVector<char> buffer(
v8::internal::kDoubleToCStringMinBufferSize);
const char* str = v8::internal::DoubleToCString(value, buffer);
if (str == nullptr) return nullptr;
std::unique_ptr<char[]> result(new char[strlen(str) + 1]);
memcpy(result.get(), str, strlen(str) + 1);
DCHECK_EQ(0, result[strlen(str)]);
return result;
}
};
} // namespace
Status ConvertCBORToJSON(span<uint8_t> cbor, std::vector<uint8_t>* json) {
Platform platform;
return v8_crdtp::json::ConvertCBORToJSON(platform, cbor, json);
}
Status ConvertJSONToCBOR(span<uint8_t> json, std::vector<uint8_t>* cbor) {
Platform platform;
return v8_crdtp::json::ConvertJSONToCBOR(platform, json, cbor);
}
Status ConvertJSONToCBOR(span<uint16_t> json, std::vector<uint8_t>* cbor) {
Platform platform;
return v8_crdtp::json::ConvertJSONToCBOR(platform, json, cbor);
}
} // namespace v8_inspector
// Copyright 2019 the V8 project 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_INSPECTOR_V8_INSPECTOR_PROTOCOL_ENCODING_H_
#define V8_INSPECTOR_V8_INSPECTOR_PROTOCOL_ENCODING_H_
#include "../../third_party/inspector_protocol/crdtp/span.h"
#include "../../third_party/inspector_protocol/crdtp/status.h"
namespace v8_inspector {
v8_crdtp::Status ConvertCBORToJSON(v8_crdtp::span<uint8_t> cbor,
std::vector<uint8_t>* json);
v8_crdtp::Status ConvertJSONToCBOR(v8_crdtp::span<uint8_t> json,
std::vector<uint8_t>* cbor);
v8_crdtp::Status ConvertJSONToCBOR(v8_crdtp::span<uint16_t> json,
std::vector<uint8_t>* cbor);
} // namespace v8_inspector
#endif // V8_INSPECTOR_V8_INSPECTOR_PROTOCOL_ENCODING_H_
......@@ -4,6 +4,8 @@
#include "src/inspector/v8-inspector-session-impl.h"
#include "../../third_party/inspector_protocol/crdtp/cbor.h"
#include "../../third_party/inspector_protocol/crdtp/json.h"
#include "src/base/logging.h"
#include "src/base/macros.h"
#include "src/inspector/injected-script.h"
......@@ -17,7 +19,6 @@
#include "src/inspector/v8-debugger.h"
#include "src/inspector/v8-heap-profiler-agent-impl.h"
#include "src/inspector/v8-inspector-impl.h"
#include "src/inspector/v8-inspector-protocol-encoding.h"
#include "src/inspector/v8-profiler-agent-impl.h"
#include "src/inspector/v8-runtime-agent-impl.h"
#include "src/inspector/v8-schema-agent-impl.h"
......@@ -26,14 +27,16 @@ namespace v8_inspector {
namespace {
using v8_crdtp::span;
using v8_crdtp::SpanFrom;
using IPEStatus = v8_crdtp::Status;
using v8_crdtp::Status;
using v8_crdtp::json::ConvertCBORToJSON;
using v8_crdtp::json::ConvertJSONToCBOR;
bool IsCBORMessage(const StringView& msg) {
return msg.is8Bit() && msg.length() >= 2 && msg.characters8()[0] == 0xd8 &&
msg.characters8()[1] == 0x5a;
}
IPEStatus ConvertToCBOR(const StringView& state, std::vector<uint8_t>* cbor) {
Status ConvertToCBOR(const StringView& state, std::vector<uint8_t>* cbor) {
return state.is8Bit()
? ConvertJSONToCBOR(
span<uint8_t>(state.characters8(), state.length()), cbor)
......@@ -168,7 +171,7 @@ std::unique_ptr<StringBuffer> V8InspectorSessionImpl::serializeForFrontend(
return std::unique_ptr<StringBuffer>(
new BinaryStringBuffer(std::move(cbor)));
std::vector<uint8_t> json;
IPEStatus status = ConvertCBORToJSON(SpanFrom(cbor), &json);
Status status = ConvertCBORToJSON(SpanFrom(cbor), &json);
DCHECK(status.ok());
USE(status);
String16 string16(reinterpret_cast<const char*>(json.data()), json.size());
......
......@@ -4,6 +4,12 @@
import("../../gni/v8.gni")
config("crdtp_config") {
visibility = [ "../../src/inspector:*" ]
configs = [ "../../:internal_config" ]
include_dirs = [ "../../include" ]
}
v8_source_set("crdtp") {
sources = [
"crdtp/cbor.cc",
......@@ -12,37 +18,59 @@ v8_source_set("crdtp") {
"crdtp/glue.h",
"crdtp/json.cc",
"crdtp/json.h",
"crdtp/json_platform.h",
"crdtp/parser_handler.h",
"crdtp/span.h",
"crdtp/status.cc",
"crdtp/status.h",
]
configs = [ "../../:internal_config" ]
configs = [ ":crdtp_config" ]
deps = [
":crdtp_platform",
]
}
# crdtp_test is part of the unittests, defined in
# test/unittests/BUILD.gn.
# A small adapter library which only :crdtp may depend on.
v8_source_set("crdtp_platform") {
visibility = [ ":crdtp" ]
sources = [
"crdtp/json_platform.h",
"crdtp/json_platform_v8.cc",
]
public_deps = [
"../..:v8_libbase",
]
configs = [ ":crdtp_config" ]
}
# These tests are linked into test/unittests.
v8_source_set("crdtp_test") {
testonly = true
sources = [
"crdtp/cbor_test.cc",
"crdtp/glue_test.cc",
"crdtp/json_test.cc",
"crdtp/span_test.cc",
"crdtp/status_test.cc",
"crdtp/test_platform.cc",
"crdtp/test_platform.h",
]
configs = [
"../..:external_config",
"../..:internal_config_base",
]
configs = [ ":crdtp_config" ]
deps = [
":crdtp_test_platform",
]
testonly = true
}
# A small adapter library which only :crdtp_test may depend on.
v8_source_set("crdtp_test_platform") {
sources = [
"crdtp/test_platform.h",
"crdtp/test_platform_v8.cc",
]
configs = [ ":crdtp_config" ]
public_deps = [
":crdtp",
"../..:v8_libbase",
"../../src/inspector:inspector_string_conversions",
"//testing/gmock",
"//testing/gtest",
]
testonly = true
}
......@@ -14,21 +14,5 @@ https://cs.chromium.org/chromium/src/v8/third_party/inspector_protocol/
See also [Contributing to Chrome Devtools Protocol](https://docs.google.com/document/d/1c-COD2kaK__5iMM5SEx-PzNA7HFmgttcYfOHHX0HaOM/edit).
We're working on enabling standalone builds for parts of this package for
testing and development.
If you're familiar with
[Chromium's development process](https://www.chromium.org/developers/contributing-code)
and have the depot_tools installed, you may use these commands
to fetch the package (and dependencies) and build and run the tests:
fetch inspector_protocol
cd src
gn gen out/Release
ninja -C out/Release encoding_test bindings_test
out/Release/encoding_test
out/Release/bindings_test
You'll probably also need to install g++, since Clang uses this to find the
standard C++ headers. E.g.,
sudo apt-get install g++-8
To build and run the tests of the crdtp library, see
[CRDTP - Chrome DevTools Protocol](crdtp/README.md).
......@@ -2,7 +2,7 @@ Name: inspector protocol
Short Name: inspector_protocol
URL: https://chromium.googlesource.com/deps/inspector_protocol/
Version: 0
Revision: 4c2a3acaea9f2e7958081dd361f81e20e9eff5e7
Revision: 7a44a37f66b58358dd8ab85ccde1998fafa95e53
License: BSD
License File: LICENSE
Security Critical: no
......
......@@ -2,6 +2,5 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// This file is V8 specific.
// It is not rolled from the upstream project.
// This file is V8 specific. It's not rolled from the upstream project.
// CRDTP doesn't export symbols from V8, so it's empty.
......@@ -12,10 +12,10 @@
#include <stack>
#include "cbor.h"
#include "json_platform.h"
namespace v8_crdtp {
namespace json {
// =============================================================================
// json::NewJSONEncoder - for encoding streaming parser events as JSON
// =============================================================================
......@@ -104,8 +104,7 @@ void Base64Encode(const span<uint8_t>& in, C* out) {
template <typename C>
class JSONEncoder : public ParserHandler {
public:
JSONEncoder(const Platform* platform, C* out, Status* status)
: platform_(platform), out_(out), status_(status) {
JSONEncoder(C* out, Status* status) : out_(out), status_(status) {
*status_ = Status();
state_.emplace(Container::NONE);
}
......@@ -283,14 +282,14 @@ class JSONEncoder : public ParserHandler {
Emit("null");
return;
}
std::unique_ptr<char[]> str_value = platform_->DToStr(value);
std::string str_value = json::platform::DToStr(value);
// DToStr may fail to emit a 0 before the decimal dot. E.g. this is
// the case in base::NumberToString in Chromium (which is based on
// dmg_fp). So, much like
// https://cs.chromium.org/chromium/src/base/json/json_writer.cc
// we probe for this and emit the leading 0 anyway if necessary.
const char* chars = str_value.get();
const char* chars = str_value.c_str();
if (chars[0] == '.') {
Emit('0');
} else if (chars[0] == '-' && chars[1] == '.') {
......@@ -336,25 +335,22 @@ class JSONEncoder : public ParserHandler {
out_->insert(out_->end(), str.begin(), str.end());
}
const Platform* platform_;
C* out_;
Status* status_;
std::stack<State> state_;
};
} // namespace
std::unique_ptr<ParserHandler> NewJSONEncoder(const Platform* platform,
std::vector<uint8_t>* out,
std::unique_ptr<ParserHandler> NewJSONEncoder(std::vector<uint8_t>* out,
Status* status) {
return std::unique_ptr<ParserHandler>(
new JSONEncoder<std::vector<uint8_t>>(platform, out, status));
new JSONEncoder<std::vector<uint8_t>>(out, status));
}
std::unique_ptr<ParserHandler> NewJSONEncoder(const Platform* platform,
std::string* out,
std::unique_ptr<ParserHandler> NewJSONEncoder(std::string* out,
Status* status) {
return std::unique_ptr<ParserHandler>(
new JSONEncoder<std::string>(platform, out, status));
new JSONEncoder<std::string>(out, status));
}
// =============================================================================
......@@ -387,8 +383,7 @@ const char* const kFalseString = "false";
template <typename Char>
class JsonParser {
public:
JsonParser(const Platform* platform, ParserHandler* handler)
: platform_(platform), handler_(handler) {}
explicit JsonParser(ParserHandler* handler) : handler_(handler) {}
void Parse(const Char* start, size_t length) {
start_pos_ = start;
......@@ -412,12 +407,12 @@ class JsonParser {
return false;
buffer.push_back(static_cast<char>(chars[ii]));
}
return platform_->StrToD(buffer.c_str(), result);
return platform::StrToD(buffer.c_str(), result);
}
bool CharsToDouble(const uint8_t* chars, size_t length, double* result) {
std::string buffer(reinterpret_cast<const char*>(chars), length);
return platform_->StrToD(buffer.c_str(), result);
return platform::StrToD(buffer.c_str(), result);
}
static bool ParseConstToken(const Char* start,
......@@ -966,22 +961,17 @@ class JsonParser {
const Char* start_pos_ = nullptr;
bool error_ = false;
const Platform* platform_;
ParserHandler* handler_;
};
} // namespace
void ParseJSON(const Platform& platform,
span<uint8_t> chars,
ParserHandler* handler) {
JsonParser<uint8_t> parser(&platform, handler);
void ParseJSON(span<uint8_t> chars, ParserHandler* handler) {
JsonParser<uint8_t> parser(handler);
parser.Parse(chars.data(), chars.size());
}
void ParseJSON(const Platform& platform,
span<uint16_t> chars,
ParserHandler* handler) {
JsonParser<uint16_t> parser(&platform, handler);
void ParseJSON(span<uint16_t> chars, ParserHandler* handler) {
JsonParser<uint16_t> parser(handler);
parser.Parse(chars.data(), chars.size());
}
......@@ -989,58 +979,43 @@ void ParseJSON(const Platform& platform,
// json::ConvertCBORToJSON, json::ConvertJSONToCBOR - for transcoding
// =============================================================================
template <typename C>
Status ConvertCBORToJSONTmpl(const Platform& platform,
span<uint8_t> cbor,
C* json) {
Status ConvertCBORToJSONTmpl(span<uint8_t> cbor, C* json) {
Status status;
std::unique_ptr<ParserHandler> json_writer =
NewJSONEncoder(&platform, json, &status);
std::unique_ptr<ParserHandler> json_writer = NewJSONEncoder(json, &status);
cbor::ParseCBOR(cbor, json_writer.get());
return status;
}
Status ConvertCBORToJSON(const Platform& platform,
span<uint8_t> cbor,
std::vector<uint8_t>* json) {
return ConvertCBORToJSONTmpl(platform, cbor, json);
Status ConvertCBORToJSON(span<uint8_t> cbor, std::vector<uint8_t>* json) {
return ConvertCBORToJSONTmpl(cbor, json);
}
Status ConvertCBORToJSON(const Platform& platform,
span<uint8_t> cbor,
std::string* json) {
return ConvertCBORToJSONTmpl(platform, cbor, json);
Status ConvertCBORToJSON(span<uint8_t> cbor, std::string* json) {
return ConvertCBORToJSONTmpl(cbor, json);
}
template <typename T, typename C>
Status ConvertJSONToCBORTmpl(const Platform& platform, span<T> json, C* cbor) {
Status ConvertJSONToCBORTmpl(span<T> json, C* cbor) {
Status status;
std::unique_ptr<ParserHandler> encoder = cbor::NewCBOREncoder(cbor, &status);
ParseJSON(platform, json, encoder.get());
ParseJSON(json, encoder.get());
return status;
}
Status ConvertJSONToCBOR(const Platform& platform,
span<uint8_t> json,
std::string* cbor) {
return ConvertJSONToCBORTmpl(platform, json, cbor);
Status ConvertJSONToCBOR(span<uint8_t> json, std::string* cbor) {
return ConvertJSONToCBORTmpl(json, cbor);
}
Status ConvertJSONToCBOR(const Platform& platform,
span<uint16_t> json,
std::string* cbor) {
return ConvertJSONToCBORTmpl(platform, json, cbor);
Status ConvertJSONToCBOR(span<uint16_t> json, std::string* cbor) {
return ConvertJSONToCBORTmpl(json, cbor);
}
Status ConvertJSONToCBOR(const Platform& platform,
span<uint8_t> json,
std::vector<uint8_t>* cbor) {
return ConvertJSONToCBORTmpl(platform, json, cbor);
Status ConvertJSONToCBOR(span<uint8_t> json, std::vector<uint8_t>* cbor) {
return ConvertJSONToCBORTmpl(json, cbor);
}
Status ConvertJSONToCBOR(const Platform& platform,
span<uint16_t> json,
std::vector<uint8_t>* cbor) {
return ConvertJSONToCBORTmpl(platform, json, cbor);
Status ConvertJSONToCBOR(span<uint16_t> json, std::vector<uint8_t>* cbor) {
return ConvertJSONToCBORTmpl(json, cbor);
}
} // namespace json
} // namespace v8_crdtp
......@@ -6,9 +6,7 @@
#define V8_CRDTP_JSON_H_
#include <memory>
#include "export.h"
#include "json_platform.h"
#include "parser_handler.h"
namespace v8_crdtp {
......@@ -23,45 +21,34 @@ namespace json {
// Except for calling the HandleError routine at any time, the client
// code must call the Handle* methods in an order in which they'd occur
// in valid JSON; otherwise we may crash (the code uses assert).
std::unique_ptr<ParserHandler> NewJSONEncoder(const Platform* platform,
std::vector<uint8_t>* out,
Status* status);
std::unique_ptr<ParserHandler> NewJSONEncoder(const Platform* platform,
std::string* out,
std::unique_ptr<ParserHandler> NewJSONEncoder(std::vector<uint8_t>* out,
Status* status);
std::unique_ptr<ParserHandler> NewJSONEncoder(std::string* out, Status* status);
// =============================================================================
// json::ParseJSON - for receiving streaming parser events for JSON
// =============================================================================
void ParseJSON(const Platform& platform,
span<uint8_t> chars,
ParserHandler* handler);
void ParseJSON(const Platform& platform,
span<uint16_t> chars,
ParserHandler* handler);
void ParseJSON(span<uint8_t> chars, ParserHandler* handler);
void ParseJSON(span<uint16_t> chars, ParserHandler* handler);
// =============================================================================
// json::ConvertCBORToJSON, json::ConvertJSONToCBOR - for transcoding
// =============================================================================
Status ConvertCBORToJSON(const Platform& platform,
span<uint8_t> cbor,
std::string* json);
Status ConvertCBORToJSON(const Platform& platform,
span<uint8_t> cbor,
std::vector<uint8_t>* json);
Status ConvertJSONToCBOR(const Platform& platform,
span<uint8_t> json,
std::vector<uint8_t>* cbor);
Status ConvertJSONToCBOR(const Platform& platform,
span<uint16_t> json,
std::vector<uint8_t>* cbor);
Status ConvertJSONToCBOR(const Platform& platform,
span<uint8_t> json,
std::string* cbor);
Status ConvertJSONToCBOR(const Platform& platform,
span<uint16_t> json,
std::string* cbor);
Status ConvertCBORToJSON(span<uint8_t> cbor, std::string* json);
Status ConvertCBORToJSON(span<uint8_t> cbor, std::vector<uint8_t>* json);
Status ConvertJSONToCBOR(span<uint8_t> json, std::vector<uint8_t>* cbor);
Status ConvertJSONToCBOR(span<uint16_t> json, std::vector<uint8_t>* cbor);
Status ConvertJSONToCBOR(span<uint8_t> json, std::string* cbor);
Status ConvertJSONToCBOR(span<uint16_t> json, std::string* cbor);
} // namespace json
} // namespace v8_crdtp
......
......@@ -5,23 +5,21 @@
#ifndef V8_CRDTP_JSON_PLATFORM_H_
#define V8_CRDTP_JSON_PLATFORM_H_
#include <memory>
#include "export.h"
#include <string>
namespace v8_crdtp {
namespace json {
// Client code must provide an instance. Implementation should delegate
// to whatever is appropriate.
class Platform {
public:
virtual ~Platform() = default;
// Parses |str| into |result|. Returns false iff there are
// leftover characters or parsing errors.
virtual bool StrToD(const char* str, double* result) const = 0;
// These routines are implemented in json_platform.cc, or in a
// platform-dependent (code-base dependent) custom replacement.
// E.g., json_platform_chromium.cc, json_platform_v8.cc.
namespace platform {
// Parses |str| into |result|. Returns false iff there are
// leftover characters or parsing errors.
bool StrToD(const char* str, double* result);
// Prints |value| in a format suitable for JSON.
virtual std::unique_ptr<char[]> DToStr(double value) const = 0;
};
// Prints |value| in a format suitable for JSON.
std::string DToStr(double value);
} // namespace platform
} // namespace json
} // namespace v8_crdtp
......
// Copyright 2019 The V8 Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// This file is V8 specific. It's not rolled from the upstream project.
#include "json_platform.h"
#include <cmath>
#include "../../../src/numbers/conversions.h"
#include "../../../src/utils/vector.h"
namespace v8_crdtp {
namespace json {
namespace platform {
// Parses |str| into |result|. Returns false iff there are
// leftover characters or parsing errors.
bool StrToD(const char* str, double* result) {
*result = v8::internal::StringToDouble(str, v8::internal::NO_FLAGS);
return std::isfinite(*result);
}
// Prints |value| in a format suitable for JSON.
std::string DToStr(double value) {
v8::internal::ScopedVector<char> buffer(
v8::internal::kDoubleToCStringMinBufferSize);
const char* str = v8::internal::DoubleToCString(value, buffer);
return (str == nullptr) ? "" : std::string(str);
}
} // namespace platform
} // namespace json
} // namespace v8_crdtp
......@@ -2,8 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// This file is V8 specific, to make encoding_test.cc work.
// It is not rolled from the upstream project.
// This file is V8 specific. It's not rolled from the upstream project.
#ifndef V8_INSPECTOR_PROTOCOL_CRDTP_TEST_PLATFORM_H_
#define V8_INSPECTOR_PROTOCOL_CRDTP_TEST_PLATFORM_H_
......
......@@ -2,8 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// This file is V8 specific, to make encoding_test.cc work.
// It is not rolled from the upstream project.
// This file is V8 specific. It's not rolled from the upstream project.
#include "test_platform.h"
......
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