Signal/Android
1
0
Fork 0
mirror of https://github.com/signalapp/Signal-Android.git synced 2025-12-24 21:15:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix incorrect left group in local state bug.

Browse Source
This commit is contained in:
Cody Henthorne
2025-01-15 11:50:00 -05:00
committed by GitHub
parent 8777c1ff89
commit 93604f53d4
6 changed files with 156 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
app/src/main/java/org/thoughtcrime/securesms/database/GroupTable.kt
View File

@@ -30,6 +30,7 @@ import org.signal.core.util.requireLong
import org.signal.core.util.requireNonNullString
import org.signal.core.util.requireString
import org.signal.core.util.select
import org.signal.core.util.toInt
import org.signal.core.util.update
import org.signal.core.util.withinTransaction
import org.signal.libsignal.zkgroup.InvalidInputException
@@ -510,6 +511,13 @@ class GroupTable(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT
return Reader(cursor)
}
fun getInactiveGroups(): Reader {
val query = SqlUtil.buildQuery("$TABLE_NAME.$ACTIVE = ?", false.toInt())
val select = "${joinedGroupSelect()} WHERE ${query.where}"
return Reader(readableDatabase.query(select, query.whereArgs))
}
fun getActiveGroupCount(): Int {
return readableDatabase
.select("COUNT(*)")

28
app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.kt
View File

@@ -112,14 +112,15 @@ class GroupsV2StateProcessor private constructor(
@WorkerThread
@Throws(IOException::class, GroupNotAMemberException::class)
fun forceSanityUpdateFromServer(timestamp: Long): GroupUpdateResult {
val currentLocalState: DecryptedGroup? = SignalDatabase.groups.getGroup(groupId).map { it.requireV2GroupProperties().decryptedGroup }.orNull()
val groupRecord = SignalDatabase.groups.getGroup(groupId).orNull()
val currentLocalState: DecryptedGroup? = groupRecord?.requireV2GroupProperties()?.decryptedGroup
if (currentLocalState == null) {
Log.i(TAG, "$logPrefix No local state to force update")
return GroupUpdateResult.CONSISTENT_OR_AHEAD
}
return when (val result = updateToLatestViaServer(timestamp, currentLocalState, reconstructChange = true)) {
return when (val result = updateToLatestViaServer(timestamp, currentLocalState, reconstructChange = true, forceUpdate = !groupRecord.isActive)) {
InternalUpdateResult.NoUpdateNeeded -> GroupUpdateResult.CONSISTENT_OR_AHEAD
is InternalUpdateResult.Updated -> GroupUpdateResult(GroupUpdateResult.UpdateStatus.GROUP_UPDATED, result.updatedLocalState)
is InternalUpdateResult.NotAMember -> throw result.exception
@@ -267,7 +268,8 @@ class GroupsV2StateProcessor private constructor(
timestamp = timestamp,
serverGuid = serverGuid,
groupStateDiff = groupStateDiff,
groupSendEndorsements = null
groupSendEndorsements = null,
forceSave = forceApply
)
}
@@ -281,7 +283,7 @@ class GroupsV2StateProcessor private constructor(
if (targetRevision == LATEST && (currentLocalState == null || currentLocalState.revision == RESTORE_PLACEHOLDER_REVISION)) {
Log.i(TAG, "$logPrefix Latest revision only, update to latest directly")
return updateToLatestViaServer(timestamp, currentLocalState, reconstructChange = false)
return updateToLatestViaServer(timestamp, currentLocalState, reconstructChange = false, forceUpdate = false)
}
Log.i(TAG, "$logPrefix Paging from server targetRevision: ${if (targetRevision == LATEST) "latest" else targetRevision}")
@@ -292,7 +294,7 @@ class GroupsV2StateProcessor private constructor(
val joinedAtFailure = InternalUpdateResult.from(joinedAtResult.getCause()!!)
if (joinedAtFailure is InternalUpdateResult.NotAMember) {
Log.i(TAG, "$logPrefix Not a member, try to update to latest directly")
return updateToLatestViaServer(timestamp, currentLocalState, reconstructChange = currentLocalState != null)
return updateToLatestViaServer(timestamp, currentLocalState, reconstructChange = currentLocalState != null, forceUpdate = true)
} else {
return joinedAtFailure
}
@@ -322,7 +324,9 @@ class GroupsV2StateProcessor private constructor(
val applyGroupStateDiffResult: AdvanceGroupStateResult = GroupStatePatcher.applyGroupStateDiff(remoteGroupStateDiff, targetRevision)
val updatedGroupState: DecryptedGroup? = applyGroupStateDiffResult.updatedGroupState
if (updatedGroupState == null || updatedGroupState == remoteGroupStateDiff.previousGroupState) {
if (groupRecord.map { it.isActive }.orNull() == false && updatedGroupState != null && updatedGroupState == remoteGroupStateDiff.previousGroupState) {
Log.w(TAG, "$logPrefix Local state is not active, but server is returning state for us, apply regardless of revision")
} else if (updatedGroupState == null || updatedGroupState == remoteGroupStateDiff.previousGroupState) {
Log.i(TAG, "$logPrefix Local state is at or later than server revision: ${currentLocalState?.revision ?: "null"}")
if (currentLocalState != null && applyGroupStateDiffResult.remainingRemoteGroupChanges.isEmpty()) {
@@ -391,7 +395,7 @@ class GroupsV2StateProcessor private constructor(
return InternalUpdateResult.Updated(currentLocalState!!)
}
private fun updateToLatestViaServer(timestamp: Long, currentLocalState: DecryptedGroup?, reconstructChange: Boolean): InternalUpdateResult {
private fun updateToLatestViaServer(timestamp: Long, currentLocalState: DecryptedGroup?, reconstructChange: Boolean, forceUpdate: Boolean): InternalUpdateResult {
val result = groupsApi.getGroupAsResult(groupSecretParams, groupsV2Authorization.getAuthorizationForToday(serviceIds, groupSecretParams))
val groupResponse = if (result is NetworkResult.Success) {
@@ -407,7 +411,8 @@ class GroupsV2StateProcessor private constructor(
timestamp = timestamp,
serverGuid = null,
groupStateDiff = remoteGroupStateDiff,
groupSendEndorsements = groupOperations.receiveGroupSendEndorsements(serviceIds.aci, groupResponse.group, groupResponse.groupSendEndorsementsResponse)
groupSendEndorsements = groupOperations.receiveGroupSendEndorsements(serviceIds.aci, groupResponse.group, groupResponse.groupSendEndorsementsResponse),
forceSave = forceUpdate && groupResponse.group.members.asSequence().mapNotNull { ACI.parseOrNull(it.aciBytes) }.any { serviceIds.matches(it) }
)
}
@@ -480,13 +485,16 @@ class GroupsV2StateProcessor private constructor(
timestamp: Long,
serverGuid: String?,
groupStateDiff: GroupStateDiff,
groupSendEndorsements: ReceivedGroupSendEndorsements?
groupSendEndorsements: ReceivedGroupSendEndorsements?,
forceSave: Boolean
): InternalUpdateResult {
val currentLocalState: DecryptedGroup? = groupStateDiff.previousGroupState
val applyGroupStateDiffResult = GroupStatePatcher.applyGroupStateDiff(groupStateDiff, GroupStatePatcher.LATEST)
val updatedGroupState = applyGroupStateDiffResult.updatedGroupState
if (updatedGroupState == null || updatedGroupState == groupStateDiff.previousGroupState) {
if (forceSave && updatedGroupState != null) {
Log.w(TAG, "$logPrefix Forcing local state update regardless of revision")
} else if (updatedGroupState == null || updatedGroupState == groupStateDiff.previousGroupState) {
Log.i(TAG, "$logPrefix Local state and server state are equal")
if (groupSendEndorsements != null) {

2
app/src/main/java/org/thoughtcrime/securesms/jobs/JobManagerFactories.java
View File

@@ -67,6 +67,7 @@ import org.thoughtcrime.securesms.migrations.EmojiDownloadMigrationJob;
import org.thoughtcrime.securesms.migrations.EmojiSearchIndexCheckMigrationJob;
import org.thoughtcrime.securesms.migrations.GooglePlayBillingPurchaseTokenMigrationJob;
import org.thoughtcrime.securesms.migrations.IdentityTableCleanupMigrationJob;
import org.thoughtcrime.securesms.migrations.InactiveGroupCheckMigrationJob;
import org.thoughtcrime.securesms.migrations.LegacyMigrationJob;
import org.thoughtcrime.securesms.migrations.MigrationCompleteJob;
import org.thoughtcrime.securesms.migrations.OptimizeMessageSearchIndexMigrationJob;
@@ -293,6 +294,7 @@ public final class JobManagerFactories {
put(EmojiSearchIndexCheckMigrationJob.KEY, new EmojiSearchIndexCheckMigrationJob.Factory());
put(GooglePlayBillingPurchaseTokenMigrationJob.KEY, new GooglePlayBillingPurchaseTokenMigrationJob.Factory());
put(IdentityTableCleanupMigrationJob.KEY, new IdentityTableCleanupMigrationJob.Factory());
put(InactiveGroupCheckMigrationJob.KEY, new InactiveGroupCheckMigrationJob.Factory());
put(LegacyMigrationJob.KEY, new LegacyMigrationJob.Factory());
put(MigrationCompleteJob.KEY, new MigrationCompleteJob.Factory());
put(OptimizeMessageSearchIndexMigrationJob.KEY, new OptimizeMessageSearchIndexMigrationJob.Factory());

7
app/src/main/java/org/thoughtcrime/securesms/migrations/ApplicationMigrations.java
View File

@@ -168,9 +168,10 @@ public class ApplicationMigrations {
static final int GPB_TOKEN_MIGRATION = 124;
static final int GROUP_ADD_MIGRATION = 125;
static final int SSRE2_CAPABILITY = 126;
static final int FIX_INACTIVE_GROUPS = 127;
}
public static final int CURRENT_VERSION = 126;
public static final int CURRENT_VERSION = 127;
/**
* This *must* be called after the {@link JobManager} has been instantiated, but *before* the call
@@ -773,6 +774,10 @@ public class ApplicationMigrations {
jobs.put(Version.SSRE2_CAPABILITY, new AttributesMigrationJob());
}
if (lastSeenVersion < Version.FIX_INACTIVE_GROUPS) {
jobs.put(Version.FIX_INACTIVE_GROUPS, new InactiveGroupCheckMigrationJob());
}
return jobs;
}

67
app/src/main/java/org/thoughtcrime/securesms/migrations/InactiveGroupCheckMigrationJob.kt Normal file
View File

@@ -0,0 +1,67 @@
package org.thoughtcrime.securesms.migrations
import org.signal.core.util.logging.Log
import org.signal.storageservice.protos.groups.local.DecryptedGroup
import org.thoughtcrime.securesms.database.SignalDatabase
import org.thoughtcrime.securesms.dependencies.AppDependencies
import org.thoughtcrime.securesms.jobmanager.Job
import org.thoughtcrime.securesms.jobs.RequestGroupV2InfoJob
import org.thoughtcrime.securesms.keyvalue.SignalStore
import org.whispersystems.signalservice.api.push.ServiceId
import org.whispersystems.signalservice.api.push.ServiceIds
/**
* Migration to fix groups that we locally marked as inactive because of the server
* but may not actually be left.
*/
internal class InactiveGroupCheckMigrationJob(
parameters: Parameters = Parameters.Builder().build()
) : MigrationJob(parameters) {
companion object {
val TAG = Log.tag(InactiveGroupCheckMigrationJob::class.java)
const val KEY = "InactiveGroupCheckMigrationJob"
}
override fun getFactoryKey(): String = KEY
override fun isUiBlocking(): Boolean = false
override fun performMigration() {
if (SignalStore.account.aci == null) {
Log.w(TAG, "ACI missing, abort")
return
}
val serviceIds = SignalStore.account.getServiceIds()
SignalDatabase
.groups
.getInactiveGroups()
.use { reader ->
reader
.asSequence()
.filter { it.isV2Group }
.filter { it.requireV2GroupProperties().decryptedGroup.isMember(serviceIds) }
.forEach {
AppDependencies.jobManager.add(RequestGroupV2InfoJob(it.id.requireV2()))
}
}
}
private fun DecryptedGroup.isMember(serviceIds: ServiceIds): Boolean {
return this
.members
.asSequence()
.mapNotNull { ServiceId.ACI.Companion.parseOrNull(it.aciBytes) }
.any { serviceIds.matches(it) }
}
override fun shouldRetry(e: Exception): Boolean = false
class Factory : Job.Factory<InactiveGroupCheckMigrationJob> {
override fun create(parameters: Parameters, serializedData: ByteArray?): InactiveGroupCheckMigrationJob {
return InactiveGroupCheckMigrationJob(parameters)
}
}
}

55
app/src/test/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessorTest.kt
View File

@@ -990,4 +990,59 @@ class GroupsV2StateProcessorTest {
verify { groupTable.update(masterKey, result.latestServer!!, null) }
}
/**
* If we think a group is not active locally, but the server returns state for us indicating it thinks we are active,
* apply the state regardless of revision.
*/
@Test
fun localInactiveButActiveOnServer() {
given {
localState(
active = false,
revision = 5,
members = selfAndOthers
)
changeSet {
}
apiCallParameters(requestedRevision = 5, includeFirst = false)
joinedAtRevision = 0
expectTableUpdate = true
}
val result = processor.updateLocalGroupToRevision(
targetRevision = GroupsV2StateProcessor.LATEST,
timestamp = 0
)
assertThat(result.updateStatus, "inactive local is still updated with same revision").isEqualTo(GroupUpdateResult.UpdateStatus.GROUP_UPDATED)
}
/**
* If we think a group is not active locally, but the server returns state for us indicating it thinks we are active,
* apply the state regardless of revision.
*/
@Test
fun forceUpdateLocalInactiveButActiveOnServer() {
given {
localState(
active = false,
revision = 5,
members = selfAndOthers
)
serverState(
revision = 5,
members = selfAndOthers
)
apiCallParameters(requestedRevision = 5, includeFirst = false)
joinedAtRevision = 0
expectTableUpdate = true
}
val result = processor.forceSanityUpdateFromServer(
timestamp = 0
)
assertThat(result.updateStatus, "inactive local is still updated given same revision from server").isEqualTo(GroupUpdateResult.UpdateStatus.GROUP_UPDATED)
}
}