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

report: fix network queries in getReport libuv with exclude-network #55602

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
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
52 changes: 48 additions & 4 deletions doc/api/report.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ is provided below for reference.
```json
{
"header": {
"reportVersion": 3,
"reportVersion": 4,
"event": "exception",
"trigger": "Exception",
"filename": "report.20181221.005011.8974.0.001.json",
Expand Down Expand Up @@ -322,6 +322,50 @@ is provided below for reference.
"is_active": true,
"address": "0x000055fc7b2cb180",
"loopIdleTimeSeconds": 22644.8
},
{
"type": "tcp",
"is_active": true,
"is_referenced": true,
"address": "0x000055e70fcb85d8",
"localEndpoint": {
"host": "localhost",
"ip4": "127.0.0.1",
"port": 48986
},
"remoteEndpoint": {
"host": "localhost",
"ip4": "127.0.0.1",
"port": 38573
},
"sendBufferSize": 2626560,
"recvBufferSize": 131072,
"fd": 24,
"writeQueueSize": 0,
"readable": true,
"writable": true
},
{
"type": "tcp",
"is_active": true,
"is_referenced": true,
"address": "0x000055e70fcd68c8",
"localEndpoint": {
"host": "ip6-localhost",
"ip6": "::1",
"port": 52266
},
"remoteEndpoint": {
"host": "ip6-localhost",
"ip6": "::1",
"port": 38573
},
"sendBufferSize": 2626560,
"recvBufferSize": 131072,
"fd": 25,
"writeQueueSize": 0,
"readable": false,
"writable": false
}
],
"workers": [],
Expand Down Expand Up @@ -461,9 +505,9 @@ meaning of `SIGUSR2` for the said purposes.
* `--report-signal` Sets or resets the signal for report generation
(not supported on Windows). Default signal is `SIGUSR2`.

* `--report-exclude-network` Exclude `header.networkInterfaces` from the
diagnostic report. By default this is not set and the network interfaces
are included.
* `--report-exclude-network` Exclude `header.networkInterfaces` and disable the reverse DNS queries
in `libuv.*.(remote|local)Endpoint.host` from the diagnostic report.
By default this is not set and the network interfaces are included.

A report can also be triggered via an API call from a JavaScript application:

Expand Down
6 changes: 4 additions & 2 deletions src/node_report.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#include <cwctype>
#include <fstream>

constexpr int NODE_REPORT_VERSION = 3;
constexpr int NODE_REPORT_VERSION = 4;
constexpr int NANOS_PER_SEC = 1000 * 1000 * 1000;
constexpr double SEC_PER_MICROS = 1e-6;
constexpr int MAX_FRAME_COUNT = node::kMaxFrameCountForLogging;
Expand Down Expand Up @@ -202,7 +202,9 @@ static void WriteNodeReport(Isolate* isolate,

writer.json_arraystart("libuv");
if (env != nullptr) {
uv_walk(env->event_loop(), WalkHandle, static_cast<void*>(&writer));
uv_walk(env->event_loop(),
exclude_network ? WalkHandleNoNetwork : WalkHandleNetwork,
static_cast<void*>(&writer));

writer.json_start();
writer.json_keyvalue("type", "loop");
Expand Down
3 changes: 2 additions & 1 deletion src/node_report.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
namespace node {
namespace report {
// Function declarations - utility functions in src/node_report_utils.cc
void WalkHandle(uv_handle_t* h, void* arg);
void WalkHandleNetwork(uv_handle_t* h, void* arg);
void WalkHandleNoNetwork(uv_handle_t* h, void* arg);

template <typename T>
std::string ValueToHexString(T value) {
Expand Down
55 changes: 35 additions & 20 deletions src/node_report_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,43 +12,51 @@ static constexpr auto null = JSONWriter::Null{};
static void ReportEndpoint(uv_handle_t* h,
struct sockaddr* addr,
const char* name,
JSONWriter* writer) {
JSONWriter* writer,
bool exclude_network) {
if (addr == nullptr) {
writer->json_keyvalue(name, null);
return;
}

uv_getnameinfo_t endpoint;
char* host = nullptr;
char hostbuf[INET6_ADDRSTRLEN];
const int family = addr->sa_family;
const int port = ntohs(family == AF_INET ?
reinterpret_cast<sockaddr_in*>(addr)->sin_port :
reinterpret_cast<sockaddr_in6*>(addr)->sin6_port);

if (uv_getnameinfo(h->loop, &endpoint, nullptr, addr, NI_NUMERICSERV) == 0) {
writer->json_objectstart(name);
if (!exclude_network &&
uv_getnameinfo(h->loop, &endpoint, nullptr, addr, NI_NUMERICSERV) == 0) {
host = endpoint.host;
DCHECK_EQ(port, std::stoi(endpoint.service));
writer->json_keyvalue("host", host);
}

if (family == AF_INET) {
char ipbuf[INET_ADDRSTRLEN];
if (uv_ip4_name(
reinterpret_cast<sockaddr_in*>(addr), ipbuf, sizeof(ipbuf)) == 0) {
writer->json_keyvalue("ip4", ipbuf);
Tofandel marked this conversation as resolved.
Show resolved Hide resolved
if (host == nullptr) writer->json_keyvalue("host", ipbuf);
}
} else {
const void* src = family == AF_INET ?
static_cast<void*>(
&(reinterpret_cast<sockaddr_in*>(addr)->sin_addr)) :
static_cast<void*>(
&(reinterpret_cast<sockaddr_in6*>(addr)->sin6_addr));
if (uv_inet_ntop(family, src, hostbuf, sizeof(hostbuf)) == 0) {
host = hostbuf;
char ipbuf[INET6_ADDRSTRLEN];
if (uv_ip6_name(
reinterpret_cast<sockaddr_in6*>(addr), ipbuf, sizeof(ipbuf)) == 0) {
writer->json_keyvalue("ip6", ipbuf);
if (host == nullptr) writer->json_keyvalue("host", ipbuf);
}
}
writer->json_objectstart(name);
if (host != nullptr) {
writer->json_keyvalue("host", host);
}
writer->json_keyvalue("port", port);
writer->json_objectend();
}

// Utility function to format libuv socket information.
static void ReportEndpoints(uv_handle_t* h, JSONWriter* writer) {
static void ReportEndpoints(uv_handle_t* h,
JSONWriter* writer,
bool exclude_network) {
struct sockaddr_storage addr_storage;
struct sockaddr* addr = reinterpret_cast<sockaddr*>(&addr_storage);
uv_any_handle* handle = reinterpret_cast<uv_any_handle*>(h);
Expand All @@ -65,7 +73,8 @@ static void ReportEndpoints(uv_handle_t* h, JSONWriter* writer) {
default:
break;
}
ReportEndpoint(h, rc == 0 ? addr : nullptr, "localEndpoint", writer);
ReportEndpoint(
h, rc == 0 ? addr : nullptr, "localEndpoint", writer, exclude_network);

switch (h->type) {
case UV_UDP:
Expand All @@ -77,7 +86,8 @@ static void ReportEndpoints(uv_handle_t* h, JSONWriter* writer) {
default:
break;
}
ReportEndpoint(h, rc == 0 ? addr : nullptr, "remoteEndpoint", writer);
ReportEndpoint(
h, rc == 0 ? addr : nullptr, "remoteEndpoint", writer, exclude_network);
}

// Utility function to format libuv pipe information.
Expand Down Expand Up @@ -155,7 +165,7 @@ static void ReportPath(uv_handle_t* h, JSONWriter* writer) {
}

// Utility function to walk libuv handles.
void WalkHandle(uv_handle_t* h, void* arg) {
void WalkHandle(uv_handle_t* h, void* arg, bool exclude_network = false) {
const char* type = uv_handle_type_name(h->type);
JSONWriter* writer = static_cast<JSONWriter*>(arg);
uv_any_handle* handle = reinterpret_cast<uv_any_handle*>(h);
Expand All @@ -177,7 +187,7 @@ void WalkHandle(uv_handle_t* h, void* arg) {
break;
case UV_TCP:
case UV_UDP:
ReportEndpoints(h, writer);
ReportEndpoints(h, writer, exclude_network);
break;
case UV_NAMED_PIPE:
ReportPipeEndpoints(h, writer);
Expand Down Expand Up @@ -267,6 +277,11 @@ void WalkHandle(uv_handle_t* h, void* arg) {
}
writer->json_end();
}

void WalkHandleNetwork(uv_handle_t* h, void* arg) {
WalkHandle(h, arg, false);
}
void WalkHandleNoNetwork(uv_handle_t* h, void* arg) {
WalkHandle(h, arg, true);
}
} // namespace report
} // namespace node
2 changes: 1 addition & 1 deletion test/common/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ function _validateContent(report, fields = []) {
'glibcVersionRuntime', 'glibcVersionCompiler', 'cwd',
'reportVersion', 'networkInterfaces', 'threadId'];
checkForUnknownFields(header, headerFields);
assert.strictEqual(header.reportVersion, 3); // Increment as needed.
assert.strictEqual(header.reportVersion, 4); // Increment as needed.
assert.strictEqual(typeof header.event, 'string');
assert.strictEqual(typeof header.trigger, 'string');
assert(typeof header.filename === 'string' || header.filename === null);
Expand Down
57 changes: 57 additions & 0 deletions test/report/test-report-exclude-network.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';
require('../common');
const http = require('node:http');
const assert = require('node:assert');
const { spawnSync } = require('node:child_process');
const tmpdir = require('../common/tmpdir');
Expand Down Expand Up @@ -38,4 +39,60 @@ describe('report exclude network option', () => {
const report = process.report.getReport();
assert.strictEqual(report.header.networkInterfaces, undefined);
});

it('should not do DNS queries in libuv if exclude network', async () => {
const server = http.createServer(function(req, res) {
res.writeHead(200, { 'Content-Type': 'text/plain' });
res.end();
});
let ipv6Available = true;
const port = await new Promise((resolve) => server.listen(0, async () => {
await Promise.all([
fetch('http://127.0.0.1:' + server.address().port),
fetch('http://[::1]:' + server.address().port).catch(() => ipv6Available = false),
]);
resolve(server.address().port);
server.close();
}));
process.report.excludeNetwork = false;
let report = process.report.getReport();
let tcp = report.libuv.filter((uv) => uv.type === 'tcp' && uv.remoteEndpoint?.port === port);
assert.strictEqual(tcp.length, ipv6Available ? 2 : 1);
const findHandle = (local, ip4 = true) => {
return tcp.find(
({ [local ? 'localEndpoint' : 'remoteEndpoint']: ep }) =>
(ep[ip4 ? 'ip4' : 'ip6'] === (ip4 ? '127.0.0.1' : '::1')),
)?.[local ? 'localEndpoint' : 'remoteEndpoint'];
};
try {
// The reverse DNS of 127.0.0.1 can be a lot of things other than localhost
// it could resolve to the server name for instance
assert.notStrictEqual(findHandle(true)?.host, '127.0.0.1');
assert.notStrictEqual(findHandle(false)?.host, '127.0.0.1');

if (ipv6Available) {
assert.notStrictEqual(findHandle(true, false)?.host, '::1');
assert.notStrictEqual(findHandle(false, false)?.host, '::1');
}
} catch (e) {
throw new Error(e.message + ' in ' + JSON.stringify(tcp, null, 2));
}

process.report.excludeNetwork = true;
report = process.report.getReport();
tcp = report.libuv.filter((uv) => uv.type === 'tcp' && uv.remoteEndpoint?.port === port);

try {
assert.strictEqual(tcp.length, ipv6Available ? 2 : 1);
assert.strictEqual(findHandle(true)?.host, '127.0.0.1');
assert.strictEqual(findHandle(false)?.host, '127.0.0.1');

if (ipv6Available) {
assert.strictEqual(findHandle(true, false)?.host, '::1');
assert.strictEqual(findHandle(false, false)?.host, '::1');
}
} catch (e) {
throw new Error(e.message + ' in ' + JSON.stringify(tcp, null, 2));
}
});
});
Loading