From 2310409a89d2d76fdc87289b99011750203b79fb Mon Sep 17 00:00:00 2001 From: Adrien Foulon <6115458+Tofandel@users.noreply.github.com> Date: Tue, 12 Nov 2024 16:57:59 +0100 Subject: [PATCH] report: fix network queries in getReport libuv with exclude-network MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/55602 Reviewed-By: Ethan Arrowood Reviewed-By: Gireesh Punathil Reviewed-By: Chengzhong Wu Reviewed-By: Vinícius Lourenço Claro Cardoso --- doc/api/report.md | 52 ++++++++++++++++++-- src/node_report.cc | 6 ++- src/node_report.h | 3 +- src/node_report_utils.cc | 57 ++++++++++++++-------- test/common/report.js | 2 +- test/report/test-report-exclude-network.js | 57 ++++++++++++++++++++++ 6 files changed, 148 insertions(+), 29 deletions(-) diff --git a/doc/api/report.md b/doc/api/report.md index babd698d183..a9711b39fd5 100644 --- a/doc/api/report.md +++ b/doc/api/report.md @@ -35,7 +35,7 @@ is provided below for reference. ```json { "header": { - "reportVersion": 3, + "reportVersion": 4, "event": "exception", "trigger": "Exception", "filename": "report.20181221.005011.8974.0.001.json", @@ -325,6 +325,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": [], @@ -464,9 +508,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. * `--report-exclude-env` Exclude `environmentVariables` from the diagnostic report. By default this is not set and the environment diff --git a/src/node_report.cc b/src/node_report.cc index 74271cca9c2..4f430ee2821 100644 --- a/src/node_report.cc +++ b/src/node_report.cc @@ -23,7 +23,7 @@ #include #include -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; @@ -205,7 +205,9 @@ static void WriteNodeReport(Isolate* isolate, writer.json_arraystart("libuv"); if (env != nullptr) { - uv_walk(env->event_loop(), WalkHandle, static_cast(&writer)); + uv_walk(env->event_loop(), + exclude_network ? WalkHandleNoNetwork : WalkHandleNetwork, + static_cast(&writer)); writer.json_start(); writer.json_keyvalue("type", "loop"); diff --git a/src/node_report.h b/src/node_report.h index 7a2e817ac82..98be339ae90 100644 --- a/src/node_report.h +++ b/src/node_report.h @@ -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 std::string ValueToHexString(T value) { diff --git a/src/node_report_utils.cc b/src/node_report_utils.cc index 516eac22dc6..d4eb52c1ed8 100644 --- a/src/node_report_utils.cc +++ b/src/node_report_utils.cc @@ -12,7 +12,8 @@ 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; @@ -20,35 +21,42 @@ static void ReportEndpoint(uv_handle_t* h, 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(addr)->sin_port : reinterpret_cast(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)); - } else { - const void* src = family == AF_INET ? - static_cast( - &(reinterpret_cast(addr)->sin_addr)) : - static_cast( - &(reinterpret_cast(addr)->sin6_addr)); - if (uv_inet_ntop(family, src, hostbuf, sizeof(hostbuf)) == 0) { - host = hostbuf; - } - } - writer->json_objectstart(name); - if (host != nullptr) { writer->json_keyvalue("host", host); } + + if (family == AF_INET) { + char ipbuf[INET_ADDRSTRLEN]; + if (uv_ip4_name( + reinterpret_cast(addr), ipbuf, sizeof(ipbuf)) == 0) { + writer->json_keyvalue("ip4", ipbuf); + if (host == nullptr) writer->json_keyvalue("host", ipbuf); + } + } else { + char ipbuf[INET6_ADDRSTRLEN]; + if (uv_ip6_name( + reinterpret_cast(addr), ipbuf, sizeof(ipbuf)) == 0) { + writer->json_keyvalue("ip6", ipbuf); + if (host == nullptr) writer->json_keyvalue("host", ipbuf); + } + } 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(&addr_storage); uv_any_handle* handle = reinterpret_cast(h); @@ -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: @@ -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. @@ -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(arg); uv_any_handle* handle = reinterpret_cast(h); @@ -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); @@ -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 diff --git a/test/common/report.js b/test/common/report.js index 02b403d934a..3280116feb8 100644 --- a/test/common/report.js +++ b/test/common/report.js @@ -110,7 +110,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); diff --git a/test/report/test-report-exclude-network.js b/test/report/test-report-exclude-network.js index c5e50135482..7d0eaa08997 100644 --- a/test/report/test-report-exclude-network.js +++ b/test/report/test-report-exclude-network.js @@ -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'); @@ -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), { cause: e }); + } + + 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), { cause: e }); + } + }); });