From aafdc2fcad51ec0f4ecd99424d6d2055d7878fab Mon Sep 17 00:00:00 2001 From: James M Snell Date: Thu, 9 Jul 2020 12:22:55 -0700 Subject: [PATCH] quic: replace ipv6Only option with `'udp6-only'` type Since the `ipv6Only` option was mutually exclusive with using `'udp6'`, making it it's own type simplifies things a bit. PR-URL: https://github.com/nodejs/node/pull/34283 Reviewed-By: Anna Henningsen --- doc/api/quic.md | 22 +++------ lib/internal/quic/core.js | 49 +++++++++---------- lib/internal/quic/util.js | 27 ++++------ .../test-quic-errors-quicsocket-connect.js | 7 --- .../test-quic-errors-quicsocket-create.js | 7 --- test/parallel/test-quic-ipv6only.js | 34 ++----------- 6 files changed, 42 insertions(+), 104 deletions(-) diff --git a/doc/api/quic.md b/doc/api/quic.md index f6628bf9803fb1..0c8f73a0e5c8d1 100644 --- a/doc/api/quic.md +++ b/doc/api/quic.md @@ -260,12 +260,9 @@ added: REPLACEME IPv6 address or a host name. If a host name is given, it will be resolved 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. **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`. + * `type` {string} Can be one of `'udp4'`, `'upd6'`, or `'udp6-only'` to + use IPv4, IPv6, or IPv6 with dual-stack mode disabled. + **Default**: `'udp4'`. * `lookup` {Function} A custom DNS lookup function. Default `dns.lookup()`. * `maxConnections` {number} The maximum number of total active inbound connections. @@ -1450,12 +1447,9 @@ added: REPLACEME IPv6 address or a host name. If a host name is given, it will be resolved 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. **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`. + * `type` {string} Can be one of `'udp4'`, `'upd6'`, or `'udp6-only'` to + use IPv4, IPv6, or IPv6 with dual-stack mode disabled. + **Default**: `'udp4'`. * Returns: {QuicEndpoint} Creates and adds a new `QuicEndpoint` to the `QuicSocket` instance. @@ -1600,10 +1594,6 @@ added: REPLACEME `SSL_OP_CIPHER_SERVER_PREFERENCE` to be set in `secureOptions`, see [OpenSSL Options][] for more information. * `idleTimeout` {number} - * `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 diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js index e46e2763241914..908e01b625b665 100644 --- a/lib/internal/quic/core.js +++ b/lib/internal/quic/core.js @@ -611,10 +611,10 @@ class QuicEndpoint { const state = this[kInternalState]; state.socket = socket; state.address = address || (type === AF_INET6 ? '::' : '0.0.0.0'); - state.ipv6Only = !!ipv6Only; + state.ipv6Only = ipv6Only; state.lookup = lookup || (type === AF_INET6 ? lookup6 : lookup4); state.port = port; - state.reuseAddr = !!reuseAddr; + state.reuseAddr = reuseAddr; state.type = type; state.udpSocket = dgram.createSocket(type === AF_INET6 ? 'udp6' : 'udp4'); @@ -696,20 +696,26 @@ class QuicEndpoint { state.socket[kEndpointBound](this); } - [kDestroy](error) { + destroy(error) { + if (this.destroyed) + return; + + const state = this[kInternalState]; + state.state = kSocketDestroyed; + const handle = this[kHandle]; - if (handle !== undefined) { - this[kHandle] = undefined; - handle[owner_symbol] = undefined; - handle.ondone = () => { - const state = this[kInternalState]; - state.udpSocket.close((err) => { - if (err) error = err; - state.socket[kEndpointClose](this, error); - }); - }; - handle.waitForPendingCallbacks(); - } + if (handle === undefined) + return; + + this[kHandle] = undefined; + handle[owner_symbol] = undefined; + handle.ondone = () => { + state.udpSocket.close((err) => { + if (err) error = err; + state.socket[kEndpointClose](this, error); + }); + }; + handle.waitForPendingCallbacks(); } // If the QuicEndpoint is bound, returns an object detailing @@ -825,14 +831,6 @@ class QuicEndpoint { state.udpSocket.unref(); return this; } - - destroy(error) { - const state = this[kInternalState]; - if (this.destroyed) - return; - state.state = kSocketDestroyed; - this[kDestroy](error); - } } // QuicSocket wraps a UDP socket plus the associated TLS context and QUIC @@ -1195,7 +1193,7 @@ class QuicSocket extends EventEmitter { port, type = 'udp4', } = { ...preferredAddress }; - const typeVal = getSocketType(type); + const [ typeVal ] = getSocketType(type); // If preferred address is set, we need to perform a lookup on it // to get the IP address. Only after that lookup completes can we // continue with the listen operation, passing in the resolved @@ -2331,7 +2329,6 @@ class QuicClientSession extends QuicSession { autoStart: true, dcid: undefined, handshakeStarted: false, - ipv6Only: undefined, minDHSize: undefined, port: undefined, ready: 0, @@ -2355,7 +2352,6 @@ class QuicClientSession extends QuicSession { autoStart, alpn, dcid, - ipv6Only, minDHSize, port, preferredAddressPolicy, @@ -2383,7 +2379,6 @@ class QuicClientSession extends QuicSession { state.autoStart = autoStart; state.handshakeStarted = autoStart; state.dcid = dcid; - state.ipv6Only = ipv6Only; state.minDHSize = minDHSize; state.port = port || 0; state.preferredAddressPolicy = preferredAddressPolicy; diff --git a/lib/internal/quic/util.js b/lib/internal/quic/util.js index 31087ef96e5000..6e3087ea2938e1 100644 --- a/lib/internal/quic/util.js +++ b/lib/internal/quic/util.js @@ -140,8 +140,9 @@ function validateNumber(value, name, function getSocketType(type = 'udp4') { switch (type) { - case 'udp4': return AF_INET; - case 'udp6': return AF_INET6; + case 'udp4': return [AF_INET, false]; + case 'udp6': return [AF_INET6, false]; + case 'udp6-only': return [AF_INET6, true]; } throw new ERR_INVALID_ARG_VALUE('options.type', type); } @@ -366,7 +367,6 @@ function validateQuicClientSessionOptions(options = {}) { address = 'localhost', alpn = '', dcid: dcid_value, - ipv6Only = false, minDHSize = 1024, port = 0, preferredAddressPolicy = 'ignore', @@ -434,7 +434,6 @@ function validateQuicClientSessionOptions(options = {}) { if (preferredAddressPolicy !== undefined) validateString(preferredAddressPolicy, 'options.preferredAddressPolicy'); - validateBoolean(ipv6Only, 'options.ipv6Only'); validateBoolean(requestOCSP, 'options.requestOCSP'); validateBoolean(verifyHostnameIdentity, 'options.verifyHostnameIdentity'); validateBoolean(qlog, 'options.qlog'); @@ -444,7 +443,6 @@ function validateQuicClientSessionOptions(options = {}) { address, alpn, dcid, - ipv6Only, minDHSize, port, preferredAddressPolicy: @@ -495,7 +493,6 @@ function validateQuicEndpointOptions(options = {}, name = 'options') { throw new ERR_INVALID_ARG_TYPE('options', 'Object', options); const { address, - ipv6Only = false, lookup, port = 0, reuseAddr = false, @@ -507,17 +504,17 @@ function validateQuicEndpointOptions(options = {}, name = 'options') { validatePort(port, 'options.port'); validateString(type, 'options.type'); validateLookup(lookup); - validateBoolean(ipv6Only, 'options.ipv6Only'); validateBoolean(reuseAddr, 'options.reuseAddr'); validateBoolean(preferred, 'options.preferred'); + const [typeVal, ipv6Only] = getSocketType(type); return { - address, + type: typeVal, ipv6Only, + address, lookup, port, preferred, reuseAddr, - type: getSocketType(type), }; } @@ -536,15 +533,13 @@ function validateQuicSocketOptions(options = {}) { retryTokenTimeout = DEFAULT_RETRYTOKEN_EXPIRATION, server = {}, statelessResetSecret, - type = endpoint.type || 'udp4', validateAddressLRU = false, validateAddress = false, } = options; - validateQuicEndpointOptions(endpoint, 'options.endpoint'); + const { type } = validateQuicEndpointOptions(endpoint, 'options.endpoint'); validateObject(client, 'options.client'); validateObject(server, 'options.server'); - validateString(type, 'options.type'); validateLookup(lookup); validateBoolean(validateAddress, 'options.validateAddress'); validateBoolean(validateAddressLRU, 'options.validateAddressLRU'); @@ -595,7 +590,7 @@ function validateQuicSocketOptions(options = {}) { maxStatelessResetsPerHost, retryTokenTimeout, server, - type: getSocketType(type), + type, validateAddress: validateAddress || validateAddressLRU, validateAddressLRU, qlog, @@ -642,10 +637,8 @@ function validateQuicSocketConnectOptions(options = {}) { } = options; if (address !== undefined) validateString(address, 'options.address'); - return { - type: getSocketType(type), - address, - }; + const [typeVal] = getSocketType(type); + return { type: typeVal, address }; } function validateCreateSecureContextOptions(options = {}) { diff --git a/test/parallel/test-quic-errors-quicsocket-connect.js b/test/parallel/test-quic-errors-quicsocket-connect.js index 7b545a4ad40a41..2713c0ec4af5f8 100644 --- a/test/parallel/test-quic-errors-quicsocket-connect.js +++ b/test/parallel/test-quic-errors-quicsocket-connect.js @@ -131,12 +131,6 @@ const client = createQuicSocket(); }); }); -['a', 1n, 1, [], {}].forEach((ipv6Only) => { - assert.throws(() => client.connect({ ipv6Only }), { - code: 'ERR_INVALID_ARG_TYPE' - }); -}); - [1, 1n, false, [], {}].forEach((preferredAddressPolicy) => { assert.throws(() => client.connect({ preferredAddressPolicy }), { code: 'ERR_INVALID_ARG_TYPE' @@ -202,7 +196,6 @@ assert.throws(() => client.connect(), { // Client QuicSession Related: // // [x] idleTimeout - must be a number greater than zero -// [x] ipv6Only - must be a boolean // [x] activeConnectionIdLimit - must be a number between 2 and 8 // [x] maxAckDelay - must be a number greater than zero // [x] maxData - must be a number greater than zero diff --git a/test/parallel/test-quic-errors-quicsocket-create.js b/test/parallel/test-quic-errors-quicsocket-create.js index 64c27a50f2701a..570f02b1a95661 100644 --- a/test/parallel/test-quic-errors-quicsocket-create.js +++ b/test/parallel/test-quic-errors-quicsocket-create.js @@ -47,13 +47,6 @@ const { createQuicSocket } = require('net'); }); }); -// Test invalid QuicSocket ipv6Only argument option -[1, NaN, 1n, null, {}, []].forEach((ipv6Only) => { - assert.throws(() => createQuicSocket({ endpoint: { ipv6Only } }), { - code: 'ERR_INVALID_ARG_TYPE' - }); -}); - // Test invalid QuicSocket reuseAddr argument option [1, NaN, 1n, null, {}, []].forEach((reuseAddr) => { assert.throws(() => createQuicSocket({ endpoint: { reuseAddr } }), { diff --git a/test/parallel/test-quic-ipv6only.js b/test/parallel/test-quic-ipv6only.js index 4c6695804e3231..5b4a23dd66efbe 100644 --- a/test/parallel/test-quic-ipv6only.js +++ b/test/parallel/test-quic-ipv6only.js @@ -19,29 +19,10 @@ const { once } = require('events'); const kALPN = 'zzz'; -// 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.mustNotCall()); - server.listen({ key, cert, ca, alpn: kALPN }); - await once(server, 'ready'); - server.close(); -} - // 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', - ipv6Only: false - } }); + const server = createQuicSocket({ endpoint: { type: 'udp6' } }); const client = createQuicSocket({ client: { key, cert, ca, alpn: kALPN } }); server.listen({ key, cert, ca, alpn: kALPN }); @@ -54,8 +35,7 @@ async function ipv6() { const session = client.connect({ address: common.localhostIPv4, - port: server.endpoints[0].address.port, - ipv6Only: true, + port: server.endpoints[0].address.port }); await once(session, 'secure'); @@ -77,11 +57,7 @@ async function ipv6() { // When the `ipv6Only` set to `true`, a client cann't connect to it // through "127.0.0.1". async function ipv6Only() { - const server = createQuicSocket({ - endpoint: { - type: 'udp6', - ipv6Only: true - } }); + const server = createQuicSocket({ endpoint: { type: 'udp6-only' } }); const client = createQuicSocket({ client: { key, cert, ca, alpn: kALPN } }); server.listen({ key, cert, ca, alpn: kALPN }); @@ -95,7 +71,6 @@ async function ipv6Only() { address: common.localhostIPv4, port: server.endpoints[0].address.port, idleTimeout: common.platformTimeout(1), - ipv6Only: true, }); session.on('secure', common.mustNotCall()); @@ -144,8 +119,7 @@ async function mismatch() { ]); } -ipv4() - .then(ipv6) +ipv6() .then(ipv6Only) .then(mismatch) .then(common.mustCall());