diff --git a/ts/jobs/AttachmentBackupManager.ts b/ts/jobs/AttachmentBackupManager.ts index 3965bbaab8..7be3aaf202 100644 --- a/ts/jobs/AttachmentBackupManager.ts +++ b/ts/jobs/AttachmentBackupManager.ts @@ -25,7 +25,6 @@ import { ReencyptedDigestMismatchError, } from '../AttachmentCrypto'; import { getBackupKey } from '../services/backups/crypto'; -import { isMoreRecentThan } from '../util/timestamp'; import type { AttachmentBackupJobType, CoreAttachmentBackupJobType, @@ -35,8 +34,8 @@ import { encryptAndUploadAttachment } from '../util/uploadAttachment'; import { getMediaIdFromMediaName } from '../services/backups/util/mediaId'; import { fromBase64 } from '../Bytes'; import type { WebAPIType } from '../textsecure/WebAPI'; +import { mightStillBeOnTransitTier } from '../types/Attachment'; -const TIME_ON_TRANSIT_TIER = 30 * durations.DAY; const MAX_CONCURRENT_JOBS = 3; const RETRY_CONFIG = { // As long as we have the file locally, we're always going to keep trying @@ -195,13 +194,7 @@ async function runAttachmentBackupJobInner( cdnNumber: transitCdnNumber, uploadTimestamp: transitCdnUploadTimestamp, } = transitCdnInfo; - if ( - transitCdnKey && - transitCdnNumber != null && - (!transitCdnUploadTimestamp || - isMoreRecentThan(transitCdnUploadTimestamp, TIME_ON_TRANSIT_TIER)) - ) { - // If we have transit CDN info, optimistically try to copy to the backup tier + if (mightStillBeOnTransitTier(transitCdnInfo)) { try { await copyToBackupTier({ cdnKey: transitCdnKey, diff --git a/ts/jobs/AttachmentDownloadManager.ts b/ts/jobs/AttachmentDownloadManager.ts index d33624f291..dc8c17ce5b 100644 --- a/ts/jobs/AttachmentDownloadManager.ts +++ b/ts/jobs/AttachmentDownloadManager.ts @@ -11,7 +11,7 @@ import { coreAttachmentDownloadJobSchema, } from '../types/AttachmentDownload'; import { - AttachmentNotFoundOnCdnError, + AttachmentPermanentlyUndownloadableError, downloadAttachment, } from '../util/downloadAttachment'; import dataInterface from '../sql/Client'; @@ -240,7 +240,7 @@ async function runDownloadAttachmentJob( return { status: 'finished' }; } - if (error instanceof AttachmentNotFoundOnCdnError) { + if (error instanceof AttachmentPermanentlyUndownloadableError) { await addAttachmentToMessage( message.id, _markAttachmentAsPermanentlyErrored(job.attachment), diff --git a/ts/test-electron/util/downloadAttachment_test.ts b/ts/test-electron/util/downloadAttachment_test.ts new file mode 100644 index 0000000000..59a8828cfa --- /dev/null +++ b/ts/test-electron/util/downloadAttachment_test.ts @@ -0,0 +1,242 @@ +// Copyright 2024 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from 'chai'; +import * as sinon from 'sinon'; +import { IMAGE_PNG } from '../../types/MIME'; +import { + AttachmentPermanentlyUndownloadableError, + downloadAttachment, +} from '../../util/downloadAttachment'; +import { MediaTier } from '../../types/AttachmentDownload'; +import { HTTPError } from '../../textsecure/Errors'; +import { getCdnNumberForBackupTier } from '../../textsecure/downloadAttachment'; +import { MASTER_KEY } from '../backup/helpers'; +import { getMediaIdFromMediaName } from '../../services/backups/util/mediaId'; + +describe('utils/downloadAttachment', () => { + const baseAttachment = { + size: 100, + contentType: IMAGE_PNG, + }; + + let sandbox: sinon.SinonSandbox; + const fakeServer = {}; + beforeEach(() => { + sandbox = sinon.createSandbox(); + sandbox.stub(window, 'textsecure').value({ server: fakeServer }); + }); + afterEach(() => { + sandbox.restore(); + }); + + it('downloads from transit tier first if no backup information', async () => { + const stubDownload = sinon.stub(); + const attachment = { + ...baseAttachment, + cdnKey: 'cdnKey', + cdnNumber: 2, + }; + await downloadAttachment(attachment, { + downloadAttachmentFromServer: stubDownload, + }); + assert.equal(stubDownload.callCount, 1); + assert.deepEqual(stubDownload.getCall(0).args, [ + fakeServer, + attachment, + { mediaTier: MediaTier.STANDARD }, + ]); + }); + + it('throw permanently missing error if attachment fails with 404 and no backup information', async () => { + const stubDownload = sinon + .stub() + .onFirstCall() + .throws(new HTTPError('not found', { code: 404, headers: {} })); + + const attachment = { + ...baseAttachment, + cdnKey: 'cdnKey', + cdnNumber: 2, + }; + await assert.isRejected( + downloadAttachment(attachment, { + downloadAttachmentFromServer: stubDownload, + }), + AttachmentPermanentlyUndownloadableError + ); + + assert.equal(stubDownload.callCount, 1); + assert.deepEqual(stubDownload.getCall(0).args, [ + fakeServer, + attachment, + { mediaTier: MediaTier.STANDARD }, + ]); + }); + + it('downloads from backup tier first if there is backup information', async () => { + const stubDownload = sinon.stub(); + const attachment = { + ...baseAttachment, + cdnKey: 'cdnKey', + cdnNumber: 2, + backupLocator: { + mediaName: 'medianame', + }, + }; + await downloadAttachment(attachment, { + downloadAttachmentFromServer: stubDownload, + }); + assert.equal(stubDownload.callCount, 1); + assert.deepEqual(stubDownload.getCall(0).args, [ + fakeServer, + attachment, + { mediaTier: MediaTier.BACKUP }, + ]); + }); + + it('falls back to transit tier if backup download fails with 404', async () => { + const stubDownload = sinon + .stub() + .onFirstCall() + .throws(new HTTPError('not found', { code: 404, headers: {} })); + + const attachment = { + ...baseAttachment, + cdnKey: 'cdnKey', + cdnNumber: 2, + backupLocator: { + mediaName: 'medianame', + }, + }; + await downloadAttachment(attachment, { + downloadAttachmentFromServer: stubDownload, + }); + assert.equal(stubDownload.callCount, 2); + assert.deepEqual(stubDownload.getCall(0).args, [ + fakeServer, + attachment, + { mediaTier: MediaTier.BACKUP }, + ]); + assert.deepEqual(stubDownload.getCall(1).args, [ + fakeServer, + attachment, + { mediaTier: MediaTier.STANDARD }, + ]); + }); + + it('falls back to transit tier if backup download fails with any other error', async () => { + const stubDownload = sinon + .stub() + .onFirstCall() + .throws(new Error('could not decrypt!')); + + const attachment = { + ...baseAttachment, + cdnKey: 'cdnKey', + cdnNumber: 2, + backupLocator: { + mediaName: 'medianame', + }, + }; + await downloadAttachment(attachment, { + downloadAttachmentFromServer: stubDownload, + }); + assert.equal(stubDownload.callCount, 2); + assert.deepEqual(stubDownload.getCall(0).args, [ + fakeServer, + attachment, + { mediaTier: MediaTier.BACKUP }, + ]); + assert.deepEqual(stubDownload.getCall(1).args, [ + fakeServer, + attachment, + { mediaTier: MediaTier.STANDARD }, + ]); + }); + + it('does not throw permanently missing error if not found on transit tier but there is backuplocator', async () => { + const stubDownload = sinon + .stub() + .throws(new HTTPError('not found', { code: 404, headers: {} })); + + const attachment = { + ...baseAttachment, + cdnKey: 'cdnKey', + cdnNumber: 2, + backupLocator: { + mediaName: 'medianame', + }, + }; + + await assert.isRejected( + downloadAttachment(attachment, { + downloadAttachmentFromServer: stubDownload, + }), + HTTPError + ); + assert.equal(stubDownload.callCount, 2); + assert.deepEqual(stubDownload.getCall(0).args, [ + fakeServer, + attachment, + { mediaTier: MediaTier.BACKUP }, + ]); + assert.deepEqual(stubDownload.getCall(1).args, [ + fakeServer, + attachment, + { mediaTier: MediaTier.STANDARD }, + ]); + }); +}); + +describe('getCdnNumberForBackupTier', () => { + let sandbox: sinon.SinonSandbox; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + sandbox.stub(window.storage, 'get').callsFake(key => { + if (key === 'masterKey') { + return MASTER_KEY; + } + return undefined; + }); + }); + + afterEach(async () => { + await window.Signal.Data.clearAllBackupCdnObjectMetadata(); + sandbox.restore(); + }); + + const baseAttachment = { + size: 100, + contentType: IMAGE_PNG, + }; + it('uses cdnNumber on attachment', async () => { + const result = await getCdnNumberForBackupTier({ + ...baseAttachment, + backupLocator: { mediaName: 'mediaName', cdnNumber: 4 }, + }); + assert.equal(result, 4); + }); + it('uses default cdn number if none on attachment', async () => { + const result = await getCdnNumberForBackupTier({ + ...baseAttachment, + backupLocator: { mediaName: 'mediaName' }, + }); + assert.equal(result, 3); + }); + it('uses cdn number in DB if none on attachment', async () => { + await window.Signal.Data.saveBackupCdnObjectMetadata([ + { + mediaId: getMediaIdFromMediaName('mediaName').string, + cdnNumber: 42, + sizeOnBackupCdn: 128, + }, + ]); + const result = await getCdnNumberForBackupTier({ + ...baseAttachment, + backupLocator: { mediaName: 'mediaName' }, + }); + assert.equal(result, 42); + }); +}); diff --git a/ts/textsecure/downloadAttachment.ts b/ts/textsecure/downloadAttachment.ts index 4e152cb668..87df7e0db7 100644 --- a/ts/textsecure/downloadAttachment.ts +++ b/ts/textsecure/downloadAttachment.ts @@ -10,7 +10,11 @@ import { ensureFile } from 'fs-extra'; import * as log from '../logging/log'; import * as Errors from '../types/errors'; import { strictAssert } from '../util/assert'; -import { AttachmentSizeError, type AttachmentType } from '../types/Attachment'; +import { + AttachmentSizeError, + mightBeOnBackupTier, + type AttachmentType, +} from '../types/Attachment'; import * as MIME from '../types/MIME'; import * as Bytes from '../Bytes'; import { @@ -47,16 +51,25 @@ function getBackupMediaKeyMaterial( return deriveBackupMediaKeyMaterial(backupKey, mediaId.bytes); } -async function getCdnNumberForBackupTier( +export async function getCdnNumberForBackupTier( attachment: ProcessedAttachment ): Promise { strictAssert( attachment.backupLocator, 'Attachment was missing backupLocator' ); - const backupCdnNumber = attachment.backupLocator.cdnNumber; - // TODO (DESKTOP-6983): get backupNumber by querying for all media - return backupCdnNumber || DEFAULT_BACKUP_CDN_NUMBER; + let backupCdnNumber = attachment.backupLocator.cdnNumber; + + if (backupCdnNumber == null) { + const mediaId = getMediaIdForAttachment(attachment); + const backupCdnInfo = await backupsService.getBackupCdnInfo(mediaId.string); + if (backupCdnInfo.isInBackupTier) { + backupCdnNumber = backupCdnInfo.cdnNumber; + } else { + backupCdnNumber = DEFAULT_BACKUP_CDN_NUMBER; + } + } + return backupCdnNumber; } export async function downloadAttachment( @@ -65,11 +78,11 @@ export async function downloadAttachment( options?: { disableRetries?: boolean; timeout?: number; - onlyFromTransitTier?: boolean; + mediaTier?: MediaTier; logPrefix?: string; } ): Promise { - const logId = `${options?.logPrefix}/downloadAttachmentV2`; + const logId = `${options?.logPrefix}/downloadAttachment`; const { digest, key, size, contentType } = attachment; @@ -77,11 +90,9 @@ export async function downloadAttachment( strictAssert(key, `${logId}: missing key`); strictAssert(isNumber(size), `${logId}: missing size`); - // TODO (DESKTOP-7043): allow downloading from transit tier even if there is a backup - // locator (as fallback) - const mediaTier = attachment.backupLocator - ? MediaTier.BACKUP - : MediaTier.STANDARD; + const mediaTier = + options?.mediaTier ?? + (mightBeOnBackupTier(attachment) ? MediaTier.BACKUP : MediaTier.STANDARD); let downloadedPath: string; if (mediaTier === MediaTier.STANDARD) { diff --git a/ts/types/Attachment.ts b/ts/types/Attachment.ts index 32d4daaee2..d6fa681b7e 100644 --- a/ts/types/Attachment.ts +++ b/ts/types/Attachment.ts @@ -28,6 +28,8 @@ import { ReadStatus } from '../messages/MessageReadStatus'; import type { MessageStatusType } from '../components/conversation/Message'; import { strictAssert } from '../util/assert'; import type { SignalService as Proto } from '../protobuf'; +import { isMoreRecentThan } from '../util/timestamp'; +import { DAY } from '../util/durations'; const MAX_WIDTH = 300; const MAX_HEIGHT = MAX_WIDTH * 1.5; @@ -1049,13 +1051,48 @@ export function isReencryptableToSameDigest( ); } +const TIME_ON_TRANSIT_TIER = 30 * DAY; +// Extend range in case the attachment is actually still there (this function is meant to +// be optimistic) +const BUFFERED_TIME_ON_TRANSIT_TIER = TIME_ON_TRANSIT_TIER + 5 * DAY; + +export function mightStillBeOnTransitTier( + attachment: Pick +): boolean { + if (!attachment.cdnKey) { + return false; + } + if (attachment.cdnNumber == null) { + return false; + } + + if (!attachment.uploadTimestamp) { + // Let's be conservative and still assume it might be downloadable + return true; + } + + if ( + isMoreRecentThan(attachment.uploadTimestamp, BUFFERED_TIME_ON_TRANSIT_TIER) + ) { + return true; + } + + return false; +} + +export function mightBeOnBackupTier( + attachment: Pick +): boolean { + return Boolean(attachment.backupLocator?.mediaName); +} + export function isDownloadableFromTransitTier( attachment: AttachmentType ): attachment is AttachmentDownloadableFromTransitTier { if (!isDecryptable(attachment)) { return false; } - if (attachment.cdnKey && attachment.cdnNumber) { + if (attachment.cdnKey && attachment.cdnNumber != null) { return true; } return false; diff --git a/ts/util/downloadAttachment.ts b/ts/util/downloadAttachment.ts index d89b1a4d69..841fbbeac4 100644 --- a/ts/util/downloadAttachment.ts +++ b/ts/util/downloadAttachment.ts @@ -1,20 +1,30 @@ // Copyright 2020 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -import type { AttachmentType } from '../types/Attachment'; +import { type AttachmentType, mightBeOnBackupTier } from '../types/Attachment'; import { downloadAttachment as doDownloadAttachment } from '../textsecure/downloadAttachment'; +import { MediaTier } from '../types/AttachmentDownload'; +import * as log from '../logging/log'; +import { redactGenericText } from './privacy'; +import { HTTPError } from '../textsecure/Errors'; +import { toLogFormat } from '../types/errors'; + +export class AttachmentPermanentlyUndownloadableError extends Error {} -export class AttachmentNotFoundOnCdnError extends Error {} export async function downloadAttachment( - attachmentData: AttachmentType + attachmentData: AttachmentType, + dependencies = { downloadAttachmentFromServer: doDownloadAttachment } ): Promise { - let migratedAttachment: AttachmentType; + const redactedDigest = redactGenericText(attachmentData.digest ?? ''); + const logId = `downloadAttachment(${redactedDigest})`; const { server } = window.textsecure; if (!server) { throw new Error('window.textsecure.server is not available!'); } + let migratedAttachment: AttachmentType; + const { id: legacyId } = attachmentData; if (legacyId === undefined) { migratedAttachment = attachmentData; @@ -25,17 +35,54 @@ export async function downloadAttachment( }; } - let downloaded; - try { - downloaded = await doDownloadAttachment(server, migratedAttachment); - } catch (error) { - // Attachments on the server expire after 30 days, then start returning 404 or 403 - if (error && (error.code === 404 || error.code === 403)) { - throw new AttachmentNotFoundOnCdnError(error.code); + if (mightBeOnBackupTier(migratedAttachment)) { + try { + return await dependencies.downloadAttachmentFromServer( + server, + migratedAttachment, + { + mediaTier: MediaTier.BACKUP, + } + ); + } catch (error) { + if (error instanceof HTTPError && error.code === 404) { + // This is an expected occurrence if restoring from a backup before the + // attachment has been moved to the backup tier + log.warn(`${logId}: attachment not found on backup CDN`); + } else { + // We also just log this error instead of throwing, since we want to still try to + // find it on the attachment tier. + log.error( + `${logId}: error when downloading from backup CDN`, + toLogFormat(error) + ); + } } - - throw error; } - return downloaded; + try { + return await dependencies.downloadAttachmentFromServer( + server, + migratedAttachment, + { + mediaTier: MediaTier.STANDARD, + } + ); + } catch (error) { + if (mightBeOnBackupTier(migratedAttachment)) { + // We don't want to throw the AttachmentPermanentlyUndownloadableError because we + // may just need to wait for this attachment to end up on the backup tier + throw error; + } + // Attachments on the transit tier expire after 30 days, then start returning 404 or + // 403 + if ( + error instanceof HTTPError && + (error.code === 404 || error.code === 403) + ) { + throw new AttachmentPermanentlyUndownloadableError(); + } else { + throw error; + } + } }