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: #34283 Reviewed-By: Anna Henningsen <[email protected]>
nodejs · Jul 16, 2020 · aafdc2f · aafdc2f
1 parent 48404a1
commit aafdc2f
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 104 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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());