From d8ea785f4ef73bca89e9d089046a0521f75d8335 Mon Sep 17 00:00:00 2001 From: Josh Perez <60019601+josh-signal@users.noreply.github.com> Date: Mon, 7 Aug 2023 12:36:37 -0400 Subject: [PATCH] Do not confirm DOE or edit until it is processed --- ts/background.ts | 24 ++-- ts/messageModifiers/Deletes.ts | 151 +++++++++++------------- ts/messageModifiers/Edits.ts | 24 ++-- ts/models/messages.ts | 13 +- ts/textsecure/MessageReceiver.ts | 7 ++ ts/textsecure/messageReceiverEvents.ts | 2 + ts/util/deleteForEveryone.ts | 21 ++-- ts/util/handleEditMessage.ts | 5 +- ts/util/modifyTargetMessage.ts | 4 +- ts/util/onStoryRecipientUpdate.ts | 16 ++- ts/util/sendDeleteForEveryoneMessage.ts | 4 +- ts/util/sendEditedMessage.ts | 12 +- 12 files changed, 145 insertions(+), 138 deletions(-) diff --git a/ts/background.ts b/ts/background.ts index db21a883cce3..8b5ccead7d74 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -113,7 +113,7 @@ import type { BadgesStateType } from './state/ducks/badges'; import { areAnyCallsActiveOrRinging } from './state/selectors/calling'; import { badgeImageFileDownloader } from './badges/badgeImageFileDownloader'; import { actionCreators } from './state/actions'; -import { Deletes } from './messageModifiers/Deletes'; +import * as Deletes from './messageModifiers/Deletes'; import type { EditAttributesType } from './messageModifiers/Edits'; import * as Edits from './messageModifiers/Edits'; import { @@ -2482,16 +2482,14 @@ export async function startApp(): Promise { strictAssert(fromConversation, 'Delete missing fromConversation'); const attributes: DeleteAttributesType = { + envelopeId: data.envelopeId, targetSentTimestamp: del.targetSentTimestamp, serverTimestamp: data.serverTimestamp, fromId: fromConversation.id, + removeFromMessageReceiverCache: confirm, }; - const deleteModel = Deletes.getSingleton().add(attributes); + drop(Deletes.onDelete(attributes)); - // Note: We do not wait for completion here - void Deletes.getSingleton().onDelete(deleteModel); - - confirm(); return; } @@ -2512,16 +2510,17 @@ export async function startApp(): Promise { }); const editAttributes: EditAttributesType = { + envelopeId: data.envelopeId, conversationId: message.attributes.conversationId, fromId: fromConversation.id, fromDevice: data.sourceDevice ?? 1, message: copyDataMessageIntoMessage(data.message, message.attributes), targetSentTimestamp: editedMessageTimestamp, + removeFromMessageReceiverCache: confirm, }; drop(Edits.onEdit(editAttributes)); - confirm(); return; } @@ -2815,14 +2814,13 @@ export async function startApp(): Promise { log.info('Queuing sent DOE for', del.targetSentTimestamp); const attributes: DeleteAttributesType = { + envelopeId: data.envelopeId, targetSentTimestamp: del.targetSentTimestamp, serverTimestamp: data.serverTimestamp, fromId: window.ConversationController.getOurConversationIdOrThrow(), + removeFromMessageReceiverCache: confirm, }; - const deleteModel = Deletes.getSingleton().add(attributes); - // Note: We do not wait for completion here - void Deletes.getSingleton().onDelete(deleteModel); - confirm(); + drop(Deletes.onDelete(attributes)); return; } @@ -2837,16 +2835,16 @@ export async function startApp(): Promise { }); const editAttributes: EditAttributesType = { + envelopeId: data.envelopeId, conversationId: message.attributes.conversationId, fromId: window.ConversationController.getOurConversationIdOrThrow(), fromDevice: window.storage.user.getDeviceId() ?? 1, message: copyDataMessageIntoMessage(data.message, message.attributes), targetSentTimestamp: editedMessageTimestamp, + removeFromMessageReceiverCache: confirm, }; drop(Edits.onEdit(editAttributes)); - - confirm(); return; } diff --git a/ts/messageModifiers/Deletes.ts b/ts/messageModifiers/Deletes.ts index e745ef861410..e01672f35ada 100644 --- a/ts/messageModifiers/Deletes.ts +++ b/ts/messageModifiers/Deletes.ts @@ -1,109 +1,98 @@ // Copyright 2020 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -/* eslint-disable max-classes-per-file */ - -import { Collection, Model } from 'backbone'; -import type { MessageModel } from '../models/messages'; +import type { MessageAttributesType } from '../model-types.d'; import { getContactId } from '../messages/helpers'; import * as log from '../logging/log'; import * as Errors from '../types/errors'; import { deleteForEveryone } from '../util/deleteForEveryone'; import { drop } from '../util/drop'; +import { filter, size } from '../util/iterables'; import { getMessageSentTimestampSet } from '../util/getMessageSentTimestampSet'; export type DeleteAttributesType = { + envelopeId: string; targetSentTimestamp: number; serverTimestamp: number; fromId: string; + removeFromMessageReceiverCache: () => unknown; }; -export class DeleteModel extends Model {} +const deletes = new Map(); -let singleton: Deletes | undefined; +export function forMessage( + messageAttributes: MessageAttributesType +): Array { + const sentTimestamps = getMessageSentTimestampSet(messageAttributes); + const matchingDeletes = filter(deletes, ([_envelopeId, item]) => { + return ( + item.fromId === getContactId(messageAttributes) && + sentTimestamps.has(item.targetSentTimestamp) + ); + }); -export class Deletes extends Collection { - static getSingleton(): Deletes { - if (!singleton) { - singleton = new Deletes(); - } - - return singleton; - } - - forMessage(message: MessageModel): Array { - const sentTimestamps = getMessageSentTimestampSet(message.attributes); - const matchingDeletes = this.filter(item => { - return ( - item.get('fromId') === getContactId(message.attributes) && - sentTimestamps.has(item.get('targetSentTimestamp')) - ); + if (size(matchingDeletes) > 0) { + log.info('Found early DOE for message'); + const result = Array.from(matchingDeletes); + result.forEach(([envelopeId, del]) => { + del.removeFromMessageReceiverCache(); + deletes.delete(envelopeId); }); - - if (matchingDeletes.length > 0) { - log.info('Found early DOE for message'); - this.remove(matchingDeletes); - return matchingDeletes; - } - - return []; + return result.map(([_envelopeId, item]) => item); } - async onDelete(del: DeleteModel): Promise { - try { - // The conversation the deleted message was in; we have to find it in the database - // to to figure that out. - const targetConversation = - await window.ConversationController.getConversationForTargetMessage( - del.get('fromId'), - del.get('targetSentTimestamp') - ); + return []; +} - if (!targetConversation) { - log.info( - 'No target conversation for DOE', - del.get('fromId'), - del.get('targetSentTimestamp') - ); +export async function onDelete(del: DeleteAttributesType): Promise { + deletes.set(del.envelopeId, del); - return; - } + const logId = `Deletes.onDelete(timestamp=${del.targetSentTimestamp})`; - // Do not await, since this can deadlock the queue - drop( - targetConversation.queueJob('Deletes.onDelete', async () => { - log.info('Handling DOE for', del.get('targetSentTimestamp')); - - const messages = await window.Signal.Data.getMessagesBySentAt( - del.get('targetSentTimestamp') - ); - - const targetMessage = messages.find( - m => del.get('fromId') === getContactId(m) && !m.deletedForEveryone - ); - - if (!targetMessage) { - log.info( - 'No message for DOE', - del.get('fromId'), - del.get('targetSentTimestamp') - ); - - return; - } - - const message = window.MessageController.register( - targetMessage.id, - targetMessage - ); - - await deleteForEveryone(message, del); - - this.remove(del); - }) + try { + // The conversation the deleted message was in; we have to find it in the database + // to to figure that out. + const targetConversation = + await window.ConversationController.getConversationForTargetMessage( + del.fromId, + del.targetSentTimestamp ); - } catch (error) { - log.error('Deletes.onDelete error:', Errors.toLogFormat(error)); + + if (!targetConversation) { + log.info(`${logId}: No message for DOE`); + return; } + + // Do not await, since this can deadlock the queue + drop( + targetConversation.queueJob('Deletes.onDelete', async () => { + log.info(`${logId}: Handling DOE`); + + const messages = await window.Signal.Data.getMessagesBySentAt( + del.targetSentTimestamp + ); + + const targetMessage = messages.find( + m => del.fromId === getContactId(m) && !m.deletedForEveryone + ); + + if (!targetMessage) { + log.info(`${logId}: No message for DOE 2`); + return; + } + + const message = window.MessageController.register( + targetMessage.id, + targetMessage + ); + + await deleteForEveryone(message, del); + + deletes.delete(del.envelopeId); + del.removeFromMessageReceiverCache(); + }) + ); + } catch (error) { + log.error(`${logId}: error`, Errors.toLogFormat(error)); } } diff --git a/ts/messageModifiers/Edits.ts b/ts/messageModifiers/Edits.ts index 1e697bfc139f..df2d49a3f8c8 100644 --- a/ts/messageModifiers/Edits.ts +++ b/ts/messageModifiers/Edits.ts @@ -13,17 +13,19 @@ import { getMessageSentTimestamp } from '../util/getMessageSentTimestamp'; export type EditAttributesType = { conversationId: string; + envelopeId: string; fromId: string; fromDevice: number; message: MessageAttributesType; targetSentTimestamp: number; + removeFromMessageReceiverCache: () => unknown; }; -const edits = new Set(); +const edits = new Map(); export function forMessage(message: MessageModel): Array { const sentAt = getMessageSentTimestamp(message.attributes, { log }); - const matchingEdits = filter(edits, item => { + const matchingEdits = filter(edits, ([_envelopeId, item]) => { return ( item.targetSentTimestamp === sentAt && item.fromId === getContactId(message.attributes) @@ -31,13 +33,20 @@ export function forMessage(message: MessageModel): Array { }); if (size(matchingEdits) > 0) { - const result = Array.from(matchingEdits); - const editsLogIds = result.map(x => x.message.sent_at); + const result: Array = []; + const editsLogIds: Array = []; + + Array.from(matchingEdits).forEach(([envelopeId, item]) => { + result.push(item); + editsLogIds.push(item.message.sent_at); + edits.delete(envelopeId); + item.removeFromMessageReceiverCache(); + }); + log.info( `Edits.forMessage(${message.get('sent_at')}): ` + `Found early edits for message ${editsLogIds.join(', ')}` ); - filter(matchingEdits, item => edits.delete(item)); return result; } @@ -45,7 +54,7 @@ export function forMessage(message: MessageModel): Array { } export async function onEdit(edit: EditAttributesType): Promise { - edits.add(edit); + edits.set(edit.envelopeId, edit); const logId = `Edits.onEdit(timestamp=${edit.message.timestamp};target=${edit.targetSentTimestamp})`; @@ -93,7 +102,8 @@ export async function onEdit(edit: EditAttributesType): Promise { await handleEditMessage(message.attributes, edit); - edits.delete(edit); + edits.delete(edit.envelopeId); + edit.removeFromMessageReceiverCache(); }) ); } catch (error) { diff --git a/ts/models/messages.ts b/ts/models/messages.ts index 977007324b19..eab500cb8a85 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -21,7 +21,7 @@ import type { } from '../model-types.d'; import { filter, find, map, repeat, zipObject } from '../util/iterables'; import * as GoogleChrome from '../util/GoogleChrome'; -import type { DeleteModel } from '../messageModifiers/Deletes'; +import type { DeleteAttributesType } from '../messageModifiers/Deletes'; import type { SentEventData } from '../textsecure/messageReceiverEvents'; import { isNotNil } from '../util/isNotNil'; import { isNormalNumber } from '../util/isNormalNumber'; @@ -3230,7 +3230,10 @@ export class MessageModel extends window.Backbone.Model { } async handleDeleteForEveryone( - del: DeleteModel, + del: Pick< + DeleteAttributesType, + 'fromId' | 'targetSentTimestamp' | 'serverTimestamp' + >, shouldPersist = true ): Promise { if (this.deletingForEveryone || this.get('deletedForEveryone')) { @@ -3239,10 +3242,10 @@ export class MessageModel extends window.Backbone.Model { log.info('Handling DOE.', { messageId: this.id, - fromId: del.get('fromId'), - targetSentTimestamp: del.get('targetSentTimestamp'), + fromId: del.fromId, + targetSentTimestamp: del.targetSentTimestamp, messageServerTimestamp: this.get('serverTimestamp'), - deleteServerTimestamp: del.get('serverTimestamp'), + deleteServerTimestamp: del.serverTimestamp, }); try { diff --git a/ts/textsecure/MessageReceiver.ts b/ts/textsecure/MessageReceiver.ts index 3ad1ac6fbed3..799e42b290da 100644 --- a/ts/textsecure/MessageReceiver.ts +++ b/ts/textsecure/MessageReceiver.ts @@ -2076,6 +2076,7 @@ export default class MessageReceiver const ev = new SentEvent( { + envelopeId: envelope.id, destination: dropNull(destination), destinationUuid, timestamp: timestamp?.toNumber(), @@ -2178,6 +2179,7 @@ export default class MessageReceiver log.warn(`${logId}: envelope is a sent group story`); const ev = new SentEvent( { + envelopeId: envelope.id, destinationUuid: { aci: envelope.destinationUuid.toString(), }, @@ -2250,6 +2252,7 @@ export default class MessageReceiver distributionListToSentUuid.forEach((sentToUuids, listId) => { const ev = new SentEvent( { + envelopeId: envelope.id, destinationUuid: { aci: envelope.destinationUuid.toString(), pni: undefined, @@ -2287,6 +2290,7 @@ export default class MessageReceiver log.warn(`${logId}: envelope is a received story`); const ev = new MessageEvent( { + envelopeId: envelope.id, source: envelope.source, sourceUuid: envelope.sourceUuid, sourceDevice: envelope.sourceDevice, @@ -2352,6 +2356,7 @@ export default class MessageReceiver const ev = new MessageEvent( { + envelopeId: envelope.id, source: envelope.source, sourceUuid: envelope.sourceUuid, sourceDevice: envelope.sourceDevice, @@ -2459,6 +2464,7 @@ export default class MessageReceiver const ev = new MessageEvent( { + envelopeId: envelope.id, source: envelope.source, sourceUuid: envelope.sourceUuid, sourceDevice: envelope.sourceDevice, @@ -3085,6 +3091,7 @@ export default class MessageReceiver const ev = new SentEvent( { + envelopeId: envelope.id, destination: dropNull(destination), destinationUuid, timestamp: envelope.timestamp, diff --git a/ts/textsecure/messageReceiverEvents.ts b/ts/textsecure/messageReceiverEvents.ts index 0019b5557e9f..ec4553032340 100644 --- a/ts/textsecure/messageReceiverEvents.ts +++ b/ts/textsecure/messageReceiverEvents.ts @@ -177,6 +177,7 @@ export class RetryRequestEvent extends ConfirmableEvent { } export type SentEventData = Readonly<{ + envelopeId: string; destination?: string; destinationUuid?: TaggedUUIDStringType; timestamp?: number; @@ -213,6 +214,7 @@ export class ProfileKeyUpdateEvent extends ConfirmableEvent { } export type MessageEventData = Readonly<{ + envelopeId: string; source?: string; sourceUuid: UUIDStringType; sourceDevice?: number; diff --git a/ts/util/deleteForEveryone.ts b/ts/util/deleteForEveryone.ts index 9597c9495613..ce77b4b36437 100644 --- a/ts/util/deleteForEveryone.ts +++ b/ts/util/deleteForEveryone.ts @@ -1,7 +1,7 @@ // Copyright 2020 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -import type { DeleteModel } from '../messageModifiers/Deletes'; +import type { DeleteAttributesType } from '../messageModifiers/Deletes'; import type { MessageModel } from '../models/messages'; import * as log from '../logging/log'; import { DAY } from './durations'; @@ -11,7 +11,10 @@ import { isStory } from '../state/selectors/message'; export async function deleteForEveryone( message: MessageModel, - doe: DeleteModel, + doe: Pick< + DeleteAttributesType, + 'fromId' | 'targetSentTimestamp' | 'serverTimestamp' + >, shouldPersist = true ): Promise { if (isDeletionByMe(message, doe)) { @@ -32,11 +35,11 @@ export async function deleteForEveryone( if (isDeletionTooOld(message, doe)) { log.warn('Received late DOE. Dropping.', { - fromId: doe.get('fromId'), - targetSentTimestamp: doe.get('targetSentTimestamp'), + fromId: doe.fromId, + targetSentTimestamp: doe.targetSentTimestamp, messageServerTimestamp: message.get('serverTimestamp'), messageSentAt: message.get('sent_at'), - deleteServerTimestamp: doe.get('serverTimestamp'), + deleteServerTimestamp: doe.serverTimestamp, }); return; } @@ -46,22 +49,22 @@ export async function deleteForEveryone( function isDeletionByMe( message: Readonly, - doe: Readonly + doe: Pick ): boolean { const ourConversationId = window.ConversationController.getOurConversationIdOrThrow(); return ( getContactId(message.attributes) === ourConversationId && - doe.get('fromId') === ourConversationId + doe.fromId === ourConversationId ); } function isDeletionTooOld( message: Readonly, - doe: Readonly + doe: Pick ): boolean { const messageTimestamp = message.get('serverTimestamp') || message.get('sent_at') || 0; - const delta = Math.abs(doe.get('serverTimestamp') - messageTimestamp); + const delta = Math.abs(doe.serverTimestamp - messageTimestamp); return delta > DAY; } diff --git a/ts/util/handleEditMessage.ts b/ts/util/handleEditMessage.ts index 5f5f94add02b..a49c209cb962 100644 --- a/ts/util/handleEditMessage.ts +++ b/ts/util/handleEditMessage.ts @@ -25,7 +25,10 @@ import { modifyTargetMessage } from './modifyTargetMessage'; export async function handleEditMessage( mainMessage: MessageAttributesType, - editAttributes: EditAttributesType + editAttributes: Pick< + EditAttributesType, + 'message' | 'conversationId' | 'fromDevice' | 'fromId' + > ): Promise { const idLog = `handleEditMessage(${getMessageIdForLogging(mainMessage)})`; diff --git a/ts/util/modifyTargetMessage.ts b/ts/util/modifyTargetMessage.ts index 8b3e5d378acd..a1555bd9f682 100644 --- a/ts/util/modifyTargetMessage.ts +++ b/ts/util/modifyTargetMessage.ts @@ -8,7 +8,7 @@ import type { SendStateByConversationId } from '../messages/MessageSendState'; import * as Edits from '../messageModifiers/Edits'; import * as log from '../logging/log'; -import { Deletes } from '../messageModifiers/Deletes'; +import * as Deletes from '../messageModifiers/Deletes'; import { MessageReceipts, MessageReceiptType, @@ -242,7 +242,7 @@ export async function modifyTargetMessage( // Does message message have any pending, previously-received associated // delete for everyone messages? - const deletes = Deletes.getSingleton().forMessage(message); + const deletes = Deletes.forMessage(message.attributes); await Promise.all( deletes.map(async del => { await deleteForEveryone(message, del, false); diff --git a/ts/util/onStoryRecipientUpdate.ts b/ts/util/onStoryRecipientUpdate.ts index 9bdae0284b72..56a6c0105222 100644 --- a/ts/util/onStoryRecipientUpdate.ts +++ b/ts/util/onStoryRecipientUpdate.ts @@ -2,10 +2,8 @@ // SPDX-License-Identifier: AGPL-3.0-only import { isEqual } from 'lodash'; -import type { DeleteAttributesType } from '../messageModifiers/Deletes'; import type { StoryRecipientUpdateEvent } from '../textsecure/messageReceiverEvents'; import * as log from '../logging/log'; -import { DeleteModel } from '../messageModifiers/Deletes'; import { SendStatus } from '../messages/MessageSendState'; import { getConversationIdForLogging } from './idForLogging'; import { isStory } from '../state/selectors/message'; @@ -175,12 +173,6 @@ export async function onStoryRecipientUpdate( messageId: item.id, storyDistributionListId, }); - const delAttributes: DeleteAttributesType = { - fromId: ourConversationId, - serverTimestamp: Number(item.serverTimestamp), - targetSentTimestamp: item.timestamp, - }; - const doe = new DeleteModel(delAttributes); // There are no longer any remaining members for this message so lets // run it through deleteForEveryone which marks the message as @@ -189,7 +181,13 @@ export async function onStoryRecipientUpdate( // NOTE: We don't call `Deletes.onDelete()` so the message lookup by // sent timestamp doesn't happen (it would return all copies of the // story, not just the one we want to delete). - void message.handleDeleteForEveryone(doe); + drop( + message.handleDeleteForEveryone({ + fromId: ourConversationId, + serverTimestamp: Number(item.serverTimestamp), + targetSentTimestamp: item.timestamp, + }) + ); } else { message.set({ sendStateByConversationId: nextSendStateByConversationId, diff --git a/ts/util/sendDeleteForEveryoneMessage.ts b/ts/util/sendDeleteForEveryoneMessage.ts index 97490fe05814..4799ded146a0 100644 --- a/ts/util/sendDeleteForEveryoneMessage.ts +++ b/ts/util/sendDeleteForEveryoneMessage.ts @@ -6,7 +6,6 @@ import type { ConversationQueueJobData } from '../jobs/conversationJobQueue'; import * as Errors from '../types/errors'; import * as durations from './durations'; import * as log from '../logging/log'; -import { DeleteModel } from '../messageModifiers/Deletes'; import { conversationJobQueue, conversationQueueJobEnum, @@ -91,10 +90,9 @@ export async function sendDeleteForEveryoneMessage( throw error; } - const deleteModel = new DeleteModel({ + await deleteForEveryone(message, { targetSentTimestamp: targetTimestamp, serverTimestamp: Date.now(), fromId: window.ConversationController.getOurConversationIdOrThrow(), }); - await deleteForEveryone(message, deleteModel); } diff --git a/ts/util/sendEditedMessage.ts b/ts/util/sendEditedMessage.ts index 07a99d1c2a99..99b5d8e00154 100644 --- a/ts/util/sendEditedMessage.ts +++ b/ts/util/sendEditedMessage.ts @@ -194,18 +194,14 @@ export async function sendEditedMessage( type: 'outgoing', }; - // Building up the dependencies for handling the edit message - const editAttributes = { + // Takes care of putting the message in the edit history, replacing the + // main message's values, and updating the conversation's properties. + await handleEditMessage(targetMessage.attributes, { conversationId, fromId, fromDevice: window.storage.user.getDeviceId() ?? 1, message: tmpMessage, - targetSentTimestamp, - }; - - // Takes care of putting the message in the edit history, replacing the - // main message's values, and updating the conversation's properties. - await handleEditMessage(targetMessage.attributes, editAttributes); + }); // Inserting the send into a job and saving it to the message await timeAndLogIfTooLong(