inspector: don't bind to 0.0.0.0 by default

bnoordhuis · rvagg · commit 08a150fccad8 · 2018-08-13T21:01:13.000+10:00
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>
diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
@@ -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);
 }
 
@@ -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();
 
@@ -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_;
@@ -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),
@@ -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;
@@ -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);
@@ -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/@";
@@ -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);
@@ -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
@@ -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_);
   }
@@ -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();
@@ -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() {
diff --git a/src/inspector_agent.h b/src/inspector_agent.h
@@ -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();
 
diff --git a/src/node.cc b/src/node.cc
@@ -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
@@ -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
   }
@@ -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);
diff --git a/test/inspector/inspector-helper.js b/test/inspector/inspector-helper.js
@@ -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);
diff --git a/test/sequential/test-inspector-address.js b/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();
