Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inspector: don't bind to 0.0.0.0 by default (v6.x) #21376

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions configure
Expand Up @@ -572,8 +572,8 @@ def try_check_compiler(cc, lang):

values = (proc.communicate()[0].split() + ['0'] * 7)[0:7]
is_clang = values[0] == '1'
gcc_version = '%s.%s.%s' % tuple(values[1:1+3])
clang_version = '%s.%s.%s' % tuple(values[4:4+3])
gcc_version = tuple(values[1:1+3])
clang_version = tuple(values[4:4+3])

return (True, is_clang, clang_version, gcc_version)

Expand Down Expand Up @@ -610,7 +610,7 @@ def get_llvm_version(cc):

def get_xcode_version(cc):
return get_version_helper(
cc, r"(^Apple LLVM version) ([5-9]\.[0-9]+)")
cc, r"(^Apple LLVM version) ([0-9]+\.[0-9]+)")

def get_gas_version(cc):
try:
Expand Down Expand Up @@ -647,13 +647,13 @@ def check_compiler(o):
ok, is_clang, clang_version, gcc_version = try_check_compiler(CXX, 'c++')
if not ok:
warn('failed to autodetect C++ compiler version (CXX=%s)' % CXX)
elif clang_version < '3.4.2' if is_clang else gcc_version < '4.8.0':
elif clang_version < (3, 4, 2) if is_clang else gcc_version < (4, 8, 0):
warn('C++ compiler too old, need g++ 4.8 or clang++ 3.4.2 (CXX=%s)' % CXX)

ok, is_clang, clang_version, gcc_version = try_check_compiler(CC, 'c')
if not ok:
warn('failed to autodetect C compiler version (CC=%s)' % CC)
elif not is_clang and gcc_version < '4.2.0':
elif not is_clang and gcc_version < (4, 2, 0):
# clang 3.2 is a little white lie because any clang version will probably
# do for the C bits. However, we might as well encourage people to upgrade
# to a version that is not completely ancient.
Expand Down
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();
2 changes: 1 addition & 1 deletion tools/gyp/pylib/gyp/xcode_emulation.py
Expand Up @@ -1262,7 +1262,7 @@ def XcodeVersion():
except:
version = CLTVersion()
if version:
version = re.match(r'(\d\.\d\.?\d*)', version).groups()[0]
version = re.match(r'(\d+\.\d+\.?\d*)', version).groups()[0]
else:
raise GypError("No Xcode or CLT version detected!")
# The CLT has no build information, so we return an empty string.
Expand Down