Skip to content

Commit 0827c80

Browse files
alexkozyalexkozy
committed
inspector: implemented V8InspectorClient::resourceNameToUrl
This method is required by inspector to report normalized urls over the protocol. Fixes nodejs#22223 PR-URL: nodejs#22251 Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
1 parent cf340fe commit 0827c80

10 files changed

+90
-16
lines changed

‎src/inspector_agent.cc‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "inspector/tracing_agent.h"
77
#include "node/inspector/protocol/Protocol.h"
88
#include "node_internals.h"
9+
#include "node_url.h"
910
#include "v8-inspector.h"
1011
#include "v8-platform.h"
1112

@@ -369,6 +370,25 @@ void NotifyClusterWorkersDebugEnabled(Environment* env) {
369370
MakeCallback(env->isolate(), process_object, emit_fn.As<Function>(),
370371
arraysize(argv), argv, {0, 0});
371372
}
373+
374+
#ifdef _WIN32
375+
bool IsFilePath(const std::string& path) {
376+
// '\\'
377+
if (path.length() > 2 && path[0] == '\\' && path[1] == '\\')
378+
return true;
379+
// '[A-Z]:[/\\]'
380+
if (path.length() < 3)
381+
return false;
382+
if ((path[0] >= 'A' && path[0] <= 'Z') || (path[0] >= 'a' && path[0] <= 'z'))
383+
return path[1] == ':' && (path[2] == '/' || path[2] == '\\');
384+
return false;
385+
}
386+
#else
387+
bool IsFilePath(const std::string& path) {
388+
return path.length() && path[0] == '/';
389+
}
390+
#endif // __POSIX__
391+
372392
} // namespace
373393

374394
class NodeInspectorClient : public V8InspectorClient {
@@ -592,6 +612,18 @@ class NodeInspectorClient : public V8InspectorClient {
592612
return env_->isolate_data()->platform()->CurrentClockTimeMillis();
593613
}
594614

615+
std::unique_ptr<StringBuffer> resourceNameToUrl(
616+
const StringView& resource_name_view) override {
617+
std::string resource_name =
618+
protocol::StringUtil::StringViewToUtf8(resource_name_view);
619+
if (!IsFilePath(resource_name))
620+
return nullptr;
621+
node::url::URL url = node::url::URL::FromFilePath(resource_name);
622+
// TODO(ak239spb): replace this code with url.href().
623+
// Refs: https://github.com/nodejs/node/issues/22610
624+
return Utf8ToStringView(url.protocol() + "//" + url.path());
625+
}
626+
595627
node::Environment* env_;
596628
bool running_nested_loop_ = false;
597629
std::unique_ptr<V8Inspector> client_;

‎test/cctest/test_url.cc‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,6 @@ TEST_F(URLTest, FromFilePath) {
124124
file_url = URL::FromFilePath("b:\\a\\%%.js");
125125
EXPECT_EQ("file:", file_url.protocol());
126126
EXPECT_EQ("/b:/a/%25%25.js", file_url.path());
127-
128-
file_url = URL::FromFilePath("\\\\host\\a\\b\\c");
129-
EXPECT_EQ("file:", file_url.protocol());
130-
EXPECT_EQ("host/a/b/c", file_url.path());
131127
#else
132128
file_url = URL::FromFilePath("/");
133129
EXPECT_EQ("file:", file_url.protocol());

‎test/parallel/test-inspector-multisession-js.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34

@@ -6,6 +7,7 @@ common.skipIfInspectorDisabled();
67
const assert = require('assert');
78
const { Session } = require('inspector');
89
const path = require('path');
10+
const { pathToFileURL } = require('url');
911

1012
function debugged() {
1113
return 42;
@@ -32,8 +34,8 @@ async function test() {
3234

3335
await new Promise((resolve, reject) => {
3436
session1.post('Debugger.setBreakpointByUrl', {
35-
'lineNumber': 9,
36-
'url': path.resolve(__dirname, __filename),
37+
'lineNumber': 12,
38+
'url': pathToFileURL(path.resolve(__dirname, __filename)).toString(),
3739
'columnNumber': 0,
3840
'condition': ''
3941
}, (error, result) => {

‎test/parallel/test-v8-coverage.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ function getFixtureCoverage(fixtureFile, coverageDirectory) {
9797
for (const coverageFile of coverageFiles) {
9898
const coverage = require(path.join(coverageDirectory, coverageFile));
9999
for (const fixtureCoverage of coverage.result) {
100-
if (fixtureCoverage.url.indexOf(`${path.sep}${fixtureFile}`) !== -1) {
100+
if (fixtureCoverage.url.indexOf(`/${fixtureFile}`) !== -1) {
101101
return fixtureCoverage;
102102
}
103103
}

‎test/sequential/test-inspector-bindings.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1+
// Flags: --expose-internals
12
'use strict';
23
const common = require('../common');
34
common.skipIfInspectorDisabled();
45
const assert = require('assert');
56
const inspector = require('inspector');
67
const path = require('path');
8+
const { pathToFileURL } = require('url');
79

810
// This test case will set a breakpoint 4 lines below
911
function debuggedFunction() {
@@ -84,8 +86,8 @@ function testSampleDebugSession() {
8486
}, TypeError);
8587
session.post('Debugger.enable', () => cbAsSecondArgCalled = true);
8688
session.post('Debugger.setBreakpointByUrl', {
87-
'lineNumber': 12,
88-
'url': path.resolve(__dirname, __filename),
89+
'lineNumber': 14,
90+
'url': pathToFileURL(path.resolve(__dirname, __filename)).toString(),
8991
'columnNumber': 0,
9092
'condition': ''
9193
});

‎test/sequential/test-inspector-break-when-eval.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ common.skipIfInspectorDisabled();
55
const assert = require('assert');
66
const { NodeInstance } = require('../common/inspector-helper.js');
77
const fixtures = require('../common/fixtures');
8+
const { pathToFileURL } = require('url');
89

910
const script = fixtures.path('inspector-global-function.js');
1011

@@ -26,7 +27,7 @@ async function breakOnLine(session) {
2627
const commands = [
2728
{ 'method': 'Debugger.setBreakpointByUrl',
2829
'params': { 'lineNumber': 9,
29-
'url': script,
30+
'url': pathToFileURL(script).toString(),
3031
'columnNumber': 0,
3132
'condition': ''
3233
}
@@ -45,7 +46,7 @@ async function breakOnLine(session) {
4546
}
4647
];
4748
session.send(commands);
48-
await session.waitForBreakOnLine(9, script);
49+
await session.waitForBreakOnLine(9, pathToFileURL(script).toString());
4950
}
5051

5152
async function stepOverConsoleStatement(session) {

‎test/sequential/test-inspector-debug-brk-flag.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ async function testBreakpointOnStart(session) {
2424
];
2525

2626
session.send(commands);
27-
await session.waitForBreakOnLine(0, session.scriptPath());
27+
await session.waitForBreakOnLine(0, session.scriptURL());
2828
}
2929

3030
async function runTests() {

‎test/sequential/test-inspector-exception.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ common.skipIfInspectorDisabled();
77

88
const assert = require('assert');
99
const { NodeInstance } = require('../common/inspector-helper.js');
10+
const { pathToFileURL } = require('url');
1011

1112
const script = fixtures.path('throws_error.js');
1213

@@ -29,7 +30,7 @@ async function testBreakpointOnStart(session) {
2930
];
3031

3132
await session.send(commands);
32-
await session.waitForBreakOnLine(21, script);
33+
await session.waitForBreakOnLine(21, pathToFileURL(script).toString());
3334
}
3435

3536

‎test/sequential/test-inspector-resource-name-to-url.js‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
common.skipIfInspectorDisabled();
5+
6+
(async function test() {
7+
const { strictEqual } = require('assert');
8+
const { Session } = require('inspector');
9+
const { promisify } = require('util');
10+
const vm = require('vm');
11+
const session = new Session();
12+
session.connect();
13+
session.post = promisify(session.post);
14+
await session.post('Debugger.enable');
15+
await check('http://example.com', 'http://example.com');
16+
await check(undefined, 'evalmachine.<anonymous>');
17+
await check('file:///foo.js', 'file:///foo.js');
18+
await check('file:///foo.js', 'file:///foo.js');
19+
await check('foo.js', 'foo.js');
20+
await check('[eval]', '[eval]');
21+
await check('%.js', '%.js');
22+
23+
if (common.isWindows) {
24+
await check('C:\\foo.js', 'file:///C:/foo.js');
25+
await check('C:\\a\\b\\c\\foo.js', 'file:///C:/a/b/c/foo.js');
26+
await check('a:\\%.js', 'file:///a:/%25.js');
27+
} else {
28+
await check('/foo.js', 'file:///foo.js');
29+
await check('/a/b/c/d/foo.js', 'file:///a/b/c/d/foo.js');
30+
await check('/%%%.js', 'file:///%25%25%25.js');
31+
}
32+
33+
async function check(filename, expected) {
34+
const promise =
35+
new Promise((resolve) => session.once('inspectorNotification', resolve));
36+
new vm.Script('42', { filename }).runInThisContext();
37+
const { params: { url } } = await promise;
38+
strictEqual(url, expected);
39+
}
40+
})();

‎test/sequential/test-inspector.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,15 @@ async function testBreakpointOnStart(session) {
6969
];
7070

7171
await session.send(commands);
72-
await session.waitForBreakOnLine(0, session.scriptPath());
72+
await session.waitForBreakOnLine(0, session.scriptURL());
7373
}
7474

7575
async function testBreakpoint(session) {
7676
console.log('[test]', 'Setting a breakpoint and verifying it is hit');
7777
const commands = [
7878
{ 'method': 'Debugger.setBreakpointByUrl',
7979
'params': { 'lineNumber': 5,
80-
'url': session.scriptPath(),
80+
'url': session.scriptURL(),
8181
'columnNumber': 0,
8282
'condition': ''
8383
}
@@ -92,7 +92,7 @@ async function testBreakpoint(session) {
9292
`Script source is wrong: ${scriptSource}`);
9393

9494
await session.waitForConsoleOutput('log', ['A message', 5]);
95-
const paused = await session.waitForBreakOnLine(5, session.scriptPath());
95+
const paused = await session.waitForBreakOnLine(5, session.scriptURL());
9696
const scopeId = paused.params.callFrames[0].scopeChain[0].object.objectId;
9797

9898
console.log('[test]', 'Verify we can read current application state');

0 commit comments

Comments
 (0)