From b80108c033325c00dabbde945522e7f3535a3977 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Thu, 9 Jul 2020 13:40:23 -0700
Subject: [PATCH] quic: restrict addEndpoint to before QuicSocket bind

Restricting this to pre-bind keeps things simple

PR-URL: https://github.com/nodejs/node/pull/34283
Reviewed-By: Anna Henningsen <anna@addaleax.net>
---
 doc/api/quic.md                                 |  5 ++++-
 lib/internal/quic/core.js                       | 15 ++++++---------
 test/parallel/test-quic-quicendpoint-address.js |  5 +++++
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/doc/api/quic.md b/doc/api/quic.md
index b21d432642580b..d16b0a853e19fa 100644
--- 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
index bbddf0851d0c48..3aebecd47c8370 100644
--- 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
index 0554104748e777..3a3c2c04831b27 100644
--- 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);