Skip to content

Commit f8c9a58

Browse files
alexkozyBridgeAR
authored andcommitted
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 <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
1 parent 40a1a11 commit f8c9a58

10 files changed

+148
-22
lines changed

doc/api/cli.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,13 @@ default) is not firewall-protected.**
400400

401401
See the [debugging security implications][] section for more information.
402402

403+
### `--inspect-publish-uid=stderr,http`
404+
405+
Specify ways of the inspector web socket url exposure.
406+
407+
By default inspector websocket url is available in stderr and under `/json/list`
408+
endpoint on `http://host:port/json/list`.
409+
403410
### `--loader=file`
404411
<!-- YAML
405412
added: v9.0.0
@@ -992,6 +999,7 @@ Node.js options that are allowed are:
992999
- `--inspect`
9931000
- `--inspect-brk`
9941001
- `--inspect-port`
1002+
- `--inspect-publish-uid`
9951003
- `--loader`
9961004
- `--max-http-header-size`
9971005
- `--napi-modules`

src/inspector_agent.cc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -792,7 +792,10 @@ bool Agent::StartIoThread() {
792792

793793
CHECK_NOT_NULL(client_);
794794

795-
io_ = InspectorIo::Start(client_->getThreadHandle(), path_, host_port_);
795+
io_ = InspectorIo::Start(client_->getThreadHandle(),
796+
path_,
797+
host_port_,
798+
debug_options_.inspect_publish_uid);
796799
if (io_ == nullptr) {
797800
return false;
798801
}

src/inspector_io.cc

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -242,9 +242,13 @@ class InspectorIoDelegate: public node::inspector::SocketServerDelegate {
242242
std::unique_ptr<InspectorIo> InspectorIo::Start(
243243
std::shared_ptr<MainThreadHandle> main_thread,
244244
const std::string& path,
245-
std::shared_ptr<HostPort> host_port) {
245+
std::shared_ptr<HostPort> host_port,
246+
const InspectPublishUid& inspect_publish_uid) {
246247
auto io = std::unique_ptr<InspectorIo>(
247-
new InspectorIo(main_thread, path, host_port));
248+
new InspectorIo(main_thread,
249+
path,
250+
host_port,
251+
inspect_publish_uid));
248252
if (io->request_queue_->Expired()) { // Thread is not running
249253
return nullptr;
250254
}
@@ -253,9 +257,11 @@ std::unique_ptr<InspectorIo> InspectorIo::Start(
253257

254258
InspectorIo::InspectorIo(std::shared_ptr<MainThreadHandle> main_thread,
255259
const std::string& path,
256-
std::shared_ptr<HostPort> host_port)
260+
std::shared_ptr<HostPort> host_port,
261+
const InspectPublishUid& inspect_publish_uid)
257262
: main_thread_(main_thread),
258263
host_port_(host_port),
264+
inspect_publish_uid_(inspect_publish_uid),
259265
thread_(),
260266
script_name_(path),
261267
id_(GenerateID()) {
@@ -293,7 +299,8 @@ void InspectorIo::ThreadMain() {
293299
InspectorSocketServer server(std::move(delegate),
294300
&loop,
295301
host_port_->host(),
296-
host_port_->port());
302+
host_port_->port(),
303+
inspect_publish_uid_);
297304
request_queue_ = queue->handle();
298305
// Its lifetime is now that of the server delegate
299306
queue.reset();

src/inspector_io.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ class InspectorIo {
4848
static std::unique_ptr<InspectorIo> Start(
4949
std::shared_ptr<MainThreadHandle> main_thread,
5050
const std::string& path,
51-
std::shared_ptr<HostPort> host_port);
51+
std::shared_ptr<HostPort> host_port,
52+
const InspectPublishUid& inspect_publish_uid);
5253

5354
// Will block till the transport thread shuts down
5455
~InspectorIo();
@@ -61,7 +62,8 @@ class InspectorIo {
6162
private:
6263
InspectorIo(std::shared_ptr<MainThreadHandle> handle,
6364
const std::string& path,
64-
std::shared_ptr<HostPort> host_port);
65+
std::shared_ptr<HostPort> host_port,
66+
const InspectPublishUid& inspect_publish_uid);
6567

6668
// Wrapper for agent->ThreadMain()
6769
static void ThreadMain(void* agent);
@@ -76,6 +78,7 @@ class InspectorIo {
7678
// running
7779
std::shared_ptr<RequestQueue> request_queue_;
7880
std::shared_ptr<HostPort> host_port_;
81+
InspectPublishUid inspect_publish_uid_;
7982

8083
// The IO thread runs its own uv_loop to implement the TCP server off
8184
// the main thread.

src/inspector_socket_server.cc

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -94,14 +94,20 @@ const char* MatchPathSegment(const char* path, const char* expected) {
9494
return nullptr;
9595
}
9696

97-
void SendHttpResponse(InspectorSocket* socket, const std::string& response) {
98-
const char HEADERS[] = "HTTP/1.0 200 OK\r\n"
97+
void SendHttpResponse(InspectorSocket* socket,
98+
const std::string& response,
99+
int code) {
100+
const char HEADERS[] = "HTTP/1.0 %d OK\r\n"
99101
"Content-Type: application/json; charset=UTF-8\r\n"
100102
"Cache-Control: no-cache\r\n"
101103
"Content-Length: %zu\r\n"
102104
"\r\n";
103105
char header[sizeof(HEADERS) + 20];
104-
int header_len = snprintf(header, sizeof(header), HEADERS, response.size());
106+
int header_len = snprintf(header,
107+
sizeof(header),
108+
HEADERS,
109+
code,
110+
response.size());
105111
socket->Write(header, header_len);
106112
socket->Write(response.data(), response.size());
107113
}
@@ -110,7 +116,11 @@ void SendVersionResponse(InspectorSocket* socket) {
110116
std::map<std::string, std::string> response;
111117
response["Browser"] = "node.js/" NODE_VERSION;
112118
response["Protocol-Version"] = "1.1";
113-
SendHttpResponse(socket, MapToString(response));
119+
SendHttpResponse(socket, MapToString(response), 200);
120+
}
121+
122+
void SendHttpNotFound(InspectorSocket* socket) {
123+
SendHttpResponse(socket, "", 404);
114124
}
115125

116126
void SendProtocolJson(InspectorSocket* socket) {
@@ -131,7 +141,7 @@ void SendProtocolJson(InspectorSocket* socket) {
131141
CHECK_EQ(Z_STREAM_END, inflate(&strm, Z_FINISH));
132142
CHECK_EQ(0, strm.avail_out);
133143
CHECK_EQ(Z_OK, inflateEnd(&strm));
134-
SendHttpResponse(socket, data);
144+
SendHttpResponse(socket, data, 200);
135145
}
136146
} // namespace
137147

@@ -224,8 +234,9 @@ void PrintDebuggerReadyMessage(
224234
const std::string& host,
225235
const std::vector<InspectorSocketServer::ServerSocketPtr>& server_sockets,
226236
const std::vector<std::string>& ids,
237+
bool publish_uid_stderr,
227238
FILE* out) {
228-
if (out == nullptr) {
239+
if (!publish_uid_stderr || out == nullptr) {
229240
return;
230241
}
231242
for (const auto& server_socket : server_sockets) {
@@ -241,9 +252,15 @@ void PrintDebuggerReadyMessage(
241252

242253
InspectorSocketServer::InspectorSocketServer(
243254
std::unique_ptr<SocketServerDelegate> delegate, uv_loop_t* loop,
244-
const std::string& host, int port, FILE* out)
245-
: loop_(loop), delegate_(std::move(delegate)), host_(host), port_(port),
246-
next_session_id_(0), out_(out) {
255+
const std::string& host, int port,
256+
const InspectPublishUid& inspect_publish_uid, FILE* out)
257+
: loop_(loop),
258+
delegate_(std::move(delegate)),
259+
host_(host),
260+
port_(port),
261+
inspect_publish_uid_(inspect_publish_uid),
262+
next_session_id_(0),
263+
out_(out) {
247264
delegate_->AssignServer(this);
248265
state_ = ServerState::kNew;
249266
}
@@ -280,8 +297,11 @@ void InspectorSocketServer::SessionTerminated(int session_id) {
280297
if (connected_sessions_.empty()) {
281298
if (was_attached && state_ == ServerState::kRunning
282299
&& !server_sockets_.empty()) {
283-
PrintDebuggerReadyMessage(host_, server_sockets_,
284-
delegate_->GetTargetIds(), out_);
300+
PrintDebuggerReadyMessage(host_,
301+
server_sockets_,
302+
delegate_->GetTargetIds(),
303+
inspect_publish_uid_.console,
304+
out_);
285305
}
286306
if (state_ == ServerState::kStopped) {
287307
delegate_.reset();
@@ -294,6 +314,10 @@ bool InspectorSocketServer::HandleGetRequest(int session_id,
294314
const std::string& path) {
295315
SocketSession* session = Session(session_id);
296316
InspectorSocket* socket = session->ws_socket();
317+
if (!inspect_publish_uid_.http) {
318+
SendHttpNotFound(socket);
319+
return true;
320+
}
297321
const char* command = MatchPathSegment(path.c_str(), "/json");
298322
if (command == nullptr)
299323
return false;
@@ -342,7 +366,7 @@ void InspectorSocketServer::SendListResponse(InspectorSocket* socket,
342366
formatted_address);
343367
target_map["webSocketDebuggerUrl"] = FormatAddress(detected_host, id, true);
344368
}
345-
SendHttpResponse(socket, MapsToString(response));
369+
SendHttpResponse(socket, MapsToString(response), 200);
346370
}
347371

348372
std::string InspectorSocketServer::GetFrontendURL(bool is_compat,
@@ -397,8 +421,11 @@ bool InspectorSocketServer::Start() {
397421
}
398422
delegate_.swap(delegate_holder);
399423
state_ = ServerState::kRunning;
400-
PrintDebuggerReadyMessage(host_, server_sockets_,
401-
delegate_->GetTargetIds(), out_);
424+
PrintDebuggerReadyMessage(host_,
425+
server_sockets_,
426+
delegate_->GetTargetIds(),
427+
inspect_publish_uid_.console,
428+
out_);
402429
return true;
403430
}
404431

src/inspector_socket_server.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ class InspectorSocketServer {
4343
uv_loop_t* loop,
4444
const std::string& host,
4545
int port,
46+
const InspectPublishUid& inspect_publish_uid,
4647
FILE* out = stderr);
4748
~InspectorSocketServer();
4849

@@ -88,6 +89,7 @@ class InspectorSocketServer {
8889
std::unique_ptr<SocketServerDelegate> delegate_;
8990
const std::string host_;
9091
int port_;
92+
InspectPublishUid inspect_publish_uid_;
9193
std::vector<ServerSocketPtr> server_sockets_;
9294
std::map<int, std::pair<std::string, std::unique_ptr<SocketSession>>>
9395
connected_sessions_;

src/node_options.cc

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,21 @@ void DebugOptions::CheckOptions(std::vector<std::string>* errors) {
4444
errors->push_back("[DEP0062]: `node --inspect --debug-brk` is deprecated. "
4545
"Please use `node --inspect-brk` instead.");
4646
}
47+
48+
std::vector<std::string> destinations =
49+
SplitString(inspect_publish_uid_string, ',');
50+
inspect_publish_uid.console = false;
51+
inspect_publish_uid.http = false;
52+
for (const std::string& destination : destinations) {
53+
if (destination == "stderr") {
54+
inspect_publish_uid.console = true;
55+
} else if (destination == "http") {
56+
inspect_publish_uid.http = true;
57+
} else {
58+
errors->push_back("--inspect-publish-uid destination can be "
59+
"stderr or http");
60+
}
61+
}
4762
}
4863

4964
void PerProcessOptions::CheckOptions(std::vector<std::string>* errors) {
@@ -276,6 +291,12 @@ DebugOptionsParser::DebugOptionsParser() {
276291
AddOption("--debug-brk", "", &DebugOptions::break_first_line);
277292
Implies("--debug-brk", "--debug");
278293
AddAlias("--debug-brk=", { "--inspect-port", "--debug-brk" });
294+
295+
AddOption("--inspect-publish-uid",
296+
"comma separated list of destinations for inspector uid"
297+
"(default: stderr,http)",
298+
&DebugOptions::inspect_publish_uid_string,
299+
kAllowedInEnvironment);
279300
}
280301

281302
EnvironmentOptionsParser::EnvironmentOptionsParser() {

src/node_options.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ class Options {
5050
virtual ~Options() = default;
5151
};
5252

53+
struct InspectPublishUid {
54+
bool console;
55+
bool http;
56+
};
57+
5358
// These options are currently essentially per-Environment, but it can be nice
5459
// to keep them separate since they are a group of options applying to a very
5560
// specific part of Node. It might also make more sense for them to be
@@ -70,6 +75,10 @@ class DebugOptions : public Options {
7075
bool break_first_line = false;
7176
// --inspect-brk-node
7277
bool break_node_first_line = false;
78+
// --inspect-publish-uid
79+
std::string inspect_publish_uid_string = "stderr,http";
80+
81+
InspectPublishUid inspect_publish_uid;
7382

7483
enum { kDefaultInspectorPort = 9229 };
7584

test/cctest/test_inspector_socket_server.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "inspector_socket_server.h"
22

33
#include "node.h"
4+
#include "node_options.h"
45
#include "util-inl.h"
56
#include "gtest/gtest.h"
67

@@ -358,8 +359,11 @@ ServerHolder::ServerHolder(bool has_targets, uv_loop_t* loop,
358359
targets = { MAIN_TARGET_ID };
359360
std::unique_ptr<TestSocketServerDelegate> delegate(
360361
new TestSocketServerDelegate(this, targets));
362+
node::InspectPublishUid inspect_publish_uid;
363+
inspect_publish_uid.console = true;
364+
inspect_publish_uid.http = true;
361365
server_ = std::make_unique<InspectorSocketServer>(
362-
std::move(delegate), loop, host, port, out);
366+
std::move(delegate), loop, host, port, inspect_publish_uid, out);
363367
}
364368

365369
static void TestHttpRequest(int port, const std::string& path,
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
common.skipIfInspectorDisabled();
5+
6+
const assert = require('assert');
7+
const { spawnSync } = require('child_process');
8+
9+
(async function test() {
10+
await testArg('stderr');
11+
await testArg('http');
12+
await testArg('http,stderr');
13+
})();
14+
15+
async function testArg(argValue) {
16+
console.log('Checks ' + argValue + '..');
17+
const hasHttp = argValue.split(',').includes('http');
18+
const hasStderr = argValue.split(',').includes('stderr');
19+
20+
const nodeProcess = spawnSync(process.execPath, [
21+
'--inspect=0',
22+
`--inspect-publish-uid=${argValue}`,
23+
'-e', `(${scriptMain.toString()})(${hasHttp ? 200 : 404})`
24+
]);
25+
const hasWebSocketInStderr = checkStdError(
26+
nodeProcess.stderr.toString('utf8'));
27+
assert.strictEqual(hasWebSocketInStderr, hasStderr);
28+
29+
function checkStdError(data) {
30+
const matches = data.toString('utf8').match(/ws:\/\/.+:(\d+)\/.+/);
31+
return !!matches;
32+
}
33+
34+
function scriptMain(code) {
35+
const url = require('inspector').url();
36+
const { host } = require('url').parse(url);
37+
require('http').get('http://' + host + '/json/list', (response) => {
38+
assert.strictEqual(response.statusCode, code);
39+
response.destroy();
40+
});
41+
}
42+
}

0 commit comments

Comments
 (0)