Improve recovery from corrupted downloads

This commit is contained in:
trevor-signal 2025-02-25 14:18:34 -05:00 committed by GitHub
parent a46a4a67b9
commit 572849b9dd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 21 additions and 10 deletions

View file

@ -19,6 +19,7 @@ import {
ValidatingPassThrough, ValidatingPassThrough,
} from '@signalapp/libsignal-client/dist/incremental_mac'; } from '@signalapp/libsignal-client/dist/incremental_mac';
import type { ChunkSizeChoice } from '@signalapp/libsignal-client/dist/incremental_mac'; import type { ChunkSizeChoice } from '@signalapp/libsignal-client/dist/incremental_mac';
import { isAbsolute } from 'path';
import * as log from './logging/log'; import * as log from './logging/log';
import { import {
@ -37,7 +38,7 @@ import { finalStream } from './util/finalStream';
import { getIvAndDecipher } from './util/getIvAndDecipher'; import { getIvAndDecipher } from './util/getIvAndDecipher';
import { getMacAndUpdateHmac } from './util/getMacAndUpdateHmac'; import { getMacAndUpdateHmac } from './util/getMacAndUpdateHmac';
import { trimPadding } from './util/trimPadding'; import { trimPadding } from './util/trimPadding';
import { strictAssert } from './util/assert'; import { assertDev, strictAssert } from './util/assert';
import * as Errors from './types/errors'; import * as Errors from './types/errors';
import { isNotNil } from './util/isNotNil'; import { isNotNil } from './util/isNotNil';
import { missingCaseError } from './util/missingCaseError'; import { missingCaseError } from './util/missingCaseError';
@ -733,11 +734,17 @@ export function getPlaintextHashForInMemoryAttachment(
* Unlinks a file without throwing an error if it doesn't exist. * Unlinks a file without throwing an error if it doesn't exist.
* Throws an error if it fails to unlink for any other reason. * Throws an error if it fails to unlink for any other reason.
*/ */
export async function safeUnlink(filePath: string): Promise<void> { export async function safeUnlink(absoluteFilePath: string): Promise<void> {
assertDev(
isAbsolute(absoluteFilePath),
'safeUnlink: a relative path was passed instead of an absolute one'
);
try { try {
await unlink(filePath); await unlink(absoluteFilePath);
} catch (error) { } catch (error) {
// Ignore if file doesn't exist // Ignore if file doesn't exist
if (error.code !== 'ENOENT') { if (error.code !== 'ENOENT') {
log.error('Failed to unlink', error); log.error('Failed to unlink', error);
throw error; throw error;

View file

@ -134,7 +134,9 @@ describe('utils/ensureAttachmentIsReencryptable', async () => {
after(async () => { after(async () => {
if (path) { if (path) {
await safeUnlink(path); await safeUnlink(
window.Signal.Migrations.getAbsoluteAttachmentPath(path)
);
} }
}); });

View file

@ -126,10 +126,12 @@ export async function downloadAttachment(
let downloadResult: Awaited<ReturnType<typeof downloadToDisk>>; let downloadResult: Awaited<ReturnType<typeof downloadToDisk>>;
let { downloadPath } = attachment; let { downloadPath } = attachment;
const absoluteDownloadPath = downloadPath
? window.Signal.Migrations.getAbsoluteAttachmentPath(downloadPath)
: undefined;
let downloadOffset = 0; let downloadOffset = 0;
if (downloadPath) {
const absoluteDownloadPath = if (absoluteDownloadPath) {
window.Signal.Migrations.getAbsoluteDownloadsPath(downloadPath);
try { try {
({ size: downloadOffset } = await stat(absoluteDownloadPath)); ({ size: downloadOffset } = await stat(absoluteDownloadPath));
} catch (error) { } catch (error) {
@ -139,7 +141,7 @@ export async function downloadAttachment(
Errors.toLogFormat(error) Errors.toLogFormat(error)
); );
try { try {
await safeUnlink(downloadPath); await safeUnlink(absoluteDownloadPath);
} catch { } catch {
downloadPath = undefined; downloadPath = undefined;
} }
@ -148,9 +150,9 @@ export async function downloadAttachment(
} }
// Start over if we go over the size // Start over if we go over the size
if (downloadOffset >= size && downloadPath) { if (downloadOffset >= size && absoluteDownloadPath) {
log.warn('downloadAttachment: went over, retrying'); log.warn('downloadAttachment: went over, retrying');
await safeUnlink(downloadPath); await safeUnlink(absoluteDownloadPath);
downloadOffset = 0; downloadOffset = 0;
} }