dns: accept 'IPv4' and 'IPv6' for family

aduh95 · danielleadams · commit 0bb84b09a55e · 2022-06-15T21:28:47.000-04:00
Refs: #43014 PR-URL: #43054 Fixes: #43014 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Beth Griggs <bgriggs@redhat.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
diff --git a/doc/api/dgram.md b/doc/api/dgram.md
@@ -114,6 +114,9 @@ exist and calls such as `socket.address()` and `socket.setTTL()` will fail.
 <!-- YAML
 added: v0.1.99
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/43054
+    description: The `family` property now returns a string instead of a number.
   - version: v18.0.0
     pr-url: https://github.com/nodejs/node/pull/41431
     description: The `family` property now returns a number instead of a string.
@@ -125,7 +128,7 @@ The event handler function is passed two arguments: `msg` and `rinfo`.
 * `msg` {Buffer} The message.
 * `rinfo` {Object} Remote address information.
   * `address` {string} The sender address.
-  * `family` {number} The address family (`4` for IPv4 or `6` for IPv6).
+  * `family` {string} The address family (`'IPv4'` or `'IPv6'`).
   * `port` {number} The sender port.
   * `size` {number} The message size.
 
diff --git a/doc/api/dns.md b/doc/api/dns.md
@@ -179,6 +179,11 @@ section if a custom port is used.
 <!-- YAML
 added: v0.1.90
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/43054
+    description: For compatibility with `node:net`, when passing an option
+                 object the `family` option can be the string `'IPv4'` or the
+                 string `'IPv6'`.
   - version: v18.0.0
     pr-url: https://github.com/nodejs/node/pull/41678
     description: Passing an invalid callback to the `callback` argument
@@ -197,9 +202,10 @@ changes:
 
 * `hostname` {string}
 * `options` {integer | Object}
-  * `family` {integer} The record family. Must be `4`, `6`, or `0`. The value
-    `0` indicates that IPv4 and IPv6 addresses are both returned. **Default:**
-    `0`.
+  * `family` {integer|string} The record family. Must be `4`, `6`, or `0`. For
+    backward compatibility reasons,`'IPv4'` and `'IPv6'` are interpreted as `4`
+    and `6` respectively. The value `0` indicates that IPv4 and IPv6 addresses
+    are both returned. **Default:** `0`.
   * `hints` {number} One or more [supported `getaddrinfo` flags][]. Multiple
     flags may be passed by bitwise `OR`ing their values.
   * `all` {boolean} When `true`, the callback returns all resolved addresses in
@@ -219,8 +225,8 @@ changes:
 
 Resolves a host name (e.g. `'nodejs.org'`) into the first found A (IPv4) or
 AAAA (IPv6) record. All `option` properties are optional. If `options` is an
-integer, then it must be `4` or `6` – if `options` is not provided, then IPv4
-and IPv6 addresses are both returned if found.
+integer, then it must be `4` or `6` – if `options` is `0` or not provided, then
+IPv4 and IPv6 addresses are both returned if found.
 
 With the `all` option set to `true`, the arguments for `callback` change to
 `(err, addresses)`, with `addresses` being an array of objects with the
diff --git a/doc/api/net.md b/doc/api/net.md
@@ -286,6 +286,9 @@ Emitted when the server has been bound after calling [`server.listen()`][].
 <!-- YAML
 added: v0.1.90
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/43054
+    description: The `family` property now returns a string instead of a number.
   - version: v18.0.0
     pr-url: https://github.com/nodejs/node/pull/41431
     description: The `family` property now returns a number instead of a string.
@@ -296,7 +299,7 @@ changes:
 Returns the bound `address`, the address `family` name, and `port` of the server
 as reported by the operating system if listening on an IP socket
 (useful to find which port was assigned when getting an OS-assigned address):
-`{ port: 12346, family: 4, address: '127.0.0.1' }`.
+`{ port: 12346, family: 'IPv4', address: '127.0.0.1' }`.
 
 For a server listening on a pipe or Unix domain socket, the name is returned
 as a string.
@@ -743,6 +746,9 @@ See also: [`socket.setTimeout()`][].
 <!-- YAML
 added: v0.1.90
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/43054
+    description: The `family` property now returns a string instead of a number.
   - version: v18.0.0
     pr-url: https://github.com/nodejs/node/pull/41431
     description: The `family` property now returns a number instead of a string.
@@ -752,7 +758,7 @@ changes:
 
 Returns the bound `address`, the address `family` name and `port` of the
 socket as reported by the operating system:
-`{ port: 12346, family: 4, address: '127.0.0.1' }`
+`{ port: 12346, family: 'IPv4', address: '127.0.0.1' }`
 
 ### `socket.bufferSize`
 
diff --git a/doc/api/os.md b/doc/api/os.md
@@ -225,6 +225,9 @@ always `[0, 0, 0]`.
 <!-- YAML
 added: v0.6.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/43054
+    description: The `family` property now returns a string instead of a number.
   - version: v18.0.0
     pr-url: https://github.com/nodejs/node/pull/41431
     description: The `family` property now returns a number instead of a string.
@@ -242,12 +245,12 @@ The properties available on the assigned network address object include:
 
 * `address` {string} The assigned IPv4 or IPv6 address
 * `netmask` {string} The IPv4 or IPv6 network mask
-* `family` {number} Either `4` (for IPv4) or `6` (for IPv6)
+* `family` {string} Either `IPv4` or `IPv6`
 * `mac` {string} The MAC address of the network interface
 * `internal` {boolean} `true` if the network interface is a loopback or
   similar interface that is not remotely accessible; otherwise `false`
 * `scopeid` {number} The numeric IPv6 scope ID (only specified when `family`
-  is `6`)
+  is `IPv6`)
 * `cidr` {string} The assigned IPv4 or IPv6 address with the routing prefix
   in CIDR notation. If the `netmask` is invalid, this property is set
   to `null`.
@@ -260,15 +263,15 @@ The properties available on the assigned network address object include:
     {
       address: '127.0.0.1',
       netmask: '255.0.0.0',
-      family: 4,
+      family: 'IPv4',
       mac: '00:00:00:00:00:00',
       internal: true,
       cidr: '127.0.0.1/8'
     },
     {
       address: '::1',
       netmask: 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff',
-      family: 6,
+      family: 'IPv6',
       mac: '00:00:00:00:00:00',
       scopeid: 0,
       internal: true,
@@ -279,15 +282,15 @@ The properties available on the assigned network address object include:
     {
       address: '192.168.1.108',
       netmask: '255.255.255.0',
-      family: 4,
+      family: 'IPv4',
       mac: '01:02:03:0a:0b:0c',
       internal: false,
       cidr: '192.168.1.108/24'
     },
     {
       address: 'fe80::a00:27ff:fe4e:66a1',
       netmask: 'ffff:ffff:ffff:ffff::',
-      family: 6,
+      family: 'IPv6',
       mac: '01:02:03:0a:0b:0c',
       scopeid: 1,
       internal: false,
diff --git a/doc/api/tls.md b/doc/api/tls.md
@@ -956,6 +956,9 @@ tlsSocket.once('session', (session) => {
 <!-- YAML
 added: v0.11.4
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/43054
+    description: The `family` property now returns a string instead of a number.
   - version: v18.0.0
     pr-url: https://github.com/nodejs/node/pull/41431
     description: The `family` property now returns a number instead of a string.
@@ -965,7 +968,7 @@ changes:
 
 Returns the bound `address`, the address `family` name, and `port` of the
 underlying socket as reported by the operating system:
-`{ port: 12346, family: 4, address: '127.0.0.1' }`.
+`{ port: 12346, family: 'IPv4', address: '127.0.0.1' }`.
 
 ### `tlsSocket.authorizationError`
 
diff --git a/lib/dns.js b/lib/dns.js
@@ -145,8 +145,18 @@ function lookup(hostname, options, callback) {
       validateHints(hints);
     }
     if (options?.family != null) {
-      validateOneOf(options.family, 'options.family', validFamilies, true);
-      family = options.family;
+      switch (options.family) {
+        case 'IPv4':
+          family = 4;
+          break;
+        case 'IPv6':
+          family = 6;
+          break;
+        default:
+          validateOneOf(options.family, 'options.family', validFamilies, true);
+          family = options.family;
+          break;
+      }
     }
     if (options?.all != null) {
       validateBoolean(options.all, 'options.all');
diff --git a/lib/net.js b/lib/net.js
@@ -800,9 +800,7 @@ protoGetter('remoteAddress', function remoteAddress() {
 });
 
 protoGetter('remoteFamily', function remoteFamily() {
-  const { family } = this._getpeername();
-
-  return family ? `IPv${family}` : family;
+  return 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 === 6) {
+  if (family === 'IPv6') {
     split = ':';
     range = 16;
     groupLength = 16;
@@ -248,7 +248,7 @@ function getCIDR(address, netmask, family) {
  * @returns {Record<string, Array<{
  *  address: string,
  *  netmask: string,
- *  family: 4 | 6,
+ *  family: 'IPv4' | 'IPv6',
  *  mac: string,
  *  internal: boolean,
  *  scopeid: number,
diff --git a/src/env.h b/src/env.h
@@ -301,6 +301,8 @@ class NoArrayBufferZeroFillScope {
   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,8 +174,7 @@ static void GetInterfaceAddresses(const FunctionCallbackInfo<Value>& args) {
   char ip[INET6_ADDRSTRLEN];
   char netmask[INET6_ADDRSTRLEN];
   std::array<char, 18> mac;
-  Local<String> name;
-  Local<Integer> family;
+  Local<String> name, family;
 
   int err = uv_interface_addresses(&interfaces, &count);
 
@@ -215,14 +214,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 = Integer::New(env->isolate(), 4);
+      family = env->ipv4_string();
     } 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 = Integer::New(env->isolate(), 6);
+      family = env->ipv6_string();
     } else {
       strncpy(ip, "<unknown sa family>", INET6_ADDRSTRLEN);
-      family = Integer::New(env->isolate(), 0);
+      family = env->unknown_string();
     }
 
     result.emplace_back(name);
diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc
@@ -402,9 +402,7 @@ MaybeLocal<Object> AddressToJS(Environment* env,
     info->Set(env->context(),
               env->address_string(),
               OneByteString(env->isolate(), ip)).Check();
-    info->Set(env->context(),
-              env->family_string(),
-              Integer::New(env->isolate(), 6)).Check();
+    info->Set(env->context(), env->family_string(), env->ipv6_string()).Check();
     info->Set(env->context(),
               env->port_string(),
               Integer::New(env->isolate(), port)).Check();
@@ -417,9 +415,7 @@ MaybeLocal<Object> AddressToJS(Environment* env,
     info->Set(env->context(),
               env->address_string(),
               OneByteString(env->isolate(), ip)).Check();
-    info->Set(env->context(),
-              env->family_string(),
-              Integer::New(env->isolate(), 4)).Check();
+    info->Set(env->context(), env->family_string(), env->ipv4_string()).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
@@ -854,7 +854,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 === 6);
+             iFaces[name].some(({ family }) => family === 'IPv6');
     });
   },
 
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: 4, port: 1337 });
+      Object.assign(obj, { address: '127.0.0.1', family: 'IPv4', port: 1337 });
       return 0;
     };
 
@@ -72,8 +72,8 @@ class FakeUDPWrap extends EventEmitter {
 
     let familyInt;
     switch (family) {
-      case 4: familyInt = 4; break;
-      case 6: familyInt = 6; break;
+      case 'IPv4': familyInt = 4; break;
+      case 'IPv6': 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 === 6 ? `[${iface.address}]` : iface?.address,
+    hostname: iface?.family === 'IPv6' ? `[${iface?.address}]` : iface?.address,
     listenOptions: {
       host: iface?.address,
-      ipv6Only: iface?.family === 6
+      ipv6Only: iface?.family === 'IPv6'
     }
   });
 }
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 === 4) {
+    if (!localInterface.internal && localInterface.family === 'IPv4') {
       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 && `IPv${localInterface.family}` === FAM) {
+      if (!localInterface.internal && 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 === 4) {
+    if (!localInterface.internal && localInterface.family === 'IPv4') {
       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 === 6) {
+    if (!localInterface.internal && localInterface.family === 'IPv6') {
       sourceAddress = localInterface.address;
       break get_sourceAddress;
     }
diff --git a/test/internet/test-dns-lookup.js b/test/internet/test-dns-lookup.js
@@ -47,7 +47,7 @@ dns.lookup(addresses.NOT_FOUND, {
 
 assert.throws(
   () => dnsPromises.lookup(addresses.NOT_FOUND, {
-    family: 'IPv4',
+    family: 'ipv4',
     all: 'all'
   }),
   { code: 'ERR_INVALID_ARG_VALUE' }
diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status
@@ -31,6 +31,10 @@ test-crypto-dh-stateless: SKIP
 test-crypto-keygen: SKIP
 
 [$system==solaris] # Also applies to SmartOS
+# https://github.com/nodejs/node/pull/43054
+test-net-socket-connect-without-cb: SKIP
+test-net-socket-ready-without-cb: SKIP
+test-tcp-wrap-listen: SKIP
 
 [$system==freebsd]
 # https://github.com/nodejs/node/issues/31727
@@ -39,6 +43,10 @@ test-fs-stat-bigint: PASS,FLAKY
 test-worker-message-port-message-before-close: PASS,FLAKY
 
 [$system==aix]
+# https://github.com/nodejs/node/pull/43054
+test-net-socket-connect-without-cb: SKIP
+test-net-socket-ready-without-cb: SKIP
+test-tcp-wrap-listen: SKIP
 
 [$system==ibmi]
 # https://github.com/nodejs/node/pull/30819
diff --git a/test/parallel/test-dgram-address.js b/test/parallel/test-dgram-address.js
diff --git a/test/parallel/test-dgram-udp6-link-local-address.js b/test/parallel/test-dgram-udp6-link-local-address.js
diff --git a/test/parallel/test-net-listen-invalid-port.js b/test/parallel/test-net-listen-invalid-port.js
diff --git a/test/parallel/test-net-socket-connect-without-cb.js b/test/parallel/test-net-socket-connect-without-cb.js
diff --git a/test/parallel/test-os.js b/test/parallel/test-os.js
diff --git a/test/sequential/test-net-server-address.js b/test/sequential/test-net-server-address.js
diff --git a/test/sequential/test-net-server-bind.js b/test/sequential/test-net-server-bind.js
