Skip to content

Commit

Permalink
inspector: don't bind to 0.0.0.0 by default
Browse files Browse the repository at this point in the history
Change the bind address from 0.0.0.0 to 127.0.0.1 and start respecting
the address part of `--inspect=<address>:<port>` so that the bind
address can be overridden by the user.

Fixes: #21349
PR-URL: #21376
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
bnoordhuis authored and rvagg committed Aug 13, 2018
1 parent 58b9497 commit 08a150f
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 25 deletions.
52 changes: 34 additions & 18 deletions src/inspector_agent.cc
Expand Up @@ -37,20 +37,30 @@ static const uint8_t PROTOCOL_JSON[] = {
#include "v8_inspector_protocol_json.h" // NOLINT(build/include_order)
};

std::string GetWsUrl(int port, const std::string& id) {
std::string GetWsUrl(const sockaddr_in& address, const std::string& id) {
char buf[1024];
snprintf(buf, sizeof(buf), "127.0.0.1:%d/%s", port, id.c_str());
char name[64];

if (uv_ip4_name(&address, name, sizeof(name))) *name = '\0';
const int port = ntohs(address.sin_port);
snprintf(buf, sizeof(buf), "%s:%d/%s", name, port, id.c_str());

return buf;
}

void PrintDebuggerReadyMessage(int port, const std::string& id) {
fprintf(stderr, "Debugger listening on port %d.\n"
void PrintDebuggerReadyMessage(const sockaddr_in& address,
const std::string& id) {
const std::string ws_url = GetWsUrl(address, id);
const size_t slash_pos = ws_url.find('/');
CHECK_NE(slash_pos, std::string::npos);

fprintf(stderr, "Debugger listening on %.*s.\n"
"Warning: This is an experimental feature and could change at any time.\n"
"To start debugging, open the following URL in Chrome:\n"
" chrome-devtools://devtools/remote/serve_file/"
"@" V8_INSPECTOR_REVISION "/inspector.html?"
"experiments=true&v8only=true&ws=%s\n",
port, GetWsUrl(port, id).c_str());
static_cast<int>(slash_pos), ws_url.c_str(), ws_url.c_str());
fflush(stderr);
}

Expand Down Expand Up @@ -187,7 +197,8 @@ class AgentImpl {
~AgentImpl();

// Start the inspector agent thread
bool Start(v8::Platform* platform, const char* path, int port, bool wait);
bool Start(v8::Platform* platform, const char* path,
const char* host, int port, bool wait);
// Stop the inspector agent
void Stop();

Expand Down Expand Up @@ -234,7 +245,7 @@ class AgentImpl {
uv_thread_t thread_;
uv_loop_t child_loop_;

int port_;
struct sockaddr_in saddr_;
bool wait_;
bool shutting_down_;
State state_;
Expand Down Expand Up @@ -380,7 +391,7 @@ class V8NodeInspector : public v8_inspector::V8InspectorClient {
std::unique_ptr<v8_inspector::V8InspectorSession> session_;
};

AgentImpl::AgentImpl(Environment* env) : port_(0),
AgentImpl::AgentImpl(Environment* env) : saddr_(),
wait_(false),
shutting_down_(false),
state_(State::kNew),
Expand Down Expand Up @@ -473,7 +484,10 @@ void InspectorWrapConsoleCall(const v8::FunctionCallbackInfo<v8::Value>& args) {
}

bool AgentImpl::Start(v8::Platform* platform, const char* path,
int port, bool wait) {
const char* address, int port, bool wait) {
if (*address == '\0') address = "127.0.0.1";
if (uv_ip4_addr(address, port, &saddr_)) return false;

auto env = parent_env_;
inspector_ = new V8NodeInspector(this, env, platform);
platform_ = platform;
Expand All @@ -485,7 +499,6 @@ bool AgentImpl::Start(v8::Platform* platform, const char* path,
int err = uv_loop_init(&child_loop_);
CHECK_EQ(err, 0);

port_ = port;
wait_ = wait;

err = uv_thread_create(&thread_, AgentImpl::ThreadCbIO, this);
Expand Down Expand Up @@ -661,7 +674,7 @@ void AgentImpl::SendListResponse(InspectorSocket* socket) {
Escape(&response["url"]);

if (!client_socket_) {
std::string address = GetWsUrl(port_, id_);
std::string address = GetWsUrl(saddr_, id_);

std::ostringstream frontend_url;
frontend_url << "https://chrome-devtools-frontend.appspot.com/serve_file/@";
Expand Down Expand Up @@ -715,7 +728,6 @@ void AgentImpl::WriteCbIO(uv_async_t* async) {
}

void AgentImpl::WorkerRunIO() {
sockaddr_in addr;
uv_tcp_t server;
int err = uv_loop_init(&child_loop_);
CHECK_EQ(err, 0);
Expand All @@ -729,14 +741,18 @@ void AgentImpl::WorkerRunIO() {
uv_fs_req_cleanup(&req);
}
uv_tcp_init(&child_loop_, &server);
uv_ip4_addr("0.0.0.0", port_, &addr);
server.data = this;
err = uv_tcp_bind(&server,
reinterpret_cast<const struct sockaddr*>(&addr), 0);
reinterpret_cast<const struct sockaddr*>(&saddr_), 0);
if (err == 0) {
err = uv_listen(reinterpret_cast<uv_stream_t*>(&server), 1,
OnSocketConnectionIO);
}
if (err == 0) {
int namelen = sizeof(saddr_);
err = uv_tcp_getsockname(&server, reinterpret_cast<sockaddr*>(&saddr_),
&namelen);
}
if (err != 0) {
fprintf(stderr, "Unable to open devtools socket: %s\n", uv_strerror(err));
state_ = State::kError; // Safe, main thread is waiting on semaphore
Expand All @@ -746,7 +762,7 @@ void AgentImpl::WorkerRunIO() {
uv_sem_post(&start_sem_);
return;
}
PrintDebuggerReadyMessage(port_, id_);
PrintDebuggerReadyMessage(saddr_, id_);
if (!wait_) {
uv_sem_post(&start_sem_);
}
Expand Down Expand Up @@ -830,7 +846,7 @@ void AgentImpl::DispatchMessages() {
if (shutting_down_) {
state_ = State::kDone;
} else {
PrintDebuggerReadyMessage(port_, id_);
PrintDebuggerReadyMessage(saddr_, id_);
state_ = State::kAccepting;
}
inspector_->quitMessageLoopOnPause();
Expand Down Expand Up @@ -858,8 +874,8 @@ Agent::~Agent() {
}

bool Agent::Start(v8::Platform* platform, const char* path,
int port, bool wait) {
return impl->Start(platform, path, port, wait);
const char* host, int port, bool wait) {
return impl->Start(platform, path, host, port, wait);
}

void Agent::Stop() {
Expand Down
3 changes: 2 additions & 1 deletion src/inspector_agent.h
Expand Up @@ -29,7 +29,8 @@ class Agent {
~Agent();

// Start the inspector agent thread
bool Start(v8::Platform* platform, const char* path, int port, bool wait);
bool Start(v8::Platform* platform, const char* path,
const char* address, int port, bool wait);
// Stop the inspector agent
void Stop();

Expand Down
12 changes: 7 additions & 5 deletions src/node.cc
Expand Up @@ -232,9 +232,10 @@ static struct {
}

bool StartInspector(Environment *env, const char* script_path,
int port, bool wait) {
const char* host, int port, bool wait) {
#if HAVE_INSPECTOR
return env->inspector_agent()->Start(platform_, script_path, port, wait);
return env->inspector_agent()->Start(platform_, script_path,
host, port, wait);
#else
return true;
#endif // HAVE_INSPECTOR
Expand All @@ -246,7 +247,7 @@ static struct {
void PumpMessageLoop(Isolate* isolate) {}
void Dispose() {}
bool StartInspector(Environment *env, const char* script_path,
int port, bool wait) {
const char* host, int port, bool wait) {
env->ThrowError("Node compiled with NODE_USE_V8_PLATFORM=0");
return false; // make compiler happy
}
Expand Down Expand Up @@ -4092,8 +4093,9 @@ static void DispatchMessagesDebugAgentCallback(Environment* env) {
static void StartDebug(Environment* env, const char* path, bool wait) {
CHECK(!debugger_running);
if (use_inspector) {
debugger_running = v8_platform.StartInspector(env, path, inspector_port,
wait);
debugger_running = v8_platform.StartInspector(env, path,
inspector_host.c_str(),
inspector_port, wait);
} else {
env->debugger_agent()->set_dispatch_handler(
DispatchMessagesDebugAgentCallback);
Expand Down
2 changes: 1 addition & 1 deletion test/inspector/inspector-helper.js
Expand Up @@ -446,7 +446,7 @@ exports.startNodeForInspectorTest = function(callback) {
clearTimeout(timeoutId);
console.log('[err]', text);
if (found) return;
const match = text.match(/Debugger listening on port (\d+)/);
const match = text.match(/Debugger listening on 127\.0\.0\.1:(\d+)\./);
found = true;
child.stderr.removeListener('data', dataCallback);
assert.ok(match, text);
Expand Down
44 changes: 44 additions & 0 deletions test/sequential/test-inspector-address.js
@@ -0,0 +1,44 @@
'use strict';
const { PORT, mustCall, skipIfInspectorDisabled } = require('../common');

skipIfInspectorDisabled();

const assert = require('assert');
const { spawn } = require('child_process');

function test(arg, expected, done) {
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', (hadErr) => assert(!hadErr));
proc.stderr.on('close', (hadErr) => assert(!hadErr));
proc.stderr.on('data', () => {
if (!stderr.includes('\n')) return;
assert(/Debugger listening on (.+)\./.test(stderr));
assert.strictEqual(RegExp.$1, expected);
});
proc.on('exit', (exitCode, signalCode) => {
assert.strictEqual(exitCode, 0);
assert.strictEqual(signalCode, null);
done();
});
}

function one() {
test(`--inspect=${PORT}`, `127.0.0.1:${PORT}`, mustCall(two));
}

function two() {
test(`--inspect=0.0.0.0:${PORT}`, `0.0.0.0:${PORT}`, mustCall(three));
}

function three() {
test(`--inspect=127.0.0.1:${PORT}`, `127.0.0.1:${PORT}`, mustCall());
}

one();

0 comments on commit 08a150f

Please sign in to comment.