Fix storage service crash when matching a local contact without an ID.
It's possible that we could match a local contact that doesn't have a storageId, which would crash when we tried to make a model from it for merging. This isn't an impossible case -- it could be that the manifest has record of a user that is newly registered (or just registered at some point and never deleted) and so we need to give our local record a storageId for merging.
This commit is contained in:
parent
69ebee3eeb
commit
60690208de
8 changed files with 24 additions and 24 deletions
|
@ -2716,12 +2716,16 @@ public class RecipientDatabase extends Database {
|
||||||
ApplicationDependencies.getRecipientCache().clear();
|
ApplicationDependencies.getRecipientCache().clear();
|
||||||
}
|
}
|
||||||
|
|
||||||
public void updateStorageKeys(@NonNull Map<RecipientId, byte[]> keys) {
|
public void updateStorageId(@NonNull RecipientId recipientId, byte[] id) {
|
||||||
|
updateStorageIds(Collections.singletonMap(recipientId, id));
|
||||||
|
}
|
||||||
|
|
||||||
|
public void updateStorageIds(@NonNull Map<RecipientId, byte[]> ids) {
|
||||||
SQLiteDatabase db = databaseHelper.getWritableDatabase();
|
SQLiteDatabase db = databaseHelper.getWritableDatabase();
|
||||||
db.beginTransaction();
|
db.beginTransaction();
|
||||||
|
|
||||||
try {
|
try {
|
||||||
for (Map.Entry<RecipientId, byte[]> entry : keys.entrySet()) {
|
for (Map.Entry<RecipientId, byte[]> entry : ids.entrySet()) {
|
||||||
ContentValues values = new ContentValues();
|
ContentValues values = new ContentValues();
|
||||||
values.put(STORAGE_SERVICE_ID, Base64.encodeBytes(entry.getValue()));
|
values.put(STORAGE_SERVICE_ID, Base64.encodeBytes(entry.getValue()));
|
||||||
db.update(TABLE_NAME, values, ID_WHERE, new String[] { entry.getKey().serialize() });
|
db.update(TABLE_NAME, values, ID_WHERE, new String[] { entry.getKey().serialize() });
|
||||||
|
@ -2732,7 +2736,7 @@ public class RecipientDatabase extends Database {
|
||||||
db.endTransaction();
|
db.endTransaction();
|
||||||
}
|
}
|
||||||
|
|
||||||
for (RecipientId id : keys.keySet()) {
|
for (RecipientId id : ids.keySet()) {
|
||||||
Recipient.live(id).refresh();
|
Recipient.live(id).refresh();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -279,7 +279,7 @@ public class StorageSyncJob extends BaseJob {
|
||||||
clearIds.add(Recipient.self().getId());
|
clearIds.add(Recipient.self().getId());
|
||||||
|
|
||||||
recipientDatabase.clearDirtyState(clearIds);
|
recipientDatabase.clearDirtyState(clearIds);
|
||||||
recipientDatabase.updateStorageKeys(localWriteResult.get().getStorageKeyUpdates());
|
recipientDatabase.updateStorageIds(localWriteResult.get().getStorageKeyUpdates());
|
||||||
|
|
||||||
needsMultiDeviceSync = true;
|
needsMultiDeviceSync = true;
|
||||||
|
|
||||||
|
|
|
@ -395,7 +395,7 @@ public class StorageSyncJobV2 extends BaseJob {
|
||||||
clearIds.add(Recipient.self().getId());
|
clearIds.add(Recipient.self().getId());
|
||||||
|
|
||||||
recipientDatabase.clearDirtyState(clearIds);
|
recipientDatabase.clearDirtyState(clearIds);
|
||||||
recipientDatabase.updateStorageKeys(localWriteResult.get().getStorageKeyUpdates());
|
recipientDatabase.updateStorageIds(localWriteResult.get().getStorageKeyUpdates());
|
||||||
|
|
||||||
needsMultiDeviceSync = true;
|
needsMultiDeviceSync = true;
|
||||||
|
|
||||||
|
|
|
@ -9,19 +9,14 @@ import org.signal.core.util.logging.Log;
|
||||||
import org.thoughtcrime.securesms.database.DatabaseFactory;
|
import org.thoughtcrime.securesms.database.DatabaseFactory;
|
||||||
import org.thoughtcrime.securesms.database.RecipientDatabase;
|
import org.thoughtcrime.securesms.database.RecipientDatabase;
|
||||||
import org.thoughtcrime.securesms.recipients.Recipient;
|
import org.thoughtcrime.securesms.recipients.Recipient;
|
||||||
import org.thoughtcrime.securesms.recipients.RecipientId;
|
|
||||||
import org.whispersystems.libsignal.util.guava.Optional;
|
import org.whispersystems.libsignal.util.guava.Optional;
|
||||||
import org.whispersystems.signalservice.api.storage.SignalAccountRecord;
|
import org.whispersystems.signalservice.api.storage.SignalAccountRecord;
|
||||||
import org.whispersystems.signalservice.api.storage.SignalAccountRecord.PinnedConversation;
|
import org.whispersystems.signalservice.api.storage.SignalAccountRecord.PinnedConversation;
|
||||||
import org.whispersystems.signalservice.api.storage.StorageId;
|
|
||||||
import org.whispersystems.signalservice.internal.storage.protos.AccountRecord;
|
import org.whispersystems.signalservice.internal.storage.protos.AccountRecord;
|
||||||
|
|
||||||
import java.util.Arrays;
|
import java.util.Arrays;
|
||||||
import java.util.Collection;
|
|
||||||
import java.util.HashSet;
|
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Objects;
|
import java.util.Objects;
|
||||||
import java.util.Set;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Processes {@link SignalAccountRecord}s. Unlike some other {@link StorageRecordProcessor}s, this
|
* Processes {@link SignalAccountRecord}s. Unlike some other {@link StorageRecordProcessor}s, this
|
||||||
|
@ -66,7 +61,7 @@ public class AccountRecordProcessor extends DefaultStorageRecordProcessor<Signal
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public @NonNull Optional<SignalAccountRecord> getMatching(@NonNull SignalAccountRecord record) {
|
public @NonNull Optional<SignalAccountRecord> getMatching(@NonNull SignalAccountRecord record, @NonNull StorageKeyGenerator keyGenerator) {
|
||||||
return Optional.of(localAccountRecord);
|
return Optional.of(localAccountRecord);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -16,9 +16,6 @@ import org.whispersystems.signalservice.api.storage.SignalContactRecord;
|
||||||
import org.whispersystems.signalservice.internal.storage.protos.ContactRecord.IdentityState;
|
import org.whispersystems.signalservice.internal.storage.protos.ContactRecord.IdentityState;
|
||||||
|
|
||||||
import java.util.Arrays;
|
import java.util.Arrays;
|
||||||
import java.util.Collection;
|
|
||||||
import java.util.LinkedList;
|
|
||||||
import java.util.List;
|
|
||||||
import java.util.Objects;
|
import java.util.Objects;
|
||||||
import java.util.UUID;
|
import java.util.UUID;
|
||||||
|
|
||||||
|
@ -63,13 +60,21 @@ public class ContactRecordProcessor extends DefaultStorageRecordProcessor<Signal
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@NonNull Optional<SignalContactRecord> getMatching(@NonNull SignalContactRecord remote) {
|
@NonNull Optional<SignalContactRecord> getMatching(@NonNull SignalContactRecord remote, @NonNull StorageKeyGenerator keyGenerator) {
|
||||||
SignalServiceAddress address = remote.getAddress();
|
SignalServiceAddress address = remote.getAddress();
|
||||||
Optional<RecipientId> byUuid = address.getUuid().isPresent() ? recipientDatabase.getByUuid(address.getUuid().get()) : Optional.absent();
|
Optional<RecipientId> byUuid = address.getUuid().isPresent() ? recipientDatabase.getByUuid(address.getUuid().get()) : Optional.absent();
|
||||||
Optional<RecipientId> byE164 = address.getNumber().isPresent() ? recipientDatabase.getByE164(address.getNumber().get()) : Optional.absent();
|
Optional<RecipientId> byE164 = address.getNumber().isPresent() ? recipientDatabase.getByE164(address.getNumber().get()) : Optional.absent();
|
||||||
|
|
||||||
return byUuid.or(byE164).transform(recipientDatabase::getRecipientSettingsForSync)
|
return byUuid.or(byE164).transform(recipientDatabase::getRecipientSettingsForSync)
|
||||||
.transform(StorageSyncModels::localToRemoteRecord)
|
.transform(settings -> {
|
||||||
|
if (settings.getStorageId() != null) {
|
||||||
|
return StorageSyncModels.localToRemoteRecord(settings);
|
||||||
|
} else {
|
||||||
|
Log.w(TAG, "Newly discovering a registered user via storage service. Saving a storageId for them.");
|
||||||
|
recipientDatabase.updateStorageId(settings.getId(), keyGenerator.generate());
|
||||||
|
return StorageSyncModels.localToRemoteRecord(recipientDatabase.getRecipientSettingsForSync(settings.getId()));
|
||||||
|
}
|
||||||
|
})
|
||||||
.transform(r -> r.getContact().get());
|
.transform(r -> r.getContact().get());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -51,7 +51,7 @@ abstract class DefaultStorageRecordProcessor<E extends SignalRecord> implements
|
||||||
if (isInvalid(remote)) {
|
if (isInvalid(remote)) {
|
||||||
remoteDeletes.add(remote);
|
remoteDeletes.add(remote);
|
||||||
} else {
|
} else {
|
||||||
Optional<E> local = getMatching(remote);
|
Optional<E> local = getMatching(remote, keyGenerator);
|
||||||
|
|
||||||
if (local.isPresent()) {
|
if (local.isPresent()) {
|
||||||
E merged = merge(remote, local.get(), keyGenerator);
|
E merged = merge(remote, local.get(), keyGenerator);
|
||||||
|
@ -88,7 +88,7 @@ abstract class DefaultStorageRecordProcessor<E extends SignalRecord> implements
|
||||||
* Only records that pass the validity check (i.e. return false from {@link #isInvalid(SignalRecord)}
|
* Only records that pass the validity check (i.e. return false from {@link #isInvalid(SignalRecord)}
|
||||||
* make it to here, so you can assume all records are valid.
|
* make it to here, so you can assume all records are valid.
|
||||||
*/
|
*/
|
||||||
abstract @NonNull Optional<E> getMatching(@NonNull E remote);
|
abstract @NonNull Optional<E> getMatching(@NonNull E remote, @NonNull StorageKeyGenerator keyGenerator);
|
||||||
|
|
||||||
abstract @NonNull E merge(@NonNull E remote, @NonNull E local, @NonNull StorageKeyGenerator keyGenerator);
|
abstract @NonNull E merge(@NonNull E remote, @NonNull E local, @NonNull StorageKeyGenerator keyGenerator);
|
||||||
abstract void insertLocal(@NonNull E record) throws IOException;
|
abstract void insertLocal(@NonNull E record) throws IOException;
|
||||||
|
|
|
@ -14,8 +14,6 @@ import org.thoughtcrime.securesms.groups.GroupId;
|
||||||
import org.thoughtcrime.securesms.recipients.RecipientId;
|
import org.thoughtcrime.securesms.recipients.RecipientId;
|
||||||
import org.whispersystems.libsignal.util.guava.Optional;
|
import org.whispersystems.libsignal.util.guava.Optional;
|
||||||
import org.whispersystems.signalservice.api.storage.SignalGroupV1Record;
|
import org.whispersystems.signalservice.api.storage.SignalGroupV1Record;
|
||||||
import org.whispersystems.signalservice.api.storage.SignalGroupV2Record;
|
|
||||||
import org.whispersystems.signalservice.api.storage.SignalStorageRecord;
|
|
||||||
|
|
||||||
import java.util.Arrays;
|
import java.util.Arrays;
|
||||||
|
|
||||||
|
@ -64,7 +62,7 @@ public final class GroupV1RecordProcessor extends DefaultStorageRecordProcessor<
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@NonNull Optional<SignalGroupV1Record> getMatching(@NonNull SignalGroupV1Record record) {
|
@NonNull Optional<SignalGroupV1Record> getMatching(@NonNull SignalGroupV1Record record, @NonNull StorageKeyGenerator keyGenerator) {
|
||||||
GroupId.V1 groupId = GroupId.v1orThrow(record.getGroupId());
|
GroupId.V1 groupId = GroupId.v1orThrow(record.getGroupId());
|
||||||
|
|
||||||
Optional<RecipientId> recipientId = recipientDatabase.getByGroupId(groupId);
|
Optional<RecipientId> recipientId = recipientDatabase.getByGroupId(groupId);
|
||||||
|
|
|
@ -13,10 +13,8 @@ import org.thoughtcrime.securesms.database.RecipientDatabase;
|
||||||
import org.thoughtcrime.securesms.groups.GroupId;
|
import org.thoughtcrime.securesms.groups.GroupId;
|
||||||
import org.thoughtcrime.securesms.groups.GroupsV1MigrationUtil;
|
import org.thoughtcrime.securesms.groups.GroupsV1MigrationUtil;
|
||||||
import org.thoughtcrime.securesms.recipients.RecipientId;
|
import org.thoughtcrime.securesms.recipients.RecipientId;
|
||||||
import org.thoughtcrime.securesms.util.ParcelUtil;
|
|
||||||
import org.whispersystems.libsignal.util.guava.Optional;
|
import org.whispersystems.libsignal.util.guava.Optional;
|
||||||
import org.whispersystems.signalservice.api.storage.SignalGroupV2Record;
|
import org.whispersystems.signalservice.api.storage.SignalGroupV2Record;
|
||||||
import org.whispersystems.signalservice.internal.storage.protos.GroupV2Record;
|
|
||||||
|
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
import java.util.Arrays;
|
import java.util.Arrays;
|
||||||
|
@ -46,7 +44,7 @@ public final class GroupV2RecordProcessor extends DefaultStorageRecordProcessor<
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@NonNull Optional<SignalGroupV2Record> getMatching(@NonNull SignalGroupV2Record record) {
|
@NonNull Optional<SignalGroupV2Record> getMatching(@NonNull SignalGroupV2Record record, @NonNull StorageKeyGenerator keyGenerator) {
|
||||||
GroupId.V2 groupId = GroupId.v2(record.getMasterKeyOrThrow());
|
GroupId.V2 groupId = GroupId.v2(record.getMasterKeyOrThrow());
|
||||||
|
|
||||||
Optional<RecipientId> recipientId = recipientDatabase.getByGroupId(groupId);
|
Optional<RecipientId> recipientId = recipientDatabase.getByGroupId(groupId);
|
||||||
|
|
Loading…
Add table
Reference in a new issue