inspector: added --inspect-publish-uid

This flag specifies how inspector websocket url should be reported. Tthre options are supported: - stderr - reports websocket as a message to stderr, - http - exposes /json/list endpoint that contains inspector websocket url, - binding - require('inspector').url(). Related discussion: nodejs/diagnostics#303 PR-URL: #27741 Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
nodejs · Jun 17, 2019 · f8c9a58 · f8c9a58
1 parent 40a1a11
commit f8c9a58
Show file tree

Hide file tree

Showing 10 changed files with 148 additions and 22 deletions.
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -400,6 +400,13 @@ default) is not firewall-protected.**
 
 See the [debugging security implications][] section for more information.
 
+### `--inspect-publish-uid=stderr,http`
+
+Specify ways of the inspector web socket url exposure.
+
+By default inspector websocket url is available in stderr and under `/json/list`
+endpoint on `http://host:port/json/list`.
+
 ### `--loader=file`
 <!-- YAML
 added: v9.0.0
@@ -992,6 +999,7 @@ Node.js options that are allowed are:
 - `--inspect`
 - `--inspect-brk`
 - `--inspect-port`
+- `--inspect-publish-uid`
 - `--loader`
 - `--max-http-header-size`
 - `--napi-modules`

diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
@@ -792,7 +792,10 @@ bool Agent::StartIoThread() {
 
   CHECK_NOT_NULL(client_);
 
-  io_ = InspectorIo::Start(client_->getThreadHandle(), path_, host_port_);
+  io_ = InspectorIo::Start(client_->getThreadHandle(),
+                           path_,
+                           host_port_,
+                           debug_options_.inspect_publish_uid);
   if (io_ == nullptr) {
     return false;
   }

diff --git a/src/inspector_io.cc b/src/inspector_io.cc
@@ -242,9 +242,13 @@ class InspectorIoDelegate: public node::inspector::SocketServerDelegate {
 std::unique_ptr<InspectorIo> InspectorIo::Start(
     std::shared_ptr<MainThreadHandle> main_thread,
     const std::string& path,
-    std::shared_ptr<HostPort> host_port) {
+    std::shared_ptr<HostPort> host_port,
+    const InspectPublishUid& inspect_publish_uid) {
   auto io = std::unique_ptr<InspectorIo>(
-      new InspectorIo(main_thread, path, host_port));
+      new InspectorIo(main_thread,
+                      path,
+                      host_port,
+                      inspect_publish_uid));
   if (io->request_queue_->Expired()) {  // Thread is not running
     return nullptr;
   }
@@ -253,9 +257,11 @@ std::unique_ptr<InspectorIo> InspectorIo::Start(
 
 InspectorIo::InspectorIo(std::shared_ptr<MainThreadHandle> main_thread,
                          const std::string& path,
-                         std::shared_ptr<HostPort> host_port)
+                         std::shared_ptr<HostPort> host_port,
+                         const InspectPublishUid& inspect_publish_uid)
     : main_thread_(main_thread),
       host_port_(host_port),
+      inspect_publish_uid_(inspect_publish_uid),
       thread_(),
       script_name_(path),
       id_(GenerateID()) {
@@ -293,7 +299,8 @@ void InspectorIo::ThreadMain() {
   InspectorSocketServer server(std::move(delegate),
                                &loop,
                                host_port_->host(),
-                               host_port_->port());
+                               host_port_->port(),
+                               inspect_publish_uid_);
   request_queue_ = queue->handle();
   // Its lifetime is now that of the server delegate
   queue.reset();

diff --git a/src/inspector_io.h b/src/inspector_io.h
@@ -48,7 +48,8 @@ class InspectorIo {
   static std::unique_ptr<InspectorIo> Start(
       std::shared_ptr<MainThreadHandle> main_thread,
       const std::string& path,
-      std::shared_ptr<HostPort> host_port);
+      std::shared_ptr<HostPort> host_port,
+      const InspectPublishUid& inspect_publish_uid);
 
   // Will block till the transport thread shuts down
   ~InspectorIo();
@@ -61,7 +62,8 @@ class InspectorIo {
  private:
   InspectorIo(std::shared_ptr<MainThreadHandle> handle,
               const std::string& path,
-              std::shared_ptr<HostPort> host_port);
+              std::shared_ptr<HostPort> host_port,
+              const InspectPublishUid& inspect_publish_uid);
 
   // Wrapper for agent->ThreadMain()
   static void ThreadMain(void* agent);
@@ -76,6 +78,7 @@ class InspectorIo {
   // running
   std::shared_ptr<RequestQueue> request_queue_;
   std::shared_ptr<HostPort> host_port_;
+  InspectPublishUid inspect_publish_uid_;
 
   // The IO thread runs its own uv_loop to implement the TCP server off
   // the main thread.

diff --git a/src/inspector_socket_server.cc b/src/inspector_socket_server.cc
@@ -94,14 +94,20 @@ const char* MatchPathSegment(const char* path, const char* expected) {
   return nullptr;
 }
 
-void SendHttpResponse(InspectorSocket* socket, const std::string& response) {
-  const char HEADERS[] = "HTTP/1.0 200 OK\r\n"
+void SendHttpResponse(InspectorSocket* socket,
+                      const std::string& response,
+                      int code) {
+  const char HEADERS[] = "HTTP/1.0 %d OK\r\n"
                          "Content-Type: application/json; charset=UTF-8\r\n"
                          "Cache-Control: no-cache\r\n"
                          "Content-Length: %zu\r\n"
                          "\r\n";
   char header[sizeof(HEADERS) + 20];
-  int header_len = snprintf(header, sizeof(header), HEADERS, response.size());
+  int header_len = snprintf(header,
+                            sizeof(header),
+                            HEADERS,
+                            code,
+                            response.size());
   socket->Write(header, header_len);
   socket->Write(response.data(), response.size());
 }
@@ -110,7 +116,11 @@ void SendVersionResponse(InspectorSocket* socket) {
   std::map<std::string, std::string> response;
   response["Browser"] = "node.js/" NODE_VERSION;
   response["Protocol-Version"] = "1.1";
-  SendHttpResponse(socket, MapToString(response));
+  SendHttpResponse(socket, MapToString(response), 200);
+}
+
+void SendHttpNotFound(InspectorSocket* socket) {
+  SendHttpResponse(socket, "", 404);
 }
 
 void SendProtocolJson(InspectorSocket* socket) {
@@ -131,7 +141,7 @@ void SendProtocolJson(InspectorSocket* socket) {
   CHECK_EQ(Z_STREAM_END, inflate(&strm, Z_FINISH));
   CHECK_EQ(0, strm.avail_out);
   CHECK_EQ(Z_OK, inflateEnd(&strm));
-  SendHttpResponse(socket, data);
+  SendHttpResponse(socket, data, 200);
 }
 }  // namespace
 
@@ -224,8 +234,9 @@ void PrintDebuggerReadyMessage(
     const std::string& host,
     const std::vector<InspectorSocketServer::ServerSocketPtr>& server_sockets,
     const std::vector<std::string>& ids,
+    bool publish_uid_stderr,
     FILE* out) {
-  if (out == nullptr) {
+  if (!publish_uid_stderr || out == nullptr) {
     return;
   }
   for (const auto& server_socket : server_sockets) {
@@ -241,9 +252,15 @@ void PrintDebuggerReadyMessage(
 
 InspectorSocketServer::InspectorSocketServer(
     std::unique_ptr<SocketServerDelegate> delegate, uv_loop_t* loop,
-    const std::string& host, int port, FILE* out)
-    : loop_(loop), delegate_(std::move(delegate)), host_(host), port_(port),
-      next_session_id_(0), out_(out) {
+    const std::string& host, int port,
+    const InspectPublishUid& inspect_publish_uid, FILE* out)
+    : loop_(loop),
+      delegate_(std::move(delegate)),
+      host_(host),
+      port_(port),
+      inspect_publish_uid_(inspect_publish_uid),
+      next_session_id_(0),
+      out_(out) {
   delegate_->AssignServer(this);
   state_ = ServerState::kNew;
 }
@@ -280,8 +297,11 @@ void InspectorSocketServer::SessionTerminated(int session_id) {
   if (connected_sessions_.empty()) {
     if (was_attached && state_ == ServerState::kRunning
         && !server_sockets_.empty()) {
-      PrintDebuggerReadyMessage(host_, server_sockets_,
-                                delegate_->GetTargetIds(), out_);
+      PrintDebuggerReadyMessage(host_,
+                                server_sockets_,
+                                delegate_->GetTargetIds(),
+                                inspect_publish_uid_.console,
+                                out_);
     }
     if (state_ == ServerState::kStopped) {
       delegate_.reset();
@@ -294,6 +314,10 @@ bool InspectorSocketServer::HandleGetRequest(int session_id,
                                              const std::string& path) {
   SocketSession* session = Session(session_id);
   InspectorSocket* socket = session->ws_socket();
+  if (!inspect_publish_uid_.http) {
+    SendHttpNotFound(socket);
+    return true;
+  }
   const char* command = MatchPathSegment(path.c_str(), "/json");
   if (command == nullptr)
     return false;
@@ -342,7 +366,7 @@ void InspectorSocketServer::SendListResponse(InspectorSocket* socket,
                                                              formatted_address);
     target_map["webSocketDebuggerUrl"] = FormatAddress(detected_host, id, true);
   }
-  SendHttpResponse(socket, MapsToString(response));
+  SendHttpResponse(socket, MapsToString(response), 200);
 }
 
 std::string InspectorSocketServer::GetFrontendURL(bool is_compat,
@@ -397,8 +421,11 @@ bool InspectorSocketServer::Start() {
   }
   delegate_.swap(delegate_holder);
   state_ = ServerState::kRunning;
-  PrintDebuggerReadyMessage(host_, server_sockets_,
-                            delegate_->GetTargetIds(), out_);
+  PrintDebuggerReadyMessage(host_,
+                            server_sockets_,
+                            delegate_->GetTargetIds(),
+                            inspect_publish_uid_.console,
+                            out_);
   return true;
 }
 

diff --git a/src/inspector_socket_server.h b/src/inspector_socket_server.h
@@ -43,6 +43,7 @@ class InspectorSocketServer {
                         uv_loop_t* loop,
                         const std::string& host,
                         int port,
+                        const InspectPublishUid& inspect_publish_uid,
                         FILE* out = stderr);
   ~InspectorSocketServer();
 
@@ -88,6 +89,7 @@ class InspectorSocketServer {
   std::unique_ptr<SocketServerDelegate> delegate_;
   const std::string host_;
   int port_;
+  InspectPublishUid inspect_publish_uid_;
   std::vector<ServerSocketPtr> server_sockets_;
   std::map<int, std::pair<std::string, std::unique_ptr<SocketSession>>>
       connected_sessions_;

diff --git a/src/node_options.cc b/src/node_options.cc
@@ -44,6 +44,21 @@ void DebugOptions::CheckOptions(std::vector<std::string>* errors) {
     errors->push_back("[DEP0062]: `node --inspect --debug-brk` is deprecated. "
                       "Please use `node --inspect-brk` instead.");
   }
+
+  std::vector<std::string> destinations =
+      SplitString(inspect_publish_uid_string, ',');
+  inspect_publish_uid.console = false;
+  inspect_publish_uid.http = false;
+  for (const std::string& destination : destinations) {
+    if (destination == "stderr") {
+      inspect_publish_uid.console = true;
+    } else if (destination == "http") {
+      inspect_publish_uid.http = true;
+    } else {
+      errors->push_back("--inspect-publish-uid destination can be "
+                        "stderr or http");
+    }
+  }
 }
 
 void PerProcessOptions::CheckOptions(std::vector<std::string>* errors) {
@@ -276,6 +291,12 @@ DebugOptionsParser::DebugOptionsParser() {
   AddOption("--debug-brk", "", &DebugOptions::break_first_line);
   Implies("--debug-brk", "--debug");
   AddAlias("--debug-brk=", { "--inspect-port", "--debug-brk" });
+
+  AddOption("--inspect-publish-uid",
+            "comma separated list of destinations for inspector uid"
+            "(default: stderr,http)",
+            &DebugOptions::inspect_publish_uid_string,
+            kAllowedInEnvironment);
 }
 
 EnvironmentOptionsParser::EnvironmentOptionsParser() {

diff --git a/src/node_options.h b/src/node_options.h
@@ -50,6 +50,11 @@ class Options {
   virtual ~Options() = default;
 };
 
+struct InspectPublishUid {
+  bool console;
+  bool http;
+};
+
 // These options are currently essentially per-Environment, but it can be nice
 // to keep them separate since they are a group of options applying to a very
 // specific part of Node. It might also make more sense for them to be
@@ -70,6 +75,10 @@ class DebugOptions : public Options {
   bool break_first_line = false;
   // --inspect-brk-node
   bool break_node_first_line = false;
+  // --inspect-publish-uid
+  std::string inspect_publish_uid_string = "stderr,http";
+
+  InspectPublishUid inspect_publish_uid;
 
   enum { kDefaultInspectorPort = 9229 };
 

diff --git a/test/cctest/test_inspector_socket_server.cc b/test/cctest/test_inspector_socket_server.cc
@@ -1,6 +1,7 @@
 #include "inspector_socket_server.h"
 
 #include "node.h"
+#include "node_options.h"
 #include "util-inl.h"
 #include "gtest/gtest.h"
 
@@ -358,8 +359,11 @@ ServerHolder::ServerHolder(bool has_targets, uv_loop_t* loop,
     targets = { MAIN_TARGET_ID };
   std::unique_ptr<TestSocketServerDelegate> delegate(
       new TestSocketServerDelegate(this, targets));
+  node::InspectPublishUid inspect_publish_uid;
+  inspect_publish_uid.console = true;
+  inspect_publish_uid.http = true;
   server_ = std::make_unique<InspectorSocketServer>(
-      std::move(delegate), loop, host, port, out);
+      std::move(delegate), loop, host, port, inspect_publish_uid, out);
 }
 
 static void TestHttpRequest(int port, const std::string& path,

diff --git a/test/parallel/test-inspect-publish-uid.js b/test/parallel/test-inspect-publish-uid.js
@@ -0,0 +1,42 @@
+'use strict';
+const common = require('../common');
+
+common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+
+(async function test() {
+  await testArg('stderr');
+  await testArg('http');
+  await testArg('http,stderr');
+})();
+
+async function testArg(argValue) {
+  console.log('Checks ' + argValue + '..');
+  const hasHttp = argValue.split(',').includes('http');
+  const hasStderr = argValue.split(',').includes('stderr');
+
+  const nodeProcess = spawnSync(process.execPath, [
+    '--inspect=0',
+    `--inspect-publish-uid=${argValue}`,
+    '-e', `(${scriptMain.toString()})(${hasHttp ? 200 : 404})`
+  ]);
+  const hasWebSocketInStderr = checkStdError(
+    nodeProcess.stderr.toString('utf8'));
+  assert.strictEqual(hasWebSocketInStderr, hasStderr);
+
+  function checkStdError(data) {
+    const matches = data.toString('utf8').match(/ws:\/\/.+:(\d+)\/.+/);
+    return !!matches;
+  }
+
+  function scriptMain(code) {
+    const url = require('inspector').url();
+    const { host } = require('url').parse(url);
+    require('http').get('http://' + host + '/json/list', (response) => {
+      assert.strictEqual(response.statusCode, code);
+      response.destroy();
+    });
+  }
+}