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
This commit is contained in:
Greyson Parrelli 2022-01-18 17:08:38 -05:00
parent 64e4bcf46a
commit e50787ae20
5 changed files with 65 additions and 11 deletions

View file

@ -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);
}
}

View file

@ -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";

View file

@ -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)
}
}

View file

@ -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")

View file

@ -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