diff --git a/keyserver/src/socket/socket.js b/keyserver/src/socket/socket.js --- a/keyserver/src/socket/socket.js +++ b/keyserver/src/socket/socket.js @@ -260,7 +260,11 @@ handleAsyncPromise(extendCookieLifespan(viewer.cookieID)); } for (const response of serverResponses) { - this.sendMessage(response); + // Normally it's an anti-pattern to await in sequence like this. But in + // this case, we have a requirement that this array of serverResponses + // is delivered in order. See here: + // https://github.com/CommE2E/comm/blob/101eb34481deb49c609bfd2c785f375886e52666/keyserver/src/socket/socket.js#L566-L568 + await this.sendMessage(response); } if (clientSocketMessage.type === clientSocketMessageTypes.INITIAL) { this.onSuccessfulConnection(); @@ -277,7 +281,7 @@ errorMessage.responseTo = responseTo; } this.markActivityOccurred(); - this.sendMessage(errorMessage); + await this.sendMessage(errorMessage); return; } invariant(clientSocketMessage, 'should be set'); @@ -301,7 +305,7 @@ }, }; } - this.sendMessage(authErrorMessage); + await this.sendMessage(authErrorMessage); this.ws.close(4100, error.message); return; } else if (error.message === 'client_version_unsupported') { @@ -338,19 +342,19 @@ }, }; } - this.sendMessage(authErrorMessage); + await this.sendMessage(authErrorMessage); this.ws.close(4101, error.message); return; } if (error.payload) { - this.sendMessage({ + await this.sendMessage({ type: serverSocketMessageTypes.ERROR, responseTo, message: error.message, payload: error.payload, }); } else { - this.sendMessage({ + await this.sendMessage({ type: serverSocketMessageTypes.ERROR, responseTo, message: error.message, @@ -379,7 +383,7 @@ } }; - sendMessage = (message: ServerServerSocketMessage) => { + sendMessage = async (message: ServerServerSocketMessage) => { invariant( this.ws.readyState > 0, "shouldn't send message until connection established", @@ -405,7 +409,7 @@ return; } - const compressionResult = compressMessage(stringMessage); + const compressionResult = await compressMessage(stringMessage); if (!compressionResult.compressed) { this.ws.send(stringMessage); return; @@ -870,7 +874,7 @@ }); invariant(checkStateRequest, 'should be set'); - this.sendMessage({ + await this.sendMessage({ type: serverSocketMessageTypes.REQUESTS, payload: { serverRequests: [checkStateRequest] }, }); diff --git a/keyserver/src/utils/compress.js b/keyserver/src/utils/compress.js --- a/keyserver/src/utils/compress.js +++ b/keyserver/src/utils/compress.js @@ -1,5 +1,6 @@ // @flow +import { promisify } from 'util'; import zlib from 'zlib'; import type { CompressedData } from 'lib/types/compression-types.js'; @@ -11,15 +12,17 @@ }; const minimumSizeForCompression = 4096; // bytes +const brotliCompress = promisify(zlib.brotliCompress); + type CompressionResult = | { +compressed: true, +result: CompressedData } | { +compressed: false, +result: string }; -function compressMessage(message: string): CompressionResult { +async function compressMessage(message: string): Promise { const bytesInMessage = new Blob([message]).size; if (bytesInMessage < minimumSizeForCompression) { return { compressed: false, result: message }; } - const brotliResult = zlib.brotliCompressSync(message, brotliOptions); + const brotliResult = await brotliCompress(message, brotliOptions); const base64Encoded = brotliResult.toString('base64'); const result = { algo: 'brotli+base64', diff --git a/lib/utils/sequential-promise-resolver.js b/lib/utils/sequential-promise-resolver.js --- a/lib/utils/sequential-promise-resolver.js +++ b/lib/utils/sequential-promise-resolver.js @@ -1,11 +1,11 @@ // @flow class SequentialPromiseResolver { - onResolve: Result => mixed; + onResolve: Result => Promise; promises: Array> = []; currentlySpinning: boolean = false; - constructor(onResolve: Result => mixed) { + constructor(onResolve: Result => Promise) { this.onResolve = onResolve; } @@ -22,9 +22,11 @@ let currentPromise = this.promises.shift(); while (currentPromise) { + // It's important that we await in sequence here as the messages must be + // delivered in order. For more context, see https://phab.comm.dev/D355 const result = await currentPromise; if (result) { - this.onResolve(result); + await this.onResolve(result); } currentPromise = this.promises.shift(); }