Skip to content

Commit

Permalink
debugger: make listen address configurable
Browse files Browse the repository at this point in the history
`--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: nodejs#3306
PR-URL: nodejs#3316
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information
bnoordhuis authored and Qard committed Nov 19, 2016
1 parent 475fe96 commit 868898a
Show file tree
Hide file tree
Showing 10 changed files with 124 additions and 28 deletions.
7 changes: 4 additions & 3 deletions lib/_debug_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});

Expand Down
8 changes: 7 additions & 1 deletion src/debug-agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -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;

Expand Down Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion src/debug-agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "v8-debug.h"

#include <string.h>
#include <string>

// Forward declaration to break recursive dependency chain with src/env.h.
namespace node {
Expand Down Expand Up @@ -73,7 +74,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
Expand Down Expand Up @@ -112,6 +113,7 @@ class Agent {

State state_;

std::string host_;
int port_;
bool wait_;

Expand Down
56 changes: 48 additions & 8 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>

#include <string>
#include <vector>

#if defined(NODE_HAVE_I18N_SUPPORT)
Expand Down Expand Up @@ -139,6 +141,7 @@ static unsigned int preload_module_count = 0;
static const char** preload_modules = nullptr;
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 bool prof_process = false;
static bool v8_is_profiling = false;
Expand Down Expand Up @@ -3288,20 +3291,55 @@ 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;
} else {
return false;
}

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) {
return true;
}

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().
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;
}
}
} 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<int>(result);

return true;
}

Expand Down Expand Up @@ -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;
}
Expand Down
7 changes: 2 additions & 5 deletions test/parallel/test-cluster-disconnect-handles.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-debug-port-cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
});

Expand Down
5 changes: 2 additions & 3 deletions test/parallel/test-debug-port-from-cmdline.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]));
}
5 changes: 3 additions & 2 deletions test/parallel/test-debug-port-numbers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
10 changes: 6 additions & 4 deletions test/parallel/test-debug-signal-cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ process.on('exit', function onExit() {

const 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() {
Expand All @@ -79,5 +79,7 @@ function assertOutputLines() {
outputLines.sort();
expectedLines.sort();

assert.deepStrictEqual(outputLines, expectedLines);
assert.equal(outputLines.length, expectedLines.length);
for (var i = 0; i < expectedLines.length; i++)
assert(RegExp(expectedLines[i]).test(outputLines[i]));
}
47 changes: 47 additions & 0 deletions test/sequential/test-debug-host-port.js
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit 868898a

Please sign in to comment.