Update behavior for soon-to-expire attachments on backup CDN

This commit is contained in:
trevor-signal
2025-11-20 13:12:15 -05:00
committed by GitHub
parent 42af9b5c3d
commit a73133e534
7 changed files with 137 additions and 8 deletions

View File

@@ -86,6 +86,7 @@ import { getMessageQueueTime as doGetMessageQueueTime } from '../util/getMessage
import { JobCancelReason } from './types.std.js';
import { isAbortError } from '../util/isAbortError.std.js';
import { itemStorage } from '../textsecure/Storage.preload.js';
import { calculateExpirationTimestamp } from '../util/expirationTimer.std.js';
const { noop, omit, throttle } = lodash;
@@ -541,6 +542,8 @@ export async function runDownloadAttachmentJob({
maxAttachmentSizeInKib: options.maxAttachmentSizeInKib,
maxTextAttachmentSizeInKib: options.maxTextAttachmentSizeInKib,
dependencies,
messageExpiresAt:
calculateExpirationTimestamp(message.attributes) ?? null,
});
if (result.downloadedVariant === AttachmentVariant.ThumbnailFromBackup) {
@@ -714,10 +717,12 @@ export async function runDownloadAttachmentJobInner({
maxAttachmentSizeInKib,
maxTextAttachmentSizeInKib,
hasMediaBackups,
messageExpiresAt,
dependencies,
}: {
job: AttachmentDownloadJobType;
dependencies: Omit<DependenciesType, 'runDownloadAttachmentJobInner'>;
messageExpiresAt: number | null;
} & RunDownloadAttachmentJobOptions): Promise<DownloadAttachmentResultType> {
const { messageId, attachment, attachmentType } = job;
@@ -778,6 +783,7 @@ export async function runDownloadAttachmentJobInner({
abortSignal,
dependencies,
logId,
messageExpiresAt,
});
await addAttachmentToMessage(
messageId,
@@ -847,6 +853,7 @@ export async function runDownloadAttachmentJobInner({
abortSignal,
hasMediaBackups,
logId,
messageExpiresAt,
},
});
@@ -924,6 +931,7 @@ export async function runDownloadAttachmentJobInner({
abortSignal,
dependencies,
logId,
messageExpiresAt,
});
await addAttachmentToMessage(
@@ -981,10 +989,12 @@ async function downloadBackupThumbnail({
abortSignal,
logId,
dependencies,
messageExpiresAt,
}: {
attachment: AttachmentType;
abortSignal: AbortSignal;
logId: string;
messageExpiresAt: number | null;
dependencies: {
downloadAttachment: typeof downloadAttachmentUtil;
};
@@ -995,6 +1005,7 @@ async function downloadBackupThumbnail({
onSizeUpdate: noop,
variant: AttachmentVariant.ThumbnailFromBackup,
abortSignal,
messageExpiresAt,
hasMediaBackups: true,
logId,
},

View File

@@ -179,6 +179,7 @@ import {
import { KIBIBYTE } from '../../types/AttachmentSize.std.js';
import { itemStorage } from '../../textsecure/Storage.preload.js';
import { ChatFolderType } from '../../types/ChatFolder.std.js';
import { expiresTooSoonForBackup } from './util/expiration.std.js';
const { isNumber } = lodash;
@@ -1353,8 +1354,9 @@ export class BackupExportStream extends Readable {
}
const expirationTimestamp = calculateExpirationTimestamp(message);
if (expirationTimestamp != null && expirationTimestamp <= this.#now + DAY) {
// Message expires too soon
if (
expiresTooSoonForBackup({ messageExpiresAt: expirationTimestamp ?? null })
) {
return undefined;
}

View File

@@ -166,6 +166,7 @@ import { updateBackupMediaDownloadProgress } from '../../util/updateBackupMediaD
import { itemStorage } from '../../textsecure/Storage.preload.js';
import { ChatFolderType } from '../../types/ChatFolder.std.js';
import type { ChatFolderId, ChatFolder } from '../../types/ChatFolder.std.js';
import { expiresTooSoonForBackup } from './util/expiration.std.js';
const { isNumber } = lodash;
@@ -690,11 +691,16 @@ export class BackupImportStream extends Writable {
const conversation = this.#conversations.get(attributes.conversationId);
if (conversation && isConversationAccepted(conversation)) {
const model = new MessageModel(attributes);
const attachmentsAreLikelyExpired = expiresTooSoonForBackup({
messageExpiresAt: calculateExpirationTimestamp(attributes) ?? null,
});
attachmentDownloadJobPromises.push(
queueAttachmentDownloads(model, {
source: this.#isMediaEnabledBackup()
? AttachmentDownloadSource.BACKUP_IMPORT_WITH_MEDIA
: AttachmentDownloadSource.BACKUP_IMPORT_NO_MEDIA,
source:
this.#isMediaEnabledBackup() && !attachmentsAreLikelyExpired
? AttachmentDownloadSource.BACKUP_IMPORT_WITH_MEDIA
: AttachmentDownloadSource.BACKUP_IMPORT_NO_MEDIA,
isManualDownload: false,
})
);

View File

@@ -0,0 +1,22 @@
// Copyright 2025 Signal Messenger, LLC
// SPDX-License-Identifier: AGPL-3.0-only
import { DAY } from '../../../util/durations/constants.std.js';
// Messages that expire with 24 hours are excluded from regular backups (and their
// attachments may not be backed up), but they will be present in link & sync backups
const EXCLUDE_MESSAGE_FROM_BACKUP_IF_EXPIRING_WITHIN_MS = DAY;
export function expiresTooSoonForBackup({
messageExpiresAt,
}: {
messageExpiresAt: number | null;
}): boolean {
if (messageExpiresAt == null) {
return false;
}
return (
messageExpiresAt <=
Date.now() + EXCLUDE_MESSAGE_FROM_BACKUP_IF_EXPIRING_WITHIN_MS
);
}

View File

@@ -907,6 +907,7 @@ describe('AttachmentDownloadManager.runDownloadAttachmentJobInner', () => {
abortSignal: abortController.signal,
maxAttachmentSizeInKib: 100 * MEBIBYTE,
maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
messageExpiresAt: null,
dependencies: {
deleteAttachmentData,
deleteDownloadData,
@@ -946,6 +947,7 @@ describe('AttachmentDownloadManager.runDownloadAttachmentJobInner', () => {
abortSignal: abortController.signal,
maxAttachmentSizeInKib: 100 * MEBIBYTE,
maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
messageExpiresAt: null,
dependencies: {
deleteAttachmentData,
deleteDownloadData,
@@ -1002,6 +1004,7 @@ describe('AttachmentDownloadManager.runDownloadAttachmentJobInner', () => {
abortSignal: abortController.signal,
maxAttachmentSizeInKib: 100 * MEBIBYTE,
maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
messageExpiresAt: null,
dependencies: {
deleteAttachmentData,
deleteDownloadData,
@@ -1040,6 +1043,7 @@ describe('AttachmentDownloadManager.runDownloadAttachmentJobInner', () => {
abortSignal: abortController.signal,
maxAttachmentSizeInKib: 100 * MEBIBYTE,
maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
messageExpiresAt: null,
dependencies: {
deleteAttachmentData,
deleteDownloadData,
@@ -1079,6 +1083,7 @@ describe('AttachmentDownloadManager.runDownloadAttachmentJobInner', () => {
abortSignal: abortController.signal,
maxAttachmentSizeInKib: 100 * MEBIBYTE,
maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
messageExpiresAt: null,
dependencies: {
deleteAttachmentData,
deleteDownloadData,
@@ -1116,6 +1121,7 @@ describe('AttachmentDownloadManager.runDownloadAttachmentJobInner', () => {
abortSignal: abortController.signal,
maxAttachmentSizeInKib: 100 * MEBIBYTE,
maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
messageExpiresAt: null,
dependencies: {
deleteAttachmentData,
deleteDownloadData,
@@ -1172,6 +1178,7 @@ describe('AttachmentDownloadManager.runDownloadAttachmentJobInner', () => {
abortSignal: abortController.signal,
maxAttachmentSizeInKib: 100 * MEBIBYTE,
maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
messageExpiresAt: null,
dependencies: {
deleteAttachmentData,
deleteDownloadData,

View File

@@ -22,6 +22,7 @@ import { toHex, toBase64 } from '../../Bytes.std.js';
import { generateAttachmentKeys } from '../../AttachmentCrypto.node.js';
import { getRandomBytes } from '../../Crypto.node.js';
import { itemStorage } from '../../textsecure/Storage.preload.js';
import { DAY, HOUR } from '../../util/durations/constants.std.js';
const { noop } = lodash;
@@ -56,6 +57,7 @@ describe('utils/downloadAttachment', () => {
hasMediaBackups: true,
onSizeUpdate: noop,
abortSignal: abortController.signal,
messageExpiresAt: null,
logId: '',
},
dependencies: {
@@ -89,6 +91,7 @@ describe('utils/downloadAttachment', () => {
hasMediaBackups: true,
onSizeUpdate: noop,
abortSignal: abortController.signal,
messageExpiresAt: Date.now() + 2 * DAY,
logId: '',
},
dependencies: {
@@ -111,6 +114,60 @@ describe('utils/downloadAttachment', () => {
]);
});
it('throw permanently missing error if attachment fails with 404 and expiring from backup tier', async () => {
const stubDownload = sinon
.stub()
.throws(new HTTPError('not found', { code: 404, headers: {} }));
const attachment = backupableAttachment;
await assert.isRejected(
downloadAttachment({
attachment,
options: {
hasMediaBackups: true,
onSizeUpdate: noop,
abortSignal: abortController.signal,
messageExpiresAt: Date.now() + HOUR,
logId: '',
},
dependencies: {
downloadAttachmentFromLocalBackup: stubDownload,
downloadAttachmentFromServer: stubDownload,
},
}),
AttachmentPermanentlyUndownloadableError
);
assert.equal(stubDownload.callCount, 2);
});
it('throw permanently missing error if attachment fails with 404 and expiring from backup tier, if no transit tier info', async () => {
const stubDownload = sinon
.stub()
.throws(new HTTPError('not found', { code: 404, headers: {} }));
const attachment = { ...backupableAttachment, cdnKey: undefined };
await assert.isRejected(
downloadAttachment({
attachment,
options: {
hasMediaBackups: true,
onSizeUpdate: noop,
abortSignal: abortController.signal,
messageExpiresAt: Date.now() + HOUR,
logId: '',
},
dependencies: {
downloadAttachmentFromLocalBackup: stubDownload,
downloadAttachmentFromServer: stubDownload,
},
}),
AttachmentPermanentlyUndownloadableError
);
assert.equal(stubDownload.callCount, 1);
});
it('throw permanently missing error if attachment fails with 403 from cdn 0 and no backup information', async () => {
const stubDownload = sinon
.stub()
@@ -125,6 +182,7 @@ describe('utils/downloadAttachment', () => {
hasMediaBackups: true,
onSizeUpdate: noop,
abortSignal: abortController.signal,
messageExpiresAt: null,
logId: '',
},
dependencies: {
@@ -162,6 +220,7 @@ describe('utils/downloadAttachment', () => {
hasMediaBackups: true,
onSizeUpdate: noop,
abortSignal: abortController.signal,
messageExpiresAt: null,
logId: '',
},
dependencies: {
@@ -182,6 +241,7 @@ describe('utils/downloadAttachment', () => {
hasMediaBackups: true,
onSizeUpdate: noop,
abortSignal: abortController.signal,
messageExpiresAt: null,
logId: '',
},
dependencies: {
@@ -214,6 +274,7 @@ describe('utils/downloadAttachment', () => {
hasMediaBackups: true,
onSizeUpdate: noop,
abortSignal: abortController.signal,
messageExpiresAt: null,
logId: '',
},
dependencies: {
@@ -258,6 +319,7 @@ describe('utils/downloadAttachment', () => {
hasMediaBackups: true,
onSizeUpdate: noop,
abortSignal: abortController.signal,
messageExpiresAt: null,
logId: '',
},
dependencies: {
@@ -300,6 +362,7 @@ describe('utils/downloadAttachment', () => {
hasMediaBackups: true,
onSizeUpdate: noop,
abortSignal: abortController.signal,
messageExpiresAt: null,
logId: '',
},
dependencies: {

View File

@@ -3,6 +3,7 @@
import { ErrorCode, LibSignalErrorBase } from '@signalapp/libsignal-client';
import {
hasRequiredInformationForRemoteBackup,
hasRequiredInformationToDownloadFromTransitTier,
wasImportedFromLocalBackup,
} from './Attachment.std.js';
import {
@@ -24,6 +25,7 @@ import type { ReencryptedAttachmentV2 } from '../AttachmentCrypto.node.js';
import * as RemoteConfig from '../RemoteConfig.dom.js';
import { ToastType } from '../types/Toast.dom.js';
import { isAbortError } from './isAbortError.std.js';
import { expiresTooSoonForBackup } from '../services/backups/util/expiration.std.js';
const log = createLogger('downloadAttachment');
@@ -35,6 +37,7 @@ export async function downloadAttachment({
abortSignal,
hasMediaBackups,
logId: _logId,
messageExpiresAt,
},
dependencies = {
downloadAttachmentFromServer: doDownloadAttachment,
@@ -48,6 +51,7 @@ export async function downloadAttachment({
abortSignal: AbortSignal;
hasMediaBackups: boolean;
logId: string;
messageExpiresAt: number | null;
};
dependencies?: {
downloadAttachmentFromServer: typeof doDownloadAttachment;
@@ -61,7 +65,11 @@ export async function downloadAttachment({
const isBackupable = hasRequiredInformationForRemoteBackup(attachment);
const mightBeOnBackupTierNow = isBackupable && hasMediaBackups;
const mightBeOnBackupTierInTheFuture = isBackupable;
const mightBeExpiredFromBackupTier = expiresTooSoonForBackup({
messageExpiresAt,
});
const mightBeOnBackupTierInTheFuture =
isBackupable && !mightBeExpiredFromBackupTier;
if (wasImportedFromLocalBackup(attachment)) {
log.info(`${logId}: Downloading attachment from local backup`);
@@ -109,9 +117,13 @@ export async function downloadAttachment({
}
const shouldFallbackToTransitTier =
variant !== AttachmentVariant.ThumbnailFromBackup;
variant !== AttachmentVariant.ThumbnailFromBackup &&
hasRequiredInformationToDownloadFromTransitTier(attachment);
if (RemoteConfig.isEnabled('desktop.internalUser')) {
if (
RemoteConfig.isEnabled('desktop.internalUser') &&
!mightBeExpiredFromBackupTier
) {
window.reduxActions.toast.showToast({
toastType: ToastType.UnableToDownloadFromBackupTier,
});
@@ -124,6 +136,12 @@ export async function downloadAttachment({
`${logId}: attachment not found on backup CDN`,
shouldFallbackToTransitTier ? 'will try transit tier' : ''
);
if (!mightBeOnBackupTierInTheFuture && !shouldFallbackToTransitTier) {
throw new AttachmentPermanentlyUndownloadableError(
`HTTP ${error.code}`
);
}
} else {
// We also just log this error instead of throwing, since we want to still try to
// find it on the attachment tier.