Skip to content

Commit

Permalink
quic: eliminate "ready"/"not ready" states for QuicSession
Browse files Browse the repository at this point in the history 
QuicClientSession and QuicServerSessions are now always immediately
ready for use when they are created, so no more need to track
ready state.

PR-URL: #34283
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
@jasnell
jasnell committed Jul 16, 2020
1 parent 6665dda commit 346aeaf
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 120 deletions.
10 changes: 0 additions & 10 deletions doc/api/quic.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1269,16 +1269,6 @@ empty object when the key exchange is not ephemeral. The supported types are

For example: `{ type: 'ECDH', name: 'prime256v1', size: 256 }`.

#### quicclientsession.ready
<!-- YAML
added: REPLACEME
-->

* Type: {boolean}

Set to `true` if the `QuicClientSession` is ready for use. False if the
`QuicSocket` has not yet been bound.

#### quicclientsession.setSocket(socket])
<!-- YAML
added: REPLACEME
Expand Down
116 changes: 17 additions & 99 deletions lib/internal/quic/core.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,6 @@ const kBind = Symbol('kBind');
const kClose = Symbol('kClose');
const kCert = Symbol('kCert');
const kClientHello = Symbol('kClientHello');
const kContinueConnect = Symbol('kContinueConnect');
const kCompleteListen = Symbol('kCompleteListen');
const kDestroy = Symbol('kDestroy');
const kEndpointBound = Symbol('kEndpointBound');
Expand All @@ -226,11 +225,9 @@ const kInternalServerState = Symbol('kInternalServerState');
const kListen = Symbol('kListen');
const kMakeStream = Symbol('kMakeStream');
const kMaybeBind = Symbol('kMaybeBind');
const kMaybeReady = Symbol('kMaybeReady');
const kOnFileOpened = Symbol('kOnFileOpened');
const kOnFileUnpipe = Symbol('kOnFileUnpipe');
const kOnPipedFileHandleRead = Symbol('kOnPipedFileHandleRead');
const kSocketReady = Symbol('kSocketReady');
const kRemoveSession = Symbol('kRemove');
const kRemoveStream = Symbol('kRemoveStream');
const kServerBusy = Symbol('kServerBusy');
Expand Down Expand Up @@ -1148,9 +1145,6 @@ class QuicSocket extends EventEmitter {

state.state = kSocketBound;

for (const session of state.sessions)
session[kSocketReady]();

process.nextTick(() => {
// User code may have run before this so we need to check the
// destroyed state. If it has been destroyed, do nothing.
Expand All @@ -1175,13 +1169,6 @@ class QuicSocket extends EventEmitter {
return;
state.state = kSocketBound;

// Once the QuicSocket has been bound, we notify all currently
// existing QuicSessions. QuicSessions created after this
// point will automatically be notified that the QuicSocket
// is ready.
for (const session of state.sessions)
session[kSocketReady]();

// The ready event indicates that the QuicSocket is ready to be
// used to either listen or connect. No QuicServerSession should
// exist before this event, and all QuicClientSession will remain
Expand Down Expand Up @@ -1380,16 +1367,7 @@ class QuicSocket extends EventEmitter {
if (this.closing)
throw new ERR_INVALID_STATE('QuicSocket is closing');

const session = new QuicClientSession(this, options);

try {
session[kContinueConnect](type, ip);
} catch (err) {
session.destroy();
throw err;
}

return session;
return new QuicClientSession(this, options, type, ip);
}

[kEndpointClose](endpoint, error) {
Expand Down Expand Up @@ -2207,10 +2185,7 @@ class QuicSession extends EventEmitter {
// signaling the completion of the TLS handshake.
const makeStream = QuicSession[kMakeStream].bind(this, stream, halfOpen);
let deferred = false;
if (this.allowEarlyData && !this.ready) {
deferred = true;
this.once('ready', makeStream);
} else if (!this.handshakeComplete) {
if (!this.handshakeComplete) {
deferred = true;
this.once('secure', makeStream);
}
Expand Down Expand Up @@ -2348,10 +2323,6 @@ class QuicServerSession extends QuicSession {
} = options;
super(socket, { highWaterMark, defaultEncoding });
this[kSetHandle](handle);

// Both the handle and socket are immediately usable
// at this point so trigger the ready event.
this[kSocketReady]();
}

// Called only when a clientHello event handler is registered.
Expand Down Expand Up @@ -2388,12 +2359,6 @@ class QuicServerSession extends QuicSession {
callback.bind(this[kHandle]));
}

[kSocketReady]() {
process.nextTick(emit.bind(this, 'ready'));
}

get ready() { return true; }

get allowEarlyData() { return false; }

addContext(servername, context = {}) {
Expand All @@ -2418,7 +2383,6 @@ class QuicClientSession extends QuicSession {
handshakeStarted: false,
minDHSize: undefined,
port: undefined,
ready: 0,
remoteTransportParams: undefined,
requestOCSP: undefined,
secureContext: undefined,
Expand All @@ -2429,7 +2393,7 @@ class QuicClientSession extends QuicSession {
qlogEnabled: false,
};

constructor(socket, options) {
constructor(socket, options, type, ip) {
const sc_options = {
...options,
minVersion: 'TLSv1.3',
Expand Down Expand Up @@ -2486,33 +2450,8 @@ class QuicClientSession extends QuicSession {
remoteTransportParams !== undefined &&
sessionTicket !== undefined;

if (socket.bound)
this[kSocketReady]();
}

[kHandshakePost]() {
const { type, size } = this.ephemeralKeyInfo;
if (type === 'DH' && size < this[kInternalClientState].minDHSize) {
this.destroy(new ERR_TLS_DH_PARAM_SIZE(size));
return false;
}
return true;
}

[kCert](response) {
this.emit('OCSPResponse', response);
}

[kContinueConnect](type, ip) {
const state = this[kInternalClientState];
setTransportParams(state.transportParams);

const options =
(state.verifyHostnameIdentity ?
QUICCLIENTSESSION_OPTION_VERIFY_HOSTNAME_IDENTITY : 0) |
(state.requestOCSP ?
QUICCLIENTSESSION_OPTION_REQUEST_OCSP : 0);

const handle =
_createClientSession(
this.socket[kHandle],
Expand All @@ -2526,7 +2465,10 @@ class QuicClientSession extends QuicSession {
state.dcid,
state.preferredAddressPolicy,
this.alpnProtocol,
options,
(state.verifyHostnameIdentity ?
QUICCLIENTSESSION_OPTION_VERIFY_HOSTNAME_IDENTITY : 0) |
(state.requestOCSP ?
QUICCLIENTSESSION_OPTION_REQUEST_OCSP : 0),
state.qlogEnabled,
state.autoStart);

Expand Down Expand Up @@ -2556,45 +2498,25 @@ class QuicClientSession extends QuicSession {
}

this[kSetHandle](handle);

// Listeners may have been added before the handle was created.
// Ensure that we toggle those listeners in the handle state.

const internalState = this[kInternalState];
if (this.listenerCount('keylog') > 0) {
toggleListeners(internalState.state, 'keylog', true);
}

if (this.listenerCount('pathValidation') > 0)
toggleListeners(internalState.state, 'pathValidation', true);

if (this.listenerCount('usePreferredAddress') > 0)
toggleListeners(internalState.state, 'usePreferredAddress', true);

this[kMaybeReady](0x2);
}

[kSocketReady]() {
this[kMaybeReady](0x1);
[kHandshakePost]() {
const { type, size } = this.ephemeralKeyInfo;
if (type === 'DH' && size < this[kInternalClientState].minDHSize) {
this.destroy(new ERR_TLS_DH_PARAM_SIZE(size));
return false;
}
return true;
}

// The QuicClientSession is ready for use only after
// (a) The QuicSocket has been bound and
// (b) The internal handle has been created
[kMaybeReady](flag) {
this[kInternalClientState].ready |= flag;
if (this.ready)
process.nextTick(emit.bind(this, 'ready'));
[kCert](response) {
this.emit('OCSPResponse', response);
}

get allowEarlyData() {
return this[kInternalClientState].allowEarlyData;
}

get ready() {
return this[kInternalClientState].ready === 0x3;
}

get handshakeStarted() {
return this[kInternalClientState].handshakeStarted;
}
Expand All @@ -2608,11 +2530,7 @@ class QuicClientSession extends QuicSession {
if (state.handshakeStarted)
return;
state.handshakeStarted = true;
if (!this.ready) {
this.once('ready', () => this[kHandle].startHandshake());
} else {
this[kHandle].startHandshake();
}
this[kHandle].startHandshake();
}

get ephemeralKeyInfo() {
Expand Down
29 changes: 18 additions & 11 deletions test/parallel/test-quic-quicsession-resume.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,29 +42,36 @@ const countdown = new Countdown(2, () => {

await server.listen();

let storedTicket;
let storedParams;

const req = await client.connect({
address: common.localhostIPv4,
port: server.endpoints[0].address.port,
});

const stream = req.openStream({ halfOpen: true });
stream.end('hello');
stream.resume();
stream.on('close', () => countdown.dec());

req.on('sessionTicket', common.mustCall((ticket, params) => {
assert(ticket instanceof Buffer);
assert(params instanceof Buffer);
debug(' Ticket: %s', ticket.toString('hex'));
debug(' Params: %s', params.toString('hex'));

// Destroy this initial client session...
req.destroy();

// Wait a tick then start a new one.
setImmediate(newSession, ticket, params);
storedTicket = ticket;
storedParams = params;
}, 1));

req.on('secure', () => {
const stream = req.openStream({ halfOpen: true });
stream.end('hello');
stream.resume();
stream.on('close', () => {
req.close();
countdown.dec();
// Wait a turn then start a new session using the stored
// ticket and transportParameters
setImmediate(newSession, storedTicket, storedParams);
});
});

async function newSession(sessionTicket, remoteTransportParams) {
const req = await client.connect({
address: common.localhostIPv4,
Expand Down

0 comments on commit 346aeaf

Please sign in to comment.