Be more careful with unknown IDs during storage sync.

app/src/main/java/org/thoughtcrime/securesms/database/UnknownStorageIdDatabase.java

@@ -15,6 +15,7 @@ import org.thoughtcrime.securesms.util.SqlUtil;
 import org.whispersystems.libsignal.util.guava.Preconditions;
 import org.whispersystems.signalservice.api.storage.SignalStorageRecord;
 import org.whispersystems.signalservice.api.storage.StorageId;
+import org.whispersystems.signalservice.internal.storage.protos.ManifestRecord;
 import org.whispersystems.signalservice.internal.storage.protos.SignalStorage;
 
 import java.io.IOException;
@@ -46,10 +47,13 @@ public class UnknownStorageIdDatabase extends Database {
     super(context, databaseHelper);
   }
 
-  public List<StorageId> getAllIds() {
+  public List<StorageId> getAllUnknownIds() {
-    List<StorageId> keys = new ArrayList<>();
+    List<StorageId> keys  = new ArrayList<>();
 
-    try (Cursor cursor = databaseHelper.getReadableDatabase().query(TABLE_NAME, null, null, null, null, null, null)) {
+    String   query = TYPE + " > ?";
+    String[] args  = SqlUtil.buildArgs(StorageId.largestKnownType());
+
+    try (Cursor cursor = databaseHelper.getReadableDatabase().query(TABLE_NAME, null, query, args, null, null, null)) {
       while (cursor != null && cursor.moveToNext()) {
         String keyEncoded = cursor.getString(cursor.getColumnIndexOrThrow(STORAGE_ID));
         int    type       = cursor.getInt(cursor.getColumnIndexOrThrow(TYPE));

app/src/main/java/org/thoughtcrime/securesms/jobs/StorageSyncJob.java

@@ -20,7 +20,6 @@ import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraint;
 import org.thoughtcrime.securesms.keyvalue.SignalStore;
 import org.thoughtcrime.securesms.migrations.StorageServiceMigrationJob;
 import org.thoughtcrime.securesms.recipients.Recipient;
-import org.thoughtcrime.securesms.recipients.RecipientId;
 import org.thoughtcrime.securesms.storage.AccountRecordProcessor;
 import org.thoughtcrime.securesms.storage.ContactRecordProcessor;
 import org.thoughtcrime.securesms.storage.GroupV1RecordProcessor;
@@ -32,8 +31,6 @@ import org.thoughtcrime.securesms.storage.StorageSyncHelper.WriteOperationResult
 import org.thoughtcrime.securesms.storage.StorageSyncModels;
 import org.thoughtcrime.securesms.storage.StorageSyncValidations;
 import org.thoughtcrime.securesms.transport.RetryLaterException;
-import org.thoughtcrime.securesms.util.FeatureFlags;
-import org.thoughtcrime.securesms.util.SetUtil;
 import org.thoughtcrime.securesms.util.Stopwatch;
 import org.thoughtcrime.securesms.util.TextSecurePreferences;
 import org.thoughtcrime.securesms.util.Util;
@@ -51,7 +48,6 @@ import org.whispersystems.signalservice.api.storage.SignalStorageRecord;
 import org.whispersystems.signalservice.api.storage.StorageId;
 import org.whispersystems.signalservice.api.storage.StorageKey;
 import org.whispersystems.signalservice.internal.storage.protos.ManifestRecord;
-import org.whispersystems.signalservice.internal.storage.protos.SignalStorage;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -60,8 +56,6 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.Locale;
-import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 /**
@@ -269,8 +263,10 @@ public class StorageSyncJob extends BaseJob {
             remoteGv2.add(remote.getGroupV2().get());
           } else if (remote.getAccount().isPresent()) {
             remoteAccount.add(remote.getAccount().get());
-          } else {
+          } else if (remote.getId().isUnknown()) {
             remoteUnknown.add(remote);
+          } else {
+            Log.w(TAG, "Bad record! Type is a known value (" + remote.getId().getType() + "), but doesn't have a matching inner record. Dropping it.");
           }
         }
 
@@ -370,7 +366,7 @@ public class StorageSyncJob extends BaseJob {
   private static @NonNull List<StorageId> getAllLocalStorageIds(@NonNull Context context, @NonNull Recipient self) {
     return Util.concatenatedList(DatabaseFactory.getRecipientDatabase(context).getContactStorageSyncIds(),
                                  Collections.singletonList(StorageId.forAccount(self.getStorageServiceId())),
-                                 DatabaseFactory.getUnknownStorageIdDatabase(context).getAllIds());
+                                 DatabaseFactory.getUnknownStorageIdDatabase(context).getAllUnknownIds());
   }
 
   private static @NonNull List<SignalStorageRecord> buildLocalStorageRecords(@NonNull Context context, @NonNull Recipient self, @NonNull Collection<StorageId> ids) {

libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/SignalStorageModels.java

@@ -4,6 +4,7 @@ import com.google.protobuf.ByteString;
 
 import org.signal.zkgroup.groups.GroupMasterKey;
 import org.whispersystems.libsignal.InvalidKeyException;
+import org.whispersystems.libsignal.logging.Log;
 import org.whispersystems.signalservice.internal.storage.protos.ManifestRecord;
 import org.whispersystems.signalservice.internal.storage.protos.StorageItem;
 import org.whispersystems.signalservice.internal.storage.protos.StorageManifest;
@@ -15,6 +16,8 @@ import java.util.List;
 
 public final class SignalStorageModels {
 
+  private static final String TAG = SignalStorageModels.class.getSimpleName();
+
   public static SignalStorageManifest remoteToLocalStorageManifest(StorageManifest manifest, StorageKey storageKey) throws IOException, InvalidKeyException {
     byte[]          rawRecord      = SignalStorageCipher.decrypt(storageKey.deriveManifestKey(manifest.getVersion()), manifest.getValue().toByteArray());
     ManifestRecord  manifestRecord = ManifestRecord.parseFrom(rawRecord);
@@ -42,6 +45,9 @@ public final class SignalStorageModels {
     } else if (record.hasAccount() && type == ManifestRecord.Identifier.Type.ACCOUNT_VALUE) {
       return SignalStorageRecord.forAccount(id, new SignalAccountRecord(id, record.getAccount()));
     } else {
+      if (StorageId.isKnownType(type)) {
+        Log.w(TAG, "StorageId is of known type (" + type + "), but the data is bad! Falling back to unknown.");
+      }
       return SignalStorageRecord.forUnknown(StorageId.forType(key, type));
     }
   }

libsignal/service/src/main/java/org/whispersystems/signalservice/api/storage/StorageId.java

@@ -53,13 +53,25 @@ public class StorageId {
 
   public static boolean isKnownType(int val) {
     for (ManifestRecord.Identifier.Type type : ManifestRecord.Identifier.Type.values()) {
-      if (type.getNumber() == val) {
+      if (type != ManifestRecord.Identifier.Type.UNRECOGNIZED && type.getNumber() == val) {
         return true;
       }
     }
     return false;
   }
 
+  public static int largestKnownType() {
+    int max = 0;
+
+    for (ManifestRecord.Identifier.Type type : ManifestRecord.Identifier.Type.values()) {
+      if (type != ManifestRecord.Identifier.Type.UNRECOGNIZED) {
+        max = Math.max(type.getNumber(), max);
+      }
+    }
+
+    return max;
+  }
+
   @Override
   public boolean equals(Object o) {
     if (this == o) return true;