Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 91a3023

Browse filesBrowse files
legendecastargos
legendecas
authored and
targos
committed
inspector: fix StringUtil::CharacterCount for unicodes
`StringUtil::CharacterCount` should return the length of underlying representation storage of a protocol string. `StringUtil::CharacterCount` is only used in DictionaryValue serialization. Only `Network.Headers` is an object type, represented with DictionaryValue. PR-URL: #56788 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
1 parent 91638ee commit 91a3023
Copy full SHA for 91a3023

File tree

Expand file treeCollapse file tree

7 files changed

+122
-6
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

7 files changed

+122
-6
lines changed
Open diff view settings
Collapse file

‎Makefile‎

Copy file name to clipboardExpand all lines: Makefile
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,6 +1474,8 @@ LINT_CPP_FILES = $(filter-out $(LINT_CPP_EXCLUDE), $(wildcard \
14741474
src/*/*.h \
14751475
test/addons/*/*.cc \
14761476
test/addons/*/*.h \
1477+
test/cctest/*/*.cc \
1478+
test/cctest/*/*.h \
14771479
test/cctest/*.cc \
14781480
test/cctest/*.h \
14791481
test/embedding/*.cc \
Collapse file

‎node.gyp‎

Copy file name to clipboardExpand all lines: node.gyp
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,7 @@
432432
'test/cctest/test_quic_tokens.cc',
433433
],
434434
'node_cctest_inspector_sources': [
435+
'test/cctest/inspector/test_node_protocol.cc',
435436
'test/cctest/test_inspector_socket.cc',
436437
'test/cctest/test_inspector_socket_server.cc',
437438
],
@@ -1210,6 +1211,14 @@
12101211
'HAVE_INSPECTOR=1',
12111212
],
12121213
'sources': [ '<@(node_cctest_inspector_sources)' ],
1214+
'include_dirs': [
1215+
# TODO(legendecas): make node_inspector.gypi a dependable target.
1216+
'<(SHARED_INTERMEDIATE_DIR)', # for inspector
1217+
'<(SHARED_INTERMEDIATE_DIR)/src', # for inspector
1218+
],
1219+
'dependencies': [
1220+
'deps/inspector_protocol/inspector_protocol.gyp:crdtp',
1221+
],
12131222
}, {
12141223
'defines': [
12151224
'HAVE_INSPECTOR=0',
Collapse file

‎src/inspector/node_string.cc‎

Copy file name to clipboardExpand all lines: src/inspector/node_string.cc
+6-5Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,12 @@ const uint8_t* StringUtil::CharactersUTF8(const std::string_view s) {
8585
}
8686

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

9596
} // namespace protocol
Collapse file

‎src/inspector/node_string.h‎

Copy file name to clipboardExpand all lines: src/inspector/node_string.h
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ using StringBuilder = std::ostringstream;
3232
using ProtocolMessage = std::string;
3333

3434
// Implements StringUtil methods used in `inspector_protocol/lib/`.
35+
// Refer to
36+
// https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/inspector/v8_inspector_string.h;l=40;drc=2b15d6974a49d3a14d3d67ae099a649d523a828d
37+
// for more details about the interface.
3538
struct StringUtil {
3639
// Convert Utf16 in local endianness to Utf8 if needed.
3740
static String StringViewToUtf8(v8_inspector::StringView view);
Collapse file

‎test/cctest/inspector/test_node_protocol.cc‎

Copy file name to clipboard
+33Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
#include "crdtp/json.h"
2+
#include "gtest/gtest.h"
3+
#include "inspector/node_json.h"
4+
#include "node/inspector/protocol/Protocol.h"
5+
6+
namespace node {
7+
namespace inspector {
8+
namespace protocol {
9+
namespace {
10+
11+
TEST(InspectorProtocol, Utf8StringSerDes) {
12+
constexpr const char* kKey = "unicode_key";
13+
constexpr const char* kValue = "CJK 汉字 🍱 🧑‍🧑‍🧒‍🧒";
14+
std::unique_ptr<DictionaryValue> val = DictionaryValue::create();
15+
val->setString(kKey, kValue);
16+
17+
std::vector<uint8_t> cbor = val->Serialize();
18+
std::string json;
19+
crdtp::Status status =
20+
crdtp::json::ConvertCBORToJSON(crdtp::SpanFrom(cbor), &json);
21+
CHECK(status.ok());
22+
23+
std::unique_ptr<DictionaryValue> parsed =
24+
DictionaryValue::cast(JsonUtil::parseJSON(json));
25+
std::string parsed_value;
26+
CHECK(parsed->getString(kKey, &parsed_value));
27+
CHECK_EQ(parsed_value, std::string(kValue));
28+
}
29+
30+
} // namespace
31+
} // namespace protocol
32+
} // namespace inspector
33+
} // namespace node
Collapse file

‎test/common/inspector-helper.js‎

Copy file name to clipboardExpand all lines: test/common/inspector-helper.js
+28-1Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const http = require('http');
66
const fixtures = require('../common/fixtures');
77
const { spawn } = require('child_process');
88
const { URL, pathToFileURL } = require('url');
9-
const { EventEmitter } = require('events');
9+
const { EventEmitter, once } = require('events');
1010

1111
const _MAINSCRIPT = fixtures.path('loop.js');
1212
const DEBUG = false;
@@ -544,6 +544,33 @@ function fires(promise, error, timeoutMs) {
544544
]);
545545
}
546546

547+
/**
548+
* When waiting for inspector events, there might be no handles on the event
549+
* loop, and leads to process exits.
550+
*
551+
* This function provides a utility to wait until a inspector event for a certain
552+
* time.
553+
*/
554+
function waitUntil(session, eventName, timeout = 1000) {
555+
const resolvers = Promise.withResolvers();
556+
const timer = setTimeout(() => {
557+
resolvers.reject(new Error(`Wait for inspector event ${eventName} timed out`));
558+
}, timeout);
559+
560+
once(session, eventName)
561+
.then((res) => {
562+
resolvers.resolve(res);
563+
clearTimeout(timer);
564+
}, (error) => {
565+
// This should never happen.
566+
resolvers.reject(error);
567+
clearTimeout(timer);
568+
});
569+
570+
return resolvers.promise;
571+
}
572+
547573
module.exports = {
548574
NodeInstance,
575+
waitUntil,
549576
};
Collapse file

‎test/parallel/test-inspector-network-arbitrary-data.js‎

Copy file name to clipboard
+41Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Flags: --inspect=0 --experimental-network-inspection
2+
'use strict';
3+
const common = require('../common');
4+
5+
common.skipIfInspectorDisabled();
6+
7+
const inspector = require('node:inspector/promises');
8+
const { Network } = require('node:inspector');
9+
const test = require('node:test');
10+
const assert = require('node:assert');
11+
const { waitUntil } = require('../common/inspector-helper');
12+
13+
const session = new inspector.Session();
14+
session.connect();
15+
16+
test('should emit Network.requestWillBeSent with unicode', async () => {
17+
await session.post('Network.enable');
18+
const expectedValue = 'CJK 汉字 🍱 🧑‍🧑‍🧒‍🧒';
19+
20+
const requestWillBeSentFuture = waitUntil(session, 'Network.requestWillBeSent')
21+
.then(([event]) => {
22+
assert.strictEqual(event.params.request.url, expectedValue);
23+
assert.strictEqual(event.params.request.method, expectedValue);
24+
assert.strictEqual(event.params.request.headers.mKey, expectedValue);
25+
});
26+
27+
Network.requestWillBeSent({
28+
requestId: '1',
29+
timestamp: 1,
30+
wallTime: 1,
31+
request: {
32+
url: expectedValue,
33+
method: expectedValue,
34+
headers: {
35+
mKey: expectedValue,
36+
},
37+
},
38+
});
39+
40+
await requestWillBeSentFuture;
41+
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.