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

PR-URL: #55602 Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
nodejs · Nov 16, 2024 · 5a437c4 · 5a437c4
1 parent e0db9ed
commit 5a437c4
Show file tree

Hide file tree

Showing 6 changed files with 147 additions and 28 deletions.
diff --git 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
@@ -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;
@@ -205,7 +205,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");

diff --git 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 <typename T>
 std::string ValueToHexString(T value) {

diff --git a/src/node_report_utils.cc b/src/node_report_utils.cc
@@ -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);
+      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);
@@ -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<JSONWriter*>(arg);
   uv_any_handle* handle = reinterpret_cast<uv_any_handle*>(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
@@ -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
@@ -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 });
+    }
+  });
 });