Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert the change of network interfaces family from string to integer #43054

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion doc/api/dgram.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.

Expand Down
16 changes: 11 additions & 5 deletions doc/api/dns.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
10 changes: 8 additions & 2 deletions doc/api/net.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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`

Expand Down
15 changes: 9 additions & 6 deletions doc/api/os.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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`.
Expand All @@ -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,
Expand All @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion doc/api/tls.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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`

Expand Down
14 changes: 12 additions & 2 deletions lib/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,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');
Expand Down
4 changes: 1 addition & 3 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -799,9 +799,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() {
Expand Down
4 changes: 2 additions & 2 deletions lib/os.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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") \
Expand Down
9 changes: 4 additions & 5 deletions src/node_os.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down
8 changes: 2 additions & 6 deletions src/tcp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
},

Expand Down
6 changes: 3 additions & 3 deletions test/common/udppair.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down Expand Up @@ -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');
}

Expand Down
4 changes: 2 additions & 2 deletions test/es-module/test-http-imports.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
});
}
Expand Down
2 changes: 1 addition & 1 deletion test/internet/test-dgram-broadcast-multi-process.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion test/internet/test-dgram-multicast-set-interface-lo.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
2 changes: 1 addition & 1 deletion test/internet/test-dgram-multicast-ssm-multi-process.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion test/internet/test-dgram-multicast-ssmv6-multi-process.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion test/internet/test-dns-lookup.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' }
Expand Down
8 changes: 8 additions & 0 deletions test/parallel/parallel.status
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading