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 <[email protected]> Reviewed-By: Trevor Norris <[email protected]>

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;
 
@@ -211,6 +213,10 @@ void Agent::InitAdaptor(Environment* env){
  Local<Object> api = t->GetFunction()->NewInstance();
  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

@@ -30,6 +30,7 @@
 #include"v8-debug.h"
 
 #include<string.h>
+#include<string>
 
 // Forward declaration to break recursive dependency chain with src/env.h.
 namespacenode{
@@ -73,7 +74,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
@@ -112,6 +113,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)
@@ -139,6 +141,7 @@ static unsigned int preload_module_count = 0;
 staticconstchar** preload_modules = nullptr;
 staticbool use_debug_agent = false;
 staticbool debug_wait_connect = false;
+static std::string debug_host; // NOLINT(runtime/string)
 staticint debug_port = 5858;
 staticbool prof_process = false;
 staticbool v8_is_profiling = false;
@@ -3288,20 +3291,55 @@ 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;
  } else{
 returnfalse;
  }
 
-if (port != nullptr){
- debug_port = atoi(port);
-if (debug_port < 1024 || debug_port > 65535){
-fprintf(stderr, "Debug port must be in range 1024 to 65535.\n");
-PrintHelp();
-exit(12);
+if (port == nullptr){
+returntrue;
+ }
+
+ std::string* const the_host = &debug_host;
+int* const the_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;
+ }
  }
+ } 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;
 }
 
@@ -3539,9 +3577,11 @@ static void StartDebug(Environment* env, bool wait){
 
  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;
  }

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

@@ -65,11 +65,11 @@ process.on('exit', function onExit(){
 
 constexpectedLines=[
 '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,7 @@ function assertOutputLines(){
 outputLines.sort();
 expectedLines.sort();
 
-assert.deepStrictEqual(outputLines,expectedLines);
+assert.equal(outputLines.length,expectedLines.length);
+for(vari=0;i<expectedLines.length;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.