diff --git a/ts/messageModifiers/DeletesForMe.ts b/ts/messageModifiers/DeletesForMe.ts index 27b772bc52..db755ae492 100644 --- a/ts/messageModifiers/DeletesForMe.ts +++ b/ts/messageModifiers/DeletesForMe.ts @@ -111,7 +111,8 @@ export async function onDelete(item: DeleteForMeAttributesType): Promise { item.message, item.deleteAttachmentData, { - deleteOnDisk: window.Signal.Migrations.deleteAttachmentData, + deleteAttachmentOnDisk: + window.Signal.Migrations.deleteAttachmentData, deleteDownloadOnDisk: window.Signal.Migrations.deleteDownloadData, logId, } diff --git a/ts/signal.ts b/ts/signal.ts index 865a3ebe5f..1443bceb33 100644 --- a/ts/signal.ts +++ b/ts/signal.ts @@ -231,7 +231,7 @@ export function initializeMigrations({ const loadQuoteData = MessageType.loadQuoteData(loadAttachmentData); const loadStickerData = MessageType.loadStickerData(loadAttachmentData); const getAbsoluteAttachmentPath = createAbsolutePathGetter(attachmentsPath); - const deleteOnDisk = Attachments.createDeleter(attachmentsPath); + const deleteAttachmentOnDisk = Attachments.createDeleter(attachmentsPath); const writeNewAttachmentData = createEncryptedWriterForNew(attachmentsPath); const doesAttachmentExist = createDoesExist(attachmentsPath); @@ -282,16 +282,13 @@ export function initializeMigrations({ attachmentsPath, copyStickerIntoAttachmentsDirectory, copyAttachmentIntoTempDirectory, - deleteAttachmentData: deleteOnDisk, + deleteAttachmentData: deleteAttachmentOnDisk, deleteAvatar, deleteDownloadData: deleteDownloadOnDisk, deleteDraftFile, deleteExternalMessageFiles: MessageType.deleteAllExternalFiles({ - deleteAttachmentData: Type.deleteData({ - deleteOnDisk, - deleteDownloadOnDisk, - }), - deleteOnDisk, + deleteAttachmentOnDisk, + deleteDownloadOnDisk, }), deleteSticker, deleteTempFile, @@ -327,7 +324,6 @@ export function initializeMigrations({ getImageDimensions, makeImageThumbnail, makeVideoScreenshot, - deleteOnDisk, logger, }), processNewSticker: (stickerData: Uint8Array) => @@ -349,7 +345,7 @@ export function initializeMigrations({ const { maxVersion } = options; return MessageType.upgradeSchema(message, { - deleteOnDisk, + deleteAttachmentOnDisk, doesAttachmentExist, getImageDimensions, getRegionCode, diff --git a/ts/sql/Server.ts b/ts/sql/Server.ts index c35dec0d09..97291003cc 100644 --- a/ts/sql/Server.ts +++ b/ts/sql/Server.ts @@ -267,6 +267,7 @@ import type { } from '../types/Colors.js'; import { sqlLogger } from './sqlLogger.js'; import { permissiveMessageAttachmentSchema } from './server/messageAttachments.js'; +import { getFilePathsOwnedByMessage } from '../util/messageFilePaths.js'; const { forEach, @@ -8132,97 +8133,6 @@ function getMessageServerGuidsForSpam( .all({ conversationId }); } -function getExternalFilesForMessage(message: MessageType): { - externalAttachments: Array; - externalDownloads: Array; -} { - const { attachments, bodyAttachment, contact, quote, preview, sticker } = - message; - const externalAttachments: Array = []; - const externalDownloads: Array = []; - - forEach(attachments, attachment => { - const { - path: file, - thumbnail, - screenshot, - thumbnailFromBackup, - downloadPath, - } = attachment; - if (file) { - externalAttachments.push(file); - } - - // downloadPath is relative to downloads folder and has to be tracked - // separately. - if (downloadPath) { - externalDownloads.push(downloadPath); - } - - if (thumbnail && thumbnail.path) { - externalAttachments.push(thumbnail.path); - } - - if (screenshot && screenshot.path) { - externalAttachments.push(screenshot.path); - } - - if (thumbnailFromBackup && thumbnailFromBackup.path) { - externalAttachments.push(thumbnailFromBackup.path); - } - }); - - if (bodyAttachment?.path) { - externalAttachments.push(bodyAttachment.path); - } - - for (const editHistory of message.editHistory ?? []) { - if (editHistory.bodyAttachment?.path) { - externalAttachments.push(editHistory.bodyAttachment.path); - } - } - - if (quote && quote.attachments && quote.attachments.length) { - forEach(quote.attachments, attachment => { - const { thumbnail } = attachment; - - if (thumbnail && thumbnail.path) { - externalAttachments.push(thumbnail.path); - } - }); - } - - if (contact && contact.length) { - forEach(contact, item => { - const { avatar } = item; - - if (avatar && avatar.avatar && avatar.avatar.path) { - externalAttachments.push(avatar.avatar.path); - } - }); - } - - if (preview && preview.length) { - forEach(preview, item => { - const { image } = item; - - if (image && image.path) { - externalAttachments.push(image.path); - } - }); - } - - if (sticker && sticker.data && sticker.data.path) { - externalAttachments.push(sticker.data.path); - - if (sticker.data.thumbnail && sticker.data.thumbnail.path) { - externalAttachments.push(sticker.data.thumbnail.path); - } - } - - return { externalAttachments, externalDownloads }; -} - function getExternalFilesForConversation( conversation: Pick ): Array { @@ -8277,7 +8187,7 @@ function getKnownMessageAttachments( const { messages, cursor: newCursor } = pageMessages(db, innerCursor); for (const message of messages) { const { externalAttachments, externalDownloads } = - getExternalFilesForMessage(message); + getFilePathsOwnedByMessage(message); externalAttachments.forEach(file => attachments.add(file)); externalDownloads.forEach(file => downloads.add(file)); } diff --git a/ts/test-electron/cleanupOrphanedAttachments_test.ts b/ts/test-electron/cleanupOrphanedAttachments_test.ts index 32fe6e68f3..7472843d48 100644 --- a/ts/test-electron/cleanupOrphanedAttachments_test.ts +++ b/ts/test-electron/cleanupOrphanedAttachments_test.ts @@ -17,6 +17,7 @@ import { import { generateAci } from '../types/ServiceId.js'; import { IMAGE_JPEG, LONG_MESSAGE } from '../types/MIME.js'; +import type { MessageAttributesType } from '../model-types.js'; const { emptyDir, ensureFile } = fsExtra; @@ -49,7 +50,7 @@ async function writeFiles( ) { for (let i = 0; i < num; i += 1) { // eslint-disable-next-line no-await-in-loop - await writeFile(`file${i}`, type); + await writeFile(`${type}${i}`, type); } } @@ -57,10 +58,16 @@ function listFiles(type: 'attachment' | 'download' | 'draft'): Array { return readdirSync(dirname(getAbsolutePath('fakename', type))); } +let attachmentIndex = 0; +let downloadIndex = 0; + describe('cleanupOrphanedAttachments', () => { // TODO (DESKTOP-8613): stickers & badges beforeEach(async () => { await DataWriter.removeAll(); + + attachmentIndex = 0; + downloadIndex = 0; await emptyDir(getPath(window.SignalContext.config.userDataPath)); await emptyDir(getDownloadsPath(window.SignalContext.config.userDataPath)); await emptyDir(getDraftPath(window.SignalContext.config.userDataPath)); @@ -72,14 +79,59 @@ describe('cleanupOrphanedAttachments', () => { await emptyDir(getDraftPath(window.SignalContext.config.userDataPath)); }); + function getAttachmentFilePath() { + attachmentIndex += 1; + return `attachment${attachmentIndex}`; + } + function getDownloadFilePath() { + downloadIndex += 1; + return `download${downloadIndex}`; + } + + function composeAttachment() { + return { + contentType: IMAGE_JPEG, + size: 128, + path: getAttachmentFilePath(), + downloadPath: getDownloadFilePath(), + thumbnail: { + contentType: IMAGE_JPEG, + size: 128, + path: getAttachmentFilePath(), + }, + screenshot: { + contentType: IMAGE_JPEG, + size: 128, + path: getAttachmentFilePath(), + }, + thumbnailFromBackup: { + contentType: IMAGE_JPEG, + size: 128, + path: getAttachmentFilePath(), + }, + }; + } + + function composeBodyAttachment() { + return { + contentType: LONG_MESSAGE, + size: 128, + path: getAttachmentFilePath(), + downloadPath: getDownloadFilePath(), + }; + } + it('deletes paths if not referenced', async () => { await writeFiles(2, 'attachment'); await writeFiles(2, 'draft'); await writeFiles(2, 'download'); - assert.sameDeepMembers(listFiles('attachment'), ['file0', 'file1']); - assert.sameDeepMembers(listFiles('draft'), ['file0', 'file1']); - assert.sameDeepMembers(listFiles('download'), ['file0', 'file1']); + assert.sameDeepMembers(listFiles('attachment'), [ + 'attachment0', + 'attachment1', + ]); + assert.sameDeepMembers(listFiles('draft'), ['draft0', 'draft1']); + assert.sameDeepMembers(listFiles('download'), ['download0', 'download1']); await DataWriter.cleanupOrphanedAttachments({ _block: true }); @@ -97,109 +149,54 @@ describe('cleanupOrphanedAttachments', () => { version: 2, expireTimerVersion: 2, avatar: { - path: 'file0', + path: 'attachment0', }, profileAvatar: { - path: 'file1', + path: 'attachment1', }, }); await DataWriter.cleanupOrphanedAttachments({ _block: true }); - assert.sameDeepMembers(listFiles('attachment'), ['file0', 'file1']); + assert.sameDeepMembers(listFiles('attachment'), [ + 'attachment0', + 'attachment1', + ]); }); - it('does not delete message attachments (including thumbnails, previews, avatars, etc.)', async () => { - await writeFiles(20, 'attachment'); - await writeFiles(6, 'download'); - - // Save with legacy (un-normalized) sattachment format (attachments in JSON) - await DataWriter.saveMessage( - { + describe('message attachments', () => { + // Update these if more paths are added to composeMessageWithAllAttachments + const NUM_ATTACHMENT_FILES_IN_MESSAGE = 26; + const NUM_DOWNLOAD_FILES_IN_MESSAGE = 8; + function composeMessageWithAllAttachments(): MessageAttributesType { + return { id: generateUuid(), type: 'outgoing', sent_at: Date.now(), timestamp: Date.now(), received_at: Date.now(), conversationId: generateUuid(), - - attachments: [ - { - contentType: IMAGE_JPEG, - size: 128, - path: 'file0', - downloadPath: 'file0', - thumbnail: { - contentType: IMAGE_JPEG, - size: 128, - path: 'file1', - }, - screenshot: { - contentType: IMAGE_JPEG, - size: 128, - path: 'file2', - }, - thumbnailFromBackup: { - contentType: IMAGE_JPEG, - size: 128, - path: 'file3', - }, - }, - ], - }, - { - ourAci: generateAci(), - forceSave: true, - _testOnlyAvoidNormalizingAttachments: true, - postSaveUpdates: () => Promise.resolve(), - } - ); - - // Save one with attachments normalized - await DataWriter.saveMessage( - { - id: generateUuid(), - type: 'outgoing', - sent_at: Date.now(), - timestamp: Date.now(), - received_at: Date.now(), - conversationId: generateUuid(), - bodyAttachment: { - contentType: IMAGE_JPEG, - size: 128, - path: 'file4', - }, + bodyAttachment: composeBodyAttachment(), + attachments: [composeAttachment(), composeAttachment()], contact: [ { avatar: { isProfile: false, - avatar: { - contentType: IMAGE_JPEG, - size: 128, - path: 'file5', - }, + avatar: composeAttachment(), }, }, ], preview: [ { url: 'url', - image: { - contentType: IMAGE_JPEG, - size: 128, - path: 'file6', - }, + image: composeAttachment(), }, ], editHistory: [ { timestamp: Date.now(), received_at: Date.now(), - bodyAttachment: { - contentType: LONG_MESSAGE, - size: 128, - path: 'file7', - }, + bodyAttachment: composeBodyAttachment(), }, ], quote: { @@ -209,12 +206,7 @@ describe('cleanupOrphanedAttachments', () => { attachments: [ { contentType: IMAGE_JPEG, - - thumbnail: { - contentType: IMAGE_JPEG, - size: 128, - path: 'file8', - }, + thumbnail: composeAttachment(), }, ], }, @@ -222,40 +214,204 @@ describe('cleanupOrphanedAttachments', () => { packId: 'packId', stickerId: 42, packKey: 'packKey', - data: { - contentType: IMAGE_JPEG, - size: 128, - path: 'file9', - thumbnail: { - contentType: IMAGE_JPEG, - size: 128, - path: 'file10', - }, - }, + data: composeAttachment(), }, - }, - { + }; + } + + it('does not delete message attachments (including thumbnails, previews, avatars, etc.)', async () => { + await writeFiles(NUM_ATTACHMENT_FILES_IN_MESSAGE + 5, 'attachment'); + await writeFiles(NUM_DOWNLOAD_FILES_IN_MESSAGE + 5, 'download'); + + await DataWriter.saveMessage(composeMessageWithAllAttachments(), { ourAci: generateAci(), forceSave: true, postSaveUpdates: () => Promise.resolve(), - } - ); + }); - await DataWriter.cleanupOrphanedAttachments({ _block: true }); + await DataWriter.cleanupOrphanedAttachments({ _block: true }); - assert.sameDeepMembers(listFiles('attachment'), [ - 'file0', - 'file1', - 'file2', - 'file3', - 'file4', - 'file5', - 'file6', - 'file7', - 'file8', - 'file9', - 'file10', - ]); - assert.sameDeepMembers(listFiles('download'), ['file0']); + assert.strictEqual(attachmentIndex, NUM_ATTACHMENT_FILES_IN_MESSAGE); + assert.strictEqual(downloadIndex, NUM_DOWNLOAD_FILES_IN_MESSAGE); + + const attachmentFiles = listFiles('attachment'); + const downloadFiles = listFiles('download'); + + assert.strictEqual( + attachmentFiles.length, + NUM_ATTACHMENT_FILES_IN_MESSAGE + ); + assert.sameDeepMembers( + attachmentFiles, + new Array(attachmentIndex) + .fill(null) + .map((_, idx) => `attachment${idx + 1}`) + ); + + assert.strictEqual(downloadFiles.length, NUM_DOWNLOAD_FILES_IN_MESSAGE); + assert.sameDeepMembers( + downloadFiles, + new Array(downloadIndex) + .fill(null) + .map((_, idx) => `download${idx + 1}`) + ); + }); + + it('works with non-normalized message attachments', async () => { + await writeFiles(NUM_ATTACHMENT_FILES_IN_MESSAGE + 5, 'attachment'); + await writeFiles(NUM_DOWNLOAD_FILES_IN_MESSAGE + 5, 'download'); + + await DataWriter.saveMessage(composeMessageWithAllAttachments(), { + ourAci: generateAci(), + forceSave: true, + // Save one with attachments not normalized + _testOnlyAvoidNormalizingAttachments: true, + postSaveUpdates: () => Promise.resolve(), + }); + + await DataWriter.cleanupOrphanedAttachments({ _block: true }); + + assert.strictEqual(attachmentIndex, NUM_ATTACHMENT_FILES_IN_MESSAGE); + assert.strictEqual(downloadIndex, NUM_DOWNLOAD_FILES_IN_MESSAGE); + + const attachmentFiles = listFiles('attachment'); + const downloadFiles = listFiles('download'); + + assert.strictEqual( + attachmentFiles.length, + NUM_ATTACHMENT_FILES_IN_MESSAGE + ); + assert.sameDeepMembers( + attachmentFiles, + new Array(attachmentIndex) + .fill(null) + .map((_, idx) => `attachment${idx + 1}`) + ); + + assert.strictEqual(downloadFiles.length, NUM_DOWNLOAD_FILES_IN_MESSAGE); + assert.sameDeepMembers( + downloadFiles, + new Array(downloadIndex) + .fill(null) + .map((_, idx) => `download${idx + 1}`) + ); + }); + + it('will NOT delete copied quote attachments if there is at least one strong reference', async () => { + await writeFiles(10, 'attachment'); + + const quotedMessage = { + id: generateUuid(), + type: 'outgoing', + sent_at: Date.now(), + timestamp: Date.now(), + received_at: Date.now(), + conversationId: generateUuid(), + attachments: [ + { + contentType: IMAGE_JPEG, + size: 128, + path: 'attachment1', + thumbnail: { + contentType: IMAGE_JPEG, + size: 42, + // strong reference + path: 'attachment2', + }, + }, + ], + } as const; + + const quotingMessage = { + id: generateUuid(), + type: 'outgoing', + sent_at: Date.now(), + timestamp: Date.now(), + received_at: Date.now(), + conversationId: generateUuid(), + quote: { + id: quotedMessage.sent_at, + isViewOnce: false, + referencedMessageNotFound: false, + attachments: [ + { + contentType: IMAGE_JPEG, + thumbnail: { + contentType: IMAGE_JPEG, + size: 42, + // weak (copied) reference + path: 'attachment2', + copied: true, + }, + }, + ], + }, + } as const; + + // Make sure we constructed the test correctly: both attachments reference the same + // path on disk + assert.strictEqual( + quotedMessage.attachments[0].thumbnail.path, + quotingMessage.quote.attachments[0].thumbnail.path + ); + + await DataWriter.saveMessages([quotedMessage, quotingMessage], { + ourAci: generateAci(), + forceSave: true, + postSaveUpdates: () => Promise.resolve(), + }); + + await DataWriter.cleanupOrphanedAttachments({ _block: true }); + + const attachmentFilesLeftOnDisk = listFiles('attachment'); + + assert.strictEqual(attachmentFilesLeftOnDisk.length, 2); + + assert.sameDeepMembers(attachmentFilesLeftOnDisk, [ + 'attachment1', + 'attachment2', + ]); + }); + + it('will delete quote attachments if there are only weak references', async () => { + await writeFiles(10, 'attachment'); + + const quotingMessage = { + id: generateUuid(), + type: 'outgoing', + sent_at: Date.now(), + timestamp: Date.now(), + received_at: Date.now(), + conversationId: generateUuid(), + quote: { + id: Date.now(), + isViewOnce: false, + referencedMessageNotFound: false, + attachments: [ + { + contentType: IMAGE_JPEG, + thumbnail: { + contentType: IMAGE_JPEG, + size: 42, + path: 'attachment1', + copied: true, + }, + }, + ], + }, + } as const; + + await DataWriter.saveMessage(quotingMessage, { + ourAci: generateAci(), + forceSave: true, + postSaveUpdates: () => Promise.resolve(), + }); + + await DataWriter.cleanupOrphanedAttachments({ _block: true }); + + const attachmentFilesLeftOnDisk = listFiles('attachment'); + + assert.strictEqual(attachmentFilesLeftOnDisk.length, 0); + }); }); }); diff --git a/ts/test-electron/deleteMessageAttachments_test.ts b/ts/test-electron/deleteMessageAttachments_test.ts new file mode 100644 index 0000000000..e5f0a9c463 --- /dev/null +++ b/ts/test-electron/deleteMessageAttachments_test.ts @@ -0,0 +1,295 @@ +// Copyright 2025 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from 'chai'; +import fsExtra from 'fs-extra'; +import { v4 as generateUuid } from 'uuid'; +import { readdirSync } from 'node:fs'; +import { dirname } from 'node:path'; + +import { missingCaseError } from '../util/missingCaseError.js'; +import { getDownloadsPath, getPath } from '../windows/main/attachments.js'; + +import { IMAGE_JPEG, LONG_MESSAGE } from '../types/MIME.js'; +import type { MessageAttributesType } from '../model-types.js'; +import { + type AttachmentType, + deleteAllAttachmentFilesOnDisk, +} from '../types/Attachment.js'; +import { strictAssert } from '../util/assert.js'; + +const { emptyDir, ensureFile } = fsExtra; + +function getAbsolutePath( + path: string, + type: 'attachment' | 'download' | 'draft' +) { + switch (type) { + case 'attachment': + return window.Signal.Migrations.getAbsoluteAttachmentPath(path); + case 'download': + return window.Signal.Migrations.getAbsoluteDownloadsPath(path); + case 'draft': + return window.Signal.Migrations.getAbsoluteDraftPath(path); + default: + throw missingCaseError(type); + } +} + +async function writeFile( + path: string, + type: 'attachment' | 'download' | 'draft' +) { + await ensureFile(getAbsolutePath(path, type)); +} + +async function writeFiles( + num: number, + type: 'attachment' | 'download' | 'draft' +) { + for (let i = 1; i <= num; i += 1) { + // eslint-disable-next-line no-await-in-loop + await writeFile(`${type}${i}`, type); + } +} + +function listFiles(type: 'attachment' | 'download' | 'draft'): Array { + return readdirSync(dirname(getAbsolutePath('fakename', type))); +} + +let attachmentIndex = 0; +let downloadIndex = 0; + +describe('Attachment deletion', () => { + beforeEach(async () => { + attachmentIndex = 0; + downloadIndex = 0; + await emptyDir(getPath(window.SignalContext.config.userDataPath)); + await emptyDir(getDownloadsPath(window.SignalContext.config.userDataPath)); + }); + + afterEach(async () => { + await emptyDir(getPath(window.SignalContext.config.userDataPath)); + await emptyDir(getDownloadsPath(window.SignalContext.config.userDataPath)); + }); + + function getAttachmentFilePath() { + attachmentIndex += 1; + return `attachment${attachmentIndex}`; + } + function getDownloadFilePath() { + downloadIndex += 1; + return `download${downloadIndex}`; + } + + function composeAttachment(): AttachmentType { + return { + contentType: IMAGE_JPEG, + size: 128, + path: getAttachmentFilePath(), + downloadPath: getDownloadFilePath(), + thumbnail: { + contentType: IMAGE_JPEG, + size: 128, + path: getAttachmentFilePath(), + }, + screenshot: { + contentType: IMAGE_JPEG, + size: 128, + path: getAttachmentFilePath(), + }, + thumbnailFromBackup: { + contentType: IMAGE_JPEG, + size: 128, + path: getAttachmentFilePath(), + }, + }; + } + + function composeBodyAttachment() { + return { + contentType: LONG_MESSAGE, + size: 128, + path: getAttachmentFilePath(), + downloadPath: getDownloadFilePath(), + }; + } + + it('deleteAllAttachmentFilesOnDisk deletes all paths referenced', async () => { + await writeFiles(5, 'attachment'); + await writeFiles(3, 'download'); + + await deleteAllAttachmentFilesOnDisk({ + deleteAttachmentOnDisk: window.Signal.Migrations.deleteAttachmentData, + deleteDownloadOnDisk: window.Signal.Migrations.deleteDownloadData, + })(composeAttachment()); + + assert.strictEqual(attachmentIndex, 4); + assert.sameDeepMembers(listFiles('attachment'), ['attachment5']); + assert.sameDeepMembers(listFiles('download'), ['download2', 'download3']); + }); + + it('deleteAllAttachmentFilesOnDisk does not delete files for copied attachments', async () => { + await writeFiles(5, 'attachment'); + await writeFiles(5, 'download'); + + const attachment = composeAttachment(); + attachment.copied = true; + + await deleteAllAttachmentFilesOnDisk({ + deleteAttachmentOnDisk: window.Signal.Migrations.deleteAttachmentData, + deleteDownloadOnDisk: window.Signal.Migrations.deleteDownloadData, + })(attachment); + + assert.sameDeepMembers(listFiles('attachment'), [ + 'attachment1', + 'attachment2', + 'attachment3', + 'attachment4', + 'attachment5', + ]); + assert.sameDeepMembers(listFiles('download'), [ + 'download1', + 'download2', + 'download3', + 'download4', + 'download5', + ]); + }); + // Update these if more paths are added to composeMessageWithAllAttachments + const NUM_ATTACHMENT_FILES_IN_MESSAGE = 42; + const NUM_DOWNLOAD_FILES_IN_MESSAGE = 12; + function composeMessageWithAllAttachments(): MessageAttributesType { + const message: MessageAttributesType = { + id: generateUuid(), + type: 'outgoing', + sent_at: Date.now(), + timestamp: Date.now(), + received_at: Date.now(), + conversationId: generateUuid(), + bodyAttachment: composeBodyAttachment(), + attachments: [composeAttachment(), composeAttachment()], + contact: [ + { + avatar: { + isProfile: false, + avatar: composeAttachment(), + }, + }, + ], + preview: [ + { + url: 'url', + image: composeAttachment(), + }, + ], + + quote: { + id: Date.now(), + isViewOnce: false, + referencedMessageNotFound: false, + attachments: [ + { + contentType: IMAGE_JPEG, + thumbnail: composeAttachment(), + }, + ], + }, + sticker: { + packId: 'packId', + stickerId: 42, + packKey: 'packKey', + data: composeAttachment(), + }, + }; + message.editHistory = [ + { + timestamp: Date.now(), + received_at: Date.now(), + bodyAttachment: message.bodyAttachment, + attachments: message.attachments, + preview: message.preview, + quote: message.quote, + }, + { + timestamp: Date.now(), + received_at: Date.now(), + bodyAttachment: composeBodyAttachment(), + attachments: [composeAttachment(), composeAttachment()], + preview: [ + { + url: 'url', + image: composeAttachment(), + }, + ], + quote: { + id: Date.now(), + isViewOnce: false, + referencedMessageNotFound: false, + attachments: [ + { + contentType: IMAGE_JPEG, + thumbnail: composeAttachment(), + }, + ], + }, + }, + ]; + return message; + } + + it('deleteExternalMessageFiles deletes all message attachments, including editHistory', async () => { + await writeFiles(NUM_ATTACHMENT_FILES_IN_MESSAGE + 3, 'attachment'); + await writeFiles(NUM_DOWNLOAD_FILES_IN_MESSAGE + 3, 'download'); + const message = composeMessageWithAllAttachments(); + + await window.Signal.Migrations.deleteExternalMessageFiles(message); + + assert.strictEqual(attachmentIndex, NUM_ATTACHMENT_FILES_IN_MESSAGE); + assert.strictEqual(downloadIndex, NUM_DOWNLOAD_FILES_IN_MESSAGE); + + assert.sameDeepMembers(listFiles('attachment'), [ + 'attachment43', + 'attachment44', + 'attachment45', + ]); + + assert.sameDeepMembers(listFiles('download'), [ + 'download13', + 'download14', + 'download15', + ]); + }); + + it('deleteExternalMessageFiles does not delete copied quote attachments', async () => { + await writeFiles(NUM_ATTACHMENT_FILES_IN_MESSAGE + 3, 'attachment'); + await writeFiles(NUM_DOWNLOAD_FILES_IN_MESSAGE + 3, 'download'); + const message = composeMessageWithAllAttachments(); + + const quotedThumbnail = message.quote?.attachments[0].thumbnail; + strictAssert(quotedThumbnail, 'thumbnail exists'); + quotedThumbnail.copied = true; + + await window.Signal.Migrations.deleteExternalMessageFiles(message); + + assert.strictEqual(attachmentIndex, NUM_ATTACHMENT_FILES_IN_MESSAGE); + assert.strictEqual(downloadIndex, NUM_DOWNLOAD_FILES_IN_MESSAGE); + + assert.sameDeepMembers(listFiles('attachment'), [ + quotedThumbnail.path, + quotedThumbnail.thumbnail?.path, + quotedThumbnail.thumbnailFromBackup?.path, + quotedThumbnail.screenshot?.path, + 'attachment43', + 'attachment44', + 'attachment45', + ]); + + assert.sameDeepMembers(listFiles('download'), [ + quotedThumbnail.downloadPath, + 'download13', + 'download14', + 'download15', + ]); + }); +}); diff --git a/ts/test-node/types/Message2_test.ts b/ts/test-node/types/Message2_test.ts index 2ae61c3281..e7d7b0a093 100644 --- a/ts/test-node/types/Message2_test.ts +++ b/ts/test-node/types/Message2_test.ts @@ -97,7 +97,7 @@ describe('Message', () => { localKey: '123', plaintextHash: 'hash', }), - deleteOnDisk: async (_path: string) => undefined, + deleteAttachmentOnDisk: async (_path: string) => undefined, ...props, }; } diff --git a/ts/types/Attachment.ts b/ts/types/Attachment.ts index c9c9be9cf5..0aa0733f74 100644 --- a/ts/types/Attachment.ts +++ b/ts/types/Attachment.ts @@ -38,6 +38,7 @@ import { import { missingCaseError } from '../util/missingCaseError.js'; import type { MakeVideoScreenshotResultType } from './VisualAttachment.js'; import type { MessageAttachmentType } from './AttachmentDownload.js'; +import { getFilePathsOwnedByAttachment } from '../util/messageFilePaths.js'; import { strictAssert } from '../util/assert.js'; const { @@ -438,15 +439,17 @@ export function loadData( }; } -export function deleteData({ - deleteOnDisk, +export function deleteAllAttachmentFilesOnDisk({ + deleteAttachmentOnDisk, deleteDownloadOnDisk, }: { - deleteOnDisk: (path: string) => Promise; + deleteAttachmentOnDisk: (path: string) => Promise; deleteDownloadOnDisk: (path: string) => Promise; }): (attachment?: AttachmentType) => Promise { - if (!isFunction(deleteOnDisk)) { - throw new TypeError('deleteData: deleteOnDisk must be a function'); + if (!isFunction(deleteAttachmentOnDisk)) { + throw new TypeError( + 'deleteAttachmentOnDisk: deleteAttachmentOnDisk must be a function' + ); } return async (attachment?: AttachmentType): Promise => { @@ -454,28 +457,11 @@ export function deleteData({ throw new TypeError('deleteData: attachment is not valid'); } - const { path, downloadPath, thumbnail, screenshot, thumbnailFromBackup } = - attachment; - - if (isString(path)) { - await deleteOnDisk(path); - } - - if (isString(downloadPath)) { - await deleteDownloadOnDisk(downloadPath); - } - - if (thumbnail && isString(thumbnail.path)) { - await deleteOnDisk(thumbnail.path); - } - - if (screenshot && isString(screenshot.path)) { - await deleteOnDisk(screenshot.path); - } - - if (thumbnailFromBackup && isString(thumbnailFromBackup.path)) { - await deleteOnDisk(thumbnailFromBackup.path); - } + const result = getFilePathsOwnedByAttachment(attachment); + await Promise.all( + [...result.externalAttachments].map(deleteAttachmentOnDisk) + ); + await Promise.all([...result.externalDownloads].map(deleteDownloadOnDisk)); }; } diff --git a/ts/types/Message2.ts b/ts/types/Message2.ts index 37da97a5d1..48d856ae81 100644 --- a/ts/types/Message2.ts +++ b/ts/types/Message2.ts @@ -50,6 +50,7 @@ import { deepClone } from '../util/deepClone.js'; import * as Bytes from '../Bytes.js'; import { isBodyTooLong } from '../util/longAttachment.js'; import type { MessageAttachmentType } from './AttachmentDownload.js'; +import { getFilePathsOwnedByMessage } from '../util/messageFilePaths.js'; const { isFunction, isObject, identity } = lodash; @@ -89,7 +90,7 @@ export type ContextType = { ) => Promise; writeNewAttachmentData: (data: Uint8Array) => Promise; writeNewStickerData: (data: Uint8Array) => Promise; - deleteOnDisk: (path: string) => Promise; + deleteAttachmentOnDisk: (path: string) => Promise; }; // Schema version history @@ -714,7 +715,7 @@ export const upgradeSchema = async ( makeImageThumbnail, makeVideoScreenshot, writeNewStickerData, - deleteOnDisk, + deleteAttachmentOnDisk, logger, maxVersion = CURRENT_SCHEMA_VERSION, }: ContextType, @@ -752,7 +753,7 @@ export const upgradeSchema = async ( logger, getRegionCode, writeNewStickerData, - deleteOnDisk, + deleteAttachmentOnDisk, }); } catch (e) { // Throw the error if we were unable to upgrade the message at all @@ -794,7 +795,6 @@ export const processNewAttachment = async ( | 'makeImageThumbnail' | 'makeVideoScreenshot' | 'logger' - | 'deleteOnDisk' > ): Promise => { if (!isFunction(writeNewAttachmentData)) { @@ -1042,99 +1042,37 @@ export const loadStickerData = ( }; export const deleteAllExternalFiles = ({ - deleteAttachmentData, - deleteOnDisk, + deleteAttachmentOnDisk, + deleteDownloadOnDisk, }: { - deleteAttachmentData: (attachment: AttachmentType) => Promise; - deleteOnDisk: (path: string) => Promise; + deleteAttachmentOnDisk: (path: string) => Promise; + deleteDownloadOnDisk: (path: string) => Promise; }): ((message: MessageAttributesType) => Promise) => { - if (!isFunction(deleteAttachmentData)) { + if (!isFunction(deleteAttachmentOnDisk)) { throw new TypeError( - 'deleteAllExternalFiles: deleteAttachmentData must be a function' + 'deleteAllExternalFiles: deleteAttachmentOnDisk must be a function' ); } - if (!isFunction(deleteOnDisk)) { + if (!isFunction(deleteDownloadOnDisk)) { throw new TypeError( - 'deleteAllExternalFiles: deleteOnDisk must be a function' + 'deleteAllExternalFiles: deleteDownloadOnDisk must be a function' ); } - return async (message: MessageAttributesType) => { - const { - attachments, - bodyAttachment, - editHistory, - quote, - contact, - preview, - sticker, - } = message; + const { externalAttachments, externalDownloads } = + getFilePathsOwnedByMessage(message); - if (attachments && attachments.length) { - await Promise.all(attachments.map(deleteAttachmentData)); - } - - if (bodyAttachment) { - await deleteAttachmentData(bodyAttachment); - } - - if (quote && quote.attachments && quote.attachments.length) { - await Promise.all( - quote.attachments.map(async attachment => { - const { thumbnail } = attachment; - - // To prevent spoofing, we copy the original image from the quoted message. - // If so, it will have a 'copied' field. We don't want to delete it if it has - // that field set to true. - if (thumbnail && thumbnail.path && !thumbnail.copied) { - await deleteOnDisk(thumbnail.path); - } - }) - ); - } - - if (contact && contact.length) { - await Promise.all( - contact.map(async item => { - const { avatar } = item; - - if (avatar && avatar.avatar && avatar.avatar.path) { - await deleteOnDisk(avatar.avatar.path); - } - }) - ); - } - - if (preview && preview.length) { - await deletePreviews(preview, deleteOnDisk); - } - - if (sticker && sticker.data && sticker.data.path) { - await deleteOnDisk(sticker.data.path); - - if (sticker.data.thumbnail && sticker.data.thumbnail.path) { - await deleteOnDisk(sticker.data.thumbnail.path); - } - } - - if (editHistory && editHistory.length) { - await Promise.all( - editHistory.map(async edit => { - if (edit.bodyAttachment) { - await deleteAttachmentData(edit.bodyAttachment); - } - - if (!edit.attachments || !edit.attachments.length) { - return; - } - return Promise.all(edit.attachments.map(deleteAttachmentData)); - }) - ); - await Promise.all( - editHistory.map(edit => deletePreviews(edit.preview, deleteOnDisk)) - ); - } + await Promise.all( + [...externalAttachments].map(attachmentPath => + deleteAttachmentOnDisk(attachmentPath) + ) + ); + await Promise.all( + [...externalDownloads].map(downloadPath => + deleteDownloadOnDisk(downloadPath) + ) + ); }; }; @@ -1166,29 +1104,6 @@ export async function migrateBodyAttachmentToDisk( }; } -async function deletePreviews( - preview: MessageAttributesType['preview'], - deleteOnDisk: (path: string) => Promise -): Promise> { - if (!preview) { - return []; - } - - return Promise.all( - preview.map(async item => { - const { image } = item; - - if (image && image.path) { - await deleteOnDisk(image.path); - } - - if (image?.thumbnail?.path) { - await deleteOnDisk(image.thumbnail.path); - } - }) - ); -} - export const isUserMessage = (message: MessageAttributesType): boolean => message.type === 'incoming' || message.type === 'outgoing'; diff --git a/ts/util/deleteForMe.ts b/ts/util/deleteForMe.ts index 766e63e6f6..766cd1de5f 100644 --- a/ts/util/deleteForMe.ts +++ b/ts/util/deleteForMe.ts @@ -5,7 +5,7 @@ import lodash from 'lodash'; import { createLogger } from '../logging/log.js'; import { DataReader, DataWriter, deleteAndCleanup } from '../sql/Client.js'; -import { deleteData } from '../types/Attachment.js'; +import { deleteAllAttachmentFilesOnDisk } from '../types/Attachment.js'; import type { MessageAttributesType } from '../model-types.js'; import type { ConversationModel } from '../models/conversations.js'; @@ -63,11 +63,11 @@ export async function deleteAttachmentFromMessage( fallbackPlaintextHash?: string; }, { - deleteOnDisk, + deleteAttachmentOnDisk, deleteDownloadOnDisk, logId, }: { - deleteOnDisk: (path: string) => Promise; + deleteAttachmentOnDisk: (path: string) => Promise; deleteDownloadOnDisk: (path: string) => Promise; logId: string; } @@ -85,7 +85,7 @@ export async function deleteAttachmentFromMessage( const message = window.MessageCache.register(new MessageModel(found)); return applyDeleteAttachmentFromMessage(message, deleteAttachmentData, { - deleteOnDisk, + deleteAttachmentOnDisk, deleteDownloadOnDisk, logId, shouldSave: true, @@ -104,12 +104,12 @@ export async function applyDeleteAttachmentFromMessage( fallbackPlaintextHash?: string; }, { - deleteOnDisk, + deleteAttachmentOnDisk, deleteDownloadOnDisk, shouldSave, logId, }: { - deleteOnDisk: (path: string) => Promise; + deleteAttachmentOnDisk: (path: string) => Promise; deleteDownloadOnDisk: (path: string) => Promise; shouldSave: boolean; logId: string; @@ -148,7 +148,10 @@ export async function applyDeleteAttachmentFromMessage( if (shouldSave) { await saveMessage(message.attributes, { ourAci, postSaveUpdates }); } - await deleteData({ deleteOnDisk, deleteDownloadOnDisk })(attachment); + await deleteAllAttachmentFilesOnDisk({ + deleteAttachmentOnDisk, + deleteDownloadOnDisk, + })(attachment); return true; } diff --git a/ts/util/messageFilePaths.ts b/ts/util/messageFilePaths.ts new file mode 100644 index 0000000000..fb18a2b67f --- /dev/null +++ b/ts/util/messageFilePaths.ts @@ -0,0 +1,115 @@ +// Copyright 2018 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import type { MessageAttributesType } from '../model-types.js'; +import type { AttachmentType } from '../types/Attachment.js'; + +export function getFilePathsOwnedByAttachment(attachment: AttachmentType): { + externalAttachments: Set; + externalDownloads: Set; +} { + const externalAttachments = new Set(); + const externalDownloads = new Set(); + + // Copied attachments weakly reference their paths and do not 'own' them + if (attachment.copied) { + return { externalAttachments, externalDownloads }; + } + + const { path, thumbnail, screenshot, thumbnailFromBackup, downloadPath } = + attachment; + if (path) { + externalAttachments.add(path); + } + + // downloadPath is relative to downloads folder and has to be tracked + // separately. + if (downloadPath) { + externalDownloads.add(downloadPath); + } + + if (thumbnail && thumbnail.path) { + externalAttachments.add(thumbnail.path); + } + + if (screenshot && screenshot.path) { + externalAttachments.add(screenshot.path); + } + + if (thumbnailFromBackup && thumbnailFromBackup.path) { + externalAttachments.add(thumbnailFromBackup.path); + } + return { externalAttachments, externalDownloads }; +} + +function getFilePathsForVersionOfMessage( + rootOrEditHistoryMessage: Partial +): { + externalAttachments: Set; + externalDownloads: Set; +} { + const externalAttachments = new Set(); + const externalDownloads = new Set(); + function addFilePathsOwnedByAttachment(attachment: AttachmentType) { + const result = getFilePathsOwnedByAttachment(attachment); + result.externalAttachments.forEach(path => externalAttachments.add(path)); + result.externalDownloads.forEach(path => externalDownloads.add(path)); + } + + const { attachments, bodyAttachment, contact, quote, preview, sticker } = + rootOrEditHistoryMessage; + + attachments?.forEach(addFilePathsOwnedByAttachment); + + if (bodyAttachment) { + addFilePathsOwnedByAttachment(bodyAttachment); + } + + if (quote?.attachments) { + quote.attachments.forEach(attachment => { + if (attachment.thumbnail) { + addFilePathsOwnedByAttachment(attachment.thumbnail); + } + }); + } + + if (contact) { + contact.forEach(item => { + if (item.avatar?.avatar) { + addFilePathsOwnedByAttachment(item.avatar.avatar); + } + }); + } + + if (preview) { + preview.forEach(item => { + if (item.image) { + addFilePathsOwnedByAttachment(item.image); + } + }); + } + + if (sticker?.data) { + addFilePathsOwnedByAttachment(sticker.data); + } + return { externalAttachments, externalDownloads }; +} + +export function getFilePathsOwnedByMessage(message: MessageAttributesType): { + externalAttachments: Array; + externalDownloads: Array; +} { + const externalAttachments = new Set(); + const externalDownloads = new Set(); + + [message, ...(message.editHistory ?? [])].forEach(version => { + const result = getFilePathsForVersionOfMessage(version); + result.externalAttachments.forEach(path => externalAttachments.add(path)); + result.externalDownloads.forEach(path => externalDownloads.add(path)); + }); + + return { + externalAttachments: [...externalAttachments], + externalDownloads: [...externalDownloads], + }; +} diff --git a/ts/util/modifyTargetMessage.ts b/ts/util/modifyTargetMessage.ts index a5b551c46f..9c2b841610 100644 --- a/ts/util/modifyTargetMessage.ts +++ b/ts/util/modifyTargetMessage.ts @@ -106,7 +106,8 @@ export async function modifyTargetMessage( { logId, shouldSave: false, - deleteOnDisk: window.Signal.Migrations.deleteAttachmentData, + deleteAttachmentOnDisk: + window.Signal.Migrations.deleteAttachmentData, deleteDownloadOnDisk: window.Signal.Migrations.deleteDownloadData, } );