From f66e9abcb36531516df6a3d395a7c9e663c8fba1 Mon Sep 17 00:00:00 2001 From: Alexey Kozyatinskiy Date: Thu, 30 Aug 2018 14:59:34 -0700 Subject: [PATCH] inspector: implemented V8InspectorClient::resourceNameToUrl This method is required by inspector to report normalized urls over the protocol. Backport-PR-URL: https://github.com/nodejs/node/pull/22918 Fixes https://github.com/nodejs/node/issues/22223 PR-URL: https://github.com/nodejs/node/pull/22251 Reviewed-By: Eugene Ostroukhov Reviewed-By: Tiancheng "Timothy" Gu --- src/inspector_agent.cc | 32 +++++++++++++++ test/cctest/test_url.cc | 4 -- .../test-inspector-multisession-js.js | 6 ++- test/parallel/test-v8-coverage.js | 2 +- test/sequential/test-inspector-bindings.js | 6 ++- .../test-inspector-break-when-eval.js | 5 ++- .../test-inspector-debug-brk-flag.js | 2 +- test/sequential/test-inspector-exception.js | 3 +- .../test-inspector-resource-name-to-url.js | 40 +++++++++++++++++++ test/sequential/test-inspector.js | 6 +-- 10 files changed, 90 insertions(+), 16 deletions(-) create mode 100644 test/sequential/test-inspector-resource-name-to-url.js diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index c8bface6bf5372..63b92663532e06 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -6,6 +6,7 @@ #include "inspector/tracing_agent.h" #include "node/inspector/protocol/Protocol.h" #include "node_internals.h" +#include "node_url.h" #include "v8-inspector.h" #include "v8-platform.h" @@ -375,6 +376,25 @@ void NotifyClusterWorkersDebugEnabled(Environment* env) { MakeCallback(env->isolate(), process_object, emit_fn.As(), arraysize(argv), argv, {0, 0}); } + +#ifdef _WIN32 +bool IsFilePath(const std::string& path) { + // '\\' + if (path.length() > 2 && path[0] == '\\' && path[1] == '\\') + return true; + // '[A-Z]:[/\\]' + if (path.length() < 3) + return false; + if ((path[0] >= 'A' && path[0] <= 'Z') || (path[0] >= 'a' && path[0] <= 'z')) + return path[1] == ':' && (path[2] == '/' || path[2] == '\\'); + return false; +} +#else +bool IsFilePath(const std::string& path) { + return path.length() && path[0] == '/'; +} +#endif // __POSIX__ + } // namespace class NodeInspectorClient : public V8InspectorClient { @@ -601,6 +621,18 @@ class NodeInspectorClient : public V8InspectorClient { return env_->isolate_data()->platform()->CurrentClockTimeMillis(); } + std::unique_ptr resourceNameToUrl( + const StringView& resource_name_view) override { + std::string resource_name = + protocol::StringUtil::StringViewToUtf8(resource_name_view); + if (!IsFilePath(resource_name)) + return nullptr; + node::url::URL url = node::url::URL::FromFilePath(resource_name); + // TODO(ak239spb): replace this code with url.href(). + // Refs: https://github.com/nodejs/node/issues/22610 + return Utf8ToStringView(url.protocol() + "//" + url.path()); + } + node::Environment* env_; bool is_main_; bool running_nested_loop_ = false; diff --git a/test/cctest/test_url.cc b/test/cctest/test_url.cc index 2e9b06e3a4783a..ddef534b57485f 100644 --- a/test/cctest/test_url.cc +++ b/test/cctest/test_url.cc @@ -124,10 +124,6 @@ TEST_F(URLTest, FromFilePath) { file_url = URL::FromFilePath("b:\\a\\%%.js"); EXPECT_EQ("file:", file_url.protocol()); EXPECT_EQ("/b:/a/%25%25.js", file_url.path()); - - file_url = URL::FromFilePath("\\\\host\\a\\b\\c"); - EXPECT_EQ("file:", file_url.protocol()); - EXPECT_EQ("host/a/b/c", file_url.path()); #else file_url = URL::FromFilePath("/"); EXPECT_EQ("file:", file_url.protocol()); diff --git a/test/parallel/test-inspector-multisession-js.js b/test/parallel/test-inspector-multisession-js.js index 097b77e2c24231..92879d3ff3a7df 100644 --- a/test/parallel/test-inspector-multisession-js.js +++ b/test/parallel/test-inspector-multisession-js.js @@ -1,3 +1,4 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); @@ -6,6 +7,7 @@ common.skipIfInspectorDisabled(); const assert = require('assert'); const { Session } = require('inspector'); const path = require('path'); +const { pathToFileURL } = require('url'); function debugged() { return 42; @@ -32,8 +34,8 @@ async function test() { await new Promise((resolve, reject) => { session1.post('Debugger.setBreakpointByUrl', { - 'lineNumber': 9, - 'url': path.resolve(__dirname, __filename), + 'lineNumber': 12, + 'url': pathToFileURL(path.resolve(__dirname, __filename)).toString(), 'columnNumber': 0, 'condition': '' }, (error, result) => { diff --git a/test/parallel/test-v8-coverage.js b/test/parallel/test-v8-coverage.js index ad5100b42ea8d9..91581a5a073460 100644 --- a/test/parallel/test-v8-coverage.js +++ b/test/parallel/test-v8-coverage.js @@ -97,7 +97,7 @@ function getFixtureCoverage(fixtureFile, coverageDirectory) { for (const coverageFile of coverageFiles) { const coverage = require(path.join(coverageDirectory, coverageFile)); for (const fixtureCoverage of coverage.result) { - if (fixtureCoverage.url.indexOf(`${path.sep}${fixtureFile}`) !== -1) { + if (fixtureCoverage.url.indexOf(`/${fixtureFile}`) !== -1) { return fixtureCoverage; } } diff --git a/test/sequential/test-inspector-bindings.js b/test/sequential/test-inspector-bindings.js index bea8b552202087..f5583c1d7bc6d6 100644 --- a/test/sequential/test-inspector-bindings.js +++ b/test/sequential/test-inspector-bindings.js @@ -1,9 +1,11 @@ +// Flags: --expose-internals 'use strict'; const common = require('../common'); common.skipIfInspectorDisabled(); const assert = require('assert'); const inspector = require('inspector'); const path = require('path'); +const { pathToFileURL } = require('url'); // This test case will set a breakpoint 4 lines below function debuggedFunction() { @@ -84,8 +86,8 @@ function testSampleDebugSession() { }, TypeError); session.post('Debugger.enable', () => cbAsSecondArgCalled = true); session.post('Debugger.setBreakpointByUrl', { - 'lineNumber': 12, - 'url': path.resolve(__dirname, __filename), + 'lineNumber': 14, + 'url': pathToFileURL(path.resolve(__dirname, __filename)).toString(), 'columnNumber': 0, 'condition': '' }); diff --git a/test/sequential/test-inspector-break-when-eval.js b/test/sequential/test-inspector-break-when-eval.js index 8be5285221d513..e7529f786a9859 100644 --- a/test/sequential/test-inspector-break-when-eval.js +++ b/test/sequential/test-inspector-break-when-eval.js @@ -5,6 +5,7 @@ common.skipIfInspectorDisabled(); const assert = require('assert'); const { NodeInstance } = require('../common/inspector-helper.js'); const fixtures = require('../common/fixtures'); +const { pathToFileURL } = require('url'); const script = fixtures.path('inspector-global-function.js'); @@ -26,7 +27,7 @@ async function breakOnLine(session) { const commands = [ { 'method': 'Debugger.setBreakpointByUrl', 'params': { 'lineNumber': 9, - 'url': script, + 'url': pathToFileURL(script).toString(), 'columnNumber': 0, 'condition': '' } @@ -45,7 +46,7 @@ async function breakOnLine(session) { } ]; session.send(commands); - await session.waitForBreakOnLine(9, script); + await session.waitForBreakOnLine(9, pathToFileURL(script).toString()); } async function stepOverConsoleStatement(session) { diff --git a/test/sequential/test-inspector-debug-brk-flag.js b/test/sequential/test-inspector-debug-brk-flag.js index 0e5df52560d2b1..d488eea2626584 100644 --- a/test/sequential/test-inspector-debug-brk-flag.js +++ b/test/sequential/test-inspector-debug-brk-flag.js @@ -24,7 +24,7 @@ async function testBreakpointOnStart(session) { ]; session.send(commands); - await session.waitForBreakOnLine(0, session.scriptPath()); + await session.waitForBreakOnLine(0, session.scriptURL()); } async function runTests() { diff --git a/test/sequential/test-inspector-exception.js b/test/sequential/test-inspector-exception.js index ef67e1d9a57264..4a41e8826a50d1 100644 --- a/test/sequential/test-inspector-exception.js +++ b/test/sequential/test-inspector-exception.js @@ -7,6 +7,7 @@ common.skipIfInspectorDisabled(); const assert = require('assert'); const { NodeInstance } = require('../common/inspector-helper.js'); +const { pathToFileURL } = require('url'); const script = fixtures.path('throws_error.js'); @@ -29,7 +30,7 @@ async function testBreakpointOnStart(session) { ]; await session.send(commands); - await session.waitForBreakOnLine(0, script); + await session.waitForBreakOnLine(0, pathToFileURL(script).toString()); } diff --git a/test/sequential/test-inspector-resource-name-to-url.js b/test/sequential/test-inspector-resource-name-to-url.js new file mode 100644 index 00000000000000..41a98ba219b24c --- /dev/null +++ b/test/sequential/test-inspector-resource-name-to-url.js @@ -0,0 +1,40 @@ +'use strict'; +const common = require('../common'); + +common.skipIfInspectorDisabled(); + +(async function test() { + const { strictEqual } = require('assert'); + const { Session } = require('inspector'); + const { promisify } = require('util'); + const vm = require('vm'); + const session = new Session(); + session.connect(); + session.post = promisify(session.post); + await session.post('Debugger.enable'); + await check('http://example.com', 'http://example.com'); + await check(undefined, 'evalmachine.'); + await check('file:///foo.js', 'file:///foo.js'); + await check('file:///foo.js', 'file:///foo.js'); + await check('foo.js', 'foo.js'); + await check('[eval]', '[eval]'); + await check('%.js', '%.js'); + + if (common.isWindows) { + await check('C:\\foo.js', 'file:///C:/foo.js'); + await check('C:\\a\\b\\c\\foo.js', 'file:///C:/a/b/c/foo.js'); + await check('a:\\%.js', 'file:///a:/%25.js'); + } else { + await check('/foo.js', 'file:///foo.js'); + await check('/a/b/c/d/foo.js', 'file:///a/b/c/d/foo.js'); + await check('/%%%.js', 'file:///%25%25%25.js'); + } + + async function check(filename, expected) { + const promise = + new Promise((resolve) => session.once('inspectorNotification', resolve)); + new vm.Script('42', { filename }).runInThisContext(); + const { params: { url } } = await promise; + strictEqual(url, expected); + } +})(); diff --git a/test/sequential/test-inspector.js b/test/sequential/test-inspector.js index d641015a3c9954..2a7a2ef4bf34ba 100644 --- a/test/sequential/test-inspector.js +++ b/test/sequential/test-inspector.js @@ -68,7 +68,7 @@ async function testBreakpointOnStart(session) { ]; await session.send(commands); - await session.waitForBreakOnLine(0, session.scriptPath()); + await session.waitForBreakOnLine(0, session.scriptURL()); } async function testBreakpoint(session) { @@ -76,7 +76,7 @@ async function testBreakpoint(session) { const commands = [ { 'method': 'Debugger.setBreakpointByUrl', 'params': { 'lineNumber': 5, - 'url': session.scriptPath(), + 'url': session.scriptURL(), 'columnNumber': 0, 'condition': '' } @@ -91,7 +91,7 @@ async function testBreakpoint(session) { `Script source is wrong: ${scriptSource}`); await session.waitForConsoleOutput('log', ['A message', 5]); - const paused = await session.waitForBreakOnLine(5, session.scriptPath()); + const paused = await session.waitForBreakOnLine(5, session.scriptURL()); const scopeId = paused.params.callFrames[0].scopeChain[0].object.objectId; console.log('[test]', 'Verify we can read current application state');