signalapp/Signal-iOS
signalapp/Signal-iOS
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases 4 Wiki Activity

Merge branch 'charlesmchen/groupsV2parsingRobustnessTweaks' into release/3.8.1

Browse Source
This commit is contained in:
Matthew Chen 2020-04-28 16:43:36 -03:00
commit f662fd159b
8 changed files with 73 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Signal/src/ViewControllers/ThreadSettings/UpdateGroupViewController.m
View File

@ -357,7 +357,7 @@ NS_ASSUME_NONNULL_BEGIN
newTitle:newTitle
newAvatarData:newAvatarData
v1Members:memberSet];
if ([self.oldGroupModel isEqualToGroupModel:newGroupModel]) {
if ([self.oldGroupModel isEqualToGroupModel:newGroupModel ignoreRevision:YES]) {
return nil;
}
return newGroupModel;

8
SignalMessaging/groups/GroupV2UpdatesImpl.swift
View File

@ -362,10 +362,10 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift {
throw OWSAssertionError("Invalid group model.")
}
guard updatedGroupModel.revision > changedGroupModel.oldGroupModel.revision else {
throw OWSAssertionError("Invalid groupV2Revision: \(changedGroupModel.newGroupModel.revision).")
throw OWSAssertionError("Invalid groupV2Revision: \(updatedGroupModel.revision) <= \(changedGroupModel.oldGroupModel.revision).")
}
guard updatedGroupModel.revision >= changedGroupModel.newGroupModel.revision else {
throw OWSAssertionError("Invalid groupV2Revision: \(changedGroupModel.newGroupModel.revision).")
throw OWSAssertionError("Invalid groupV2Revision: \(updatedGroupModel.revision) < \(changedGroupModel.newGroupModel.revision).")
}
return updatedGroupThread
}
@ -471,11 +471,11 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift {
let newGroupModel = try builder.build(transaction: transaction)
if changeRevision == oldGroupModel.revision {
if !oldGroupModel.isEqual(to: newGroupModel) {
if !oldGroupModel.isEqual(to: newGroupModel, ignoreRevision: false) {
// Sometimes we re-apply the snapshot corresponding to the
// current revision when refreshing the group from the service.
// This should match the state in the database. If it doesn't,
// this reflects a bug, perhaps\ a deviation in how the service
// this reflects a bug, perhaps a deviation in how the service
// and client apply the "group changes" to the local model.
Logger.verbose("oldGroupModel: \(oldGroupModel.debugDescription)")
Logger.verbose("newGroupModel: \(newGroupModel.debugDescription)")

56
SignalMessaging/groups/GroupsV2Changes.swift
View File

@ -107,18 +107,14 @@ public class GroupsV2Changes {
groupMembershipBuilder.remove(address)
groupMembershipBuilder.addNonPendingMember(address, role: role)
do {
guard let profileKeyCiphertextData = member.profileKey else {
throw OWSAssertionError("Missing profileKeyCiphertext.")
}
let profileKeyCiphertext = try ProfileKeyCiphertext(contents: [UInt8](profileKeyCiphertextData))
let profileKey = try groupV2Params.profileKey(forProfileKeyCiphertext: profileKeyCiphertext,
uuid: uuid)
profileKeys[uuid] = profileKey
} catch {
owsFailDebug("Error parsing profile key: \(error)")
guard let profileKeyCiphertextData = member.profileKey else {
throw OWSAssertionError("Missing profileKeyCiphertext.")
}
let profileKeyCiphertext = try ProfileKeyCiphertext(contents: [UInt8](profileKeyCiphertextData))
let profileKey = try groupV2Params.profileKey(forProfileKeyCiphertext: profileKeyCiphertext,
uuid: uuid)
profileKeys[uuid] = profileKey
}
for action in changeActionsProto.deleteMembers {
@ -168,14 +164,10 @@ public class GroupsV2Changes {
throw OWSAssertionError("Invalid membership.")
}
do {
let profileKeyCiphertext = try presentation.getProfileKeyCiphertext()
let profileKey = try groupV2Params.profileKey(forProfileKeyCiphertext: profileKeyCiphertext,
uuid: uuid)
profileKeys[uuid] = profileKey
} catch {
owsFailDebug("Error parsing profile key: \(error)")
}
let profileKeyCiphertext = try presentation.getProfileKeyCiphertext()
let profileKey = try groupV2Params.profileKey(forProfileKeyCiphertext: profileKeyCiphertext,
uuid: uuid)
profileKeys[uuid] = profileKey
}
for action in changeActionsProto.addPendingMembers {
@ -194,13 +186,19 @@ public class GroupsV2Changes {
guard let role = TSGroupMemberRole.role(for: protoRole) else {
throw OWSAssertionError("Invalid role: \(protoRole.rawValue)")
}
let uuid = try groupV2Params.uuid(forUserId: userId)
let address = SignalServiceAddress(uuid: uuid)
guard let addedByUserID = pendingMember.addedByUserID else {
throw OWSAssertionError("Group pending member missing addedByUserID.")
}
let addedByUuid = try groupV2Params.uuid(forUserId: addedByUserID)
let uuid: UUID
let addedByUuid: UUID
do {
uuid = try groupV2Params.uuid(forUserId: userId)
addedByUuid = try groupV2Params.uuid(forUserId: addedByUserID)
} catch {
owsFailDebug("Error parsing uuid: \(error)")
continue
}
let address = SignalServiceAddress(uuid: uuid)
guard !oldGroupMembership.allUsers.contains(address) else {
throw OWSAssertionError("Invalid membership.")
}
@ -242,14 +240,10 @@ public class GroupsV2Changes {
groupMembershipBuilder.remove(address)
groupMembershipBuilder.addNonPendingMember(address, role: role)
do {
let profileKeyCiphertext = try presentation.getProfileKeyCiphertext()
let profileKey = try groupV2Params.profileKey(forProfileKeyCiphertext: profileKeyCiphertext,
uuid: uuid)
profileKeys[uuid] = profileKey
} catch {
owsFailDebug("Error parsing profile key: \(error)")
}
let profileKeyCiphertext = try presentation.getProfileKeyCiphertext()
let profileKey = try groupV2Params.profileKey(forProfileKeyCiphertext: profileKeyCiphertext,
uuid: uuid)
profileKeys[uuid] = profileKey
}
if let action = changeActionsProto.modifyTitle {

31
SignalMessaging/groups/GroupsV2Protos.swift
View File

@ -242,18 +242,13 @@ public class GroupsV2Protos {
role: role)
members.append(member)
do {
let uuid = try groupV2Params.uuid(forUserId: userID)
guard let profileKeyCiphertextData = memberProto.profileKey else {
throw OWSAssertionError("Group member missing profileKeyCiphertextData.")
}
let profileKeyCiphertext = try ProfileKeyCiphertext(contents: [UInt8](profileKeyCiphertextData))
let profileKey = try groupV2Params.profileKey(forProfileKeyCiphertext: profileKeyCiphertext,
uuid: uuid)
profileKeys[uuid] = profileKey
} catch {
owsFailDebug("Error parsing profile key: \(error)")
guard let profileKeyCiphertextData = memberProto.profileKey else {
throw OWSAssertionError("Group member missing profileKeyCiphertextData.")
}
let profileKeyCiphertext = try ProfileKeyCiphertext(contents: [UInt8](profileKeyCiphertextData))
let profileKey = try groupV2Params.profileKey(forProfileKeyCiphertext: profileKeyCiphertext,
uuid: uuid)
profileKeys[uuid] = profileKey
}
var pendingMembers = [GroupV2SnapshotImpl.PendingMember]()
@ -271,13 +266,19 @@ public class GroupsV2Protos {
throw OWSAssertionError("Group pending member missing addedByUserID.")
}
let timestamp = pendingMemberProto.timestamp
let uuid = try groupV2Params.uuid(forUserId: userId)
let addedByUuid = try groupV2Params.uuid(forUserId: addedByUserId)
guard memberProto.hasRole, let role = memberProto.role else {
throw OWSAssertionError("Group member missing role.")
}
let uuid: UUID
let addedByUuid: UUID
do {
uuid = try groupV2Params.uuid(forUserId: userId)
addedByUuid = try groupV2Params.uuid(forUserId: addedByUserId)
} catch {
owsFailDebug("Error parsing uuid: \(error)")
continue
}
let pendingMember = GroupV2SnapshotImpl.PendingMember(userID: userId,
uuid: uuid,
timestamp: timestamp,

31
SignalServiceKit/src/groups/GroupManager.swift
View File

@ -10,7 +10,8 @@ public class UpsertGroupResult: NSObject {
@objc
public enum Action: UInt {
case inserted
case updated
case updatedWithUserFacingChanges
case updatedWithoutUserFacingChanges
case unchanged
}
@ -821,7 +822,7 @@ public class GroupManager: NSObject {
groupModelBuilder.groupMembership = groupMembership
let newGroupModel = try groupModelBuilder.build(transaction: transaction)
if oldGroupModel.isEqual(to: newGroupModel) {
if oldGroupModel.isEqual(to: newGroupModel, ignoreRevision: false) {
// Skip redundant update.
throw GroupsV2Error.redundantChange
}
@ -891,7 +892,7 @@ public class GroupManager: NSObject {
builder.groupV2Revision = newRevision
let newGroupModel = try builder.build(transaction: transaction)
if oldGroupModel.isEqual(to: newGroupModel) {
if oldGroupModel.isEqual(to: newGroupModel, ignoreRevision: false) {
// Skip redundant update.
throw GroupsV2Error.redundantChange
}
@ -1544,11 +1545,6 @@ public class GroupManager: NSObject {
// Step 3: Update group in database, if necessary.
let oldGroupModel = groupThread.groupModel
let updateThreadResult: UpsertGroupResult = {
guard !oldGroupModel.isEqual(to: newGroupModel) else {
// Skip redundant update.
return UpsertGroupResult(action: .unchanged, groupThread: groupThread)
}
if let newGroupModelV2 = newGroupModel as? TSGroupModelV2,
let oldGroupModelV2 = oldGroupModel as? TSGroupModelV2 {
guard newGroupModelV2.revision >= oldGroupModelV2.revision else {
@ -1566,14 +1562,25 @@ public class GroupManager: NSObject {
}
}
guard !oldGroupModel.isEqual(to: newGroupModel, ignoreRevision: false) else {
// Skip redundant update.
return UpsertGroupResult(action: .unchanged, groupThread: groupThread)
}
let hasUserFacingChange = !oldGroupModel.isEqual(to: newGroupModel, ignoreRevision: true)
groupThread.update(with: newGroupModel, transaction: transaction)
return UpsertGroupResult(action: .updated, groupThread: groupThread)
let action: UpsertGroupResult.Action = (hasUserFacingChange
? .updatedWithUserFacingChanges
: .updatedWithoutUserFacingChanges)
return UpsertGroupResult(action: action, groupThread: groupThread)
}()
if updateDMResult.action == .unchanged &&
updateThreadResult.action == .unchanged {
// Neither DM config nor thread model changed.
(updateThreadResult.action == .unchanged ||
updateThreadResult.action == .updatedWithoutUserFacingChanges) {
// Neither DM config nor thread model had user-facing changes.
return updateThreadResult
}
@ -1590,7 +1597,7 @@ public class GroupManager: NSObject {
break
}
return UpsertGroupResult(action: .updated, groupThread: groupThread)
return UpsertGroupResult(action: .updatedWithUserFacingChanges, groupThread: groupThread)
}
// MARK: - Storage Service

2
SignalServiceKit/src/groups/TSGroupModel.h
View File

@ -53,7 +53,7 @@ typedef NS_CLOSED_ENUM(
members:(NSArray<SignalServiceAddress *> *)members NS_DESIGNATED_INITIALIZER;
- (BOOL)isEqual:(id)other;
- (BOOL)isEqualToGroupModel:(TSGroupModel *)model;
- (BOOL)isEqualToGroupModel:(TSGroupModel *)model ignoreRevision:(BOOL)ignoreRevision;
#endif
@property (nonatomic, readonly) NSString *groupNameOrDefault;

6
SignalServiceKit/src/groups/TSGroupModel.m
View File

@ -153,10 +153,11 @@ NSUInteger const TSGroupModelSchemaVersion = 1;
if (!other || ![other isKindOfClass:[self class]]) {
return NO;
}
return [self isEqualToGroupModel:other];
return [self isEqualToGroupModel:other ignoreRevision:NO];
}
- (BOOL)isEqualToGroupModel:(TSGroupModel *)other {
- (BOOL)isEqualToGroupModel:(TSGroupModel *)other ignoreRevision:(BOOL)ignoreRevision
{
if (self == other) {
return YES;
}
@ -174,7 +175,6 @@ NSUInteger const TSGroupModelSchemaVersion = 1;
if (![myGroupMembersSet isEqualToSet:otherGroupMembersSet]) {
return NO;
}
if (self.groupsVersion != other.groupsVersion) {
return NO;
}

7
SignalServiceKit/src/groups/TSGroupModel.swift
View File

@ -77,8 +77,9 @@ public class TSGroupModelV2: TSGroupModel {
return Array(groupMembership.nonPendingMembers)
}
public override func isEqual(to model: TSGroupModel) -> Bool {
guard super.isEqual(to: model) else {
public override func isEqual(to model: TSGroupModel,
ignoreRevision: Bool) -> Bool {
guard super.isEqual(to: model, ignoreRevision: ignoreRevision) else {
return false
}
guard let other = model as? TSGroupModelV2 else {
@ -93,7 +94,7 @@ public class TSGroupModelV2: TSGroupModel {
guard other.secretParamsData == secretParamsData else {
return false
}
guard other.revision == revision else {
guard ignoreRevision || other.revision == revision else {
return false
}
guard other.avatarUrlPath == avatarUrlPath else {