From 399cb25f6266a17f0e5ed838b37b9c8299a87224 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sat, 27 May 2017 13:31:00 +0200 Subject: [PATCH] inspector: bind to random port with --inspect=0 Allow binding to a randomly assigned port number with `--inspect=0` or `--inspect-brk=0`. PR-URL: https://github.com/nodejs/node/pull/5025 Refs: https://github.com/nodejs/node/issues/4419 Reviewed-By: Colin Ihrig Reviewed-By: Refael Ackermann Reviewed-By: Sam Roberts --- src/inspector_io.cc | 3 +- src/inspector_io.h | 3 ++ src/node.cc | 14 +++++- src/node_debug_options.cc | 5 +- .../test-inspector-port-zero-cluster.js | 32 +++++++++++++ test/inspector/test-inspector-port-zero.js | 46 +++++++++++++++++++ 6 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 test/inspector/test-inspector-port-zero-cluster.js create mode 100644 test/inspector/test-inspector-port-zero.js diff --git a/src/inspector_io.cc b/src/inspector_io.cc index 6dcc1e0fdd3e67..489b1552dfea0a 100644 --- a/src/inspector_io.cc +++ b/src/inspector_io.cc @@ -175,7 +175,7 @@ InspectorIo::InspectorIo(Environment* env, v8::Platform* platform, io_thread_req_(), platform_(platform), dispatching_messages_(false), session_id_(0), script_name_(path), - wait_for_connect_(wait_for_connect) { + wait_for_connect_(wait_for_connect), port_(-1) { main_thread_req_ = new AsyncAndAgent({uv_async_t(), env->inspector_agent()}); CHECK_EQ(0, uv_async_init(env->event_loop(), &main_thread_req_->first, InspectorIo::MainThreadAsyncCb)); @@ -298,6 +298,7 @@ void InspectorIo::WorkerRunIO() { uv_sem_post(&start_sem_); return; } + port_ = server.port(); // Safe, main thread is waiting on semaphore. if (!wait_for_connect_) { uv_sem_post(&start_sem_); } diff --git a/src/inspector_io.h b/src/inspector_io.h index dc7e148252a0e2..b323db45042885 100644 --- a/src/inspector_io.h +++ b/src/inspector_io.h @@ -68,6 +68,8 @@ class InspectorIo { uv_close(reinterpret_cast(&io_thread_req_), nullptr); } + int port() const { return port_; } + private: template using MessageQueue = @@ -129,6 +131,7 @@ class InspectorIo { std::string script_path_; const std::string id_; const bool wait_for_connect_; + int port_; friend class DispatchMessagesTask; friend class IoSessionDelegate; diff --git a/src/node.cc b/src/node.cc index 60fba9ad2fa553..b29df6449f81cb 100644 --- a/src/node.cc +++ b/src/node.cc @@ -40,6 +40,10 @@ #include "node_i18n.h" #endif +#if HAVE_INSPECTOR +#include "inspector_io.h" +#endif + #if defined HAVE_DTRACE || defined HAVE_ETW #include "node_dtrace.h" #endif @@ -3048,7 +3052,15 @@ static Local GetFeatures(Environment* env) { static void DebugPortGetter(Local property, const PropertyCallbackInfo& info) { - info.GetReturnValue().Set(debug_options.port()); + int port = debug_options.port(); +#if HAVE_INSPECTOR + if (port == 0) { + Environment* env = Environment::GetCurrent(info); + if (env->inspector_agent()->IsStarted()) + port = env->inspector_agent()->io()->port(); + } +#endif // HAVE_INSPECTOR + info.GetReturnValue().Set(port); } diff --git a/src/node_debug_options.cc b/src/node_debug_options.cc index 3f1a2e56e89985..d82e92170a9304 100644 --- a/src/node_debug_options.cc +++ b/src/node_debug_options.cc @@ -21,8 +21,9 @@ int parse_and_validate_port(const std::string& port) { char* endptr; errno = 0; const long result = strtol(port.c_str(), &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"); + if (errno != 0 || *endptr != '\0'|| + (result != 0 && result < 1024) || result > 65535) { + fprintf(stderr, "Debug port must be 0 or in range 1024 to 65535.\n"); exit(12); } return static_cast(result); diff --git a/test/inspector/test-inspector-port-zero-cluster.js b/test/inspector/test-inspector-port-zero-cluster.js new file mode 100644 index 00000000000000..fad25d4c844fb0 --- /dev/null +++ b/test/inspector/test-inspector-port-zero-cluster.js @@ -0,0 +1,32 @@ +// Flags: --inspect=0 +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const cluster = require('cluster'); + +if (cluster.isMaster) { + const ports = []; + for (const worker of [cluster.fork(), + cluster.fork(), + cluster.fork()]) { + worker.on('message', common.mustCall((message) => { + ports.push(message.debugPort); + worker.kill(); + })); + worker.send('debugPort'); + } + process.on('exit', () => { + ports.sort(); + assert.strictEqual(ports.length, 3); + assert(ports.every((port) => port > 0)); + assert(ports.every((port) => port < 65536)); + assert.strictEqual(ports[0] + 1, ports[1]); // Ports should be consecutive. + assert.strictEqual(ports[1] + 1, ports[2]); + }); +} else { + process.on('message', (message) => { + if (message === 'debugPort') + process.send({ debugPort: process.debugPort }); + }); +} diff --git a/test/inspector/test-inspector-port-zero.js b/test/inspector/test-inspector-port-zero.js new file mode 100644 index 00000000000000..e66c4392081aa2 --- /dev/null +++ b/test/inspector/test-inspector-port-zero.js @@ -0,0 +1,46 @@ +'use strict'; + +const { mustCall } = require('../common'); +const assert = require('assert'); +const { URL } = require('url'); +const { spawn } = require('child_process'); + +function test(arg) { + const args = [arg, '-p', 'process.debugPort']; + const proc = spawn(process.execPath, args); + proc.stdout.setEncoding('utf8'); + proc.stderr.setEncoding('utf8'); + let stdout = ''; + let stderr = ''; + proc.stdout.on('data', (data) => stdout += data); + proc.stderr.on('data', (data) => stderr += data); + proc.stdout.on('close', assert.ifError); + proc.stderr.on('close', assert.ifError); + let port = ''; + proc.stderr.on('data', () => { + if (!stderr.includes('\n')) return; + assert(/Debugger listening on (.+)/.test(stderr)); + port = new URL(RegExp.$1).port; + assert(+port > 0); + }); + if (/inspect-brk/.test(arg)) { + proc.stderr.on('data', () => { + if (stderr.includes('\n') && !proc.killed) proc.kill(); + }); + } else { + let onclose = () => { + onclose = () => assert.strictEqual(port, stdout.trim()); + }; + proc.stdout.on('close', mustCall(() => onclose())); + proc.stderr.on('close', mustCall(() => onclose())); + proc.on('exit', mustCall((exitCode) => assert.strictEqual(exitCode, 0))); + } +} + +test('--inspect=0'); +test('--inspect=127.0.0.1:0'); +test('--inspect=localhost:0'); + +test('--inspect-brk=0'); +test('--inspect-brk=127.0.0.1:0'); +test('--inspect-brk=localhost:0');