diff --git a/fixtures/diff-empty.bin b/fixtures/diff-empty.bin new file mode 100644 index 0000000000..e69de29bb2 diff --git a/fixtures/diff-empty.bin.blockmap b/fixtures/diff-empty.bin.blockmap new file mode 100644 index 0000000000..c1a4a851c2 Binary files /dev/null and b/fixtures/diff-empty.bin.blockmap differ diff --git a/ts/test-node/updater/differential_test.ts b/ts/test-node/updater/differential_test.ts index 1781274f4d..e25c936b3d 100644 --- a/ts/test-node/updater/differential_test.ts +++ b/ts/test-node/updater/differential_test.ts @@ -60,6 +60,8 @@ describe('updater/differential', () => { const oldFile = 'diff-original.bin'; const oldBlockFile = getBlockMapFileName(oldFile); + const emptyFile = 'diff-empty.bin'; + const newFile = 'diff-modified.bin'; const newBlockFile = getBlockMapFileName(newFile); const newHash = @@ -109,6 +111,20 @@ describe('updater/differential', () => { return [parseInt(range[1], 10), parseInt(range[2], 10)]; }); + if (ranges.length === 1) { + res.writeHead(200, { + 'content-type': 'application/octet-stream', + }); + if (shouldTimeout === 'response') { + res.flushHeaders(); + return; + } + + const [from, to] = ranges[0]; + res.end(fullFile.slice(from, to + 1)); + return; + } + const BOUNDARY = 'f8f254ce1ba37627'; res.writeHead(206, { @@ -278,6 +294,34 @@ describe('updater/differential', () => { ); }); + it('downloads the full file with a single range', async () => { + const data = await prepareDownload({ + oldFile: path.join(FIXTURES, emptyFile), + newUrl: `${baseUrl}/${newFile}`, + sha512: newHash, + }); + + const outDir = await fs.mkdtemp(path.join(tmpdir(), 'signal-temp-')); + await fs.mkdir(outDir, { recursive: true }); + + const outFile = path.join(outDir, 'out.bin'); + const chunks = new Array(); + await download(outFile, data, { + statusCallback(size) { + chunks.push(size); + }, + }); + + const expected = await fs.readFile(path.join(FIXTURES, newFile)); + const actual = await fs.readFile(outFile); + + assert.isTrue(actual.equals(expected), 'Files do not match'); + assert.isTrue( + chunks.length > 0, + 'Expected multiple callback invocations' + ); + }); + it('handles response timeouts gracefully', async () => { const data = await prepareDownload({ oldFile: path.join(FIXTURES, oldFile), diff --git a/ts/updater/differential.ts b/ts/updater/differential.ts index 85794d365a..78e7169d8f 100644 --- a/ts/updater/differential.ts +++ b/ts/updater/differential.ts @@ -3,10 +3,11 @@ import type { FileHandle } from 'fs/promises'; import { readFile, open } from 'fs/promises'; +import type { Readable } from 'stream'; import { promisify } from 'util'; import { gunzip as nativeGunzip } from 'zlib'; import got from 'got'; -import { chunk as lodashChunk } from 'lodash'; +import { chunk as lodashChunk, noop } from 'lodash'; import pMap from 'p-map'; import Dicer from 'dicer'; @@ -197,7 +198,7 @@ export function computeDiff( last.size += size; } - return optimizedDiff; + return optimizedDiff.filter(({ size }) => size !== 0); } export async function prepareDownload({ @@ -369,45 +370,52 @@ export async function downloadRanges( const onPart = async (part: Dicer.PartStream): Promise => { const diff = await takeDiffFromPart(part, diffByRange); - let offset = 0; - for await (const chunk of part) { - strictAssert( - offset + chunk.length <= diff.size, - 'Server returned more data than expected, ' + - `written=${offset} ` + - `newChunk=${chunk.length} ` + - `maxSize=${diff.size}` - ); - - if (abortSignal?.aborted) { - return; - } - - await output.write(chunk, 0, chunk.length, offset + diff.writeOffset); - offset += chunk.length; - - chunkStatusCallback(chunk.length); - } - - strictAssert( - offset === diff.size, - `Not enough data to download from offset=${diff.readOffset} ` + - `size=${diff.size}` - ); + await saveDiffStream({ + diff, + stream: part, + abortSignal, + output, + chunkStatusCallback, + }); }; - const [{ statusCode, headers }] = await wrapEventEmitterOnce( - stream, - 'response' - ); - strictAssert(statusCode === 206, `Invalid status code: ${statusCode}`); + let boundary: string; + try { + const [{ statusCode, headers }] = await wrapEventEmitterOnce( + stream, + 'response' + ); - const match = headers['content-type']?.match( - /^multipart\/byteranges;\s*boundary=([^\s;]+)/ - ); - strictAssert(match, `Invalid Content-Type: ${headers['content-type']}`); + // When the result is single range we might get 200 status code + if (ranges.length === 1 && statusCode === 200) { + await saveDiffStream({ + diff: ranges[0], + stream, + abortSignal, + output, + chunkStatusCallback, + }); + return; + } - const dicer = new Dicer({ boundary: match[1] }); + strictAssert(statusCode === 206, `Invalid status code: ${statusCode}`); + + const match = headers['content-type']?.match( + /^multipart\/byteranges;\s*boundary=([^\s;]+)/ + ); + strictAssert(match, `Invalid Content-Type: ${headers['content-type']}`); + + // eslint-disable-next-line prefer-destructuring + boundary = match[1]; + } catch (error) { + // Ignore further errors and destroy stream early + stream.on('error', noop); + stream.destroy(); + + throw error; + } + + const dicer = new Dicer({ boundary }); const partPromises = new Array>(); dicer.on('part', part => partPromises.push(onPart(part))); @@ -472,3 +480,43 @@ async function takeDiffFromPart( return diff; } + +async function saveDiffStream({ + diff, + stream, + output, + abortSignal, + chunkStatusCallback, +}: { + diff: DiffType; + stream: Readable; + output: FileHandle; + abortSignal?: AbortSignal; + chunkStatusCallback: (chunkSize: number) => void; +}): Promise { + let offset = 0; + for await (const chunk of stream) { + strictAssert( + offset + chunk.length <= diff.size, + 'Server returned more data than expected, ' + + `written=${offset} ` + + `newChunk=${chunk.length} ` + + `maxSize=${diff.size}` + ); + + if (abortSignal?.aborted) { + return; + } + + await output.write(chunk, 0, chunk.length, offset + diff.writeOffset); + offset += chunk.length; + + chunkStatusCallback(chunk.length); + } + + strictAssert( + offset === diff.size, + `Not enough data to download from offset=${diff.readOffset} ` + + `size=${diff.size}` + ); +}