Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inspector: added --inspect-publish-uid #27741

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -980,6 +987,7 @@ Node.js options that are allowed are:
- `--inspect`
- `--inspect-brk`
- `--inspect-port`
- `--inspect-publish-uid`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also be documented in ./doc/node.1.

- `--loader`
- `--max-http-header-size`
- `--napi-modules`
Expand Down
5 changes: 4 additions & 1 deletion src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
15 changes: 11 additions & 4 deletions src/inspector_io.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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()) {
Expand Down Expand Up @@ -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();
Expand Down
7 changes: 5 additions & 2 deletions src/inspector_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
Expand All @@ -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.
Expand Down
55 changes: 41 additions & 14 deletions src/inspector_socket_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand All @@ -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) {
Expand All @@ -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

Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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();
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}

Expand Down
2 changes: 2 additions & 0 deletions src/inspector_socket_server.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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_;
Expand Down
21 changes: 21 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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() {
Expand Down
9 changes: 9 additions & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 };

Expand Down
6 changes: 5 additions & 1 deletion test/cctest/test_inspector_socket_server.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "inspector_socket_server.h"

#include "node.h"
#include "node_options.h"
#include "util-inl.h"
#include "gtest/gtest.h"

Expand Down Expand Up @@ -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,
Expand Down
42 changes: 42 additions & 0 deletions test/parallel/test-inspect-publish-uid.js
Original file line number Diff line number Diff line change
@@ -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();
});
}
}