diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8e5783be2e..12fd4eff54 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -234,7 +234,7 @@ jobs: uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 with: repository: 'signalapp/Signal-Message-Backup-Tests' - ref: '316c3d5eb15ffc923b5ce2925094fd49b34e8203' + ref: 'df09e4bfa985c68daf845ad96abae3ae8f9b07ca' path: 'backup-integration-tests' - run: xvfb-run --auto-servernum pnpm run test-electron diff --git a/ts/services/backups/export.ts b/ts/services/backups/export.ts index 007b02b86c..369fa894ee 100644 --- a/ts/services/backups/export.ts +++ b/ts/services/backups/export.ts @@ -144,7 +144,7 @@ import { import { getRoomIdFromRootKey } from '../../util/callLinksRingrtc'; import { SeenStatus } from '../../MessageSeenStatus'; import { migrateAllMessages } from '../../messages/migrateMessageData'; -import { trimBody } from '../../util/longAttachment'; +import { isBodyTooLong, trimBody } from '../../util/longAttachment'; import { generateBackupsSubscriberData } from '../../util/backupSubscriptionData'; import { getEnvironment, @@ -157,6 +157,7 @@ import { isValidE164 } from '../../util/isValidE164'; import { toDayOfWeekArray } from '../../types/NotificationProfile'; import { getLinkPreviewSetting } from '../../types/LinkPreview'; import { getTypingIndicatorSetting } from '../../types/Util'; +import { KIBIBYTE } from '../../types/AttachmentSize'; const log = createLogger('export'); @@ -170,6 +171,8 @@ const FLUSH_TIMEOUT = 30 * MINUTE; // Threshold for reporting slow flushes const REPORTING_THRESHOLD = SECOND; +const BACKUP_LONG_ATTACHMENT_TEXT_LIMIT = 128 * KIBIBYTE; + type GetRecipientIdOptionsType = | Readonly<{ serviceId: ServiceIdString; @@ -2799,10 +2802,18 @@ export class BackupExportStream extends Readable { | 'preview' | 'reactions' | 'received_at' + | 'timestamp' >; backupLevel: BackupLevel; isLocalBackup: boolean; }): Promise { + if ( + message.body && + isBodyTooLong(message.body, BACKUP_LONG_ATTACHMENT_TEXT_LIMIT) + ) { + log.warn(`${message.timestamp}: Message body is too long; will truncate`); + } + return { quote: await this.#toQuote({ message, @@ -2821,18 +2832,23 @@ export class BackupExportStream extends Readable { }) ) : undefined, - longText: message.bodyAttachment - ? await this.#processAttachment({ - attachment: message.bodyAttachment, - backupLevel, - isLocalBackup, - messageReceivedAt: message.received_at, - }) - : undefined, + longText: + // We only include the bodyAttachment if it's not downloaded; otherwise all text + // is inlined + message.bodyAttachment && !isDownloaded(message.bodyAttachment) + ? await this.#processAttachment({ + attachment: message.bodyAttachment, + backupLevel, + isLocalBackup, + messageReceivedAt: message.received_at, + }) + : undefined, text: message.body != null ? { - body: message.body ? trimBody(message.body) : undefined, + body: message.body + ? trimBody(message.body, BACKUP_LONG_ATTACHMENT_TEXT_LIMIT) + : undefined, bodyRanges: message.bodyRanges?.map(range => this.#toBodyRange(range) ), diff --git a/ts/test-electron/backup/attachments_test.ts b/ts/test-electron/backup/attachments_test.ts index dc70414b4a..424483453b 100644 --- a/ts/test-electron/backup/attachments_test.ts +++ b/ts/test-electron/backup/attachments_test.ts @@ -41,7 +41,7 @@ import { generateKeys, getPlaintextHashForInMemoryAttachment, } from '../../AttachmentCrypto'; -import { isValidAttachmentKey } from '../../types/Crypto'; +import { KIBIBYTE } from '../../types/AttachmentSize'; const CONTACT_A = generateAci(); @@ -149,6 +149,7 @@ describe('backup/attachments', () => { } return base; } + describe('long-message attachments', () => { it('preserves attachment still on message.attachments', async () => { const longMessageAttachment = composeAttachment(1, { @@ -188,7 +189,7 @@ describe('backup/attachments', () => { ], [ composeMessage(1, { - body: body.slice(0, 2048), + body, bodyAttachment: { contentType: LONG_MESSAGE, size: bodyBytes.byteLength, @@ -206,22 +207,23 @@ describe('backup/attachments', () => { assert.deepStrictEqual( expected.bodyAttachment, - // all encryption info will be generated anew - omit(msgInDB.bodyAttachment, ['digest', 'key', 'downloadPath']) + omit(msgInDB.bodyAttachment, ['localKey', 'path', 'version']) ); assert.isUndefined(msgInDB.bodyAttachment?.digest); - assert.isTrue(isValidAttachmentKey(msgInDB.bodyAttachment?.key)); }, } ); }); it('handles existing bodyAttachments', async () => { + const body = 'a'.repeat(3000); + const bodyBytes = Bytes.fromString(body); + const attachment = omit( composeAttachment(1, { contentType: LONG_MESSAGE, - size: 3000, + size: bodyBytes.byteLength, downloadPath: 'downloadPath', }), 'thumbnail' @@ -232,13 +234,17 @@ describe('backup/attachments', () => { [ composeMessage(1, { bodyAttachment: attachment, - body: 'a'.repeat(3000), + body, }), ], [ composeMessage(1, { - body: 'a'.repeat(2048), - bodyAttachment: expectedRoundtrippedFields(attachment), + body, + bodyAttachment: { + contentType: LONG_MESSAGE, + size: 3000, + plaintextHash: getPlaintextHashForInMemoryAttachment(bodyBytes), + }, }), ], { @@ -250,11 +256,105 @@ describe('backup/attachments', () => { ); assert.deepStrictEqual( - omit(expected.bodyAttachment, ['clientUuid', 'downloadPath']), - omit(msgInDB.bodyAttachment, ['clientUuid', 'downloadPath']) + expected.bodyAttachment, + omit(msgInDB.bodyAttachment, ['localKey', 'path', 'version']) + ); + }, + } + ); + }); + it('truncates at 128 KiB', async () => { + const body = 'a'.repeat(129 * KIBIBYTE); + const truncatedBody = body.slice(0, 128 * KIBIBYTE); + const bodyBytes = Bytes.fromString(body); + + const attachment = omit( + composeAttachment(1, { + contentType: LONG_MESSAGE, + size: bodyBytes.byteLength, + downloadPath: 'downloadPath', + }), + 'thumbnail' + ); + strictAssert(attachment.digest, 'must exist'); + + await asymmetricRoundtripHarness( + [ + composeMessage(1, { + bodyAttachment: attachment, + body, + }), + ], + [ + composeMessage(1, { + body: truncatedBody, + bodyAttachment: { + contentType: LONG_MESSAGE, + size: 128 * KIBIBYTE, + plaintextHash: getPlaintextHashForInMemoryAttachment( + Bytes.fromString(truncatedBody) + ), + }, + }), + ], + { + backupLevel: BackupLevel.Paid, + comparator: (expected, msgInDB) => { + assert.deepStrictEqual( + omit(expected, 'bodyAttachment'), + omit(msgInDB, 'bodyAttachment') ); - assert.isNotEmpty(msgInDB.bodyAttachment?.downloadPath); + assert.deepStrictEqual( + expected.bodyAttachment, + omit(msgInDB.bodyAttachment, ['localKey', 'path', 'version']) + ); + }, + } + ); + }); + it('includes bodyAttachment if it has not downloaded', async () => { + const truncatedBody = 'a'.repeat(2 * KIBIBYTE); + + const attachment = omit( + composeAttachment(1, { + contentType: LONG_MESSAGE, + size: 64 * KIBIBYTE, + path: undefined, + plaintextHash: undefined, + localKey: undefined, + downloadPath: undefined, + clientUuid: undefined, // clientUuids are not roundtripped for bodyAttachments + }), + 'thumbnail' + ); + strictAssert(attachment.digest, 'must exist'); + + await asymmetricRoundtripHarness( + [ + composeMessage(1, { + body: truncatedBody, + bodyAttachment: attachment, + }), + ], + [ + composeMessage(1, { + body: truncatedBody, + bodyAttachment: attachment, + }), + ], + { + backupLevel: BackupLevel.Paid, + comparator: (expected, msgInDB) => { + assert.deepStrictEqual( + omit(msgInDB, 'bodyAttachment'), + omit(expected, 'bodyAttachment') + ); + + assert.deepStrictEqual( + omit(msgInDB.bodyAttachment, ['downloadPath']), + expected.bodyAttachment + ); }, } ); diff --git a/ts/util/longAttachment.ts b/ts/util/longAttachment.ts index ff7e1b9e3d..993faef21c 100644 --- a/ts/util/longAttachment.ts +++ b/ts/util/longAttachment.ts @@ -5,8 +5,11 @@ import { unicodeSlice } from './unicodeSlice'; const LONG_ATTACHMENT_LIMIT = 2048; -export function isBodyTooLong(body: string): boolean { - return Buffer.byteLength(body) > LONG_ATTACHMENT_LIMIT; +export function isBodyTooLong( + body: string, + length = LONG_ATTACHMENT_LIMIT +): boolean { + return Buffer.byteLength(body) > length; } export function trimBody(body: string, length = LONG_ATTACHMENT_LIMIT): string {