diff options
author | Aleksei Koziatinskii <ak239spb@gmail.com> | 2019-05-16 15:33:09 -0700 |
---|---|---|
committer | Aleksei Koziatinskii <ak239spb@gmail.com> | 2019-06-03 18:15:04 +0300 |
commit | f0018a5152a9faaf2104d62f3152776a60f59390 (patch) | |
tree | 786fb50acdeb349a43cbad532670e4b41258d87a | |
parent | 7e18c650de419ae98511be3c7bc54b34efc6d3d4 (diff) | |
download | android-node-v8-f0018a5152a9faaf2104d62f3152776a60f59390.tar.gz android-node-v8-f0018a5152a9faaf2104d62f3152776a60f59390.tar.bz2 android-node-v8-f0018a5152a9faaf2104d62f3152776a60f59390.zip |
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: https://github.com/nodejs/diagnostics/issues/303
PR-URL: https://github.com/nodejs/node/pull/27741
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
-rw-r--r-- | doc/api/cli.md | 8 | ||||
-rw-r--r-- | src/inspector_agent.cc | 5 | ||||
-rw-r--r-- | src/inspector_io.cc | 15 | ||||
-rw-r--r-- | src/inspector_io.h | 7 | ||||
-rw-r--r-- | src/inspector_socket_server.cc | 55 | ||||
-rw-r--r-- | src/inspector_socket_server.h | 2 | ||||
-rw-r--r-- | src/node_options.cc | 21 | ||||
-rw-r--r-- | src/node_options.h | 9 | ||||
-rw-r--r-- | test/cctest/test_inspector_socket_server.cc | 6 | ||||
-rw-r--r-- | test/parallel/test-inspect-publish-uid.js | 42 |
10 files changed, 148 insertions, 22 deletions
diff --git a/doc/api/cli.md b/doc/api/cli.md index 40163d84b2..d755081b4d 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -389,6 +389,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 @@ -980,6 +987,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 index 09593297e0..dc00861283 100644 --- 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 index 223e3592a1..91384ca949 100644 --- 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 index 0dfb08f470..57facb2294 100644 --- 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 index 6bb0437222..092d5e7a78 100644 --- 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 index 8d38cd5054..42ab90ac90 100644 --- 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 index da7a49231f..2b17780668 100644 --- 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 index 6b4eb89a96..63892d3c47 100644 --- 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 index 9b10f57e72..4e4ebbc328 100644 --- 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 new file mode 100644 index 0000000000..58d651e97d --- /dev/null +++ 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(); + }); + } +} |