Skip to content

Commit

Permalink
quic: restrict addEndpoint to before QuicSocket bind
Browse files Browse the repository at this point in the history
Restricting this to pre-bind keeps things simple

PR-URL: #34283
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
jasnell committed Jul 16, 2020
1 parent 81c01bb commit b80108c
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 10 deletions.
5 changes: 4 additions & 1 deletion doc/api/quic.md
Expand Up @@ -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
Expand Down
15 changes: 6 additions & 9 deletions lib/internal/quic/core.js
Expand Up @@ -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;
}

Expand Down
5 changes: 5 additions & 0 deletions test/parallel/test-quic-quicendpoint-address.js
Expand Up @@ -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);
Expand Down

0 comments on commit b80108c

Please sign in to comment.