From a9a1f12b4212c0ea81a04bf2eb56efe6f8a4699b Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Mon, 5 Mar 2018 19:18:17 -0800 Subject: [PATCH] inspector: report client-visible host and port Node instance may not know the real host and port user sees when debug frontend connects through the SSH tunnel. This change fixes '/json/list' response by using the value client provided in the host header. PR-URL: https://github.com/nodejs/node/pull/19664 Reviewed-By: Tiancheng "Timothy" Gu --- src/inspector_socket.cc | 4 +- src/inspector_socket.h | 6 +- src/inspector_socket_server.cc | 75 +++++++++++-------- src/inspector_socket_server.h | 6 +- test/cctest/test_inspector_socket.cc | 4 +- test/common/inspector-helper.js | 5 +- test/parallel/test-inspector-reported-host.js | 22 ++++++ test/sequential/test-inspector.js | 5 +- 8 files changed, 85 insertions(+), 42 deletions(-) create mode 100644 test/parallel/test-inspector-reported-host.js diff --git a/src/inspector_socket.cc b/src/inspector_socket.cc index b3a810d99c31ed..fa46c45decdd8c 100644 --- a/src/inspector_socket.cc +++ b/src/inspector_socket.cc @@ -495,12 +495,12 @@ class HttpHandler : public ProtocolHandler { CancelHandshake(); return; } else if (!event.upgrade) { - delegate()->OnHttpGet(event.path); + delegate()->OnHttpGet(event.host, event.path); } else if (event.ws_key.empty()) { CancelHandshake(); return; } else { - delegate()->OnSocketUpgrade(event.path, event.ws_key); + delegate()->OnSocketUpgrade(event.host, event.path, event.ws_key); } } } diff --git a/src/inspector_socket.h b/src/inspector_socket.h index 1a3411435ee2c5..81b894f95cb88f 100644 --- a/src/inspector_socket.h +++ b/src/inspector_socket.h @@ -17,8 +17,10 @@ class InspectorSocket { public: class Delegate { public: - virtual void OnHttpGet(const std::string& path) = 0; - virtual void OnSocketUpgrade(const std::string& path, + virtual void OnHttpGet(const std::string& host, + const std::string& path) = 0; + virtual void OnSocketUpgrade(const std::string& host, + const std::string& path, const std::string& accept_key) = 0; virtual void OnWsFrame(const std::vector& frame) = 0; virtual ~Delegate() {} diff --git a/src/inspector_socket_server.cc b/src/inspector_socket_server.cc index 03e5c60e65641e..edea483cfe0e64 100644 --- a/src/inspector_socket_server.cc +++ b/src/inspector_socket_server.cc @@ -16,12 +16,23 @@ namespace inspector { // depend on inspector_socket_server.h std::string FormatWsAddress(const std::string& host, int port, const std::string& target_id, - bool include_protocol) { + bool include_protocol); +namespace { + +static const uint8_t PROTOCOL_JSON[] = { + #include "v8_inspector_protocol_json.h" // NOLINT(build/include_order) +}; + +void Escape(std::string* string) { + for (char& c : *string) { + c = (c == '\"' || c == '\\') ? '_' : c; + } +} + +std::string FormatHostPort(const std::string& host, int port) { // Host is valid (socket was bound) so colon means it's a v6 IP address bool v6 = host.find(':') != std::string::npos; std::ostringstream url; - if (include_protocol) - url << "ws://"; if (v6) { url << '['; } @@ -29,20 +40,18 @@ std::string FormatWsAddress(const std::string& host, int port, if (v6) { url << ']'; } - url << ':' << port << '/' << target_id; + url << ':' << port; return url.str(); } -namespace { - -static const uint8_t PROTOCOL_JSON[] = { - #include "v8_inspector_protocol_json.h" // NOLINT(build/include_order) -}; - -void Escape(std::string* string) { - for (char& c : *string) { - c = (c == '\"' || c == '\\') ? '_' : c; - } +std::string FormatAddress(const std::string& host, + const std::string& target_id, + bool include_protocol) { + std::ostringstream url; + if (include_protocol) + url << "ws://"; + url << host << '/' << target_id; + return url.str(); } std::string MapToString(const std::map& object) { @@ -141,6 +150,11 @@ void SendProtocolJson(InspectorSocket* socket) { } } // namespace +std::string FormatWsAddress(const std::string& host, int port, + const std::string& target_id, + bool include_protocol) { + return FormatAddress(FormatHostPort(host, port), target_id, include_protocol); +} class Closer { public: @@ -213,8 +227,8 @@ class SocketSession { ~Delegate() { server_->SessionTerminated(session_id_); } - void OnHttpGet(const std::string& path) override; - void OnSocketUpgrade(const std::string& path, + void OnHttpGet(const std::string& host, const std::string& path) override; + void OnSocketUpgrade(const std::string& host, const std::string& path, const std::string& ws_key) override; void OnWsFrame(const std::vector& data) override; @@ -320,6 +334,7 @@ void InspectorSocketServer::SessionTerminated(int session_id) { } bool InspectorSocketServer::HandleGetRequest(int session_id, + const std::string& host, const std::string& path) { SocketSession* session = Session(session_id); InspectorSocket* socket = session->ws_socket(); @@ -328,7 +343,7 @@ bool InspectorSocketServer::HandleGetRequest(int session_id, return false; if (MatchPathSegment(command, "list") || command[0] == '\0') { - SendListResponse(socket, session); + SendListResponse(socket, host, session); return true; } else if (MatchPathSegment(command, "protocol")) { SendProtocolJson(socket); @@ -336,17 +351,12 @@ bool InspectorSocketServer::HandleGetRequest(int session_id, } else if (MatchPathSegment(command, "version")) { SendVersionResponse(socket); return true; - } else if (const char* target_id = MatchPathSegment(command, "activate")) { - if (TargetExists(target_id)) { - SendHttpResponse(socket, "Target activated"); - return true; - } - return false; } return false; } void InspectorSocketServer::SendListResponse(InspectorSocket* socket, + const std::string& host, SocketSession* session) { std::vector> response; for (const std::string& id : delegate_->GetTargetIds()) { @@ -371,15 +381,18 @@ void InspectorSocketServer::SendListResponse(InspectorSocket* socket, } } if (!connected) { - std::string host = socket->GetHost(); - int port = session->server_port(); + std::string detected_host = host; + if (detected_host.empty()) { + detected_host = FormatHostPort(socket->GetHost(), + session->server_port()); + } std::ostringstream frontend_url; frontend_url << "chrome-devtools://devtools/bundled"; frontend_url << "/inspector.html?experiments=true&v8only=true&ws="; - frontend_url << FormatWsAddress(host, port, id, false); + frontend_url << FormatAddress(detected_host, id, false); target_map["devtoolsFrontendUrl"] += frontend_url.str(); target_map["webSocketDebuggerUrl"] = - FormatWsAddress(host, port, id, true); + FormatAddress(detected_host, id, true); } } SendHttpResponse(socket, MapsToString(response)); @@ -531,12 +544,14 @@ void SocketSession::Send(const std::string& message) { ws_socket_->Write(message.data(), message.length()); } -void SocketSession::Delegate::OnHttpGet(const std::string& path) { - if (!server_->HandleGetRequest(session_id_, path)) +void SocketSession::Delegate::OnHttpGet(const std::string& host, + const std::string& path) { + if (!server_->HandleGetRequest(session_id_, host, path)) Session()->ws_socket()->CancelHandshake(); } -void SocketSession::Delegate::OnSocketUpgrade(const std::string& path, +void SocketSession::Delegate::OnSocketUpgrade(const std::string& host, + const std::string& path, const std::string& ws_key) { std::string id = path.empty() ? path : path.substr(1); server_->SessionStarted(session_id_, id, ws_key); diff --git a/src/inspector_socket_server.h b/src/inspector_socket_server.h index b193e33a46d6d3..f6003c4c4b4d70 100644 --- a/src/inspector_socket_server.h +++ b/src/inspector_socket_server.h @@ -67,7 +67,8 @@ class InspectorSocketServer { // Session connection lifecycle void Accept(int server_port, uv_stream_t* server_socket); - bool HandleGetRequest(int session_id, const std::string& path); + bool HandleGetRequest(int session_id, const std::string& host, + const std::string& path); void SessionStarted(int session_id, const std::string& target_id, const std::string& ws_id); void SessionTerminated(int session_id); @@ -77,7 +78,8 @@ class InspectorSocketServer { SocketSession* Session(int session_id); private: - void SendListResponse(InspectorSocket* socket, SocketSession* session); + void SendListResponse(InspectorSocket* socket, const std::string& host, + SocketSession* session); bool TargetExists(const std::string& id); enum class ServerState {kNew, kRunning, kStopping, kStopped}; diff --git a/test/cctest/test_inspector_socket.cc b/test/cctest/test_inspector_socket.cc index ae6e1231c4c3a1..b96489db1f49d3 100644 --- a/test/cctest/test_inspector_socket.cc +++ b/test/cctest/test_inspector_socket.cc @@ -112,11 +112,11 @@ class TestInspectorDelegate : public InspectorSocket::Delegate { delegate = nullptr; } - void OnHttpGet(const std::string& path) override { + void OnHttpGet(const std::string& host, const std::string& path) override { process(kInspectorHandshakeHttpGet, path); } - void OnSocketUpgrade(const std::string& path, + void OnSocketUpgrade(const std::string& host, const std::string& path, const std::string& ws_key) override { ws_key_ = ws_key; process(kInspectorHandshakeUpgraded, path); diff --git a/test/common/inspector-helper.js b/test/common/inspector-helper.js index 7590cfc104b70a..1bc16e3714cffa 100644 --- a/test/common/inspector-helper.js +++ b/test/common/inspector-helper.js @@ -365,10 +365,11 @@ class NodeInstance { } } - httpGet(host, path) { + httpGet(host, path, hostHeaderValue) { console.log('[test]', `Testing ${path}`); + const headers = hostHeaderValue ? { 'Host': hostHeaderValue } : null; return this.portPromise.then((port) => new Promise((resolve, reject) => { - const req = http.get({ host, port, path }, (res) => { + const req = http.get({ host, port, path, headers }, (res) => { let response = ''; res.setEncoding('utf8'); res diff --git a/test/parallel/test-inspector-reported-host.js b/test/parallel/test-inspector-reported-host.js new file mode 100644 index 00000000000000..f54ea16625d218 --- /dev/null +++ b/test/parallel/test-inspector-reported-host.js @@ -0,0 +1,22 @@ +// Flags: --expose-internals +'use strict'; +const common = require('../common'); + +common.skipIfInspectorDisabled(); + +const assert = require('assert'); +const { NodeInstance } = require('../common/inspector-helper.js'); + +common.crashOnUnhandledRejection(); + +async function test() { + const madeUpHost = '111.111.111.111:11111'; + const child = new NodeInstance(undefined, 'var a = 1'); + const response = await child.httpGet(null, '/json', madeUpHost); + assert.ok( + response[0].webSocketDebuggerUrl.startsWith(`ws://${madeUpHost}`), + response[0].webSocketDebuggerUrl); + child.kill(); +} + +test(); diff --git a/test/sequential/test-inspector.js b/test/sequential/test-inspector.js index 23b4dcb9618a44..4e0315807b628a 100644 --- a/test/sequential/test-inspector.js +++ b/test/sequential/test-inspector.js @@ -11,8 +11,9 @@ function checkListResponse(response) { assert.strictEqual(1, response.length); assert.ok(response[0].devtoolsFrontendUrl); assert.ok( - /ws:\/\/127\.0\.0\.1:\d+\/[0-9A-Fa-f]{8}-/ - .test(response[0].webSocketDebuggerUrl)); + /ws:\/\/localhost:\d+\/[0-9A-Fa-f]{8}-/ + .test(response[0].webSocketDebuggerUrl), + response[0].webSocketDebuggerUrl); } function checkVersion(response) {