net: make server.address() return an integer for family

aduh95 · aduh95 · commit 3a26db9697a0 · 2022-04-12T05:50:09.000+02:00
`dns.lookup` options only accepts integer for `family` options, having a string doesn't really make sense here. PR-URL: #41431 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
diff --git a/lib/net.js b/lib/net.js
@@ -760,7 +760,7 @@ protoGetter('remoteAddress', function remoteAddress() {
 });
 
 protoGetter('remoteFamily', function remoteFamily() {
-  return this._getpeername().family;
+  return `IPv${this._getpeername().family}`;
 });
 
 protoGetter('remotePort', function remotePort() {
diff --git a/lib/os.js b/lib/os.js
@@ -216,7 +216,7 @@ function getCIDR(address, netmask, family) {
   let groupLength = 8;
   let hasZeros = false;
 
-  if (family === 'IPv6') {
+  if (family === 6) {
     split = ':';
     range = 16;
     groupLength = 16;
@@ -248,7 +248,7 @@ function getCIDR(address, netmask, family) {
  * @returns {Record<string, Array<{
  *  address: string
  *  netmask: string
- *  family: 'IPv4' | 'IPv6'
+ *  family: 4 | 6
  *  mac: string
  *  internal: boolean
  *  scopeid: number
diff --git a/src/env.h b/src/env.h
@@ -289,8 +289,6 @@ constexpr size_t kFsStatsBufferLength =
   V(input_string, "input")                                                     \
   V(internal_binding_string, "internalBinding")                                \
   V(internal_string, "internal")                                               \
-  V(ipv4_string, "IPv4")                                                       \
-  V(ipv6_string, "IPv6")                                                       \
   V(isclosing_string, "isClosing")                                             \
   V(issuer_string, "issuer")                                                   \
   V(issuercert_string, "issuerCertificate")                                    \
diff --git a/src/node_os.cc b/src/node_os.cc
@@ -174,7 +174,8 @@ static void GetInterfaceAddresses(const FunctionCallbackInfo<Value>& args) {
   char ip[INET6_ADDRSTRLEN];
   char netmask[INET6_ADDRSTRLEN];
   std::array<char, 18> mac;
-  Local<String> name, family;
+  Local<String> name;
+  Local<Integer> family;
 
   int err = uv_interface_addresses(&interfaces, &count);
 
@@ -214,14 +215,14 @@ static void GetInterfaceAddresses(const FunctionCallbackInfo<Value>& args) {
     if (interfaces[i].address.address4.sin_family == AF_INET) {
       uv_ip4_name(&interfaces[i].address.address4, ip, sizeof(ip));
       uv_ip4_name(&interfaces[i].netmask.netmask4, netmask, sizeof(netmask));
-      family = env->ipv4_string();
+      family = Integer::New(env->isolate(), 4);
     } else if (interfaces[i].address.address4.sin_family == AF_INET6) {
       uv_ip6_name(&interfaces[i].address.address6, ip, sizeof(ip));
       uv_ip6_name(&interfaces[i].netmask.netmask6, netmask, sizeof(netmask));
-      family = env->ipv6_string();
+      family = Integer::New(env->isolate(), 6);
     } else {
       strncpy(ip, "<unknown sa family>", INET6_ADDRSTRLEN);
-      family = env->unknown_string();
+      family = Integer::New(env->isolate(), 0);
     }
 
     result.emplace_back(name);
diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc
@@ -380,7 +380,7 @@ MaybeLocal<Object> AddressToJS(Environment* env,
               OneByteString(env->isolate(), ip)).Check();
     info->Set(env->context(),
               env->family_string(),
-              env->ipv6_string()).Check();
+              Integer::New(env->isolate(), 6)).Check();
     info->Set(env->context(),
               env->port_string(),
               Integer::New(env->isolate(), port)).Check();
@@ -395,7 +395,7 @@ MaybeLocal<Object> AddressToJS(Environment* env,
               OneByteString(env->isolate(), ip)).Check();
     info->Set(env->context(),
               env->family_string(),
-              env->ipv4_string()).Check();
+              Integer::New(env->isolate(), 4)).Check();
     info->Set(env->context(),
               env->port_string(),
               Integer::New(env->isolate(), port)).Check();
diff --git a/test/common/index.js b/test/common/index.js
@@ -845,7 +845,7 @@ const common = {
     const re = isWindows ? /Loopback Pseudo-Interface/ : /lo/;
     return Object.keys(iFaces).some((name) => {
       return re.test(name) &&
-             iFaces[name].some(({ family }) => family === 'IPv6');
+             iFaces[name].some(({ family }) => family === 6);
     });
   },
 
diff --git a/test/common/udppair.js b/test/common/udppair.js
@@ -16,7 +16,7 @@ class FakeUDPWrap extends EventEmitter {
     this._handle.onwrite =
       (wrap, buffers, addr) => this._write(wrap, buffers, addr);
     this._handle.getsockname = (obj) => {
-      Object.assign(obj, { address: '127.0.0.1', family: 'IPv4', port: 1337 });
+      Object.assign(obj, { address: '127.0.0.1', family: 4, port: 1337 });
       return 0;
     };
 
@@ -72,8 +72,8 @@ class FakeUDPWrap extends EventEmitter {
 
     let familyInt;
     switch (family) {
-      case 'IPv4': familyInt = 4; break;
-      case 'IPv6': familyInt = 6; break;
+      case 4: familyInt = 4; break;
+      case 6: familyInt = 6; break;
       default: throw new Error('bad family');
     }
 
diff --git a/test/es-module/test-http-imports.mjs b/test/es-module/test-http-imports.mjs
@@ -39,10 +39,10 @@ const internalInterfaces = Object.values(os.networkInterfaces()).flat().filter(
 );
 for (const iface of internalInterfaces) {
   testListeningOptions.push({
-    hostname: iface?.family === 'IPv6' ? `[${iface?.address}]` : iface?.address,
+    hostname: iface?.family === 6 ? `[${iface.address}]` : iface?.address,
     listenOptions: {
       host: iface?.address,
-      ipv6Only: iface?.family === 'IPv6'
+      ipv6Only: iface?.family === 6
     }
   });
 }
diff --git a/test/internet/test-dgram-broadcast-multi-process.js b/test/internet/test-dgram-broadcast-multi-process.js
@@ -47,7 +47,7 @@ get_bindAddress: for (const name in networkInterfaces) {
   const interfaces = networkInterfaces[name];
   for (let i = 0; i < interfaces.length; i++) {
     const localInterface = interfaces[i];
-    if (!localInterface.internal && localInterface.family === 'IPv4') {
+    if (!localInterface.internal && localInterface.family === 4) {
       bindAddress = localInterface.address;
       break get_bindAddress;
     }
diff --git a/test/internet/test-dgram-multicast-set-interface-lo.js b/test/internet/test-dgram-multicast-set-interface-lo.js
@@ -49,7 +49,7 @@ const TMPL = (tail) => `${NOW} - ${tail}`;
 const interfaceAddress = ((networkInterfaces) => {
   for (const name in networkInterfaces) {
     for (const localInterface of networkInterfaces[name]) {
-      if (!localInterface.internal && localInterface.family === FAM) {
+      if (!localInterface.internal && `IPv${localInterface.family}` === FAM) {
         let interfaceAddress = localInterface.address;
         // On Windows, IPv6 would need: `%${localInterface.scopeid}`
         if (FAM === 'IPv6')
diff --git a/test/internet/test-dgram-multicast-ssm-multi-process.js b/test/internet/test-dgram-multicast-ssm-multi-process.js
@@ -28,7 +28,7 @@ get_sourceAddress: for (const name in networkInterfaces) {
   const interfaces = networkInterfaces[name];
   for (let i = 0; i < interfaces.length; i++) {
     const localInterface = interfaces[i];
-    if (!localInterface.internal && localInterface.family === 'IPv4') {
+    if (!localInterface.internal && localInterface.family === 4) {
       sourceAddress = localInterface.address;
       break get_sourceAddress;
     }
diff --git a/test/internet/test-dgram-multicast-ssmv6-multi-process.js b/test/internet/test-dgram-multicast-ssmv6-multi-process.js
@@ -28,7 +28,7 @@ get_sourceAddress: for (const name in networkInterfaces) {
   const interfaces = networkInterfaces[name];
   for (let i = 0; i < interfaces.length; i++) {
     const localInterface = interfaces[i];
-    if (!localInterface.internal && localInterface.family === 'IPv6') {
+    if (!localInterface.internal && localInterface.family === 6) {
       sourceAddress = localInterface.address;
       break get_sourceAddress;
     }
diff --git a/test/internet/test-dns-ipv4.js b/test/internet/test-dns-ipv4.js
@@ -1,3 +1,5 @@
+// Flags: --dns-result-order=ipv4first
+
 'use strict';
 const common = require('../common');
 const { addresses } = require('../common/internet');
diff --git a/test/parallel/test-dgram-address.js b/test/parallel/test-dgram-address.js
@@ -35,7 +35,7 @@ const dgram = require('dgram');
     assert.strictEqual(typeof address.port, 'number');
     assert.ok(isFinite(address.port));
     assert.ok(address.port > 0);
-    assert.strictEqual(address.family, 'IPv4');
+    assert.strictEqual(address.family, 4);
     socket.close();
   }));
 
@@ -59,7 +59,7 @@ if (common.hasIPv6) {
     assert.strictEqual(typeof address.port, 'number');
     assert.ok(isFinite(address.port));
     assert.ok(address.port > 0);
-    assert.strictEqual(address.family, 'IPv6');
+    assert.strictEqual(address.family, 6);
     socket.close();
   }));
 
diff --git a/test/parallel/test-net-listen-invalid-port.js b/test/parallel/test-net-listen-invalid-port.js
@@ -12,7 +12,7 @@ const invalidPort = -1 >>> 0;
 
 net.Server().listen(0, function() {
   const address = this.address();
-  const key = `${address.family.slice(-1)}:${address.address}:0`;
+  const key = `${address.family}:${address.address}:0`;
 
   assert.strictEqual(this._connectionKey, key);
   this.close();
diff --git a/test/parallel/test-net-socket-connect-without-cb.js b/test/parallel/test-net-socket-connect-without-cb.js
@@ -16,5 +16,11 @@ const server = net.createServer(common.mustCall(function(conn) {
     client.end();
   }));
 
-  client.connect(server.address());
+  const address = server.address();
+  if (!common.hasIPv6 && address.family === 6) {
+    // Necessary to pass CI running inside containers.
+    client.connect(address.port);
+  } else {
+    client.connect(address);
+  }
 }));
diff --git a/test/parallel/test-net-socket-ready-without-cb.js b/test/parallel/test-net-socket-ready-without-cb.js
@@ -9,7 +9,7 @@ const net = require('net');
 const server = net.createServer(common.mustCall(function(conn) {
   conn.end();
   server.close();
-})).listen(0, common.mustCall(function() {
+})).listen(0, 'localhost', common.mustCall(function() {
   const client = new net.Socket();
 
   client.on('ready', common.mustCall(function() {
diff --git a/test/parallel/test-os.js b/test/parallel/test-os.js
@@ -138,7 +138,7 @@ switch (platform) {
     const expected = [{
       address: '127.0.0.1',
       netmask: '255.0.0.0',
-      family: 'IPv4',
+      family: 4,
       mac: '00:00:00:00:00:00',
       internal: true,
       cidr: '127.0.0.1/8'
@@ -154,7 +154,7 @@ switch (platform) {
     const expected = [{
       address: '127.0.0.1',
       netmask: '255.0.0.0',
-      family: 'IPv4',
+      family: 4,
       mac: '00:00:00:00:00:00',
       internal: true,
       cidr: '127.0.0.1/8'
diff --git a/test/parallel/test-tcp-wrap-listen.js b/test/parallel/test-tcp-wrap-listen.js
@@ -18,9 +18,9 @@ const r = (common.hasIPv6 ?
   server.bind6('::', 0) :
   server.bind('0.0.0.0', 0));
 assert.strictEqual(r, 0);
-let port = {};
+
+const port = {};
 server.getsockname(port);
-port = port.port;
 
 server.listen(128);
 
diff --git a/test/sequential/test-net-server-address.js b/test/sequential/test-net-server-address.js
@@ -26,7 +26,7 @@ const net = require('net');
 
 // Test on IPv4 Server
 {
-  const family = 'IPv4';
+  const family = 4;
   const server = net.createServer();
 
   server.on('error', common.mustNotCall());
@@ -46,7 +46,7 @@ if (!common.hasIPv6) {
   return;
 }
 
-const family6 = 'IPv6';
+const family6 = 6;
 const anycast6 = '::';
 
 // Test on IPv6 Server
diff --git a/test/sequential/test-net-server-bind.js b/test/sequential/test-net-server-bind.js
@@ -24,7 +24,7 @@ const net = require('net');
     const address = server.address();
     assert.strictEqual(address.port, common.PORT);
 
-    if (address.family === 'IPv6')
+    if (address.family === 6)
       assert.strictEqual(server._connectionKey, `6::::${address.port}`);
     else
       assert.strictEqual(server._connectionKey, `4:0.0.0.0:${address.port}`);

Original file line number	Diff line number	Diff line change
`@@ -39,10 +39,10 @@ const internalInterfaces = Object.values(os.networkInterfaces()).flat().filter(`
`39`	`39`	`);`
`40`	`40`	`for (const iface of internalInterfaces) {`
`41`	`41`	`testListeningOptions.push({`
`42`		- hostname: iface?.family === 'IPv6' ? `[${iface?.address}]` : iface?.address,
	`42`	+ hostname: iface?.family === 6 ? `[${iface.address}]` : iface?.address,
`43`	`43`	`listenOptions: {`
`44`	`44`	`host: iface?.address,`
`45`		`- ipv6Only: iface?.family === 'IPv6'`
	`45`	`+ ipv6Only: iface?.family === 6`
`46`	`46`	`}`
`47`	`47`	`});`
`48`	`48`	`}`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ get_bindAddress: for (const name in networkInterfaces) {`
`47`	`47`	`const interfaces = networkInterfaces[name];`
`48`	`48`	`for (let i = 0; i < interfaces.length; i++) {`
`49`	`49`	`const localInterface = interfaces[i];`
`50`		`- if (!localInterface.internal && localInterface.family === 'IPv4') {`
	`50`	`+ if (!localInterface.internal && localInterface.family === 4) {`
`51`	`51`	`bindAddress = localInterface.address;`
`52`	`52`	`break get_bindAddress;`
`53`	`53`	`}`