From b0fb6276f568ca07d2e73b31317b8d968fa900d3 Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Wed, 21 May 2025 09:14:58 -0500 Subject: [PATCH] Fix additional backup export validation errors Co-authored-by: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> --- ts/services/backups/export.ts | 95 ++++++++++++++++++++++++++--------- 1 file changed, 72 insertions(+), 23 deletions(-) diff --git a/ts/services/backups/export.ts b/ts/services/backups/export.ts index f158510a7b..3a91fb59ae 100644 --- a/ts/services/backups/export.ts +++ b/ts/services/backups/export.ts @@ -29,6 +29,7 @@ import { StorySendMode, MY_STORY_ID } from '../../types/Stories'; import { getStickerPacksForBackup } from '../../types/Stickers'; import { isPniString, + isServiceIdString, type AciString, type ServiceIdString, } from '../../types/ServiceId'; @@ -152,6 +153,8 @@ import { trimBody } from '../../util/longAttachment'; import { generateBackupsSubscriberData } from '../../util/backupSubscriptionData'; import { getEnvironment, isTestEnvironment } from '../../environment'; import { calculateLightness } from '../../util/getHSL'; +import { isSignalServiceId } from '../../util/isSignalConversation'; +import { isValidE164 } from '../../util/isValidE164'; import { toDayOfWeekArray } from '../../types/NotificationProfile'; const MAX_CONCURRENCY = 10; @@ -373,6 +376,7 @@ export class BackupExportStream extends Readable { }) ); + const skippedConversationIds = new Set(); for (const { attributes } of window.ConversationController.getAll()) { const recipientId = this.#getRecipientId(attributes); @@ -382,6 +386,7 @@ export class BackupExportStream extends Readable { identityKeysById ); if (recipient === undefined) { + skippedConversationIds.add(attributes.id); // Can't be backed up. continue; } @@ -518,6 +523,10 @@ export class BackupExportStream extends Readable { continue; } + if (skippedConversationIds.has(attributes.id)) { + continue; + } + const recipientId = this.#getRecipientId(attributes); let pinnedOrder: number | null = null; @@ -711,35 +720,35 @@ export class BackupExportStream extends Readable { await DataReader.pageMessages(cursor); // eslint-disable-next-line no-await-in-loop - const items = await pMap( + await pMap( messages, - message => - this.#toChatItem(message, { + async message => { + const chatItem = await this.#toChatItem(message, { aboutMe, callHistoryByCallId, backupLevel, isLocalBackup, - }), + }); + + if (chatItem === undefined) { + this.#stats.skippedMessages += 1; + // Can't be backed up. + return; + } + + this.#pushFrame({ + chatItem, + }); + + this.#stats.messages += 1; + }, { concurrency: MAX_CONCURRENCY } ); - for (const chatItem of items) { - if (chatItem === undefined) { - this.#stats.skippedMessages += 1; - // Can't be backed up. - continue; - } - - this.#pushFrame({ - chatItem, - }); - - // eslint-disable-next-line no-await-in-loop - await this.#flush(); - this.#stats.messages += 1; - } - cursor = newCursor; + + // eslint-disable-next-line no-await-in-loop + await this.#flush(); } } finally { if (cursor !== undefined) { @@ -981,6 +990,27 @@ export class BackupExportStream extends Readable { avatarColor: toAvatarColor(convo.color), }; } else if (isDirectConversation(convo)) { + // Skip story onboarding conversation and other internal conversations. + if ( + convo.serviceId != null && + (isSignalServiceId(convo.serviceId) || + !isServiceIdString(convo.serviceId)) + ) { + log.warn( + 'backups: skipping conversation with invalid serviceId', + convo.serviceId + ); + return undefined; + } + + if (convo.e164 != null && !isValidE164(convo.e164, true)) { + log.warn( + 'backups: skipping conversation with invalid e164', + convo.serviceId + ); + return undefined; + } + let visibility: Backups.Contact.Visibility; if (convo.removalStage == null) { visibility = Backups.Contact.Visibility.VISIBLE; @@ -1712,10 +1742,17 @@ export class BackupExportStream extends Readable { } if (message.changedId) { + const changedConvo = window.ConversationController.get( + message.changedId + ); + if (!changedConvo) { + throw new Error( + 'toChatItemUpdate/profileChange: changedId conversation not found!' + ); + } + // This will override authorId on the original chatItem - patch.authorId = this.#getOrPushPrivateRecipient({ - id: message.changedId, - }); + patch.authorId = this.#getOrPushPrivateRecipient(changedConvo); } const { newName, oldName } = message.profileChange; @@ -1834,6 +1871,14 @@ export class BackupExportStream extends Readable { } threadMerge.previousE164 = Long.fromString(e164); + // Conversation merges generated on Desktop side never has + // `sourceServiceId` and thus are attributed to our conversation. + // However, we need to include proper `authorId` for compatibility with + // other clients. + patch.authorId = this.#getOrPushPrivateRecipient({ + id: message.conversationId, + }); + updateMessage.threadMerge = threadMerge; return { kind: NonBubbleResultKind.Directionless, patch }; @@ -2399,6 +2444,10 @@ export class BackupExportStream extends Readable { quoteType = Backups.Quote.Type.VIEW_ONCE; } else { quoteType = Backups.Quote.Type.NORMAL; + if (quote.text == null && quote.attachments.length === 0) { + log.warn('backups: normal quote has no text or attachments'); + return null; + } } return {