diff --git a/ts/messages/helpers.ts b/ts/messages/helpers.ts index 0001adad5..44925d040 100644 --- a/ts/messages/helpers.ts +++ b/ts/messages/helpers.ts @@ -9,7 +9,22 @@ import type { QuotedMessageType, } from '../model-types.d'; import type { UUIDStringType } from '../types/UUID'; -import { isIncoming, isOutgoing, isStory } from '../state/selectors/message'; + +export function isIncoming( + message: Pick +): boolean { + return message.type === 'incoming'; +} + +export function isOutgoing( + message: Pick +): boolean { + return message.type === 'outgoing'; +} + +export function isStory(message: Pick): boolean { + return message.type === 'story'; +} export function isQuoteAMatch( message: MessageAttributesType | null | undefined, diff --git a/ts/models/messages.ts b/ts/models/messages.ts index ee152e784..60676c6c1 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -2511,6 +2511,7 @@ export class MessageModel extends window.Backbone.Model { await Attachment.migrateDataToFileSystem(downloadedAvatar, { writeNewAttachmentData: window.Signal.Migrations.writeNewAttachmentData, + logger: log, }); avatar = { ...onDiskAttachment, diff --git a/ts/sql/Client.ts b/ts/sql/Client.ts index 7341bfc0f..1762bd10f 100644 --- a/ts/sql/Client.ts +++ b/ts/sql/Client.ts @@ -18,6 +18,7 @@ import { get, groupBy, isFunction, + isTypedArray, last, map, omit, @@ -90,6 +91,7 @@ import type { import Server from './Server'; import { isCorruptionError } from './errors'; import { MINUTE } from '../util/durations'; +import { getMessageIdForLogging } from '../util/idForLogging'; // We listen to a lot of events on ipc, often on the same channel. This prevents // any warnings that might be sent to the console in that case. @@ -433,12 +435,25 @@ function _cleanData( return cleaned; } -function _cleanMessageData(data: MessageType): MessageType { +export function _cleanMessageData(data: MessageType): MessageType { // Ensure that all messages have the received_at set properly if (!data.received_at) { assert(false, 'received_at was not set on the message'); data.received_at = window.Signal.Util.incrementMessageCounter(); } + if (data.attachments) { + const logId = getMessageIdForLogging(data); + data.attachments = data.attachments.map((attachment, index) => { + if (attachment.data && !isTypedArray(attachment.data)) { + log.warn( + `_cleanMessageData/${logId}: Attachment ${index} had non-array \`data\` field; deleting.` + ); + return omit(attachment, ['data']); + } + + return attachment; + }); + } return _cleanData(omit(data, ['dataMessage'])); } diff --git a/ts/state/selectors/message.ts b/ts/state/selectors/message.ts index cf751e534..593523962 100644 --- a/ts/state/selectors/message.ts +++ b/ts/state/selectors/message.ts @@ -93,6 +93,9 @@ import * as log from '../../logging/log'; import { getConversationColorAttributes } from '../../util/getConversationColorAttributes'; import { DAY, HOUR } from '../../util/durations'; import { getStoryReplyText } from '../../util/getStoryReplyText'; +import { isIncoming, isOutgoing, isStory } from '../../messages/helpers'; + +export { isIncoming, isOutgoing, isStory }; const THREE_HOURS = 3 * HOUR; const linkify = LinkifyIt(); @@ -129,24 +132,6 @@ export type GetPropsForBubbleOptions = Readonly<{ contactNameColorSelector: ContactNameColorSelectorType; }>; -export function isIncoming( - message: Pick -): boolean { - return message.type === 'incoming'; -} - -export function isOutgoing( - message: Pick -): boolean { - return message.type === 'outgoing'; -} - -export function isStory( - message: Pick -): boolean { - return message.type === 'story'; -} - export function hasErrors( message: Pick ): boolean { diff --git a/ts/test-both/sql/cleanMessageData_test.ts b/ts/test-both/sql/cleanMessageData_test.ts new file mode 100644 index 000000000..04b282822 --- /dev/null +++ b/ts/test-both/sql/cleanMessageData_test.ts @@ -0,0 +1,51 @@ +// Copyright 2021 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { assert } from 'chai'; + +import { _cleanMessageData } from '../../sql/Client'; +import { IMAGE_GIF } from '../../types/MIME'; + +describe('_cleanMessageData', () => { + it('throws if message is missing received_at', () => { + assert.throws(() => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + _cleanMessageData({} as any); + }, 'received_at'); + }); + + it('removes `data` field in attachment if it is not a typed array', () => { + const data = new Uint8Array([1, 2, 3]); + const message = { + id: 'something', + type: 'incoming' as const, + sent_at: Date.now(), + conversationId: 'conversation-id', + received_at: Date.now(), + timestamp: Date.now(), + attachments: [ + { + contentType: IMAGE_GIF, + size: 4, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + data: 1 as any, + }, + { + contentType: IMAGE_GIF, + size: 4, + data: {}, + }, + { + size: 4, + contentType: IMAGE_GIF, + data, + }, + ], + }; + const actual = _cleanMessageData(message); + + assert.isUndefined(actual.attachments?.[0].data); + assert.isUndefined(actual.attachments?.[1].data); + assert.strictEqual(actual.attachments?.[2].data, data); + }); +}); diff --git a/ts/test-node/types/Attachment_test.ts b/ts/test-node/types/Attachment_test.ts index a84d06cb5..3e13701d6 100644 --- a/ts/test-node/types/Attachment_test.ts +++ b/ts/test-node/types/Attachment_test.ts @@ -396,6 +396,7 @@ describe('Attachment', () => { const actual = await Attachment.migrateDataToFileSystem(input, { writeNewAttachmentData, + logger, }); assert.deepEqual(actual, expected); }); @@ -417,11 +418,12 @@ describe('Attachment', () => { const actual = await Attachment.migrateDataToFileSystem(input, { writeNewAttachmentData, + logger, }); assert.deepEqual(actual, expected); }); - it('should throw error if data is not valid', async () => { + it('should clear `data` field if it is not a typed array', async () => { const input = { contentType: MIME.IMAGE_JPEG, // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -432,12 +434,12 @@ describe('Attachment', () => { const writeNewAttachmentData = async () => 'abc/abcdefgh123456789'; - await assert.isRejected( - Attachment.migrateDataToFileSystem(input, { - writeNewAttachmentData, - }), - 'Expected `attachment.data` to be a typed array; got: number' - ); + const actual = await Attachment.migrateDataToFileSystem(input, { + writeNewAttachmentData, + logger, + }); + + assert.isUndefined(actual.data); }); }); }); diff --git a/ts/test-node/types/EmbeddedContact_test.ts b/ts/test-node/types/EmbeddedContact_test.ts index 7e6faf99e..7a2d175f7 100644 --- a/ts/test-node/types/EmbeddedContact_test.ts +++ b/ts/test-node/types/EmbeddedContact_test.ts @@ -4,6 +4,7 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; +import * as logger from '../../logging/log'; import { IMAGE_GIF, IMAGE_PNG } from '../../types/MIME'; import type { MessageAttributesType } from '../../model-types.d'; import type { Avatar, Email, Phone } from '../../types/EmbeddedContact'; @@ -18,9 +19,6 @@ import { UUID } from '../../types/UUID'; describe('Contact', () => { const NUMBER = '+12025550099'; - const logger = { - error: () => undefined, - }; const writeNewAttachmentData = sinon .stub() diff --git a/ts/types/Attachment.ts b/ts/types/Attachment.ts index 33651f8a0..3cf75f258 100644 --- a/ts/types/Attachment.ts +++ b/ts/types/Attachment.ts @@ -180,8 +180,10 @@ export async function migrateDataToFileSystem( attachment: AttachmentType, { writeNewAttachmentData, + logger, }: { writeNewAttachmentData: (data: Uint8Array) => Promise; + logger: LoggerType; } ): Promise { if (!isFunction(writeNewAttachmentData)) { @@ -195,11 +197,12 @@ export async function migrateDataToFileSystem( return attachment; } + // This attachment was already broken by a roundtrip to the database - repair it now if (!isTypedArray(data)) { - throw new TypeError( - 'Expected `attachment.data` to be a typed array;' + - ` got: ${typeof attachment.data}` + logger.warn( + 'migrateDataToFileSystem: Attachment had non-array `data` field; deleting.' ); + return omit({ ...attachment }, ['data']); } const path = await writeNewAttachmentData(data); diff --git a/ts/types/EmbeddedContact.ts b/ts/types/EmbeddedContact.ts index e794e9ecd..a7504e86d 100644 --- a/ts/types/EmbeddedContact.ts +++ b/ts/types/EmbeddedContact.ts @@ -192,7 +192,7 @@ export function parseAndWriteAvatar( context: { message: MessageAttributesType; getRegionCode: () => string | undefined; - logger: Pick; + logger: LoggerType; writeNewAttachmentData: (data: Uint8Array) => Promise; } ): Promise => { diff --git a/ts/types/Message2.ts b/ts/types/Message2.ts index 0602f5713..414b60916 100644 --- a/ts/types/Message2.ts +++ b/ts/types/Message2.ts @@ -589,6 +589,7 @@ export const processNewAttachment = async ( ); const onDiskAttachment = await migrateDataToFileSystem(rotatedAttachment, { writeNewAttachmentData, + logger, }); const finalAttachment = await captureDimensionsAndScreenshot( onDiskAttachment,