From 3f68c17f401bfb62890d01e3c12f586593552186 Mon Sep 17 00:00:00 2001 From: Brian Botha Date: Wed, 10 May 2023 17:30:24 +1000 Subject: [PATCH] feat: added keepalive to connections * Added a `keepAliveDelay` parameter to client, server and connections. * Connections will send a ping frame after each interval, this should reset the timeout for both sides, given that the ping is received and responded to. * Added tests for this feature. * Fixes #4 [ci skip] --- index.d.ts | 1 + src/QUICClient.ts | 6 +- src/QUICConnection.ts | 27 +++ src/QUICServer.ts | 5 + src/native/napi/connection.rs | 7 + src/native/types.ts | 1 + tests/QUICClient.test.ts | 318 +++++++++++++++++++++++++++++++++- 7 files changed, 362 insertions(+), 3 deletions(-) diff --git a/index.d.ts b/index.d.ts index 00c12ecf..61b81a30 100644 --- a/index.d.ts +++ b/index.d.ts @@ -269,6 +269,7 @@ export class Connection { * https://stackoverflow.com/q/50343130/582917 */ pathStats(): Array + sendAckEliciting(): void } export class StreamIter { [Symbol.iterator](): Iterator diff --git a/src/QUICClient.ts b/src/QUICClient.ts index 4e91fa80..8e5be245 100644 --- a/src/QUICClient.ts +++ b/src/QUICClient.ts @@ -57,6 +57,7 @@ class QUICClient extends EventTarget { codeToReason, maxReadableStreamBytes, maxWritableStreamBytes, + keepaliveIntervalTime, logger = new Logger(`${this.name}`), config = {}, }: { @@ -74,6 +75,7 @@ class QUICClient extends EventTarget { codeToReason?: StreamCodeToReason; maxReadableStreamBytes?: number; maxWritableStreamBytes?: number; + keepaliveIntervalTime?: number; logger?: Logger; config?: Partial; }) { @@ -199,9 +201,9 @@ class QUICClient extends EventTarget { socket.removeEventListener('error', handleQUICSocketError); // Remove the temporary connection error handler connection.removeEventListener('error', handleConnectionError); - + // Setting up keep alive + connection.setKeepAlive(keepaliveIntervalTime); // Now we create the client - const client = new QUICClient({ crypto, socket, diff --git a/src/QUICConnection.ts b/src/QUICConnection.ts index 93e53e5a..5c8f25a6 100644 --- a/src/QUICConnection.ts +++ b/src/QUICConnection.ts @@ -61,6 +61,7 @@ class QUICConnection extends EventTarget { protected logger: Logger; protected socket: QUICSocket; protected timer?: ReturnType; + protected keepAliveInterval?: ReturnType; public readonly closedP: Promise; protected resolveCloseP?: () => void; @@ -330,6 +331,11 @@ class QUICConnection extends EventTarget { force?: boolean; } = {}) { this.logger.info(`Destroy ${this.constructor.name}`); + // Clean up keep alive + if (this.keepAliveInterval != null) { + clearTimeout(this.keepAliveInterval); + delete this.keepAliveInterval; + } // Handle destruction concurrently const destroyProms: Array> = []; for (const stream of this.streamMap.values()) { @@ -694,6 +700,27 @@ class QUICConnection extends EventTarget { }); } + /** + * Used to update or disable the keep alive interval. + * Calling this will reset the delay before the next keep alive. + */ + @ready(new errors.ErrorQUICConnectionDestroyed()) + public setKeepAlive(intervalDelay?: number) { + // Clearing timeout prior to update + if (this.keepAliveInterval != null) { + clearTimeout(this.keepAliveInterval); + delete this.keepAliveInterval; + } + // Setting up keep alive interval + if (intervalDelay != null) { + this.keepAliveInterval = setInterval(async () => { + // Trigger an ping frame and send + this.conn.sendAckEliciting(); + await this.send(); + }, intervalDelay); + } + } + // Timeout handling, these methods handle time keeping for quiche. // Quiche will request an amount of time, We then call `onTimeout()` after that time has passed. protected deadline: number = 0; diff --git a/src/QUICServer.ts b/src/QUICServer.ts index 94f4fd8a..0be295c0 100644 --- a/src/QUICServer.ts +++ b/src/QUICServer.ts @@ -49,6 +49,7 @@ class QUICServer extends EventTarget { protected codeToReason: StreamCodeToReason | undefined; protected maxReadableStreamBytes?: number | undefined; protected maxWritableStreamBytes?: number | undefined; + protected keepaliveIntervalTime?: number | undefined; protected connectionMap: QUICConnectionMap; /** @@ -75,6 +76,7 @@ class QUICServer extends EventTarget { codeToReason, maxReadableStreamBytes, maxWritableStreamBytes, + keepaliveIntervalTime, logger, }: { crypto: { @@ -91,6 +93,7 @@ class QUICServer extends EventTarget { codeToReason?: StreamCodeToReason; maxReadableStreamBytes?: number; maxWritableStreamBytes?: number; + keepaliveIntervalTime?: number; logger?: Logger; }) { super(); @@ -120,6 +123,7 @@ class QUICServer extends EventTarget { this.codeToReason = codeToReason; this.maxReadableStreamBytes = maxReadableStreamBytes; this.maxWritableStreamBytes = maxWritableStreamBytes; + this.keepaliveIntervalTime = keepaliveIntervalTime; } @ready(new errors.ErrorQUICServerNotRunning()) @@ -299,6 +303,7 @@ class QUICServer extends EventTarget { `${QUICConnection.name} ${scid.toString().slice(32)}`, ), }); + connection.setKeepAlive(this.keepaliveIntervalTime); this.dispatchEvent( new events.QUICServerConnectionEvent({ detail: connection }), diff --git a/src/native/napi/connection.rs b/src/native/napi/connection.rs index 4830ec12..3d70f845 100644 --- a/src/native/napi/connection.rs +++ b/src/native/napi/connection.rs @@ -1068,4 +1068,11 @@ impl Connection { |s| s.into() ).collect(); } + + #[napi] + pub fn send_ack_eliciting(&mut self) -> napi::Result<()> { + return self.0.send_ack_eliciting().or_else( + |err| Err(Error::from_reason(err.to_string())) + ); + } } diff --git a/src/native/types.ts b/src/native/types.ts index 34860a4c..f6c3b20c 100644 --- a/src/native/types.ts +++ b/src/native/types.ts @@ -129,6 +129,7 @@ interface Connection { localError(): ConnectionError | null; stats(): Stats; pathStats(): Array; + sendAckEliciting(): void; } interface ConnectionConstructor { diff --git a/tests/QUICClient.test.ts b/tests/QUICClient.test.ts index 3434b07e..5a253aee 100644 --- a/tests/QUICClient.test.ts +++ b/tests/QUICClient.test.ts @@ -1,7 +1,9 @@ import type { Crypto, Host, Port } from '@/types'; import type * as events from '@/events'; +import type QUICConnection from '@/QUICConnection'; import Logger, { LogLevel, StreamHandler, formatting } from '@matrixai/logger'; import { fc, testProp } from '@fast-check/jest'; +import { destroyed } from '@matrixai/async-init'; import QUICClient from '@/QUICClient'; import QUICServer from '@/QUICServer'; import * as errors from '@/errors'; @@ -10,18 +12,22 @@ import QUICSocket from '@/QUICSocket'; import * as testsUtils from './utils'; import { tlsConfigWithCaArb } from './tlsUtils'; import { sleep } from './utils'; +import * as fixtures from './fixtures/certFixtures'; describe(QUICClient.name, () => { - const logger = new Logger(`${QUICClient.name} Test`, LogLevel.WARN, [ + const logger = new Logger(`${QUICClient.name} Test`, LogLevel.DEBUG, [ new StreamHandler( formatting.format`${formatting.level}:${formatting.keys}:${formatting.msg}`, ), ]); + const host = '127.0.0.1' as Host; // This has to be setup asynchronously due to key generation let crypto: { key: ArrayBuffer; ops: Crypto; }; + let clientSocket: QUICSocket; + let serverSocket: QUICSocket; // We need to test the stream making beforeEach(async () => { @@ -33,6 +39,18 @@ describe(QUICClient.name, () => { randomBytes: testsUtils.randomBytes, }, }; + clientSocket = new QUICSocket({ crypto, logger }); + serverSocket = new QUICSocket({ crypto, logger }); + await clientSocket.start({ + host: '127.0.0.1' as Host, + }); + await serverSocket.start({ + host: '127.0.0.1' as Host, + }); + }); + afterEach(async () => { + await clientSocket.stop(true); + await serverSocket.stop(true); }); // Are we describing a dual stack client!? describe('dual stack client', () => { @@ -1086,4 +1104,302 @@ describe(QUICClient.name, () => { { numRuns: 1 }, ); }); + describe('keepalive', () => { + const tlsConfig = fixtures.tlsConfigMemRSA1; + test('connection can time out on client', async () => { + const connectionEventProm = promise(); + const server = new QUICServer({ + crypto, + logger: logger.getChild(QUICServer.name), + config: { + tlsConfig, + verifyPeer: false, + maxIdleTimeout: 1000, + }, + socket: serverSocket, + }); + server.addEventListener( + 'connection', + (e: events.QUICServerConnectionEvent) => + connectionEventProm.resolveP(e.detail), + ); + await server.start({ + host: '127.0.0.1' as Host, + }); + const client = await QUICClient.createQUICClient({ + host: host, + port: server.port, + localHost: '::' as Host, + crypto, + logger: logger.getChild(QUICClient.name), + config: { + verifyPeer: false, + maxIdleTimeout: 100, + }, + socket: clientSocket, + }); + // Setting no keepalive should cause the connection to time out + // It has cleaned up due to timeout + const clientConnection = client.connection; + const clientTimeoutProm = promise(); + clientConnection.addEventListener( + 'error', + (event: events.QUICConnectionErrorEvent) => { + if (event.detail instanceof errors.ErrorQUICConnectionTimeout) { + clientTimeoutProm.resolveP(); + } + }, + ); + await clientTimeoutProm.p; + const serverConnection = await connectionEventProm.p; + await sleep(100); + // Server and client has cleaned up + expect(clientConnection[destroyed]).toBeTrue(); + expect(serverConnection[destroyed]).toBeTrue(); + + await client.destroy(); + await server.stop(); + }); + test('connection can time out on server', async () => { + const connectionEventProm = promise(); + const server = new QUICServer({ + crypto, + logger: logger.getChild(QUICServer.name), + config: { + tlsConfig, + verifyPeer: false, + maxIdleTimeout: 100, + }, + socket: serverSocket, + }); + server.addEventListener( + 'connection', + (e: events.QUICServerConnectionEvent) => + connectionEventProm.resolveP(e.detail), + ); + await server.start({ + host: '127.0.0.1' as Host, + }); + const client = await QUICClient.createQUICClient({ + host: host, + port: server.port, + localHost: '::' as Host, + crypto, + logger: logger.getChild(QUICClient.name), + config: { + verifyPeer: false, + maxIdleTimeout: 1000, + }, + socket: clientSocket, + }); + // Setting no keepalive should cause the connection to time out + // It has cleaned up due to timeout + const clientConnection = client.connection; + const serverConnection = await connectionEventProm.p; + const serverTimeoutProm = promise(); + serverConnection.addEventListener( + 'error', + (event: events.QUICConnectionErrorEvent) => { + if (event.detail instanceof errors.ErrorQUICConnectionTimeout) { + serverTimeoutProm.resolveP(); + } + }, + ); + await serverTimeoutProm.p; + await sleep(100); + // Server and client has cleaned up + expect(clientConnection[destroyed]).toBeTrue(); + expect(serverConnection[destroyed]).toBeTrue(); + + await client.destroy(); + await server.stop(); + }); + test('keep alive prevents timeout on client', async () => { + const connectionEventProm = promise(); + const server = new QUICServer({ + crypto, + logger: logger.getChild(QUICServer.name), + config: { + tlsConfig, + verifyPeer: false, + maxIdleTimeout: 20000, + logKeys: './tmp/key1.log', + }, + socket: serverSocket, + }); + server.addEventListener( + 'connection', + (e: events.QUICServerConnectionEvent) => + connectionEventProm.resolveP(e.detail), + ); + await server.start({ + host: '127.0.0.1' as Host, + }); + const client = await QUICClient.createQUICClient({ + host: host, + port: server.port, + localHost: '::' as Host, + crypto, + logger: logger.getChild(QUICClient.name), + config: { + verifyPeer: false, + maxIdleTimeout: 100, + }, + socket: clientSocket, + keepaliveIntervalTime: 50, + }); + // Setting no keepalive should cause the connection to time out + // It has cleaned up due to timeout + const clientConnection = client.connection; + const clientTimeoutProm = promise(); + clientConnection.addEventListener( + 'error', + (event: events.QUICConnectionErrorEvent) => { + if (event.detail instanceof errors.ErrorQUICConnectionTimeout) { + clientTimeoutProm.resolveP(); + } + }, + ); + await connectionEventProm.p; + // Connection would timeout after 100ms if keep alive didn't work + await Promise.race([ + sleep(300), + clientTimeoutProm.p.then(() => { + throw Error('Connection timed out'); + }), + ]); + await client.destroy(); + await server.stop(); + }); + test('keep alive prevents timeout on server', async () => { + const connectionEventProm = promise(); + const server = new QUICServer({ + crypto, + logger: logger.getChild(QUICServer.name), + config: { + tlsConfig, + verifyPeer: false, + maxIdleTimeout: 100, + logKeys: './tmp/key1.log', + }, + socket: serverSocket, + keepaliveIntervalTime: 50, + }); + server.addEventListener( + 'connection', + (e: events.QUICServerConnectionEvent) => + connectionEventProm.resolveP(e.detail), + ); + await server.start({ + host: '127.0.0.1' as Host, + }); + const client = await QUICClient.createQUICClient({ + host: host, + port: server.port, + localHost: '::' as Host, + crypto, + logger: logger.getChild(QUICClient.name), + config: { + verifyPeer: false, + maxIdleTimeout: 20000, + }, + socket: clientSocket, + }); + // Setting no keepalive should cause the connection to time out + // It has cleaned up due to timeout + const serverConnection = await connectionEventProm.p; + const serverTimeoutProm = promise(); + serverConnection.addEventListener( + 'error', + (event: events.QUICConnectionErrorEvent) => { + if (event.detail instanceof errors.ErrorQUICConnectionTimeout) { + serverTimeoutProm.resolveP(); + } + }, + ); + // Connection would time out after 100ms if keep alive didn't work + await Promise.race([ + sleep(300), + serverTimeoutProm.p.then(() => { + throw Error('Connection timed out'); + }), + ]); + await client.destroy(); + await server.stop(); + }); + test('client keep alive prevents timeout on server', async () => { + const connectionEventProm = promise(); + const server = new QUICServer({ + crypto, + logger: logger.getChild(QUICServer.name), + config: { + tlsConfig, + verifyPeer: false, + maxIdleTimeout: 100, + logKeys: './tmp/key1.log', + }, + socket: serverSocket, + }); + server.addEventListener( + 'connection', + (e: events.QUICServerConnectionEvent) => + connectionEventProm.resolveP(e.detail), + ); + await server.start({ + host: '127.0.0.1' as Host, + }); + const client = await QUICClient.createQUICClient({ + host: host, + port: server.port, + localHost: '::' as Host, + crypto, + logger: logger.getChild(QUICClient.name), + config: { + verifyPeer: false, + maxIdleTimeout: 20000, + }, + socket: clientSocket, + keepaliveIntervalTime: 50, + }); + // Setting no keepalive should cause the connection to time out + // It has cleaned up due to timeout + const serverConnection = await connectionEventProm.p; + const serverTimeoutProm = promise(); + serverConnection.addEventListener( + 'error', + (event: events.QUICConnectionErrorEvent) => { + if (event.detail instanceof errors.ErrorQUICConnectionTimeout) { + serverTimeoutProm.resolveP(); + } + }, + ); + // Connection would time out after 100ms if keep alive didn't work + await Promise.race([ + sleep(300), + serverTimeoutProm.p.then(() => { + throw Error('Connection timed out'); + }), + ]); + await client.destroy(); + await server.stop(); + }); + test('Keep alive does not prevent connection timeout', async () => { + const clientProm = QUICClient.createQUICClient({ + host: host, + port: serverSocket.port, + localHost: '::' as Host, + crypto, + logger: logger.getChild(QUICClient.name), + config: { + verifyPeer: false, + maxIdleTimeout: 100, + }, + socket: clientSocket, + keepaliveIntervalTime: 50, + }); + await expect(clientProm).rejects.toThrow( + errors.ErrorQUICConnectionTimeout, + ); + }); + }); });