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: #19664 Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
nodejs · Apr 2, 2018 · 0bcad33 · 0bcad33
1 parent c5469bb
commit 0bcad33
Show file tree

Hide file tree

Showing 8 changed files with 85 additions and 42 deletions.
diff --git 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
@@ -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<char>& frame) = 0;
     virtual ~Delegate() {}

diff --git a/src/inspector_socket_server.cc b/src/inspector_socket_server.cc
@@ -16,33 +16,42 @@ 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 << '[';
   }
   url << host;
   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<std::string, std::string>& 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<char>& 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,25 +343,20 @@ 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);
     return true;
   } 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<std::map<std::string, std::string>> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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) {