diff --git a/lib/_debug_agent.js b/lib/_debug_agent.js index cb4684f0eddfac..03d3c1c472ec09 100644 --- a/lib/_debug_agent.js +++ b/lib/_debug_agent.js @@ -18,9 +18,10 @@ exports.start = function start() { process._rawDebug(err.stack || err); }); - agent.listen(process._debugAPI.port, function() { - var addr = this.address(); - process._rawDebug('Debugger listening on port %d', addr.port); + agent.listen(process._debugAPI.port, process._debugAPI.host, function() { + const addr = this.address(); + const host = net.isIPv6(addr.address) ? `[${addr.address}]` : addr.address; + process._rawDebug('Debugger listening on %s:%d', host, addr.port); process._debugAPI.notifyListen(); }); diff --git a/src/debug-agent.cc b/src/debug-agent.cc index a67ce96811e050..ec6ebb7ad634f6 100644 --- a/src/debug-agent.cc +++ b/src/debug-agent.cc @@ -44,6 +44,7 @@ using v8::Integer; using v8::Isolate; using v8::Local; using v8::Locker; +using v8::NewStringType; using v8::Object; using v8::String; using v8::Value; @@ -69,7 +70,7 @@ Agent::~Agent() { } -bool Agent::Start(int port, bool wait) { +bool Agent::Start(const std::string& host, int port, bool wait) { int err; if (state_ == kRunning) @@ -85,6 +86,7 @@ bool Agent::Start(int port, bool wait) { goto async_init_failed; uv_unref(reinterpret_cast(&child_signal_)); + host_ = host; port_ = port; wait_ = wait; @@ -213,6 +215,10 @@ void Agent::InitAdaptor(Environment* env) { t->GetFunction()->NewInstance(env->context()).ToLocalChecked(); api->SetAlignedPointerInInternalField(0, this); + api->Set(String::NewFromUtf8(isolate, "host", + NewStringType::kNormal).ToLocalChecked(), + String::NewFromUtf8(isolate, host_.data(), NewStringType::kNormal, + host_.size()).ToLocalChecked()); api->Set(String::NewFromUtf8(isolate, "port"), Integer::New(isolate, port_)); env->process_object()->Set(String::NewFromUtf8(isolate, "_debugAPI"), api); diff --git a/src/debug-agent.h b/src/debug-agent.h index 45aa07bf2436b7..504212653bc635 100644 --- a/src/debug-agent.h +++ b/src/debug-agent.h @@ -32,6 +32,7 @@ #include "v8-debug.h" #include +#include // Forward declaration to break recursive dependency chain with src/env.h. namespace node { @@ -75,7 +76,7 @@ class Agent { typedef void (*DispatchHandler)(node::Environment* env); // Start the debugger agent thread - bool Start(int port, bool wait); + bool Start(const std::string& host, int port, bool wait); // Listen for debug events void Enable(); // Stop the debugger agent @@ -114,6 +115,7 @@ class Agent { State state_; + std::string host_; int port_; bool wait_; diff --git a/src/node.cc b/src/node.cc index e984a3b7b209ee..0c261bd56cb154 100644 --- a/src/node.cc +++ b/src/node.cc @@ -58,6 +58,8 @@ #include #include #include + +#include #include #if defined(NODE_HAVE_I18N_SUPPORT) @@ -141,10 +143,14 @@ static unsigned int preload_module_count = 0; static const char** preload_modules = nullptr; #if HAVE_INSPECTOR static bool use_inspector = false; +#else +static const bool use_inspector = false; #endif static bool use_debug_agent = false; static bool debug_wait_connect = false; +static std::string debug_host; // NOLINT(runtime/string) static int debug_port = 5858; +static std::string inspector_host; // NOLINT(runtime/string) static int inspector_port = 9229; static const int v8_default_thread_pool_size = 4; static int v8_thread_pool_size = v8_default_thread_pool_size; @@ -203,23 +209,20 @@ static struct { platform_ = nullptr; } -#if HAVE_INSPECTOR void StartInspector(Environment *env, int port, bool wait) { +#if HAVE_INSPECTOR env->inspector_agent()->Start(platform_, port, wait); - } #endif // HAVE_INSPECTOR + } v8::Platform* platform_; #else // !NODE_USE_V8_PLATFORM void Initialize(int thread_pool_size) {} void PumpMessageLoop(Isolate* isolate) {} void Dispose() {} -#if HAVE_INSPECTOR void StartInspector(Environment *env, int port, bool wait) { env->ThrowError("Node compiled with NODE_USE_V8_PLATFORM=0"); } -#endif // HAVE_INSPECTOR - #endif // !NODE_USE_V8_PLATFORM } v8_platform; @@ -3514,6 +3517,7 @@ static bool ParseDebugOpt(const char* arg) { debug_wait_connect = true; port = arg + sizeof("--debug-brk=") - 1; } else if (!strncmp(arg, "--debug-port=", sizeof("--debug-port=") - 1)) { + // XXX(bnoordhuis) Misnomer, configures port and listen address. port = arg + sizeof("--debug-port=") - 1; #if HAVE_INSPECTOR // Specifying both --inspect and --debug means debugging is on, using Chromium @@ -3535,24 +3539,49 @@ static bool ParseDebugOpt(const char* arg) { return false; } - if (port != nullptr) { - int port_int = atoi(port); - if (port_int < 1024 || port_int > 65535) { - fprintf(stderr, "Debug port must be in range 1024 to 65535.\n"); - PrintHelp(); - exit(12); - } -#if HAVE_INSPECTOR - if (use_inspector) { - inspector_port = port_int; - } else { -#endif - debug_port = port_int; -#if HAVE_INSPECTOR + if (port == nullptr) { + return true; + } + + std::string* const the_host = use_inspector ? &inspector_host : &debug_host; + int* const the_port = use_inspector ? &inspector_port : &debug_port; + + // FIXME(bnoordhuis) Move IPv6 address parsing logic to lib/net.js. + // It seems reasonable to support [address]:port notation + // in net.Server#listen() and net.Socket#connect(). + const size_t port_len = strlen(port); + if (port[0] == '[' && port[port_len - 1] == ']') { + the_host->assign(port + 1, port_len - 2); + return true; + } + + const char* const colon = strrchr(port, ':'); + if (colon == nullptr) { + // Either a port number or a host name. Assume that + // if it's not all decimal digits, it's a host name. + for (size_t n = 0; port[n] != '\0'; n += 1) { + if (port[n] < '0' || port[n] > '9') { + *the_host = port; + return true; + } } -#endif + } else { + const bool skip = (colon > port && port[0] == '[' && colon[-1] == ']'); + the_host->assign(port + skip, colon - skip); + } + + char* endptr; + errno = 0; + const char* const digits = colon != nullptr ? colon + 1 : port; + const long result = strtol(digits, &endptr, 10); // NOLINT(runtime/int) + if (errno != 0 || *endptr != '\0' || result < 1024 || result > 65535) { + fprintf(stderr, "Debug port must be in range 1024 to 65535.\n"); + PrintHelp(); + exit(12); } + *the_port = static_cast(result); + return true; } @@ -3810,34 +3839,31 @@ static void DispatchMessagesDebugAgentCallback(Environment* env) { static void StartDebug(Environment* env, bool wait) { CHECK(!debugger_running); -#if HAVE_INSPECTOR if (use_inspector) { v8_platform.StartInspector(env, inspector_port, wait); debugger_running = true; } else { -#endif env->debugger_agent()->set_dispatch_handler( DispatchMessagesDebugAgentCallback); - debugger_running = env->debugger_agent()->Start(debug_port, wait); + debugger_running = + env->debugger_agent()->Start(debug_host, debug_port, wait); if (debugger_running == false) { - fprintf(stderr, "Starting debugger on port %d failed\n", debug_port); + fprintf(stderr, "Starting debugger on %s:%d failed\n", + debug_host.c_str(), debug_port); fflush(stderr); return; } -#if HAVE_INSPECTOR } -#endif } // Called from the main thread. static void EnableDebug(Environment* env) { CHECK(debugger_running); -#if HAVE_INSPECTOR + if (use_inspector) { return; } -#endif // Send message to enable debug in workers HandleScope handle_scope(env->isolate()); diff --git a/test/parallel/test-cluster-disconnect-handles.js b/test/parallel/test-cluster-disconnect-handles.js index e05733dbd9e953..49b8218ab9c8d3 100644 --- a/test/parallel/test-cluster-disconnect-handles.js +++ b/test/parallel/test-cluster-disconnect-handles.js @@ -25,11 +25,8 @@ cluster.schedulingPolicy = cluster.SCHED_RR; if (cluster.isMaster) { let isKilling = false; const handles = require('internal/cluster').handles; - // FIXME(bnoordhuis) lib/cluster.js scans the execArgv arguments for - // debugger flags and renumbers any port numbers it sees starting - // from the default port 5858. Add a '.' that circumvents the - // scanner but is ignored by atoi(3). Heinous hack. - cluster.setupMaster({ execArgv: [`--debug=${common.PORT}.`] }); + const address = common.hasIPv6 ? '[::1]' : common.localhostIPv4; + cluster.setupMaster({ execArgv: [`--debug=${address}:${common.PORT}`] }); const worker = cluster.fork(); worker.once('exit', common.mustCall((code, signal) => { assert.strictEqual(code, 0, 'worker did not exit normally'); diff --git a/test/parallel/test-debug-port-cluster.js b/test/parallel/test-debug-port-cluster.js index cc564b3ac1522c..3410aed2b67d27 100644 --- a/test/parallel/test-debug-port-cluster.js +++ b/test/parallel/test-debug-port-cluster.js @@ -16,7 +16,8 @@ child.stderr.setEncoding('utf8'); const checkMessages = common.mustCall(() => { for (let port = PORT_MIN; port <= PORT_MAX; port += 1) { - assert(stderr.includes(`Debugger listening on port ${port}`)); + const re = RegExp(`Debugger listening on (\\[::\\]|0\\.0\\.0\\.0):${port}`); + assert(re.test(stderr)); } }); diff --git a/test/parallel/test-debug-port-from-cmdline.js b/test/parallel/test-debug-port-from-cmdline.js index 71ed71bd63af65..527d72ee74a75f 100644 --- a/test/parallel/test-debug-port-from-cmdline.js +++ b/test/parallel/test-debug-port-from-cmdline.js @@ -39,11 +39,10 @@ function processStderrLine(line) { function assertOutputLines() { var expectedLines = [ 'Starting debugger agent.', - 'Debugger listening on port ' + debugPort + 'Debugger listening on (\\[::\\]|0\\.0\\.0\\.0):' + debugPort, ]; assert.equal(outputLines.length, expectedLines.length); for (var i = 0; i < expectedLines.length; i++) - assert.equal(outputLines[i], expectedLines[i]); - + assert(RegExp(expectedLines[i]).test(outputLines[i])); } diff --git a/test/parallel/test-debug-port-numbers.js b/test/parallel/test-debug-port-numbers.js index 33cdf6035b449a..683287340c6f8d 100644 --- a/test/parallel/test-debug-port-numbers.js +++ b/test/parallel/test-debug-port-numbers.js @@ -52,8 +52,9 @@ function kill(child) { process.on('exit', function() { for (const child of children) { - const one = RegExp(`Debugger listening on port ${child.test.port}`); - const two = RegExp(`connecting to 127.0.0.1:${child.test.port}`); + const port = child.test.port; + const one = RegExp(`Debugger listening on (\\[::\\]|0\.0\.0\.0):${port}`); + const two = RegExp(`connecting to 127.0.0.1:${port}`); assert(one.test(child.test.stdout)); assert(two.test(child.test.stdout)); } diff --git a/test/parallel/test-debug-signal-cluster.js b/test/parallel/test-debug-signal-cluster.js index e51cd9a50ab29f..c78bab9d489212 100644 --- a/test/parallel/test-debug-signal-cluster.js +++ b/test/parallel/test-debug-signal-cluster.js @@ -63,11 +63,11 @@ process.on('exit', function onExit() { var expectedLines = [ 'Starting debugger agent.', - 'Debugger listening on port ' + (port + 0), + 'Debugger listening on (\\[::\\]|0\\.0\\.0\\.0):' + (port + 0), 'Starting debugger agent.', - 'Debugger listening on port ' + (port + 1), + 'Debugger listening on (\\[::\\]|0\\.0\\.0\\.0):' + (port + 1), 'Starting debugger agent.', - 'Debugger listening on port ' + (port + 2), + 'Debugger listening on (\\[::\\]|0\\.0\\.0\\.0):' + (port + 2), ]; function assertOutputLines() { @@ -79,5 +79,5 @@ function assertOutputLines() { assert.equal(outputLines.length, expectedLines.length); for (var i = 0; i < expectedLines.length; i++) - assert.equal(outputLines[i], expectedLines[i]); + assert(RegExp(expectedLines[i]).test(outputLines[i])); } diff --git a/test/sequential/test-debug-host-port.js b/test/sequential/test-debug-host-port.js new file mode 100644 index 00000000000000..be6a0837f920b8 --- /dev/null +++ b/test/sequential/test-debug-host-port.js @@ -0,0 +1,47 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const spawn = require('child_process').spawn; + +let run = () => {}; +function test(args, re) { + const next = run; + run = () => { + const options = {encoding: 'utf8'}; + const proc = spawn(process.execPath, args.concat(['-e', '0']), options); + let stderr = ''; + proc.stderr.setEncoding('utf8'); + proc.stderr.on('data', (data) => { + stderr += data; + if (re.test(stderr)) proc.kill(); + }); + proc.on('exit', common.mustCall(() => { + assert(re.test(stderr)); + next(); + })); + }; +} + +test(['--debug-brk'], /Debugger listening on (\[::\]|0\.0\.0\.0):5858/); +test(['--debug-brk=1234'], /Debugger listening on (\[::\]|0\.0\.0\.0):1234/); +test(['--debug-brk=127.0.0.1'], /Debugger listening on 127\.0\.0\.1:5858/); +test(['--debug-brk=127.0.0.1:1234'], /Debugger listening on 127\.0\.0\.1:1234/); +test(['--debug-brk=localhost'], + /Debugger listening on (\[::\]|127\.0\.0\.1):5858/); +test(['--debug-brk=localhost:1234'], + /Debugger listening on (\[::\]|127\.0\.0\.1):1234/); + +if (common.hasIPv6) { + test(['--debug-brk=::'], /Debug port must be in range 1024 to 65535/); + test(['--debug-brk=::0'], /Debug port must be in range 1024 to 65535/); + test(['--debug-brk=::1'], /Debug port must be in range 1024 to 65535/); + test(['--debug-brk=[::]'], /Debugger listening on \[::\]:5858/); + test(['--debug-brk=[::0]'], /Debugger listening on \[::\]:5858/); + test(['--debug-brk=[::]:1234'], /Debugger listening on \[::\]:1234/); + test(['--debug-brk=[::0]:1234'], /Debugger listening on \[::\]:1234/); + test(['--debug-brk=[::ffff:127.0.0.1]:1234'], + /Debugger listening on \[::ffff:127\.0\.0\.1\]:1234/); +} + +run(); // Runs tests in reverse order.