Fallback to download from transit tier if attachment not found on backup tier

Co-authored-by: trevor-signal <131492920+trevor-signal@users.noreply.github.com>

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,

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),

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);
+  });
+});

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<number> {
   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<AttachmentType> {
-  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) {

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<AttachmentType, 'cdnKey' | 'cdnNumber' | 'uploadTimestamp'>
+): 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<AttachmentType, 'backupLocator'>
+): 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;

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<AttachmentType> {
-  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;
+    }
+  }
 }