From e50787ae20088b1a3918674a337b23fd6e71d191 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 18 Jan 2022 17:08:38 -0500 Subject: [PATCH] Trim abandoned reactions from backups. When you create a backup (or do a device transfer), we skip messages with expiration timers. However, we still (unintentionally) include the reactions for those messages in the backup. These 'abandoned' reactions were being associated with newly-sent messages because the new messages had the same ID's as the expiring messages we skipped in the backup. It's worth noting that in order to hit this bug, you have to: - Have messages that are expiring, but have not expired yet - Those messages have to have reactions - Those message have to be the most recent messages in your message table Fixes #11327 --- .../securesms/backup/FullBackupExporter.java | 39 ++++++++++++++++--- .../securesms/database/MentionDatabase.java | 4 +- .../securesms/database/ReactionDatabase.kt | 16 ++++++-- .../securesms/database/SignalDatabase.kt | 1 + .../helpers/SignalDatabaseMigrations.kt | 16 +++++++- 5 files changed, 65 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupExporter.java b/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupExporter.java index 9b10384702..3771776c8f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupExporter.java +++ b/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupExporter.java @@ -27,10 +27,12 @@ import org.thoughtcrime.securesms.database.AttachmentDatabase; import org.thoughtcrime.securesms.database.EmojiSearchDatabase; import org.thoughtcrime.securesms.database.GroupReceiptDatabase; import org.thoughtcrime.securesms.database.KeyValueDatabase; +import org.thoughtcrime.securesms.database.MentionDatabase; import org.thoughtcrime.securesms.database.MmsDatabase; import org.thoughtcrime.securesms.database.MmsSmsColumns; import org.thoughtcrime.securesms.database.OneTimePreKeyDatabase; import org.thoughtcrime.securesms.database.PendingRetryReceiptDatabase; +import org.thoughtcrime.securesms.database.ReactionDatabase; import org.thoughtcrime.securesms.database.SearchDatabase; import org.thoughtcrime.securesms.database.SenderKeyDatabase; import org.thoughtcrime.securesms.database.SenderKeySharedDatabase; @@ -39,10 +41,12 @@ import org.thoughtcrime.securesms.database.SignedPreKeyDatabase; import org.thoughtcrime.securesms.database.SmsDatabase; import org.thoughtcrime.securesms.database.StickerDatabase; import org.thoughtcrime.securesms.database.model.AvatarPickerDatabase; +import org.thoughtcrime.securesms.database.model.MessageId; import org.thoughtcrime.securesms.dependencies.ApplicationDependencies; import org.thoughtcrime.securesms.keyvalue.KeyValueDataSet; import org.thoughtcrime.securesms.keyvalue.SignalStore; import org.thoughtcrime.securesms.profiles.AvatarHelper; +import org.thoughtcrime.securesms.util.CursorUtil; import org.thoughtcrime.securesms.util.SetUtil; import org.thoughtcrime.securesms.util.Stopwatch; import org.thoughtcrime.securesms.util.TextSecurePreferences; @@ -161,10 +165,14 @@ public class FullBackupExporter extends FullBackupBase { count = exportTable(table, input, outputStream, FullBackupExporter::isNonExpiringMmsMessage, null, count, estimatedCount, cancellationSignal); } else if (table.equals(SmsDatabase.TABLE_NAME)) { count = exportTable(table, input, outputStream, FullBackupExporter::isNonExpiringSmsMessage, null, count, estimatedCount, cancellationSignal); + } else if (table.equals(ReactionDatabase.TABLE_NAME)) { + count = exportTable(table, input, outputStream, cursor -> isForNonExpiringMessage(input, new MessageId(CursorUtil.requireLong(cursor, ReactionDatabase.MESSAGE_ID), CursorUtil.requireBoolean(cursor, ReactionDatabase.IS_MMS))), null, count, estimatedCount, cancellationSignal); + } else if (table.equals(MentionDatabase.TABLE_NAME)) { + count = exportTable(table, input, outputStream, cursor -> isForNonExpiringMmsMessage(input, CursorUtil.requireLong(cursor, MentionDatabase.MESSAGE_ID)), null, count, estimatedCount, cancellationSignal); } else if (table.equals(GroupReceiptDatabase.TABLE_NAME)) { - count = exportTable(table, input, outputStream, cursor -> isForNonExpiringMessage(input, cursor.getLong(cursor.getColumnIndexOrThrow(GroupReceiptDatabase.MMS_ID))), null, count, estimatedCount, cancellationSignal); + count = exportTable(table, input, outputStream, cursor -> isForNonExpiringMmsMessage(input, cursor.getLong(cursor.getColumnIndexOrThrow(GroupReceiptDatabase.MMS_ID))), null, count, estimatedCount, cancellationSignal); } else if (table.equals(AttachmentDatabase.TABLE_NAME)) { - count = exportTable(table, input, outputStream, cursor -> isForNonExpiringMessage(input, cursor.getLong(cursor.getColumnIndexOrThrow(AttachmentDatabase.MMS_ID))), (cursor, innerCount) -> exportAttachment(attachmentSecret, cursor, outputStream, innerCount, estimatedCount), count, estimatedCount, cancellationSignal); + count = exportTable(table, input, outputStream, cursor -> isForNonExpiringMmsMessage(input, cursor.getLong(cursor.getColumnIndexOrThrow(AttachmentDatabase.MMS_ID))), (cursor, innerCount) -> exportAttachment(attachmentSecret, cursor, outputStream, innerCount, estimatedCount), count, estimatedCount, cancellationSignal); } else if (table.equals(StickerDatabase.TABLE_NAME)) { count = exportTable(table, input, outputStream, cursor -> true, (cursor, innerCount) -> exportSticker(attachmentSecret, cursor, outputStream, innerCount, estimatedCount), count, estimatedCount, cancellationSignal); } else if (!BLACKLISTED_TABLES.contains(table) && !table.startsWith("sqlite_")) { @@ -470,15 +478,36 @@ public class FullBackupExporter extends FullBackupBase { return cursor.getLong(cursor.getColumnIndexOrThrow(MmsSmsColumns.EXPIRES_IN)) <= 0; } - private static boolean isForNonExpiringMessage(@NonNull SQLiteDatabase db, long mmsId) { + private static boolean isForNonExpiringMessage(@NonNull SQLiteDatabase db, @NonNull MessageId messageId) { + if (messageId.isMms()) { + return isForNonExpiringMmsMessage(db, messageId.getId()); + } else { + return isForNonExpiringSmsMessage(db, messageId.getId()); + } + } + + private static boolean isForNonExpiringSmsMessage(@NonNull SQLiteDatabase db, long smsId) { + String[] columns = new String[] { SmsDatabase.EXPIRES_IN }; + String where = SmsDatabase.ID + " = ?"; + String[] args = new String[] { String.valueOf(smsId) }; + + try (Cursor cursor = db.query(SmsDatabase.TABLE_NAME, columns, where, args, null, null, null)) { + if (cursor != null && cursor.moveToFirst()) { + return isNonExpiringSmsMessage(cursor); + } + } + + return false; + } + + private static boolean isForNonExpiringMmsMessage(@NonNull SQLiteDatabase db, long mmsId) { String[] columns = new String[] { MmsDatabase.EXPIRES_IN, MmsDatabase.VIEW_ONCE}; String where = MmsDatabase.ID + " = ?"; String[] args = new String[] { String.valueOf(mmsId) }; try (Cursor mmsCursor = db.query(MmsDatabase.TABLE_NAME, columns, where, args, null, null, null)) { if (mmsCursor != null && mmsCursor.moveToFirst()) { - return mmsCursor.getLong(mmsCursor.getColumnIndexOrThrow(MmsDatabase.EXPIRES_IN)) == 0 && - mmsCursor.getLong(mmsCursor.getColumnIndexOrThrow(MmsDatabase.VIEW_ONCE)) == 0; + return isNonExpiringMmsMessage(mmsCursor); } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MentionDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/MentionDatabase.java index c0d71bc1e1..53da2f492e 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MentionDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MentionDatabase.java @@ -23,11 +23,11 @@ import java.util.Map; public class MentionDatabase extends Database { - static final String TABLE_NAME = "mention"; + public static final String TABLE_NAME = "mention"; private static final String ID = "_id"; static final String THREAD_ID = "thread_id"; - static final String MESSAGE_ID = "message_id"; + public static final String MESSAGE_ID = "message_id"; static final String RECIPIENT_ID = "recipient_id"; private static final String RANGE_START = "range_start"; private static final String RANGE_LENGTH = "range_length"; diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/ReactionDatabase.kt b/app/src/main/java/org/thoughtcrime/securesms/database/ReactionDatabase.kt index 6bc5ffff4f..6ce8ce7a75 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/ReactionDatabase.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/ReactionDatabase.kt @@ -16,11 +16,11 @@ import org.thoughtcrime.securesms.util.SqlUtil class ReactionDatabase(context: Context, databaseHelper: SignalDatabase) : Database(context, databaseHelper) { companion object { - private const val TABLE_NAME = "reaction" + const val TABLE_NAME = "reaction" private const val ID = "_id" - private const val MESSAGE_ID = "message_id" - private const val IS_MMS = "is_mms" + const val MESSAGE_ID = "message_id" + const val IS_MMS = "is_mms" private const val AUTHOR_ID = "author_id" private const val EMOJI = "emoji" private const val DATE_SENT = "date_sent" @@ -195,4 +195,14 @@ class ReactionDatabase(context: Context, databaseHelper: SignalDatabase) : Datab databaseHelper.signalWritableDatabase.update(TABLE_NAME, values, query, args) } + + fun deleteAbandonedReactions() { + val query = """ + ($IS_MMS = 0 AND $MESSAGE_ID NOT IN (SELECT ${SmsDatabase.ID} FROM ${SmsDatabase.TABLE_NAME})) + OR + ($IS_MMS = 1 AND $MESSAGE_ID NOT IN (SELECT ${MmsDatabase.ID} FROM ${MmsDatabase.TABLE_NAME})) + """.trimIndent() + + writableDatabase.delete(TABLE_NAME, query, null) + } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/SignalDatabase.kt b/app/src/main/java/org/thoughtcrime/securesms/database/SignalDatabase.kt index 4d0aae41ec..ce8713f7d3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/SignalDatabase.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/SignalDatabase.kt @@ -238,6 +238,7 @@ open class SignalDatabase(private val context: Application, databaseSecret: Data instance!!.sms.deleteAbandonedMessages() instance!!.mms.deleteAbandonedMessages() instance!!.mms.trimEntriesForExpiredMessages() + instance!!.reactionDatabase.deleteAbandonedReactions() instance!!.rawWritableDatabase.execSQL("DROP TABLE IF EXISTS key_value") instance!!.rawWritableDatabase.execSQL("DROP TABLE IF EXISTS megaphone") instance!!.rawWritableDatabase.execSQL("DROP TABLE IF EXISTS job_spec") diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt index 86045a96a8..936eb3572a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt @@ -181,8 +181,9 @@ object SignalDatabaseMigrations { private const val PNI = 122 private const val NOTIFICATION_PROFILES = 123 private const val NOTIFICATION_PROFILES_END_FIX = 124 + private const val REACTION_BACKUP_CLEANUP = 125 - const val DATABASE_VERSION = 124 + const val DATABASE_VERSION = 125 @JvmStatic fun migrate(context: Context, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) { @@ -2232,6 +2233,19 @@ object SignalDatabaseMigrations { """.trimIndent() ) } + + if (oldVersion < REACTION_BACKUP_CLEANUP) { + db.execSQL( + // language=sql + """ + DELETE FROM reaction + WHERE + (is_mms = 0 AND message_id NOT IN (SELECT _id FROM sms)) + OR + (is_mms = 1 AND message_id NOT IN (SELECT _id FROM mms)) + """.trimIndent() + ) + } } @JvmStatic