debugger: make listen address configurable

`--debug=1.2.3.4:5678` and `--debug=example.com:5678` are now accepted,
likewise the `--debug-brk` and `--debug-port` switch. The latter is
now something of a misnomer but it's undocumented and for internal use
only so it shouldn't matter too much.
`--inspect=1.2.3.4:5678` and `--inspect=example.com:5678` are also
accepted but don't use the host name yet; they still bind to the
default address.
Fixes: #3306
PR-URL: #3316
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>

Author: bnoordhuis

lib/_debug_agent.js

@@ -18,9 +18,10 @@ exports.start = function start() {
 process._rawDebug(err.stack||err);
 });
 
-agent.listen(process._debugAPI.port,function(){
-varaddr=this.address();
-process._rawDebug('Debugger listening on port %d',addr.port);
+agent.listen(process._debugAPI.port,process._debugAPI.host,function(){
+constaddr=this.address();
+consthost=net.isIPv6(addr.address) ? `[${addr.address}]` : addr.address;
+process._rawDebug('Debugger listening on %s:%d',host,addr.port);
 process._debugAPI.notifyListen();
 });
 

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() {
 }
 
 
-boolAgent::Start(int port, bool wait) {
+boolAgent::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<uv_handle_t*>(&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);

src/debug-agent.h

@@ -32,6 +32,7 @@
 #include"v8-debug.h"
 
 #include<string.h>
+#include<string>
 
 // Forward declaration to break recursive dependency chain with src/env.h.
 namespacenode {
@@ -75,7 +76,7 @@ class Agent {
 typedefvoid (*DispatchHandler)(node::Environment* env);
 
 // Start the debugger agent thread
-boolStart(int port, bool wait);
+boolStart(const std::string& host, int port, bool wait);
 // Listen for debug events
 voidEnable();
 // Stop the debugger agent
@@ -114,6 +115,7 @@ class Agent {
 
  State state_;
 
+ std::string host_;
 int port_;
 bool wait_;
 

src/node.cc

@@ -58,6 +58,8 @@
 #include<stdlib.h>
 #include<string.h>
 #include<sys/types.h>
+
+#include<string>
 #include<vector>
 
 #if defined(NODE_HAVE_I18N_SUPPORT)
@@ -141,10 +143,14 @@ static unsigned int preload_module_count = 0;
 staticconstchar** preload_modules = nullptr;
 #if HAVE_INSPECTOR
 staticbool use_inspector = false;
+#else
+staticconstbool use_inspector = false;
 #endif
 staticbool use_debug_agent = false;
 staticbool debug_wait_connect = false;
+static std::string debug_host; // NOLINT(runtime/string)
 staticint debug_port = 5858;
+static std::string inspector_host; // NOLINT(runtime/string)
 staticint inspector_port = 9229;
 staticconstint v8_default_thread_pool_size = 4;
 staticint v8_thread_pool_size = v8_default_thread_pool_size;
@@ -203,23 +209,20 @@ static struct {
  platform_ = nullptr;
  }
 
-#if HAVE_INSPECTOR
 voidStartInspector(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
 voidInitialize(int thread_pool_size) {}
 voidPumpMessageLoop(Isolate* isolate) {}
 voidDispose() {}
-#if HAVE_INSPECTOR
 voidStartInspector(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;
  } elseif (!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) {
 returnfalse;
  }
 
-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) {
+returntrue;
+ }
+
+ 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().
+constsize_t port_len = strlen(port);
+if (port[0] == '[' && port[port_len - 1] == ']') {
+ the_host->assign(port + 1, port_len - 2);
+returntrue;
+ }
+
+constchar* 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;
+returntrue;
+ }
  }
-#endif
+ } else {
+constbool skip = (colon > port && port[0] == '[' && colon[-1] == ']');
+ the_host->assign(port + skip, colon - skip);
+ }
+
+char* endptr;
+ errno = 0;
+constchar* const digits = colon != nullptr ? colon + 1 : port;
+constlong 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<int>(result);
+
 returntrue;
 }
 
@@ -3810,34 +3839,31 @@ static void DispatchMessagesDebugAgentCallback(Environment* env) {
 
 staticvoidStartDebug(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.
 staticvoidEnableDebug(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());

test/parallel/test-cluster-disconnect-handles.js

@@ -25,11 +25,8 @@ cluster.schedulingPolicy = cluster.SCHED_RR;
 if(cluster.isMaster){
 letisKilling=false;
 consthandles=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}.`]});
+constaddress=common.hasIPv6 ? '[::1]' : common.localhostIPv4;
+cluster.setupMaster({execArgv: [`--debug=${address}:${common.PORT}`]});
 constworker=cluster.fork();
 worker.once('exit',common.mustCall((code,signal)=>{
 assert.strictEqual(code,0,'worker did not exit normally');

test/parallel/test-debug-port-cluster.js

@@ -16,7 +16,8 @@ child.stderr.setEncoding('utf8');
 
 constcheckMessages=common.mustCall(()=>{
 for(letport=PORT_MIN;port<=PORT_MAX;port+=1){
-assert(stderr.includes(`Debugger listening on port ${port}`));
+constre=RegExp(`Debugger listening on (\\[::\\]|0\\.0\\.0\\.0):${port}`);
+assert(re.test(stderr));
 }
 });
 

test/parallel/test-debug-port-from-cmdline.js

@@ -39,11 +39,10 @@ function processStderrLine(line) {
 functionassertOutputLines(){
 varexpectedLines=[
 'Starting debugger agent.',
-'Debugger listening on port '+debugPort
+'Debugger listening on (\\[::\\]|0\\.0\\.0\\.0):'+debugPort,
 ];
 
 assert.equal(outputLines.length,expectedLines.length);
 for(vari=0;i<expectedLines.length;i++)
-assert.equal(outputLines[i],expectedLines[i]);
-
+assert(RegExp(expectedLines[i]).test(outputLines[i]));
 }

test/parallel/test-debug-port-numbers.js

@@ -52,8 +52,9 @@ function kill(child) {
 
 process.on('exit',function(){
 for(constchildofchildren){
-constone=RegExp(`Debugger listening on port ${child.test.port}`);
-consttwo=RegExp(`connecting to 127.0.0.1:${child.test.port}`);
+constport=child.test.port;
+constone=RegExp(`Debugger listening on (\\[::\\]|0\.0\.0\.0):${port}`);
+consttwo=RegExp(`connecting to 127.0.0.1:${port}`);
 assert(one.test(child.test.stdout));
 assert(two.test(child.test.stdout));
 }

test/parallel/test-debug-signal-cluster.js

@@ -63,11 +63,11 @@ process.on('exit', function onExit() {
 
 varexpectedLines=[
 '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),
 ];
 
 functionassertOutputLines(){
@@ -79,5 +79,5 @@ function assertOutputLines() {
 
 assert.equal(outputLines.length,expectedLines.length);
 for(vari=0;i<expectedLines.length;i++)
-assert.equal(outputLines[i],expectedLines[i]);
+assert(RegExp(expectedLines[i]).test(outputLines[i]));
 }

test/sequential/test-debug-host-port.js

@@ -0,0 +1,47 @@
+'use strict';
+
+constcommon=require('../common');
+constassert=require('assert');
+constspawn=require('child_process').spawn;
+
+letrun=()=>{};
+functiontest(args,re){
+constnext=run;
+run=()=>{
+constoptions={encoding: 'utf8'};
+constproc=spawn(process.execPath,args.concat(['-e','0']),options);
+letstderr='';
+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'],/Debuggerlisteningon(\[::\]|0\.0\.0\.0):5858/);
+test(['--debug-brk=1234'],/Debuggerlisteningon(\[::\]|0\.0\.0\.0):1234/);
+test(['--debug-brk=127.0.0.1'],/Debuggerlisteningon127\.0\.0\.1:5858/);
+test(['--debug-brk=127.0.0.1:1234'],/Debuggerlisteningon127\.0\.0\.1:1234/);
+test(['--debug-brk=localhost'],
+/Debuggerlisteningon(\[::\]|127\.0\.0\.1):5858/);
+test(['--debug-brk=localhost:1234'],
+/Debuggerlisteningon(\[::\]|127\.0\.0\.1):1234/);
+
+if(common.hasIPv6){
+test(['--debug-brk=::'],/Debugportmustbeinrange1024to65535/);
+test(['--debug-brk=::0'],/Debugportmustbeinrange1024to65535/);
+test(['--debug-brk=::1'],/Debugportmustbeinrange1024to65535/);
+test(['--debug-brk=[::]'],/Debuggerlisteningon\[::\]:5858/);
+test(['--debug-brk=[::0]'],/Debuggerlisteningon\[::\]:5858/);
+test(['--debug-brk=[::]:1234'],/Debuggerlisteningon\[::\]:1234/);
+test(['--debug-brk=[::0]:1234'],/Debuggerlisteningon\[::\]:1234/);
+test(['--debug-brk=[::ffff:127.0.0.1]:1234'],
+/Debuggerlisteningon\[::ffff:127\.0\.0\.1\]:1234/);
+}
+
+run();// Runs tests in reverse order.