empathicqubit/Signal-Android
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Fix GV2 state change bug.

Browse source
This commit is contained in:
Cody Henthorne 2022-02-16 14:01:42 -05:00 • committed by GitHub
parent 44800cf440
commit e01574c6b4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 94 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.java
View file

@ -217,7 +217,7 @@ public final class GroupsV2StateProcessor {
if (inputGroupState == null) {
try {
return updateLocalGroupFromServerPaged(revision, localState, timestamp);
return updateLocalGroupFromServerPaged(revision, localState, timestamp, false);
} catch (GroupNotAMemberException e) {
if (localState != null && signedGroupChange != null) {
try {
@ -296,7 +296,7 @@ public final class GroupsV2StateProcessor {
/**
* Using network, attempt to bring the local copy of the group up to the revision specified via paging.
*/
private GroupUpdateResult updateLocalGroupFromServerPaged(int revision, DecryptedGroup localState, long timestamp) throws IOException, GroupNotAMemberException {
private GroupUpdateResult updateLocalGroupFromServerPaged(int revision, DecryptedGroup localState, long timestamp, boolean forceIncludeFirst) throws IOException, GroupNotAMemberException {
boolean latestRevisionOnly = revision == LATEST && (localState == null || localState.getRevision() == GroupsV2StateProcessor.RESTORE_PLACEHOLDER_REVISION);
ACI selfAci = this.selfAci;
@ -324,9 +324,16 @@ public final class GroupsV2StateProcessor {
} else {
int revisionWeWereAdded = GroupProtoUtil.findRevisionWeWereAdded(latestServerGroup, selfAci.uuid());
int logsNeededFrom = localState != null ? Math.max(localState.getRevision(), revisionWeWereAdded) : revisionWeWereAdded;
boolean includeFirstState = localState == null || localState.getRevision() < 0 || (revision == LATEST && localState.getRevision() + 1 < latestServerGroup.getRevision());
boolean includeFirstState = forceIncludeFirst ||
localState == null ||
localState.getRevision() < 0 ||
(revision == LATEST && localState.getRevision() + 1 < latestServerGroup.getRevision());
Log.i(TAG, "Requesting from server currentRevision: " + (localState != null ? localState.getRevision() : "null") + " logsNeededFrom: " + logsNeededFrom);
Log.i(TAG,
"Requesting from server currentRevision: " + (localState != null ? localState.getRevision() : "null") +
" logsNeededFrom: " + logsNeededFrom +
" includeFirstState: " + includeFirstState +
" forceIncludeFirst: " + forceIncludeFirst);
inputGroupState = getFullMemberHistoryPage(localState, selfAci, logsNeededFrom, includeFirstState);
}
@ -341,6 +348,15 @@ public final class GroupsV2StateProcessor {
DecryptedGroup newLocalState = advanceGroupStateResult.getNewGlobalGroupState().getLocalState();
Log.i(TAG, "Advanced group to revision: " + (newLocalState != null ? newLocalState.getRevision() : "null"));
if (newLocalState != null && !inputGroupState.hasMore() && !forceIncludeFirst) {
int newLocalRevision = newLocalState.getRevision();
int requestRevision = (revision == LATEST) ? latestServerGroup.getRevision() : revision;
if (newLocalRevision < requestRevision) {
Log.w(TAG, "Paging again with force first snapshot enabled due to error processing changes. New local revision [" + newLocalRevision + "] hasn't reached our desired level [" + requestRevision + "]");
return updateLocalGroupFromServerPaged(revision, localState, timestamp, true);
}
}
if (newLocalState == null || newLocalState == inputGroupState.getLocalState()) {
return new GroupUpdateResult(GroupState.GROUP_CONSISTENT_OR_AHEAD, null);
}

5
app/src/test/java/org/thoughtcrime/securesms/database/GroupTestUtil.kt
View file

@ -19,6 +19,7 @@ import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupHistoryEntry
import org.whispersystems.signalservice.api.groupsv2.GroupHistoryPage
import org.whispersystems.signalservice.api.push.ACI
import org.whispersystems.signalservice.api.push.DistributionId
import java.util.UUID
fun DecryptedGroupChange.Builder.setNewDescription(description: String) {
newDescription = DecryptedString.newBuilder().setValue(description).build()
@ -189,6 +190,10 @@ fun decryptedGroup(
return builder.build()
}
fun member(aci: UUID, role: Member.Role = Member.Role.DEFAULT, joinedAt: Int = 0): DecryptedMember {
return member(ACI.from(aci), role, joinedAt)
}
fun member(aci: ACI, role: Member.Role = Member.Role.DEFAULT, joinedAt: Int = 0): DecryptedMember {
return DecryptedMember.newBuilder()
.setRole(role)

73
app/src/test/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessorTest.kt
View file

@ -18,7 +18,9 @@ import org.mockito.Mockito.reset
import org.mockito.Mockito.verify
import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.Config
import org.signal.core.util.logging.Log
import org.signal.storageservice.protos.groups.local.DecryptedGroupChange
import org.signal.storageservice.protos.groups.local.DecryptedMember
import org.signal.storageservice.protos.groups.local.DecryptedTimer
import org.signal.zkgroup.groups.GroupMasterKey
import org.thoughtcrime.securesms.SignalStoreRule
@ -33,7 +35,10 @@ import org.thoughtcrime.securesms.dependencies.ApplicationDependencies
import org.thoughtcrime.securesms.groups.GroupId
import org.thoughtcrime.securesms.groups.GroupsV2Authorization
import org.thoughtcrime.securesms.jobs.RequestGroupV2InfoJob
import org.thoughtcrime.securesms.logging.CustomSignalProtocolLogger
import org.thoughtcrime.securesms.testutil.SystemOutLogger
import org.thoughtcrime.securesms.util.Hex.fromStringCondensed
import org.whispersystems.libsignal.logging.SignalProtocolLoggerProvider
import org.whispersystems.signalservice.api.groupsv2.GroupsV2Api
import org.whispersystems.signalservice.api.push.ACI
import java.util.UUID
@ -44,10 +49,10 @@ class GroupsV2StateProcessorTest {
companion object {
val masterKey = GroupMasterKey(fromStringCondensed("0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"))
val selfAci = ACI.from(UUID.randomUUID())
val otherAci = ACI.from(UUID.randomUUID())
val selfAndOthers = listOf(member(selfAci), member(otherAci))
val others = listOf(member(otherAci))
val selfAci: ACI = ACI.from(UUID.randomUUID())
val otherAci: ACI = ACI.from(UUID.randomUUID())
val selfAndOthers: List<DecryptedMember> = listOf(member(selfAci), member(otherAci))
val others: List<DecryptedMember> = listOf(member(otherAci))
}
private lateinit var groupDatabase: GroupDatabase
@ -63,6 +68,9 @@ class GroupsV2StateProcessorTest {
@Before
fun setUp() {
Log.initialize(SystemOutLogger())
SignalProtocolLoggerProvider.setProvider(CustomSignalProtocolLogger())
groupDatabase = mock(GroupDatabase::class.java)
recipientDatabase = mock(RecipientDatabase::class.java)
groupsV2API = mock(GroupsV2Api::class.java)
@ -372,4 +380,61 @@ class GroupsV2StateProcessorTest {
assertThat("title matches revision approved at", result.latestServer!!.title, `is`("Beam me up"))
verify(ApplicationDependencies.getJobManager()).add(isA(RequestGroupV2InfoJob::class.java))
}
@Test
fun `when failing to update fully to desired revision, then try again forcing inclusion of full group state, and then successfully update from server to latest revision`() {
val randomMembers = listOf(member(UUID.randomUUID()), member(UUID.randomUUID()))
given {
localState(
revision = 100,
title = "Title",
members = others
)
serverState(
extendGroup = localState,
revision = 101,
members = listOf(others[0], randomMembers[0], member(selfAci, joinedAt = 100))
)
changeSet {
changeLog(100) {
change {
addNewMembers(member(selfAci, joinedAt = 100))
}
}
changeLog(101) {
change {
addDeleteMembers(randomMembers[1].uuid)
addModifiedProfileKeys(randomMembers[0])
}
}
}
apiCallParameters(100, false)
}
val secondApiCallChangeSet = GroupStateTestData(masterKey).apply {
changeSet {
changeLog(100) {
fullSnapshot(
extendGroup = localState,
members = selfAndOthers + randomMembers[0] + randomMembers[1]
)
change {
addNewMembers(member(selfAci, joinedAt = 100))
}
}
changeLog(101) {
change {
addDeleteMembers(randomMembers[1].uuid)
addModifiedProfileKeys(randomMembers[0])
}
}
}
}
doReturn(secondApiCallChangeSet.changeSet!!.toApiResponse()).`when`(groupsV2API).getGroupHistoryPage(any(), eq(100), any(), eq(true))
val result = processor.updateLocalGroupToRevision(GroupsV2StateProcessor.LATEST, 0, null)
assertThat("local should update to server", result.groupState, `is`(GroupsV2StateProcessor.GroupState.GROUP_UPDATED))
assertThat("revision matches latest revision on server", result.latestServer!!.revision, `is`(101))
}
}