From 3da21d4babf392e87be5b75416101d75184d0378 Mon Sep 17 00:00:00 2001 From: Tofandel Date: Thu, 31 Oct 2024 00:29:40 +0100 Subject: [PATCH 1/9] report: fix network queries in getReport libuv with exclude-network --- src/node_report.cc | 4 +- src/node_report.h | 3 +- src/node_report_utils.cc | 55 ++++++++++++++-------- test/report/test-report-exclude-network.js | 51 ++++++++++++++++++++ 4 files changed, 91 insertions(+), 22 deletions(-) diff --git a/src/node_report.cc b/src/node_report.cc index cffb9dd1b655af..523e08c24e3f9a 100644 --- a/src/node_report.cc +++ b/src/node_report.cc @@ -202,7 +202,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 7a2e817ac82f6b..98be339ae90d8f 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 516eac22dc63a2..d4eb52c1ed89c0 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)); + 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 { - 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; + 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_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(&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/report/test-report-exclude-network.js b/test/report/test-report-exclude-network.js index c5e50135482f1a..78cd13792b787a 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,54 @@ 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(); + }); + 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), + ]); + 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, 2); + const findHandle = (host, local = true, ip4 = true) => { + return tcp.some( + ({ [local ? 'localEndpoint' : 'remoteEndpoint']: ep }) => + (ep[ip4 ? 'ip4' : 'ip6'] === (ip4 ? '127.0.0.1' : '::1') && + (Array.isArray(host) ? host.includes(ep.host) : ep.host === host)), + ); + }; + try { + assert.ok(findHandle('localhost'), 'local localhost handle not found'); + assert.ok(findHandle('localhost', false), 'remote localhost handle not found'); + + assert.ok(findHandle(['localhost', 'ip6-localhost'], true, false), 'local ip6-localhost handle not found'); + assert.ok(findHandle(['localhost', 'ip6-localhost'], false, false), 'remote ip6-localhost handle not found'); + } 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, 2); + assert.ok(findHandle('127.0.0.1'), 'local 127.0.0.1 handle not found'); + assert.ok(findHandle('127.0.0.1', false), 'remote 127.0.0.1 handle not found'); + + assert.ok(findHandle('::1', true, false), 'local ::1 handle not found'); + assert.ok(findHandle('::1', false, false), 'remote ::1 handle not found'); + } catch (e) { + throw new Error(e.message + ' in ' + JSON.stringify(tcp, null, 2)); + } + }); }); From 83420e0933796658b349551d81aa819b59a94ef4 Mon Sep 17 00:00:00 2001 From: Tofandel Date: Thu, 31 Oct 2024 21:01:27 +0100 Subject: [PATCH 2/9] Don't hardcode hostname or that leads to flaky test depending on where it runs and skip ipv6 if not supported --- test/report/test-report-exclude-network.js | 38 +++++++++++++--------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/test/report/test-report-exclude-network.js b/test/report/test-report-exclude-network.js index 78cd13792b787a..b4d4c7b18942f2 100644 --- a/test/report/test-report-exclude-network.js +++ b/test/report/test-report-exclude-network.js @@ -45,10 +45,11 @@ describe('report exclude network option', () => { 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), + fetch('http://[::1]:' + server.address().port).catch(() => ipv6Available = false), ]); resolve(server.address().port); server.close(); @@ -56,20 +57,23 @@ describe('report exclude network option', () => { 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, 2); - const findHandle = (host, local = true, ip4 = true) => { - return tcp.some( + 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') && - (Array.isArray(host) ? host.includes(ep.host) : ep.host === host)), - ); + (ep[ip4 ? 'ip4' : 'ip6'] === (ip4 ? '127.0.0.1' : '::1')), + )?.[local ? 'localEndpoint' : 'remoteEndpoint']; }; try { - assert.ok(findHandle('localhost'), 'local localhost handle not found'); - assert.ok(findHandle('localhost', false), 'remote localhost handle not found'); + // 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'); - assert.ok(findHandle(['localhost', 'ip6-localhost'], true, false), 'local ip6-localhost handle not found'); - assert.ok(findHandle(['localhost', 'ip6-localhost'], false, false), 'remote ip6-localhost handle not found'); + 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)); } @@ -79,12 +83,14 @@ describe('report exclude network option', () => { tcp = report.libuv.filter((uv) => uv.type === 'tcp' && uv.remoteEndpoint?.port === port); try { - assert.strictEqual(tcp.length, 2); - assert.ok(findHandle('127.0.0.1'), 'local 127.0.0.1 handle not found'); - assert.ok(findHandle('127.0.0.1', false), 'remote 127.0.0.1 handle not found'); + 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'); - assert.ok(findHandle('::1', true, false), 'local ::1 handle not found'); - assert.ok(findHandle('::1', false, false), 'remote ::1 handle not found'); + 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)); } From a7c18c3d560c661aba7bbed8d171d035fb24e757 Mon Sep 17 00:00:00 2001 From: Adrien Foulon <6115458+Tofandel@users.noreply.github.com> Date: Wed, 6 Nov 2024 12:30:37 +0100 Subject: [PATCH 3/9] Bump node report version --- src/node_report.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_report.cc b/src/node_report.cc index 523e08c24e3f9a..2fd4915f3d7e03 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; From f18b125cdf5d90e471da9cc78dd845636db6a6ac Mon Sep 17 00:00:00 2001 From: Adrien Foulon <6115458+Tofandel@users.noreply.github.com> Date: Wed, 6 Nov 2024 13:07:57 +0100 Subject: [PATCH 4/9] Update documentation --- doc/api/report.md | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/doc/api/report.md b/doc/api/report.md index 19a39400ff9bee..a73757f6969704 100644 --- a/doc/api/report.md +++ b/doc/api/report.md @@ -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", @@ -322,6 +322,34 @@ 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": [], @@ -461,7 +489,7 @@ 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 +* `--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. From f4da7aec766c8782ab8200fb51105342273a1678 Mon Sep 17 00:00:00 2001 From: Adrien Foulon <6115458+Tofandel@users.noreply.github.com> Date: Wed, 6 Nov 2024 13:10:45 +0100 Subject: [PATCH 5/9] Fix json --- doc/api/report.md | 64 +++++++++++++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 24 deletions(-) diff --git a/doc/api/report.md b/doc/api/report.md index a73757f6969704..41b1d0179262fb 100644 --- a/doc/api/report.md +++ b/doc/api/report.md @@ -324,32 +324,48 @@ is provided below for reference. "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": "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 + "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": [], From 0ab08f01cb6ed098864ba37bf638c2e516813e30 Mon Sep 17 00:00:00 2001 From: Adrien Foulon <6115458+Tofandel@users.noreply.github.com> Date: Wed, 6 Nov 2024 13:17:46 +0100 Subject: [PATCH 6/9] Fix report version in test --- test/common/report.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/report.js b/test/common/report.js index 6fce96590c3f54..f7093148ef844c 100644 --- a/test/common/report.js +++ b/test/common/report.js @@ -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); From d7966355643834c8f34960b626c43302ef0da107 Mon Sep 17 00:00:00 2001 From: Adrien Foulon <6115458+Tofandel@users.noreply.github.com> Date: Wed, 6 Nov 2024 17:39:40 +0100 Subject: [PATCH 7/9] Lint md --- doc/api/report.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/api/report.md b/doc/api/report.md index 41b1d0179262fb..24d6d58a81cdc9 100644 --- a/doc/api/report.md +++ b/doc/api/report.md @@ -505,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` 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-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: From a0ce11fde9948492addee8524aecba6076febb02 Mon Sep 17 00:00:00 2001 From: Adrien Foulon <6115458+Tofandel@users.noreply.github.com> Date: Thu, 7 Nov 2024 18:39:30 +0100 Subject: [PATCH 8/9] Update test/report/test-report-exclude-network.js Co-authored-by: Antoine du Hamel --- test/report/test-report-exclude-network.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/report/test-report-exclude-network.js b/test/report/test-report-exclude-network.js index b4d4c7b18942f2..4f8517c436349d 100644 --- a/test/report/test-report-exclude-network.js +++ b/test/report/test-report-exclude-network.js @@ -75,7 +75,7 @@ describe('report exclude network option', () => { assert.notStrictEqual(findHandle(false, false)?.host, '::1'); } } catch (e) { - throw new Error(e.message + ' in ' + JSON.stringify(tcp, null, 2)); + throw new Error(e?.message + ' in ' + JSON.stringify(tcp, null, 2), { cause: e }); } process.report.excludeNetwork = true; From 2165c6c578ace4b2813b9e06588c423f59de772b Mon Sep 17 00:00:00 2001 From: Adrien Foulon <6115458+Tofandel@users.noreply.github.com> Date: Thu, 7 Nov 2024 18:41:13 +0100 Subject: [PATCH 9/9] Update test/report/test-report-exclude-network.js --- test/report/test-report-exclude-network.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/report/test-report-exclude-network.js b/test/report/test-report-exclude-network.js index 4f8517c436349d..7d0eaa08997cb5 100644 --- a/test/report/test-report-exclude-network.js +++ b/test/report/test-report-exclude-network.js @@ -92,7 +92,7 @@ describe('report exclude network option', () => { assert.strictEqual(findHandle(false, false)?.host, '::1'); } } catch (e) { - throw new Error(e.message + ' in ' + JSON.stringify(tcp, null, 2)); + throw new Error(e?.message + ' in ' + JSON.stringify(tcp, null, 2), { cause: e }); } }); });