inspector: proper WS URLs when bound to 0.0.0.0

JSON target list response will now return appropriate IP address for instances listening on 0.0.0.0. Backport of #11755 PR-URL: #11850 Refs: #11591 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: James Snell <jasnell@gmail.com>
nodejs · Mar 15, 2017 · 14e3ad0 · 14e3ad0
1 parent 753adee
commit 14e3ad0
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 16 deletions.
diff --git a/src/inspector_socket_server.cc b/src/inspector_socket_server.cc
@@ -139,6 +139,28 @@ void SendProtocolJson(InspectorSocket* socket) {
   SendHttpResponse(socket, data);
 }
 
+int GetSocketHost(uv_tcp_t* socket, std::string* out_host) {
+  char ip[INET6_ADDRSTRLEN];
+  sockaddr_storage addr;
+  int len = sizeof(addr);
+  int err = uv_tcp_getsockname(socket,
+                               reinterpret_cast<struct sockaddr*>(&addr),
+                               &len);
+  if (err != 0)
+    return err;
+  if (addr.ss_family == AF_INET6) {
+    const sockaddr_in6* v6 = reinterpret_cast<const sockaddr_in6*>(&addr);
+    err = uv_ip6_name(v6, ip, sizeof(ip));
+  } else {
+    const sockaddr_in* v4 = reinterpret_cast<const sockaddr_in*>(&addr);
+    err = uv_ip4_name(v4, ip, sizeof(ip));
+  }
+  if (err != 0)
+    return err;
+  *out_host = ip;
+  return err;
+}
+
 int GetPort(uv_tcp_t* socket, int* out_port) {
   sockaddr_storage addr;
   int len = sizeof(addr);
@@ -341,7 +363,9 @@ void InspectorSocketServer::SendListResponse(InspectorSocket* socket) {
       }
     }
     if (!connected) {
-      std::string address = GetWsUrl(host_, port_, id);
+      std::string host;
+      GetSocketHost(&socket->client, &host);
+      std::string address = GetWsUrl(host, port_, id);
       std::ostringstream frontend_url;
       frontend_url << "chrome-devtools://devtools/bundled";
       frontend_url << "/inspector.html?experiments=true&v8only=true&ws=";

diff --git a/test/inspector/inspector-helper.js b/test/inspector/inspector-helper.js
@@ -80,8 +80,8 @@ function tearDown(child, err) {
   }
 }
 
-function checkHttpResponse(port, path, callback) {
-  http.get({port, path}, function(res) {
+function checkHttpResponse(host, port, path, callback, errorcb) {
+  const req = http.get({host, port, path}, function(res) {
     let response = '';
     res.setEncoding('utf8');
     res
@@ -98,6 +98,8 @@ function checkHttpResponse(port, path, callback) {
         callback(err, json);
       });
   });
+  if (errorcb)
+    req.on('error', errorcb);
 }
 
 function makeBufferingDataCallback(dataCallback) {
@@ -295,7 +297,7 @@ TestSession.prototype.disconnect = function(childDone) {
 
 TestSession.prototype.testHttpResponse = function(path, check) {
   return this.enqueue((callback) =>
-      checkHttpResponse(this.harness_.port, path, (err, response) => {
+      checkHttpResponse(null, this.harness_.port, path, (err, response) => {
         check.call(this, err, response);
         callback();
       }));
@@ -361,12 +363,17 @@ Harness.prototype.enqueue_ = function(task) {
   return this;
 };
 
-Harness.prototype.testHttpResponse = function(path, check) {
+Harness.prototype.testHttpResponse = function(host, path, check, errorcb) {
   return this.enqueue_((doneCallback) => {
-    checkHttpResponse(this.port, path, (err, response) => {
-      check.call(this, err, response);
-      doneCallback();
-    });
+    function wrap(callback) {
+      if (callback) {
+        return function() {
+          callback(...arguments);
+          doneCallback();
+        };
+      }
+    }
+    checkHttpResponse(host, this.port, path, wrap(check), wrap(errorcb));
   });
 };
 
@@ -404,7 +411,7 @@ Harness.prototype.wsHandshake = function(devtoolsUrl, tests, readyCallback) {
 
 Harness.prototype.runFrontendSession = function(tests) {
   return this.enqueue_((callback) => {
-    checkHttpResponse(this.port, '/json/list', (err, response) => {
+    checkHttpResponse(null, this.port, '/json/list', (err, response) => {
       assert.ifError(err);
       this.wsHandshake(response[0]['webSocketDebuggerUrl'], tests, callback);
     });

diff --git a/test/inspector/test-inspector-ip-detection.js b/test/inspector/test-inspector-ip-detection.js
@@ -0,0 +1,52 @@
+'use strict';
+const common = require('../common');
+common.skipIfInspectorDisabled && common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const helper = require('./inspector-helper.js');
+const os = require('os');
+
+const ip = pickIPv4Address();
+
+if (!ip) {
+  common.skip('No IP address found');
+  return;
+}
+
+function checkListResponse(instance, err, response) {
+  assert.ifError(err);
+  const res = response[0];
+  const wsUrl = res['webSocketDebuggerUrl'];
+  assert.ok(wsUrl);
+  const match = wsUrl.match(/^ws:\/\/(.*):9229\/(.*)/);
+  assert.strictEqual(ip, match[1]);
+  assert.strictEqual(res['id'], match[2]);
+  assert.strictEqual(ip, res['devtoolsFrontendUrl'].match(/.*ws=(.*):9229/)[1]);
+  instance.childInstanceDone = true;
+}
+
+function checkError(instance, error) {
+  // Some OSes will not allow us to connect
+  if (error.code === 'EHOSTUNREACH') {
+    common.skip('Unable to connect to self');
+  } else {
+    throw error;
+  }
+  instance.childInstanceDone = true;
+}
+
+function runTests(instance) {
+  instance
+    .testHttpResponse(ip, '/json/list', checkListResponse.bind(null, instance),
+                      checkError.bind(null, instance))
+    .kill();
+}
+
+function pickIPv4Address() {
+  for (const i of [].concat(...Object.values(os.networkInterfaces()))) {
+    if (i.family === 'IPv4' && i.address !== '127.0.0.1')
+      return i.address;
+  }
+}
+
+helper.startNodeForInspectorTest(runTests, '--inspect-brk=0.0.0.0');
diff --git a/test/inspector/test-inspector.js b/test/inspector/test-inspector.js
@@ -208,12 +208,12 @@ function testWaitsForFrontendDisconnect(session, harness) {
 
 function runTests(harness) {
   harness
-    .testHttpResponse('/json', checkListResponse)
-    .testHttpResponse('/json/list', checkListResponse)
-    .testHttpResponse('/json/version', checkVersion)
-    .testHttpResponse('/json/activate', checkBadPath)
-    .testHttpResponse('/json/activate/boom', checkBadPath)
-    .testHttpResponse('/json/badpath', checkBadPath)
+    .testHttpResponse(null, '/json', checkListResponse)
+    .testHttpResponse(null, '/json/list', checkListResponse)
+    .testHttpResponse(null, '/json/version', checkVersion)
+    .testHttpResponse(null, '/json/activate', checkBadPath)
+    .testHttpResponse(null, '/json/activate/boom', checkBadPath)
+    .testHttpResponse(null, '/json/badpath', checkBadPath)
     .runFrontendSession([
       testNoUrlsWhenConnected,
       testBreakpointOnStart,