Deduplicate incoming stickers from installed sticker packs

2026-02-14 23:18:54 +00:00 · 2026-02-12 12:36:53 -05:00
parent 95f131efbf
commit ae90a74cef
8 changed files with 235 additions and 194 deletions
--- a/app/attachments.node.ts
+++ b/app/attachments.node.ts
@@ -316,6 +316,7 @@ export const readAndDecryptDataFromDisk = async ({
  return Buffer.concat(chunks);
 };

+export const CURRENT_ATTACHMENT_VERSION = 2;
 export const writeNewAttachmentData = async ({
  data,
  getAbsoluteAttachmentPath,
@@ -333,7 +334,7 @@ export const writeNewAttachmentData = async ({
  });

  return {
-    version: 2,
+    version: CURRENT_ATTACHMENT_VERSION,
    plaintextHash,
    size: data.byteLength,
    path,
--- a/ts/jobs/AttachmentDownloadManager.preload.ts
+++ b/ts/jobs/AttachmentDownloadManager.preload.ts
@@ -21,7 +21,6 @@ import {
 import {
  maybeDeleteAttachmentFile,
  deleteDownloadFile,
-  doesAttachmentExist,
  processNewAttachment,
 } from '../util/migrations.preload.js';
 import { DataReader, DataWriter } from '../sql/Client.preload.js';
@@ -62,11 +61,7 @@ import {
  type JobManagerJobResultType,
  type JobManagerJobType,
 } from './JobManager.std.js';
-import {
-  IMAGE_WEBP,
-  type MIMEType,
-  stringToMIMEType,
-} from '../types/MIME.std.js';
+import { IMAGE_WEBP } from '../types/MIME.std.js';
 import { AttachmentDownloadSource } from '../sql/Interface.std.js';
 import { drop } from '../util/drop.std.js';
 import { type ReencryptedAttachmentV2 } from '../AttachmentCrypto.node.js';
@@ -92,7 +87,7 @@ import { isAbortError } from '../util/isAbortError.std.js';
 import { itemStorage } from '../textsecure/Storage.preload.js';
 import { calculateExpirationTimestamp } from '../util/expirationTimer.std.js';
 import { cleanupAttachmentFiles } from '../types/Message2.preload.js';
-import type { WithRequiredProperties } from '../types/Util.std.js';
+import { getExistingAttachmentDataForReuse } from '../util/attachments/deduplicateAttachment.preload.js';

 const { noop, omit, throttle } = lodash;

@@ -862,14 +857,19 @@ export async function runDownloadAttachmentJobInner({
      },
    });

+    const existingAttachmentData = await getExistingAttachmentDataForReuse({
+      plaintextHash: downloadedAttachment.plaintextHash,
+      contentType: attachment.contentType,
+      logId,
+    });
+
+    if (existingAttachmentData) {
+      await dependencies.maybeDeleteAttachmentFile(downloadedAttachment.path);
+    }
+
    const attachmentDataToUse: Partial<AttachmentType> = {
      ...downloadedAttachment,
-      ...(await getExistingAttachmentDataForReuse({
-        downloadedAttachment,
-        contentType: attachment.contentType,
-        logId,
-        dependencies,
-      })),
+      ...existingAttachmentData,
    };

    const upgradedAttachment = await dependencies.processNewAttachment(
@@ -1055,100 +1055,3 @@ function _markAttachmentAsTransientlyErrored(
 ): AttachmentType {
  return { ...attachment, pending: false, error: true };
 }
-
-type AttachmentDataToBeReused = WithRequiredProperties<
-  Pick<
-    AttachmentType,
-    | 'path'
-    | 'localKey'
-    | 'version'
-    | 'thumbnail'
-    | 'screenshot'
-    | 'width'
-    | 'height'
-  >,
-  'path' | 'localKey' | 'version'
->;
-async function getExistingAttachmentDataForReuse({
-  downloadedAttachment,
-  contentType,
-  logId,
-  dependencies,
-}: {
-  downloadedAttachment: ReencryptedAttachmentV2;
-  contentType: MIMEType;
-  logId: string;
-  dependencies: Pick<DependenciesType, 'maybeDeleteAttachmentFile'>;
-}): Promise<AttachmentDataToBeReused | null> {
-  const existingAttachmentData =
-    await DataWriter.getAndProtectExistingAttachmentPath({
-      plaintextHash: downloadedAttachment.plaintextHash,
-      version: downloadedAttachment.version,
-      contentType,
-    });
-
-  if (!existingAttachmentData) {
-    return null;
-  }
-
-  strictAssert(existingAttachmentData.path, 'path must exist for reuse');
-  strictAssert(
-    existingAttachmentData.version === downloadedAttachment.version,
-    'version mismatch'
-  );
-  strictAssert(existingAttachmentData.localKey, 'localKey must exist');
-
-  if (!(await doesAttachmentExist(existingAttachmentData.path))) {
-    log.warn(
-      `${logId}: Existing attachment no longer exists, using newly downloaded one`
-    );
-    return null;
-  }
-
-  log.info(`${logId}: Reusing existing attachment`);
-
-  await dependencies.maybeDeleteAttachmentFile(downloadedAttachment.path);
-
-  const dataToReuse: AttachmentDataToBeReused = {
-    path: existingAttachmentData.path,
-    localKey: existingAttachmentData.localKey,
-    version: existingAttachmentData.version,
-    width: existingAttachmentData.width ?? undefined,
-    height: existingAttachmentData.height ?? undefined,
-  };
-  const { thumbnailPath, thumbnailSize, thumbnailContentType } =
-    existingAttachmentData;
-
-  if (
-    thumbnailPath &&
-    thumbnailSize &&
-    thumbnailContentType &&
-    (await doesAttachmentExist(thumbnailPath))
-  ) {
-    dataToReuse.thumbnail = {
-      path: thumbnailPath,
-      localKey: existingAttachmentData.thumbnailLocalKey ?? undefined,
-      version: existingAttachmentData.thumbnailVersion ?? undefined,
-      size: thumbnailSize,
-      contentType: stringToMIMEType(thumbnailContentType),
-    };
-  }
-
-  const { screenshotPath, screenshotSize, screenshotContentType } =
-    existingAttachmentData;
-  if (
-    screenshotPath &&
-    screenshotSize &&
-    screenshotContentType &&
-    (await doesAttachmentExist(screenshotPath))
-  ) {
-    dataToReuse.screenshot = {
-      path: screenshotPath,
-      localKey: existingAttachmentData.screenshotLocalKey ?? undefined,
-      version: existingAttachmentData.screenshotVersion ?? undefined,
-      size: screenshotSize,
-      contentType: stringToMIMEType(screenshotContentType),
-    };
-  }
-  return dataToReuse;
-}
--- a/ts/test-electron/services/AttachmentDownloadManager_test.preload.ts
+++ b/ts/test-electron/services/AttachmentDownloadManager_test.preload.ts
@@ -1432,12 +1432,24 @@ describe('AttachmentDownloadManager.runDownloadAttachmentJobInner', () => {
      await writeAttachmentFile('existingThumbnailPath');
      await writeAttachmentFile('existingScreenshotPath');

-      // @ts-expect-error new attachment version
+      await DataWriter.saveMessages(
+        [
+          {
+            ...existingMessageWithDownloadedAttachment,
+            attachments: [{ ...existingAttachment, version: 1 }],
+          },
+        ],
+        {
+          ourAci: generateAci(),
+          postSaveUpdates: () => Promise.resolve(),
+        }
+      );
+
      downloadAttachment.callsFake(async ({ attachment }) => {
        return {
          path: 'newlyDownloadedPath',
          plaintextHash: existingAttachment.plaintextHash,
-          version: existingAttachment.version + 1,
+          version: 2,
          localKey: testAttachmentLocalKey(),
          size: existingAttachment.size,
          digest: attachment.digest,
@@ -1562,52 +1574,5 @@ describe('AttachmentDownloadManager.runDownloadAttachmentJobInner', () => {
      assert.strictEqual(attachment?.path, 'existingPath');
      assert.strictEqual(attachment?.thumbnail?.path, 'newThumbnailPath');
    });
-
-    it('does not reuse attachment if version is not the same as requested version', async () => {
-      await writeAttachmentFile('existingPath');
-      await writeAttachmentFile('existingThumbnailPath');
-      await writeAttachmentFile('existingScreenshotPath');
-
-      // @ts-expect-error version is wrong
-      downloadAttachment.callsFake(async ({ attachment }) => {
-        return {
-          path: 'newlyDownloadedPath',
-          plaintextHash: existingAttachment.plaintextHash,
-          version: existingAttachment.version + 1, // Newer version!
-          localKey: testAttachmentLocalKey(),
-          size: existingAttachment.size,
-          digest: attachment.digest,
-        };
-      });
-      const job = composeJob({
-        messageId: newMessage.id,
-        receivedAt: newMessage.received_at,
-        attachmentOverrides: undownloadedAttachment,
-      });
-
-      await runDownloadAttachmentJobInner({
-        job,
-        isForCurrentlyVisibleMessage: false,
-        hasMediaBackups: false,
-        abortSignal: abortController.signal,
-        maxAttachmentSizeInKib: 100 * MEBIBYTE,
-        maxTextAttachmentSizeInKib: 2 * MEBIBYTE,
-        messageExpiresAt: null,
-        dependencies: {
-          cleanupAttachmentFiles,
-          maybeDeleteAttachmentFile,
-          deleteDownloadFile,
-          downloadAttachment,
-          processNewAttachment,
-        },
-      });
-
-      // Cleans up newly downloaded path
-      assert.equal(maybeDeleteAttachmentFile.callCount, 0);
-
-      const updatedMessage = window.MessageCache.getById(newMessage.id);
-      const attachment = updatedMessage?.attributes.attachments?.[0];
-      assert.strictEqual(attachment?.path, 'newlyDownloadedPath');
-    });
  });
 });
--- a/ts/test-mock/storage/sticker_test.node.ts
+++ b/ts/test-mock/storage/sticker_test.node.ts
@@ -14,10 +14,15 @@ import {
  getStickerPackRecordPredicate,
  getStickerPackLink,
 } from './fixtures.node.js';
+import {
+  getMessageInTimelineByTimestamp,
+  sendTextMessage,
+} from '../helpers.node.js';
+import { strictAssert } from '../../util/assert.std.js';

 const { StickerPackOperation } = Proto.SyncMessage;

-describe('storage service', function (this: Mocha.Suite) {
+describe('stickers', function (this: Mocha.Suite) {
  this.timeout(durations.MINUTE);

  let bootstrap: Bootstrap;
@@ -251,5 +256,56 @@ describe('storage service', function (this: Mocha.Suite) {
    const finalState = await phone.expectStorageState('consistency check');

    assert.strictEqual(finalState.version, 5);
+
+    debug(
+      'verifying that stickers from packs can be received and paths are deduplicated'
+    );
+    const firstTimestamp = bootstrap.getTimestamp();
+    const secondTimestamp = bootstrap.getTimestamp();
+    await sendTextMessage({
+      from: firstContact,
+      to: desktop,
+      desktop,
+      text: undefined,
+      sticker: {
+        packId: STICKER_PACKS[0].id,
+        packKey: STICKER_PACKS[0].key,
+        stickerId: 0,
+      },
+      timestamp: firstTimestamp,
+    });
+
+    await sendTextMessage({
+      from: firstContact,
+      to: desktop,
+      desktop,
+      text: undefined,
+      sticker: {
+        packId: STICKER_PACKS[0].id,
+        packKey: STICKER_PACKS[0].key,
+        stickerId: 0,
+      },
+      timestamp: secondTimestamp,
+    });
+
+    await window.getByRole('button', { name: 'Go back' }).click();
+    await getMessageInTimelineByTimestamp(window, firstTimestamp)
+      .locator('.module-image--loaded')
+      .waitFor();
+
+    await getMessageInTimelineByTimestamp(window, secondTimestamp)
+      .locator('.module-image--loaded')
+      .waitFor();
+
+    const firstStickerData = (await app.getMessagesBySentAt(firstTimestamp))[0]
+      .sticker?.data;
+    const secondStickerData = (
+      await app.getMessagesBySentAt(secondTimestamp)
+    )[0].sticker?.data;
+
+    strictAssert(firstStickerData?.path, 'path exists');
+    strictAssert(firstStickerData?.localKey, 'localKey exists');
+    assert.strictEqual(firstStickerData.path, secondStickerData?.path);
+    assert.strictEqual(firstStickerData.localKey, secondStickerData?.localKey);
  });
 });
--- a/ts/types/Stickers.preload.ts
+++ b/ts/types/Stickers.preload.ts
@@ -13,7 +13,7 @@ import { getMessagesById } from '../messages/getMessagesById.preload.js';
 import * as Bytes from '../Bytes.std.js';
 import * as Errors from './errors.std.js';
 import { deriveStickerPackKey, decryptAttachmentV1 } from '../Crypto.node.js';
-import { IMAGE_WEBP } from './MIME.std.js';
+import { IMAGE_WEBP, type MIMEType } from './MIME.std.js';
 import { sniffImageMimeType } from '../util/sniffImageMimeType.std.js';
 import type {
  AttachmentType,
@@ -34,12 +34,10 @@ import {
  processNewEphemeralSticker,
  processNewSticker,
  deleteTempFile,
-  getAbsoluteStickerPath,
-  copyStickerIntoAttachmentsDirectory,
-  readAttachmentData,
  deleteSticker,
  readStickerData,
  writeNewStickerData,
+  writeNewAttachmentData,
 } from '../util/migrations.preload.js';
 import { drop } from '../util/drop.std.js';
 import { isNotNil } from '../util/isNotNil.std.js';
@@ -52,6 +50,7 @@ import {
  getSticker as doGetSticker,
  getStickerPackManifest,
 } from '../textsecure/WebAPI.preload.js';
+import { getExistingAttachmentDataForReuse } from '../util/attachments/deduplicateAttachment.preload.js';

 const { isNumber, reject, groupBy, values, chunk } = lodash;

@@ -1106,33 +1105,46 @@ export async function copyStickerToAttachments(
    );
  }

-  const { path: stickerPath } = sticker;
-  const absolutePath = getAbsoluteStickerPath(stickerPath);
-  const { path, size } =
-    await copyStickerIntoAttachmentsDirectory(absolutePath);
-
-  const newSticker: AttachmentType = {
-    ...sticker,
-    path,
-    size,
-
-    // Fall-back
-    contentType: IMAGE_WEBP,
-  };
-  const data = await readAttachmentData(newSticker);
+  const data = await readStickerData(sticker);
+  const size = data.byteLength;
+  const plaintextHash = getPlaintextHashForInMemoryAttachment(data);

+  let contentType: MIMEType;
  const sniffedMimeType = sniffImageMimeType(data);
  if (sniffedMimeType) {
-    newSticker.contentType = sniffedMimeType;
+    contentType = sniffedMimeType;
  } else {
    log.warn(
      'copyStickerToAttachments: Unable to sniff sticker MIME type; falling back to WebP'
    );
+    contentType = IMAGE_WEBP;
  }

-  newSticker.plaintextHash = getPlaintextHashForInMemoryAttachment(data);
+  const newSticker: AttachmentType = {
+    width: sticker.width,
+    height: sticker.height,
+    version: sticker.version,
+    size,
+    contentType,
+    plaintextHash,
+  };

-  return newSticker;
+  const existingAttachmentData = await getExistingAttachmentDataForReuse({
+    plaintextHash,
+    contentType,
+    logId: 'copyStickerToAttachments',
+  });
+
+  if (existingAttachmentData) {
+    return {
+      ...newSticker,
+      ...existingAttachmentData,
+    };
+  }
+
+  const newAttachmentData = await writeNewAttachmentData(data);
+
+  return { ...newSticker, ...newAttachmentData };
 }

 // In the case where a sticker pack is uninstalled, we want to delete it if there are no
--- a/ts/util/attachments/deduplicateAttachment.preload.ts
+++ b/ts/util/attachments/deduplicateAttachment.preload.ts
@@ -0,0 +1,108 @@
+// Copyright 2026 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { createLogger } from '../../logging/log.std.js';
+import { DataWriter } from '../../sql/Client.preload.js';
+import { doesAttachmentExist } from '../migrations.preload.js';
+
+import { type MIMEType, stringToMIMEType } from '../../types/MIME.std.js';
+import { strictAssert } from '../assert.std.js';
+import { type WithRequiredProperties } from '../../types/Util.std.js';
+import { type AttachmentType } from '../../types/Attachment.std.js';
+import { CURRENT_ATTACHMENT_VERSION } from '../../../app/attachments.node.js';
+
+const log = createLogger('deduplicateAttachment');
+
+type AttachmentDataToBeReused = WithRequiredProperties<
+  Pick<
+    AttachmentType,
+    | 'path'
+    | 'localKey'
+    | 'version'
+    | 'thumbnail'
+    | 'screenshot'
+    | 'width'
+    | 'height'
+  >,
+  'path' | 'localKey' | 'version'
+>;
+
+export async function getExistingAttachmentDataForReuse({
+  plaintextHash,
+  contentType,
+  logId,
+}: {
+  plaintextHash: string;
+  contentType: MIMEType;
+  logId?: string;
+}): Promise<AttachmentDataToBeReused | null> {
+  const existingAttachmentData =
+    await DataWriter.getAndProtectExistingAttachmentPath({
+      plaintextHash,
+      version: CURRENT_ATTACHMENT_VERSION,
+      contentType,
+    });
+
+  if (!existingAttachmentData) {
+    return null;
+  }
+
+  strictAssert(existingAttachmentData.path, 'path must exist for reuse');
+  strictAssert(
+    existingAttachmentData.version === CURRENT_ATTACHMENT_VERSION,
+    'version mismatch'
+  );
+  strictAssert(existingAttachmentData.localKey, 'localKey must exist');
+
+  if (!(await doesAttachmentExist(existingAttachmentData.path))) {
+    log.warn(
+      `${logId}: Existing attachment no longer exists, using newly downloaded one`
+    );
+    return null;
+  }
+
+  log.info(`${logId}: Reusing existing attachment`);
+
+  const dataToReuse: AttachmentDataToBeReused = {
+    path: existingAttachmentData.path,
+    localKey: existingAttachmentData.localKey,
+    version: existingAttachmentData.version,
+    width: existingAttachmentData.width ?? undefined,
+    height: existingAttachmentData.height ?? undefined,
+  };
+  const { thumbnailPath, thumbnailSize, thumbnailContentType } =
+    existingAttachmentData;
+
+  if (
+    thumbnailPath &&
+    thumbnailSize &&
+    thumbnailContentType &&
+    (await doesAttachmentExist(thumbnailPath))
+  ) {
+    dataToReuse.thumbnail = {
+      path: thumbnailPath,
+      localKey: existingAttachmentData.thumbnailLocalKey ?? undefined,
+      version: existingAttachmentData.thumbnailVersion ?? undefined,
+      size: thumbnailSize,
+      contentType: stringToMIMEType(thumbnailContentType),
+    };
+  }
+
+  const { screenshotPath, screenshotSize, screenshotContentType } =
+    existingAttachmentData;
+  if (
+    screenshotPath &&
+    screenshotSize &&
+    screenshotContentType &&
+    (await doesAttachmentExist(screenshotPath))
+  ) {
+    dataToReuse.screenshot = {
+      path: screenshotPath,
+      localKey: existingAttachmentData.screenshotLocalKey ?? undefined,
+      version: existingAttachmentData.screenshotVersion ?? undefined,
+      size: screenshotSize,
+      contentType: stringToMIMEType(screenshotContentType),
+    };
+  }
+  return dataToReuse;
+}
--- a/ts/util/migrations.preload.ts
+++ b/ts/util/migrations.preload.ts
@@ -137,12 +137,6 @@ export const getAbsoluteStickerPath = createAbsolutePathGetter(STICKERS_PATH);
 export const writeNewStickerData = createEncryptedWriterForNew(STICKERS_PATH);
 export const deleteSticker = createDeleter(STICKERS_PATH);
 export const readStickerData = createEncryptedReader(STICKERS_PATH);
-export const copyStickerIntoAttachmentsDirectory = copyIntoAttachmentsDirectory(
-  {
-    sourceDir: STICKERS_PATH,
-    targetDir: ATTACHMENTS_PATH,
-  }
-);

 export const getAbsoluteBadgeImageFilePath =
  createAbsolutePathGetter(BADGES_PATH);
--- a/ts/util/queueAttachmentDownloads.preload.ts
+++ b/ts/util/queueAttachmentDownloads.preload.ts
@@ -342,9 +342,8 @@ export async function queueAttachmentDownloads(
      try {
        log.info(`${logId}: Copying sticker from installed pack`);
        copiedSticker = true;
-        count += 1;
-        const data = await copyStickerToAttachments(packId, stickerId);

+        const data = await copyStickerToAttachments(packId, stickerId);
        // Refresh sticker attachment since we had to await above
        const freshSticker = message.get('sticker');
        strictAssert(freshSticker != null, 'Sticker is gone while copying');
@@ -460,13 +459,16 @@ export async function queueAttachmentDownloads(
    message.set({ editHistory });
  }

-  if (count <= 0) {
-    return false;
+  if (count > 0) {
+    log.info(`${logId}: Queued ${count} total attachment downloads`);
+    return true;
  }

-  log.info(`${logId}: Queued ${count} total attachment downloads`);
+  if (copiedSticker) {
+    return true;
+  }

-  return true;
+  return false;
 }

 export async function queueNormalAttachments({