From 691ab353dae4654b429fed3d005fed1e787290ff Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Wed, 22 Feb 2023 14:18:20 -0500 Subject: [PATCH] Fix possible dlist sync crash, improved debugging. Fixes #12795 --- .../database/DistributionListTables.kt | 2 +- .../securesms/database/RecipientTable.kt | 57 +++++++++------- .../storage/StorageSyncValidations.java | 68 ++++++++++++++----- .../api/storage/SignalStorageManifest.java | 4 ++ 4 files changed, 89 insertions(+), 42 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/DistributionListTables.kt b/app/src/main/java/org/thoughtcrime/securesms/database/DistributionListTables.kt index 285494fdd7..d53e9511d4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/DistributionListTables.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/DistributionListTables.kt @@ -73,7 +73,7 @@ class DistributionListTables constructor(context: Context?, databaseHelper: Sign } } - private object ListTable { + object ListTable { const val TABLE_NAME = "distribution_list" const val ID = "_id" diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt index c2149501cb..de68cc8652 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt @@ -1170,36 +1170,47 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da * @return All storage IDs for synced records, excluding the ones that need to be deleted. */ fun getContactStorageSyncIdsMap(): Map { - val inPart = "(?, ?)" - val args = SqlUtil.buildArgs(GroupType.NONE.id, Recipient.self().id, GroupType.SIGNAL_V1.id, GroupType.DISTRIBUTION_LIST.id) - - val query = """ - $STORAGE_SERVICE_ID NOT NULL AND ( - ($GROUP_TYPE = ? AND $SERVICE_ID NOT NULL AND $ID != ?) - OR - $GROUP_TYPE IN $inPart - ) - """.trimIndent() val out: MutableMap = HashMap() - readableDatabase.query(TABLE_NAME, arrayOf(ID, STORAGE_SERVICE_ID, GROUP_TYPE), query, args, null, null, null).use { cursor -> - while (cursor != null && cursor.moveToNext()) { - val id = RecipientId.from(cursor.requireLong(ID)) - val encodedKey = cursor.requireNonNullString(STORAGE_SERVICE_ID) - val groupType = GroupType.fromId(cursor.requireInt(GROUP_TYPE)) - val key = Base64.decodeOrThrow(encodedKey) + readableDatabase + .select(ID, STORAGE_SERVICE_ID, GROUP_TYPE) + .from(TABLE_NAME) + .where( + """ + $STORAGE_SERVICE_ID NOT NULL AND ( + ($GROUP_TYPE = ? AND $SERVICE_ID NOT NULL AND $ID != ?) + OR + $GROUP_TYPE = ? + OR + $DISTRIBUTION_LIST_ID NOT NULL AND $DISTRIBUTION_LIST_ID IN ( + SELECT ${DistributionListTables.ListTable.ID} + FROM ${DistributionListTables.ListTable.TABLE_NAME} + ) + ) + """.toSingleLine(), + GroupType.NONE.id, + Recipient.self().id, + GroupType.SIGNAL_V1.id + ) + .run() + .use { cursor -> + while (cursor.moveToNext()) { + val id = RecipientId.from(cursor.requireLong(ID)) + val encodedKey = cursor.requireNonNullString(STORAGE_SERVICE_ID) + val groupType = GroupType.fromId(cursor.requireInt(GROUP_TYPE)) + val key = Base64.decodeOrThrow(encodedKey) - when (groupType) { - GroupType.NONE -> out[id] = StorageId.forContact(key) - GroupType.SIGNAL_V1 -> out[id] = StorageId.forGroupV1(key) - GroupType.DISTRIBUTION_LIST -> out[id] = StorageId.forStoryDistributionList(key) - else -> throw AssertionError() + when (groupType) { + GroupType.NONE -> out[id] = StorageId.forContact(key) + GroupType.SIGNAL_V1 -> out[id] = StorageId.forGroupV1(key) + GroupType.DISTRIBUTION_LIST -> out[id] = StorageId.forStoryDistributionList(key) + else -> throw AssertionError() + } } } - } for (id in groups.getAllGroupV2Ids()) { - val recipient = Recipient.externalGroupExact(id!!) + val recipient = Recipient.externalGroupExact(id) val recipientId = recipient.id val existing: RecipientRecord = getRecordForSync(recipientId) ?: throw AssertionError() val key = existing.storageId ?: throw AssertionError() diff --git a/app/src/main/java/org/thoughtcrime/securesms/storage/StorageSyncValidations.java b/app/src/main/java/org/thoughtcrime/securesms/storage/StorageSyncValidations.java index a4a6ffc202..c58b7d42a8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/storage/StorageSyncValidations.java +++ b/app/src/main/java/org/thoughtcrime/securesms/storage/StorageSyncValidations.java @@ -9,7 +9,6 @@ import org.signal.core.util.logging.Log; import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.util.Base64; import org.signal.core.util.SetUtil; -import org.whispersystems.signalservice.api.push.SignalServiceAddress; import org.whispersystems.signalservice.api.storage.SignalContactRecord; import org.whispersystems.signalservice.api.storage.SignalStorageManifest; import org.whispersystems.signalservice.api.storage.SignalStorageRecord; @@ -103,22 +102,6 @@ public final class StorageSyncValidations { } private static void validateManifestAndInserts(@NonNull SignalStorageManifest manifest, @NonNull List inserts, @NonNull Recipient self) { - Set allSet = new HashSet<>(manifest.getStorageIds()); - Set insertSet = new HashSet<>(Stream.of(inserts).map(SignalStorageRecord::getId).toList()); - Set rawIdSet = Stream.of(allSet).map(id -> ByteBuffer.wrap(id.getRaw())).collect(Collectors.toSet()); - - if (allSet.size() != manifest.getStorageIds().size()) { - throw new DuplicateStorageIdError(); - } - - if (rawIdSet.size() != allSet.size()) { - throw new DuplicateRawIdError(); - } - - if (inserts.size() > insertSet.size()) { - throw new DuplicateInsertInWriteError(); - } - int accountCount = 0; for (StorageId id : manifest.getStorageIds()) { accountCount += id.getType() == ManifestRecord.Identifier.Type.ACCOUNT_VALUE ? 1 : 0; @@ -132,6 +115,43 @@ public final class StorageSyncValidations { throw new MissingAccountError(); } + Set allSet = new HashSet<>(manifest.getStorageIds()); + Set insertSet = new HashSet<>(Stream.of(inserts).map(SignalStorageRecord::getId).toList()); + Set rawIdSet = Stream.of(allSet).map(id -> ByteBuffer.wrap(id.getRaw())).collect(Collectors.toSet()); + + if (allSet.size() != manifest.getStorageIds().size()) { + throw new DuplicateStorageIdError(); + } + + if (rawIdSet.size() != allSet.size()) { + List ids = manifest.getStorageIdsByType().get(ManifestRecord.Identifier.Type.CONTACT_VALUE); + if (ids.size() != new HashSet<>(ids).size()) { + throw new DuplicateContactIdError(); + } + + ids = manifest.getStorageIdsByType().get(ManifestRecord.Identifier.Type.GROUPV1_VALUE); + if (ids.size() != new HashSet<>(ids).size()) { + throw new DuplicateGroupV1IdError(); + } + + ids = manifest.getStorageIdsByType().get(ManifestRecord.Identifier.Type.GROUPV2_VALUE); + if (ids.size() != new HashSet<>(ids).size()) { + throw new DuplicateGroupV2IdError(); + } + + ids = manifest.getStorageIdsByType().get(ManifestRecord.Identifier.Type.STORY_DISTRIBUTION_LIST_VALUE); + if (ids.size() != new HashSet<>(ids).size()) { + throw new DuplicateDistributionListIdError(); + } + + throw new DuplicateRawIdAcrossTypesError(); + } + + if (inserts.size() > insertSet.size()) { + throw new DuplicateInsertInWriteError(); + } + + for (SignalStorageRecord insert : inserts) { if (!allSet.contains(insert.getId())) { throw new InsertNotPresentInFullIdSetError(); @@ -161,7 +181,19 @@ public final class StorageSyncValidations { private static final class DuplicateStorageIdError extends Error { } - private static final class DuplicateRawIdError extends Error { + private static final class DuplicateRawIdAcrossTypesError extends Error { + } + + private static final class DuplicateContactIdError extends Error { + } + + private static final class DuplicateGroupV1IdError extends Error { + } + + private static final class DuplicateGroupV2IdError extends Error { + } + + private static final class DuplicateDistributionListIdError extends Error { } private static final class DuplicateInsertInWriteError extends Error { diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalStorageManifest.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalStorageManifest.java index 7e645d52c9..905f70370a 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalStorageManifest.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalStorageManifest.java @@ -69,6 +69,10 @@ public class SignalStorageManifest { } } + public Map> getStorageIdsByType() { + return storageIdsByType; + } + public byte[] serialize() { List ids = new ArrayList<>(storageIds.size());