FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

inspector: fix StringUtil::CharacterCount for unicodes · nodejs/node@91a3023 · GitHub

/ node Public

Commit 91a3023

Browse files
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

7 files changed

Lines changed: 122 additions & 6 deletions

File tree

‎Makefile‎

Lines 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 \

‎node.gyp‎

Lines 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',

‎src/inspector/node_string.cc‎

Lines 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

‎src/inspector/node_string.h‎

Lines 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);

‎test/cctest/inspector/test_node_protocol.cc‎

Lines 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

‎test/common/inspector-helper.js‎

Lines 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
};

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

Lines 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)
Back | FazBrowse Home | New Git URL