Skip to content

Commit

Permalink
quic: refactor and improve ipv6Only
Browse files Browse the repository at this point in the history
Ignore `ipv6Only: true` when binding to `'udp4'` (this differs from
dgram which will still attempt to apply the flag and will fail during
bind). Improve the test so that it should work consistently.

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #33935
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
jasnell committed Jun 22, 2020
1 parent 1b7434d commit c7d859e
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 26 deletions.
21 changes: 15 additions & 6 deletions doc/api/quic.md
Expand Up @@ -261,8 +261,11 @@ added: REPLACEME
to an IP address.
* `port` {number} The local port to bind to.
* `type` {string} Either `'udp4'` or `'upd6'` to use either IPv4 or IPv6,
respectively.
* `ipv6Only` {boolean}
respectively. **Default**: `'udp4'`.
* `ipv6Only` {boolean} If `type` is `'udp6'`, then setting `ipv6Only` to
`true` will disable dual-stack support on the UDP binding -- that is,
binding to address `'::'` will not make `'0.0.0.0'` be bound. The option
is ignored if `type` is `'udp4'`. **Default**: `false`.
* `lookup` {Function} A custom DNS lookup function. Default `dns.lookup()`.
* `maxConnections` {number} The maximum number of total active inbound
connections.
Expand Down Expand Up @@ -1387,8 +1390,11 @@ added: REPLACEME
to an IP address.
* `port` {number} The local port to bind to.
* `type` {string} Either `'udp4'` or `'upd6'` to use either IPv4 or IPv6,
respectively.
* `ipv6Only` {boolean}
respectively. **Default**: `'udp4'`.
* `ipv6Only` {boolean} If `type` is `'udp6'`, then setting `ipv6Only` to
`true` will disable dual-stack support on the UDP binding -- that is,
binding to address `'::'` will not make `'0.0.0.0'` be bound. The option
is ignored if `type` is `'udp4'`. **Default**: `false`.
* Returns: {QuicEndpoint}

Creates and adds a new `QuicEndpoint` to the `QuicSocket` instance.
Expand Down Expand Up @@ -1519,7 +1525,10 @@ added: REPLACEME
`SSL_OP_CIPHER_SERVER_PREFERENCE` to be set in `secureOptions`, see
[OpenSSL Options][] for more information.
* `idleTimeout` {number}
* `ipv6Only` {boolean}
* `ipv6Only` {boolean} If `type` is `'udp6'`, then setting `ipv6Only` to
`true` will disable dual-stack support on the UDP binding -- that is,
binding to address `'::'` will not make `'0.0.0.0'` be bound. The option
is ignored if `type` is `'udp4'`. **Default**: `false`.
* `key` {string|string[]|Buffer|Buffer[]|Object[]} Private keys in PEM format.
PEM allows the option of private keys being encrypted. Encrypted keys will
be decrypted with `options.passphrase`. Multiple keys using different
Expand Down Expand Up @@ -1578,7 +1587,7 @@ added: REPLACEME
`QuicClientSession` object.
* `type`: {string} Identifies the type of UDP socket. The value must either
be `'udp4'`, indicating UDP over IPv4, or `'udp6'`, indicating UDP over
IPv6. Defaults to `'udp4'`.
IPv6. **Default**: `'udp4'`.

Create a new `QuicClientSession`. This function can be called multiple times
to create sessions associated with different endpoints on the same
Expand Down
6 changes: 4 additions & 2 deletions lib/internal/quic/core.js
Expand Up @@ -667,6 +667,7 @@ class QuicEndpoint {
this.#lookup = lookup || (type === AF_INET6 ? lookup6 : lookup4);
this.#port = port;
this.#reuseAddr = !!reuseAddr;
this.#type = type;
this.#udpSocket = dgram.createSocket(type === AF_INET6 ? 'udp6' : 'udp4');

// kUDPHandleForTesting is only used in the Node.js test suite to
Expand All @@ -688,7 +689,7 @@ class QuicEndpoint {
const obj = {
address: this.address,
fd: this.#fd,
type: this.#type
type: this.#type === AF_INET6 ? 'udp6' : 'udp4'
};
return `QuicEndpoint ${util.format(obj)}`;
}
Expand Down Expand Up @@ -726,9 +727,10 @@ class QuicEndpoint {
// what conditions does this happen?
return;
}

const flags =
(this.#reuseAddr ? UV_UDP_REUSEADDR : 0) |
(this.#ipv6Only ? UV_UDP_IPV6ONLY : 0);
(this.#type === AF_INET6 && this.#ipv6Only ? UV_UDP_IPV6ONLY : 0);

const ret = udpHandle.bind(ip, this.#port, flags);
if (ret) {
Expand Down
44 changes: 26 additions & 18 deletions test/parallel/test-quic-ipv6only.js
Expand Up @@ -16,26 +16,24 @@ const { once } = require('events');

const kALPN = 'zzz';

// Setting `type` to `udp4` while setting `ipv6Only` to `true` is possible
// and it will throw an error.
{
// Setting `type` to `udp4` while setting `ipv6Only` to `true` is possible.
// The ipv6Only setting will be ignored.
async function ipv4() {
const server = createQuicSocket({
endpoint: {
type: 'udp4',
ipv6Only: true
} });

server.on('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'EINVAL');
assert.strictEqual(err.message, 'bind EINVAL 0.0.0.0');
}));

}
});
server.on('error', common.mustNotCall());
server.listen({ key, cert, ca, alpn: kALPN });
await once(server, 'ready');
server.close();
}

// Connecting ipv6 server by "127.0.0.1" should work when `ipv6Only`
// is set to `false`.
(async () => {
// Connecting to ipv6 server using "127.0.0.1" should work when
// `ipv6Only` is set to `false`.
async function ipv6() {
const server = createQuicSocket({
endpoint: {
type: 'udp6',
Expand All @@ -54,6 +52,7 @@ const kALPN = 'zzz';
const session = client.connect({
address: common.localhostIPv4,
port: server.endpoints[0].address.port,
ipv6Only: true,
});

await once(session, 'secure');
Expand All @@ -70,11 +69,11 @@ const kALPN = 'zzz';
once(client, 'close'),
once(server, 'close')
]);
})().then(common.mustCall());
}

// When the `ipv6Only` set to `true`, a client cann't connect to it
// through "127.0.0.1".
(async () => {
async function ipv6Only() {
const server = createQuicSocket({
endpoint: {
type: 'udp6',
Expand All @@ -87,10 +86,13 @@ const kALPN = 'zzz';

await once(server, 'ready');

// This will attempt to connect to the ipv4 localhost address
// but should fail as the connection idle timeout expires.
const session = client.connect({
address: common.localhostIPv4,
port: server.endpoints[0].address.port,
idleTimeout: common.platformTimeout(1),
ipv6Only: true,
});

session.on('secure', common.mustNotCall());
Expand All @@ -104,11 +106,11 @@ const kALPN = 'zzz';
once(client, 'close'),
once(server, 'close')
]);
})();
}

// Creating the QuicSession fails when connect type does not match the
// the connect IP address...
(async () => {
async function mismatch() {
const server = createQuicSocket({ endpoint: { type: 'udp6' } });
const client = createQuicSocket({ client: { key, cert, ca, alpn: kALPN } });

Expand Down Expand Up @@ -137,4 +139,10 @@ const kALPN = 'zzz';
once(client, 'close'),
once(server, 'close')
]);
})().then(common.mustCall());
}

ipv4()
.then(ipv6)
.then(ipv6Only)
.then(mismatch)
.then(common.mustCall());

0 comments on commit c7d859e

Please sign in to comment.