quic: restrict addEndpoint to before QuicSocket bind

Restricting this to pre-bind keeps things simple
nodejs · jasnell · Jul 9, 2020 · Jul 9, 2020 · Jul 9, 2020 · Jul 9, 2020
commit 7297f7025c5438f23eb92ce5beef01a82ccaee7a
diff --git a/doc/api/quic.md b/doc/api/quic.md
@@ -1458,7 +1458,10 @@ added: REPLACEME
     **Default**: `'udp4'`.
 * Returns: {QuicEndpoint}
 
-Creates and adds a new `QuicEndpoint` to the `QuicSocket` instance.
+Creates and adds a new `QuicEndpoint` to the `QuicSocket` instance. An
+error will be thrown if `quicsock.addEndpoint()` is called either after
+the `QuicSocket` has already started binding to the local ports or after
+the `QuicSocket` has been destroyed.
 
 #### quicsocket.bound
 <!-- YAML

diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js
@@ -1102,22 +1102,19 @@ class QuicSocket extends EventEmitter {
     });
   }
 
+  // A QuicSocket will typically bind only to a single local port, but it is
+  // possible to bind to multiple, even if those use different IP families
+  // (e.g. IPv4 or IPv6). Calls to addEndpoint() must be made before the
+  // QuicSocket is bound (e.g. before any calls to listen() or connect()).
   addEndpoint(options = {}) {
     const state = this[kInternalState];
     if (this.destroyed)
       throw new ERR_INVALID_STATE('QuicSocket is already destroyed');
-
-    // TODO(@jasnell): Also forbid adding an endpoint if
-    // the QuicSocket is closing.
+    if (state.state !== kSocketUnbound)
+      throw new ERR_INVALID_STATE('QuicSocket is already being bound');
 
     const endpoint = new QuicEndpoint(this, options);
     state.endpoints.add(endpoint);
-
-    // If the QuicSocket is already bound at this point,
-    // also bind the newly created QuicEndpoint.
-    if (state.state !== kSocketUnbound)
-      endpoint[kMaybeBind]();
-
     return endpoint;
   }
 

diff --git a/test/parallel/test-quic-quicendpoint-address.js b/test/parallel/test-quic-quicendpoint-address.js
@@ -43,6 +43,11 @@ async function Test2() {
 
   server.listen({ key, cert, ca, alpn: 'zzz' });
 
+  // Attempting to add an endpoint after fails.
+  assert.throws(() => server.addEndpoint(), {
+    code: 'ERR_INVALID_STATE'
+  });
+
   await once(server, 'ready');
 
   assert.strictEqual(server.endpoints.length, 2);