Skip to content

Commit

Permalink
inspector: proper WS URLs when bound to 0.0.0.0
Browse files Browse the repository at this point in the history
JSON target list response will now return appropriate IP address
for instances listening on 0.0.0.0.

Refs: nodejs#11591
PR-URL: nodejs#11755
Reviewed-By: James Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
Eugene Ostroukhov authored and jungx098 committed Mar 21, 2017
1 parent 015daeb commit 3aed9cd
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 16 deletions.
26 changes: 25 additions & 1 deletion src/inspector_socket_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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=";
Expand Down
25 changes: 16 additions & 9 deletions test/inspector/inspector-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -98,6 +98,8 @@ function checkHttpResponse(port, path, callback) {
callback(err, json);
});
});
if (errorcb)
req.on('error', errorcb);
}

function makeBufferingDataCallback(dataCallback) {
Expand Down Expand Up @@ -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();
}));
Expand Down Expand Up @@ -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));
});
};

Expand Down Expand Up @@ -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);
});
Expand Down
52 changes: 52 additions & 0 deletions test/inspector/test-inspector-ip-detection.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
'use strict';
const common = require('../common');
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');
12 changes: 6 additions & 6 deletions test/inspector/test-inspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,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,
Expand Down

0 comments on commit 3aed9cd

Please sign in to comment.