Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inspector: fix StringUtil::CharacterCount for unicodes #56788

Merged
merged 1 commit into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -1474,6 +1474,8 @@ LINT_CPP_FILES = $(filter-out $(LINT_CPP_EXCLUDE), $(wildcard \
src/*/*.h \
test/addons/*/*.cc \
test/addons/*/*.h \
test/cctest/*/*.cc \
test/cctest/*/*.h \
test/cctest/*.cc \
test/cctest/*.h \
test/embedding/*.cc \
Expand Down
9 changes: 9 additions & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@
'test/cctest/test_quic_tokens.cc',
],
'node_cctest_inspector_sources': [
'test/cctest/inspector/test_node_protocol.cc',
'test/cctest/test_inspector_socket.cc',
'test/cctest/test_inspector_socket_server.cc',
],
Expand Down Expand Up @@ -1209,6 +1210,14 @@
'HAVE_INSPECTOR=1',
],
'sources': [ '<@(node_cctest_inspector_sources)' ],
'include_dirs': [
# TODO(legendecas): make node_inspector.gypi a dependable target.
'<(SHARED_INTERMEDIATE_DIR)', # for inspector
'<(SHARED_INTERMEDIATE_DIR)/src', # for inspector
],
'dependencies': [
'deps/inspector_protocol/inspector_protocol.gyp:crdtp',
],
}, {
'defines': [
'HAVE_INSPECTOR=0',
Expand Down
11 changes: 6 additions & 5 deletions src/inspector/node_string.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,12 @@ const uint8_t* StringUtil::CharactersUTF8(const std::string_view s) {
}

size_t StringUtil::CharacterCount(const std::string_view s) {
// The utf32_length_from_utf8 function calls count_utf8.
// The count_utf8 function counts the number of code points
// (characters) in the string, assuming that the string is valid Unicode.
// TODO(@anonrig): Test to make sure CharacterCount returns correctly.
return simdutf::utf32_length_from_utf8(s.data(), s.length());
// Return the length of underlying representation storage.
// E.g. for std::basic_string_view<char>, return its byte length.
// If we adopt a variant underlying store string type, like
// `v8_inspector::StringView`, for UTF16, return the length of the
// underlying uint16_t store.
return s.length();
}

} // namespace protocol
Expand Down
3 changes: 3 additions & 0 deletions src/inspector/node_string.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ using StringBuilder = std::ostringstream;
using ProtocolMessage = std::string;

// Implements StringUtil methods used in `inspector_protocol/lib/`.
// Refer to
// https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/inspector/v8_inspector_string.h;l=40;drc=2b15d6974a49d3a14d3d67ae099a649d523a828d
// for more details about the interface.
struct StringUtil {
// Convert Utf16 in local endianness to Utf8 if needed.
static String StringViewToUtf8(v8_inspector::StringView view);
Expand Down
33 changes: 33 additions & 0 deletions test/cctest/inspector/test_node_protocol.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#include "crdtp/json.h"
#include "gtest/gtest.h"
#include "inspector/node_json.h"
#include "node/inspector/protocol/Protocol.h"

namespace node {
namespace inspector {
namespace protocol {
namespace {

TEST(InspectorProtocol, Utf8StringSerDes) {
constexpr const char* kKey = "unicode_key";
constexpr const char* kValue = "CJK 汉字 🍱 🧑‍🧑‍🧒‍🧒";
std::unique_ptr<DictionaryValue> val = DictionaryValue::create();
val->setString(kKey, kValue);

std::vector<uint8_t> cbor = val->Serialize();
std::string json;
crdtp::Status status =
crdtp::json::ConvertCBORToJSON(crdtp::SpanFrom(cbor), &json);
CHECK(status.ok());

std::unique_ptr<DictionaryValue> parsed =
DictionaryValue::cast(JsonUtil::parseJSON(json));
std::string parsed_value;
CHECK(parsed->getString(kKey, &parsed_value));
CHECK_EQ(parsed_value, std::string(kValue));
}

} // namespace
} // namespace protocol
} // namespace inspector
} // namespace node
29 changes: 28 additions & 1 deletion test/common/inspector-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const http = require('http');
const fixtures = require('../common/fixtures');
const { spawn } = require('child_process');
const { URL, pathToFileURL } = require('url');
const { EventEmitter } = require('events');
const { EventEmitter, once } = require('events');

const _MAINSCRIPT = fixtures.path('loop.js');
const DEBUG = false;
Expand Down Expand Up @@ -544,6 +544,33 @@ function fires(promise, error, timeoutMs) {
]);
}

/**
* When waiting for inspector events, there might be no handles on the event
* loop, and leads to process exits.
*
* This function provides a utility to wait until a inspector event for a certain
* time.
*/
function waitUntil(session, eventName, timeout = 1000) {
const resolvers = Promise.withResolvers();
const timer = setTimeout(() => {
resolvers.reject(new Error(`Wait for inspector event ${eventName} timed out`));
}, timeout);

once(session, eventName)
.then((res) => {
resolvers.resolve(res);
clearTimeout(timer);
}, (error) => {
// This should never happen.
resolvers.reject(error);
clearTimeout(timer);
});

return resolvers.promise;
}

module.exports = {
NodeInstance,
waitUntil,
};
41 changes: 41 additions & 0 deletions test/parallel/test-inspector-network-arbitrary-data.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Flags: --inspect=0 --experimental-network-inspection
'use strict';
const common = require('../common');

common.skipIfInspectorDisabled();

const inspector = require('node:inspector/promises');
const { Network } = require('node:inspector');
const test = require('node:test');
const assert = require('node:assert');
const { waitUntil } = require('../common/inspector-helper');

const session = new inspector.Session();
session.connect();

test('should emit Network.requestWillBeSent with unicode', async () => {
await session.post('Network.enable');
const expectedValue = 'CJK 汉字 🍱 🧑‍🧑‍🧒‍🧒';

const requestWillBeSentFuture = waitUntil(session, 'Network.requestWillBeSent')
.then(([event]) => {
assert.strictEqual(event.params.request.url, expectedValue);
assert.strictEqual(event.params.request.method, expectedValue);
assert.strictEqual(event.params.request.headers.mKey, expectedValue);
});

Network.requestWillBeSent({
requestId: '1',
timestamp: 1,
wallTime: 1,
request: {
url: expectedValue,
method: expectedValue,
headers: {
mKey: expectedValue,
},
},
});

await requestWillBeSentFuture;
});
Loading