Signal/Android
1
0
Fork 0
mirror of https://github.com/signalapp/Signal-Android.git synced 2026-02-21 10:17:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improve handling of unknown fields in storage service.

Browse Source 
Improve handling of unknown fields in storage service.

Found a lovely bug today where unmuting chats on mobile didn't sync to my linked devices. Turns out this was a result of the unknown field merging. 

1. When a proto has unknown fields, we store the entire proto in a column in our database.
2. After building a proto that we want to write remotely, we merge the saved proto with unknown fields into constructed proto. Most of the time this is fine.
3. _However_, if one of the values you're trying to set happens to be the same as the default value for the given data type (e.g. setting a long like mutedUntil = 0), then when the protos merge, it treats that field as unset and can override it with the field from the proto with unknown fields.
4. Because we currently have unknown fields in every GV2 record, we could never unmute a GV2 group :(

This changes the order of things so that unknown fields are the first thing applied in the record builder. I did this by requiring them in the builder constructors. That way start off with the unknown fields and then can manually set whatever you want, and it'll be guaranteed to override it.
This commit is contained in:
Greyson Parrelli
2022-02-08 11:53:48 -05:00
committed by Alex Hart
parent 83ee4c0147
commit 0877d6a25e
12 changed files with 80 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
app/src/main/java/org/thoughtcrime/securesms/storage/AccountRecordProcessor.java
View File

@@ -119,8 +119,7 @@ public class AccountRecordProcessor extends DefaultStorageRecordProcessor<Signal
} else if (matchesLocal) {
return local;
} else {
return new SignalAccountRecord.Builder(keyGenerator.generate())
.setUnknownFields(unknownFields)
return new SignalAccountRecord.Builder(keyGenerator.generate(), unknownFields)
.setGivenName(givenName)
.setFamilyName(familyName)
.setAvatarUrlPath(avatarUrlPath)

3
app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.java
View File

@@ -118,8 +118,7 @@ public class ContactRecordProcessor extends DefaultStorageRecordProcessor<Signal
} else if (matchesLocal) {
return local;
} else {
return new SignalContactRecord.Builder(keyGenerator.generate(), address)
.setUnknownFields(unknownFields)
return new SignalContactRecord.Builder(keyGenerator.generate(), address, unknownFields)
.setGivenName(givenName)
.setFamilyName(familyName)
.setProfileKey(profileKey)

3
app/src/main/java/org/thoughtcrime/securesms/storage/GroupV1RecordProcessor.java
View File

@@ -89,8 +89,7 @@ public final class GroupV1RecordProcessor extends DefaultStorageRecordProcessor<
} else if (matchesLocal) {
return local;
} else {
return new SignalGroupV1Record.Builder(keyGenerator.generate(), remote.getGroupId())
.setUnknownFields(unknownFields)
return new SignalGroupV1Record.Builder(keyGenerator.generate(), remote.getGroupId(), unknownFields)
.setBlocked(blocked)
.setProfileSharingEnabled(blocked)
.setForcedUnread(forcedUnread)

3
app/src/main/java/org/thoughtcrime/securesms/storage/GroupV2RecordProcessor.java
View File

@@ -81,8 +81,7 @@ public final class GroupV2RecordProcessor extends DefaultStorageRecordProcessor<
} else if (matchesLocal) {
return local;
} else {
return new SignalGroupV2Record.Builder(keyGenerator.generate(), remote.getMasterKeyBytes())
.setUnknownFields(unknownFields)
return new SignalGroupV2Record.Builder(keyGenerator.generate(), remote.getMasterKeyBytes(), unknownFields)
.setBlocked(blocked)
.setProfileSharingEnabled(blocked)
.setArchived(archived)

3
app/src/main/java/org/thoughtcrime/securesms/storage/StorageSyncHelper.java
View File

@@ -111,8 +111,7 @@ public final class StorageSyncHelper {
.map(recipientDatabase::getRecordForSync)
.toList();
SignalAccountRecord account = new SignalAccountRecord.Builder(self.getStorageServiceId())
.setUnknownFields(record != null ? record.getSyncExtras().getStorageProto() : null)
SignalAccountRecord account = new SignalAccountRecord.Builder(self.getStorageServiceId(), record != null ? record.getSyncExtras().getStorageProto() : null)
.setProfileKey(self.getProfileKey())
.setGivenName(self.getProfileName().getGivenName())
.setFamilyName(self.getProfileName().getFamilyName())

9
app/src/main/java/org/thoughtcrime/securesms/storage/StorageSyncModels.java
View File

@@ -97,8 +97,7 @@ public final class StorageSyncModels {
ACI aci = recipient.getAci() != null ? recipient.getAci() : ACI.UNKNOWN;
return new SignalContactRecord.Builder(rawStorageId, new SignalServiceAddress(aci, recipient.getE164()))
.setUnknownFields(recipient.getSyncExtras().getStorageProto())
return new SignalContactRecord.Builder(rawStorageId, new SignalServiceAddress(aci, recipient.getE164()), recipient.getSyncExtras().getStorageProto())
.setProfileKey(recipient.getProfileKey())
.setGivenName(recipient.getProfileName().getGivenName())
.setFamilyName(recipient.getProfileName().getFamilyName())
@@ -123,8 +122,7 @@ public final class StorageSyncModels {
throw new AssertionError("Group is not V1");
}
return new SignalGroupV1Record.Builder(rawStorageId, groupId.getDecodedId())
.setUnknownFields(recipient.getSyncExtras().getStorageProto())
return new SignalGroupV1Record.Builder(rawStorageId, groupId.getDecodedId(), recipient.getSyncExtras().getStorageProto())
.setBlocked(recipient.isBlocked())
.setProfileSharingEnabled(recipient.isProfileSharing())
.setArchived(recipient.getSyncExtras().isArchived())
@@ -148,8 +146,7 @@ public final class StorageSyncModels {
throw new AssertionError("Group master key not on recipient record");
}
return new SignalGroupV2Record.Builder(rawStorageId, groupMasterKey)
.setUnknownFields(recipient.getSyncExtras().getStorageProto())
return new SignalGroupV2Record.Builder(rawStorageId, groupMasterKey, recipient.getSyncExtras().getStorageProto())
.setBlocked(recipient.isBlocked())
.setProfileSharingEnabled(recipient.isProfileSharing())
.setArchived(recipient.getSyncExtras().isArchived())