From 534029d2e6526968199f58e0b6d6f252c763e0e3 Mon Sep 17 00:00:00 2001 From: trevor-signal <131492920+trevor-signal@users.noreply.github.com> Date: Thu, 30 May 2024 14:53:30 -0400 Subject: [PATCH] Backup support for link preview and contact attachments --- protos/Backups.proto | 2 +- ts/services/backups/export.ts | 81 ++++--- ts/services/backups/import.ts | 57 +++-- ts/test-electron/backup/attachments_test.ts | 253 ++++++++++++++++++++ ts/test-electron/backup/helpers.ts | 11 +- ts/test-mock/backups/backups_test.ts | 5 +- ts/types/message/LinkPreviews.ts | 2 +- 7 files changed, 357 insertions(+), 54 deletions(-) create mode 100644 ts/test-electron/backup/attachments_test.ts diff --git a/protos/Backups.proto b/protos/Backups.proto index 06cd4f7e760..bea6fb53074 100644 --- a/protos/Backups.proto +++ b/protos/Backups.proto @@ -479,7 +479,7 @@ message ContactAttachment { repeated Phone number = 3; repeated Email email = 4; repeated PostalAddress address = 5; - optional string avatarUrlPath = 6; + optional FilePointer avatar = 6; optional string organization = 7; } diff --git a/ts/services/backups/export.ts b/ts/services/backups/export.ts index 6f4f1dd99f1..5f4e7976558 100644 --- a/ts/services/backups/export.ts +++ b/ts/services/backups/export.ts @@ -99,6 +99,7 @@ import { import type { CoreAttachmentBackupJobType } from '../../types/AttachmentBackup'; import { AttachmentBackupManager } from '../../jobs/AttachmentBackupManager'; import { getBackupCdnInfo } from './util/mediaId'; +import { ReadStatus } from '../../messages/MessageReadStatus'; const MAX_CONCURRENCY = 10; @@ -760,23 +761,30 @@ export class BackupExportStream extends Readable { } else if (contact && contact[0]) { const contactMessage = new Backups.ContactMessage(); - // TODO (DESKTOP-6845): properly handle avatarUrlPath - - contactMessage.contact = contact.map(contactDetails => ({ - ...contactDetails, - number: contactDetails.number?.map(number => ({ - ...number, - type: numberToPhoneType(number.type), - })), - email: contactDetails.email?.map(email => ({ - ...email, - type: numberToPhoneType(email.type), - })), - address: contactDetails.address?.map(address => ({ - ...address, - type: numberToAddressType(address.type), - })), - })); + contactMessage.contact = await Promise.all( + contact.map(async contactDetails => ({ + ...contactDetails, + number: contactDetails.number?.map(number => ({ + ...number, + type: numberToPhoneType(number.type), + })), + email: contactDetails.email?.map(email => ({ + ...email, + type: numberToPhoneType(email.type), + })), + address: contactDetails.address?.map(address => ({ + ...address, + type: numberToAddressType(address.type), + })), + avatar: contactDetails.avatar?.avatar + ? await this.processAttachment({ + attachment: contactDetails.avatar.avatar, + backupLevel, + messageReceivedAt: message.received_at, + }) + : undefined, + })) + ); const reactions = this.getMessageReactions(message); if (reactions != null) { @@ -790,7 +798,13 @@ export class BackupExportStream extends Readable { stickerProto.packId = Bytes.fromHex(sticker.packId); stickerProto.packKey = Bytes.fromBase64(sticker.packKey); stickerProto.stickerId = sticker.stickerId; - // TODO (DESKTOP-6845): properly handle data FilePointer + stickerProto.data = sticker.data + ? await this.processAttachment({ + attachment: sticker.data, + backupLevel, + messageReceivedAt: message.received_at, + }) + : undefined; result.stickerMessage = { sticker: stickerProto, @@ -817,14 +831,25 @@ export class BackupExportStream extends Readable { bodyRanges: message.bodyRanges?.map(range => this.toBodyRange(range)), }, - linkPreview: message.preview?.map(preview => { - return { - url: preview.url, - title: preview.title, - description: preview.description, - date: getSafeLongFromTimestamp(preview.date), - }; - }), + linkPreview: message.preview + ? await Promise.all( + message.preview.map(async preview => { + return { + url: preview.url, + title: preview.title, + description: preview.description, + date: getSafeLongFromTimestamp(preview.date), + image: preview.image + ? await this.processAttachment({ + attachment: preview.image, + backupLevel, + messageReceivedAt: message.received_at, + }) + : undefined, + }; + }) + ) + : undefined, reactions: this.getMessageReactions(message), }; } @@ -1785,7 +1810,7 @@ export class BackupExportStream extends Readable { private getIncomingMessageDetails({ received_at_ms: receivedAtMs, serverTimestamp, - readAt, + readStatus, }: MessageAttributesType): Backups.ChatItem.IIncomingMessageDetails { return { dateReceived: @@ -1794,7 +1819,7 @@ export class BackupExportStream extends Readable { serverTimestamp != null ? getSafeLongFromTimestamp(serverTimestamp) : null, - read: Boolean(readAt), + read: readStatus === ReadStatus.Read, }; } diff --git a/ts/services/backups/import.ts b/ts/services/backups/import.ts index e9022633a28..afd497cad56 100644 --- a/ts/services/backups/import.ts +++ b/ts/services/backups/import.ts @@ -873,15 +873,32 @@ export class BackupImportStream extends Writable { data: Backups.IStandardMessage ): Partial { return { - body: data.text?.body ?? '', - attachments: data.attachments - ?.map(attachment => { - if (!attachment.pointer) { - return null; - } - return convertFilePointerToAttachment(attachment.pointer); - }) - .filter(isNotNil), + body: data.text?.body || undefined, + attachments: data.attachments?.length + ? data.attachments + .map(attachment => { + if (!attachment.pointer) { + return null; + } + return convertFilePointerToAttachment(attachment.pointer); + }) + .filter(isNotNil) + : undefined, + preview: data.linkPreview?.length + ? data.linkPreview.map(preview => { + const { url } = preview; + strictAssert(url, 'preview must have a URL'); + return { + url, + title: dropNull(preview.title), + description: dropNull(preview.description), + date: getTimestampFromLong(preview.date), + image: preview.image + ? convertFilePointerToAttachment(preview.image) + : undefined, + }; + }) + : undefined, reactions: this.fromReactions(data.reactions), }; } @@ -889,6 +906,9 @@ export class BackupImportStream extends Writable { private fromReactions( reactions: ReadonlyArray | null | undefined ): Array | undefined { + if (!reactions?.length) { + return undefined; + } return reactions?.map( ({ emoji, authorId, sentTimestamp, receivedTimestamp }) => { strictAssert(emoji != null, 'reaction must have an emoji'); @@ -938,14 +958,8 @@ export class BackupImportStream extends Writable { return { message: { contact: (chatItem.contactMessage.contact ?? []).map(details => { - const { - name, - number, - email, - address, - // TODO (DESKTOP-6845): properly handle avatarUrlPath - organization, - } = details; + const { avatar, name, number, email, address, organization } = + details; return { name: name @@ -1016,6 +1030,12 @@ export class BackupImportStream extends Writable { }) : undefined, organization: dropNull(organization), + avatar: avatar + ? { + avatar: convertFilePointerToAttachment(avatar), + isProfile: false, + } + : undefined, }; }), reactions: this.fromReactions(chatItem.contactMessage.reactions), @@ -1038,7 +1058,7 @@ export class BackupImportStream extends Writable { ); const { stickerMessage: { - sticker: { emoji, packId, packKey, stickerId }, + sticker: { emoji, packId, packKey, stickerId, data }, }, } = chatItem; strictAssert(emoji != null, 'stickerMessage must have an emoji'); @@ -1059,6 +1079,7 @@ export class BackupImportStream extends Writable { packId: Bytes.toHex(packId), packKey: Bytes.toBase64(packKey), stickerId, + data: data ? convertFilePointerToAttachment(data) : undefined, }, reactions: this.fromReactions(chatItem.stickerMessage.reactions), }, diff --git a/ts/test-electron/backup/attachments_test.ts b/ts/test-electron/backup/attachments_test.ts new file mode 100644 index 00000000000..5f294272306 --- /dev/null +++ b/ts/test-electron/backup/attachments_test.ts @@ -0,0 +1,253 @@ +// Copyright 2024 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { v4 as generateGuid } from 'uuid'; +import { BackupLevel } from '@signalapp/libsignal-client/zkgroup'; +import { omit } from 'lodash'; + +import type { ConversationModel } from '../../models/conversations'; +import * as Bytes from '../../Bytes'; +import Data from '../../sql/Client'; +import { generateAci } from '../../types/ServiceId'; +import { ReadStatus } from '../../messages/MessageReadStatus'; +import { SeenStatus } from '../../MessageSeenStatus'; +import { loadCallsHistory } from '../../services/callHistoryLoader'; +import { setupBasics, asymmetricRoundtripHarness } from './helpers'; +import { IMAGE_JPEG } from '../../types/MIME'; +import type { MessageAttributesType } from '../../model-types'; +import type { AttachmentType } from '../../types/Attachment'; +import { strictAssert } from '../../util/assert'; + +const CONTACT_A = generateAci(); + +describe('backup/attachments', () => { + let contactA: ConversationModel; + + beforeEach(async () => { + await Data._removeAllMessages(); + await Data._removeAllConversations(); + window.storage.reset(); + + await setupBasics(); + + contactA = await window.ConversationController.getOrCreateAndWait( + CONTACT_A, + 'private', + { systemGivenName: 'CONTACT_A' } + ); + + await loadCallsHistory(); + }); + + function getBase64(str: string): string { + return Bytes.toBase64(Bytes.fromString(str)); + } + + function composeAttachment( + index: number, + overrides?: Partial + ): AttachmentType { + return { + cdnKey: `cdnKey${index}`, + cdnNumber: 3, + key: getBase64(`key${index}`), + digest: getBase64(`digest${index}`), + iv: getBase64(`iv${index}`), + size: 100, + contentType: IMAGE_JPEG, + path: `/path/to/file${index}.png`, + uploadTimestamp: index, + ...overrides, + }; + } + + function composeMessage( + timestamp: number, + overrides?: Partial + ): MessageAttributesType { + return { + conversationId: contactA.id, + id: generateGuid(), + type: 'incoming', + received_at: timestamp, + received_at_ms: timestamp, + sourceServiceId: CONTACT_A, + sourceDevice: timestamp, + sent_at: timestamp, + timestamp, + readStatus: ReadStatus.Read, + seenStatus: SeenStatus.Seen, + ...overrides, + }; + } + + describe('normal attachments', () => { + it('BackupLevel.Messages, roundtrips normal attachments', async () => { + const attachment1 = composeAttachment(1); + const attachment2 = composeAttachment(2); + + await asymmetricRoundtripHarness( + [ + composeMessage(1, { + attachments: [attachment1, attachment2], + }), + ], + // path & iv will not be roundtripped + [ + composeMessage(1, { + attachments: [ + omit(attachment1, ['path', 'iv']), + omit(attachment2, ['path', 'iv']), + ], + }), + ], + BackupLevel.Messages + ); + }); + it('BackupLevel.Media, roundtrips normal attachments', async () => { + const attachment = composeAttachment(1); + strictAssert(attachment.digest, 'digest exists'); + + await asymmetricRoundtripHarness( + [ + composeMessage(1, { + attachments: [attachment], + }), + ], + [ + composeMessage(1, { + // path, iv, and uploadTimestamp will not be roundtripped, + // but there will be a backupLocator + attachments: [ + { + ...omit(attachment, ['path', 'iv', 'uploadTimestamp']), + backupLocator: { mediaName: attachment.digest }, + }, + ], + }), + ], + BackupLevel.Media + ); + }); + }); + + describe('Preview attachments', () => { + it('BackupLevel.Messages, roundtrips preview attachments', async () => { + const attachment = composeAttachment(1); + + await asymmetricRoundtripHarness( + [ + composeMessage(1, { + preview: [{ url: 'url', date: 1, image: attachment }], + }), + ], + // path & iv will not be roundtripped + [ + composeMessage(1, { + preview: [ + { url: 'url', date: 1, image: omit(attachment, ['path', 'iv']) }, + ], + }), + ], + BackupLevel.Messages + ); + }); + it('BackupLevel.Media, roundtrips preview attachments', async () => { + const attachment = composeAttachment(1); + strictAssert(attachment.digest, 'digest exists'); + + await asymmetricRoundtripHarness( + [ + composeMessage(1, { + preview: [ + { + url: 'url', + date: 1, + title: 'title', + description: 'description', + image: attachment, + }, + ], + }), + ], + [ + composeMessage(1, { + preview: [ + { + url: 'url', + date: 1, + title: 'title', + description: 'description', + image: { + // path, iv, and uploadTimestamp will not be roundtripped, + // but there will be a backupLocator + ...omit(attachment, ['path', 'iv', 'uploadTimestamp']), + backupLocator: { mediaName: attachment.digest }, + }, + }, + ], + }), + ], + BackupLevel.Media + ); + }); + }); + + describe('contact attachments', () => { + it('BackupLevel.Messages, roundtrips contact attachments', async () => { + const attachment = composeAttachment(1); + + await asymmetricRoundtripHarness( + [ + composeMessage(1, { + contact: [{ avatar: { avatar: attachment, isProfile: false } }], + }), + ], + // path & iv will not be roundtripped + [ + composeMessage(1, { + contact: [ + { + avatar: { + avatar: omit(attachment, ['path', 'iv']), + isProfile: false, + }, + }, + ], + }), + ], + BackupLevel.Messages + ); + }); + it('BackupLevel.Media, roundtrips contact attachments', async () => { + const attachment = composeAttachment(1); + strictAssert(attachment.digest, 'digest exists'); + + await asymmetricRoundtripHarness( + [ + composeMessage(1, { + contact: [{ avatar: { avatar: attachment, isProfile: false } }], + }), + ], + // path, iv, and uploadTimestamp will not be roundtripped, + // but there will be a backupLocator + [ + composeMessage(1, { + contact: [ + { + avatar: { + avatar: { + ...omit(attachment, ['path', 'iv', 'uploadTimestamp']), + backupLocator: { mediaName: attachment.digest }, + }, + isProfile: false, + }, + }, + ], + }), + ], + BackupLevel.Media + ); + }); + }); +}); diff --git a/ts/test-electron/backup/helpers.ts b/ts/test-electron/backup/helpers.ts index fc99c4a6997..826edae3691 100644 --- a/ts/test-electron/backup/helpers.ts +++ b/ts/test-electron/backup/helpers.ts @@ -8,6 +8,7 @@ import { sortBy } from 'lodash'; import { createReadStream } from 'fs'; import { mkdtemp, rm } from 'fs/promises'; import * as sinon from 'sinon'; +import { BackupLevel } from '@signalapp/libsignal-client/zkgroup'; import type { MessageAttributesType } from '../../model-types'; import type { @@ -109,9 +110,10 @@ function sortAndNormalize( } export async function symmetricRoundtripHarness( - messages: Array + messages: Array, + backupLevel: BackupLevel = BackupLevel.Messages ): Promise { - return asymmetricRoundtripHarness(messages, messages); + return asymmetricRoundtripHarness(messages, messages, backupLevel); } async function updateConvoIdToTitle() { @@ -126,7 +128,8 @@ async function updateConvoIdToTitle() { export async function asymmetricRoundtripHarness( before: Array, - after: Array + after: Array, + backupLevel: BackupLevel = BackupLevel.Messages ): Promise { const outDir = await mkdtemp(path.join(tmpdir(), 'signal-temp-')); const fetchAndSaveBackupCdnObjectMetadata = sinon.stub( @@ -138,7 +141,7 @@ export async function asymmetricRoundtripHarness( await Data.saveMessages(before, { forceSave: true, ourAci: OUR_ACI }); - await backupsService.exportToDisk(targetOutputFile); + await backupsService.exportToDisk(targetOutputFile, backupLevel); await updateConvoIdToTitle(); diff --git a/ts/test-mock/backups/backups_test.ts b/ts/test-mock/backups/backups_test.ts index 5c2cb77b77a..66364859e2d 100644 --- a/ts/test-mock/backups/backups_test.ts +++ b/ts/test-mock/backups/backups_test.ts @@ -152,6 +152,9 @@ describe('backups', function (this: Mocha.Suite) { ); } + const backupPath = bootstrap.getBackupPath('backup.bin'); + await app.exportBackupToDisk(backupPath); + const comparator = await bootstrap.createScreenshotComparator( app, async (window, snapshot) => { @@ -185,8 +188,6 @@ describe('backups', function (this: Mocha.Suite) { this.test ); - const backupPath = bootstrap.getBackupPath('backup.bin'); - await app.exportBackupToDisk(backupPath); await app.close(); // Restart diff --git a/ts/types/message/LinkPreviews.ts b/ts/types/message/LinkPreviews.ts index c086430b145..a68b9dd4e31 100644 --- a/ts/types/message/LinkPreviews.ts +++ b/ts/types/message/LinkPreviews.ts @@ -9,7 +9,7 @@ type GenericLinkPreviewType = { domain?: string; url: string; isStickerPack?: boolean; - isCallLink: boolean; + isCallLink?: boolean; image?: Readonly; date?: number; };