From 49a44bd042543bde79119a833683f2b74ed3ab8d Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 21 Jul 2021 11:22:35 +0200 Subject: [PATCH 01/10] Do not change txnId it in case of retry, if not provided in the params Also create txnId using UUID.randomUUID() instead of Random.nextInt(Integer.MAX_VALUE) for coherency --- .../sdk/internal/crypto/tasks/SendToDeviceTask.kt | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendToDeviceTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendToDeviceTask.kt index 41a5118be0..4b60d54238 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendToDeviceTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendToDeviceTask.kt @@ -22,8 +22,8 @@ import org.matrix.android.sdk.internal.crypto.model.rest.SendToDeviceBody import org.matrix.android.sdk.internal.network.GlobalErrorReceiver import org.matrix.android.sdk.internal.network.executeRequest import org.matrix.android.sdk.internal.task.Task +import java.util.UUID import javax.inject.Inject -import kotlin.random.Random internal interface SendToDeviceTask : Task { data class Params( @@ -31,7 +31,7 @@ internal interface SendToDeviceTask : Task { val eventType: String, // the content to send. Map from user_id to device_id to content dictionary. val contentMap: MXUsersDevicesMap, - // the transactionId + // the transactionId. If not provided, a transactionId will be created by the task val transactionId: String? = null ) } @@ -46,16 +46,21 @@ internal class DefaultSendToDeviceTask @Inject constructor( messages = params.contentMap.map ) + // Create a unique txnId first, to use the same value if the request is retried + val txnId = params.transactionId ?: createUniqueTxnId() + return executeRequest( globalErrorReceiver, canRetry = true, maxRetriesCount = 3 ) { cryptoApi.sendToDevice( - params.eventType, - params.transactionId ?: Random.nextInt(Integer.MAX_VALUE).toString(), - sendToDeviceBody + eventType = params.eventType, + transactionId = txnId, + body = sendToDeviceBody ) } } } + +internal fun createUniqueTxnId() = UUID.randomUUID().toString() From 7513e972d13f72ab37de7f62241bc16f306adbdf Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 21 Jul 2021 11:43:47 +0200 Subject: [PATCH 02/10] Ensure the same txnId is reused if the Worker is started again. --- .../sdk/internal/crypto/CancelGossipRequestWorker.kt | 11 ++++++++--- .../crypto/IncomingGossipingRequestManager.kt | 8 +++++--- .../crypto/OutgoingGossipingRequestManager.kt | 7 +++++-- .../sdk/internal/crypto/SendGossipRequestWorker.kt | 10 +++++++--- .../android/sdk/internal/crypto/SendGossipWorker.kt | 10 +++++++--- 5 files changed, 32 insertions(+), 14 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CancelGossipRequestWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CancelGossipRequestWorker.kt index 7e92ff3e88..178bb4233d 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CancelGossipRequestWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CancelGossipRequestWorker.kt @@ -23,12 +23,12 @@ import org.matrix.android.sdk.api.auth.data.Credentials import org.matrix.android.sdk.api.failure.shouldBeRetried import org.matrix.android.sdk.api.session.events.model.Event import org.matrix.android.sdk.api.session.events.model.EventType -import org.matrix.android.sdk.api.session.events.model.LocalEcho import org.matrix.android.sdk.api.session.events.model.toContent import org.matrix.android.sdk.internal.crypto.model.MXUsersDevicesMap import org.matrix.android.sdk.internal.crypto.model.rest.ShareRequestCancellation import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore import org.matrix.android.sdk.internal.crypto.tasks.SendToDeviceTask +import org.matrix.android.sdk.internal.crypto.tasks.createUniqueTxnId import org.matrix.android.sdk.internal.session.SessionComponent import org.matrix.android.sdk.internal.worker.SessionSafeCoroutineWorker import org.matrix.android.sdk.internal.worker.SessionWorkerParams @@ -43,6 +43,9 @@ internal class CancelGossipRequestWorker(context: Context, override val sessionId: String, val requestId: String, val recipients: Map>, + // The txnId for the sendToDevice request. Nullable for compatibility reason, but MUST always be provided + // to use the same value if this worker is retried. + val txnId: String? = null, override val lastFailureMessage: String? = null ) : SessionWorkerParams { companion object { @@ -51,6 +54,7 @@ internal class CancelGossipRequestWorker(context: Context, sessionId = sessionId, requestId = request.requestId, recipients = request.recipients, + txnId = createUniqueTxnId(), lastFailureMessage = null ) } @@ -66,7 +70,8 @@ internal class CancelGossipRequestWorker(context: Context, } override suspend fun doSafeWork(params: Params): Result { - val localId = LocalEcho.createLocalEchoId() + // (temporary code) + val txnId = params.txnId ?: createUniqueTxnId() val contentMap = MXUsersDevicesMap() val toDeviceContent = ShareRequestCancellation( requestingDeviceId = credentials.deviceId, @@ -92,7 +97,7 @@ internal class CancelGossipRequestWorker(context: Context, SendToDeviceTask.Params( eventType = EventType.ROOM_KEY_REQUEST, contentMap = contentMap, - transactionId = localId + transactionId = txnId ) ) cryptoStore.updateOutgoingGossipingRequestState(params.requestId, OutgoingGossipingRequestState.CANCELLED) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/IncomingGossipingRequestManager.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/IncomingGossipingRequestManager.kt index 4a0a274f4f..e8640d5011 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/IncomingGossipingRequestManager.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/IncomingGossipingRequestManager.kt @@ -35,6 +35,7 @@ import org.matrix.android.sdk.internal.crypto.model.rest.GossipingDefaultContent import org.matrix.android.sdk.internal.crypto.model.rest.GossipingToDeviceObject import org.matrix.android.sdk.internal.crypto.model.rest.RoomKeyRequestBody import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore +import org.matrix.android.sdk.internal.crypto.tasks.createUniqueTxnId import org.matrix.android.sdk.internal.di.SessionId import org.matrix.android.sdk.internal.session.SessionScope import org.matrix.android.sdk.internal.util.MatrixCoroutineDispatchers @@ -356,7 +357,8 @@ internal class IncomingGossipingRequestManager @Inject constructor( secretValue = secretValue, requestUserId = request.userId, requestDeviceId = request.deviceId, - requestId = request.requestId + requestId = request.requestId, + txnId = createUniqueTxnId() ) cryptoStore.updateGossipingRequestState(request, GossipingRequestState.ACCEPTING) @@ -376,13 +378,13 @@ internal class IncomingGossipingRequestManager @Inject constructor( } request.share = { secretValue -> - val params = SendGossipWorker.Params( sessionId = userId, secretValue = secretValue, requestUserId = request.userId, requestDeviceId = request.deviceId, - requestId = request.requestId + requestId = request.requestId, + txnId = createUniqueTxnId() ) cryptoStore.updateGossipingRequestState(request, GossipingRequestState.ACCEPTING) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OutgoingGossipingRequestManager.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OutgoingGossipingRequestManager.kt index c86f2be0a3..6005fae854 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OutgoingGossipingRequestManager.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OutgoingGossipingRequestManager.kt @@ -26,6 +26,7 @@ import org.matrix.android.sdk.internal.worker.WorkerParamsFactory import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.delay import kotlinx.coroutines.launch +import org.matrix.android.sdk.internal.crypto.tasks.createUniqueTxnId import timber.log.Timber import javax.inject.Inject @@ -131,7 +132,8 @@ internal class OutgoingGossipingRequestManager @Inject constructor( val params = SendGossipRequestWorker.Params( sessionId = sessionId, keyShareRequest = request as? OutgoingRoomKeyRequest, - secretShareRequest = request as? OutgoingSecretRequest + secretShareRequest = request as? OutgoingSecretRequest, + txnId = createUniqueTxnId() ) cryptoStore.updateOutgoingGossipingRequestState(request.requestId, OutgoingGossipingRequestState.SENDING) val workRequest = gossipingWorkManager.createWork(WorkerParamsFactory.toData(params), true) @@ -154,7 +156,8 @@ internal class OutgoingGossipingRequestManager @Inject constructor( if (resend) { val reSendParams = SendGossipRequestWorker.Params( sessionId = sessionId, - keyShareRequest = request.copy(requestId = LocalEcho.createLocalEchoId()) + keyShareRequest = request.copy(requestId = LocalEcho.createLocalEchoId()), + txnId = createUniqueTxnId() ) val reSendWorkRequest = gossipingWorkManager.createWork(WorkerParamsFactory.toData(reSendParams), true) gossipingWorkManager.postWork(reSendWorkRequest) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipRequestWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipRequestWorker.kt index e8d567b944..cff4002610 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipRequestWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipRequestWorker.kt @@ -23,7 +23,6 @@ import org.matrix.android.sdk.api.auth.data.Credentials import org.matrix.android.sdk.api.failure.shouldBeRetried import org.matrix.android.sdk.api.session.events.model.Event import org.matrix.android.sdk.api.session.events.model.EventType -import org.matrix.android.sdk.api.session.events.model.LocalEcho import org.matrix.android.sdk.api.session.events.model.toContent import org.matrix.android.sdk.internal.crypto.model.MXUsersDevicesMap import org.matrix.android.sdk.internal.crypto.model.rest.GossipingToDeviceObject @@ -31,6 +30,7 @@ import org.matrix.android.sdk.internal.crypto.model.rest.RoomKeyShareRequest import org.matrix.android.sdk.internal.crypto.model.rest.SecretShareRequest import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore import org.matrix.android.sdk.internal.crypto.tasks.SendToDeviceTask +import org.matrix.android.sdk.internal.crypto.tasks.createUniqueTxnId import org.matrix.android.sdk.internal.session.SessionComponent import org.matrix.android.sdk.internal.worker.SessionSafeCoroutineWorker import org.matrix.android.sdk.internal.worker.SessionWorkerParams @@ -46,6 +46,9 @@ internal class SendGossipRequestWorker(context: Context, override val sessionId: String, val keyShareRequest: OutgoingRoomKeyRequest? = null, val secretShareRequest: OutgoingSecretRequest? = null, + // The txnId for the sendToDevice request. Nullable for compatibility reason, but MUST always be provided + // to use the same value if this worker is retried. + val txnId: String? = null, override val lastFailureMessage: String? = null ) : SessionWorkerParams @@ -58,7 +61,8 @@ internal class SendGossipRequestWorker(context: Context, } override suspend fun doSafeWork(params: Params): Result { - val localId = LocalEcho.createLocalEchoId() + // (temporary code) + val txnId = params.txnId ?: createUniqueTxnId() val contentMap = MXUsersDevicesMap() val eventType: String val requestId: String @@ -122,7 +126,7 @@ internal class SendGossipRequestWorker(context: Context, SendToDeviceTask.Params( eventType = eventType, contentMap = contentMap, - transactionId = localId + transactionId = txnId ) ) cryptoStore.updateOutgoingGossipingRequestState(requestId, OutgoingGossipingRequestState.SENT) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipWorker.kt index 8c68057056..e6531e72ac 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipWorker.kt @@ -23,7 +23,6 @@ import org.matrix.android.sdk.api.auth.data.Credentials import org.matrix.android.sdk.api.failure.shouldBeRetried import org.matrix.android.sdk.api.session.events.model.Event import org.matrix.android.sdk.api.session.events.model.EventType -import org.matrix.android.sdk.api.session.events.model.LocalEcho import org.matrix.android.sdk.api.session.events.model.toContent import org.matrix.android.sdk.internal.crypto.actions.EnsureOlmSessionsForDevicesAction import org.matrix.android.sdk.internal.crypto.actions.MessageEncrypter @@ -31,6 +30,7 @@ import org.matrix.android.sdk.internal.crypto.model.MXUsersDevicesMap import org.matrix.android.sdk.internal.crypto.model.event.SecretSendEventContent import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore import org.matrix.android.sdk.internal.crypto.tasks.SendToDeviceTask +import org.matrix.android.sdk.internal.crypto.tasks.createUniqueTxnId import org.matrix.android.sdk.internal.session.SessionComponent import org.matrix.android.sdk.internal.worker.SessionSafeCoroutineWorker import org.matrix.android.sdk.internal.worker.SessionWorkerParams @@ -48,6 +48,9 @@ internal class SendGossipWorker(context: Context, val requestUserId: String?, val requestDeviceId: String?, val requestId: String?, + // The txnId for the sendToDevice request. Nullable for compatibility reason, but MUST always be provided + // to use the same value if this worker is retried. + val txnId: String? = null, override val lastFailureMessage: String? = null ) : SessionWorkerParams @@ -62,7 +65,8 @@ internal class SendGossipWorker(context: Context, } override suspend fun doSafeWork(params: Params): Result { - val localId = LocalEcho.createLocalEchoId() + // (temporary code) + val txnId = params.txnId ?: createUniqueTxnId() val eventType: String = EventType.SEND_SECRET val toDeviceContent = SecretSendEventContent( @@ -127,7 +131,7 @@ internal class SendGossipWorker(context: Context, SendToDeviceTask.Params( eventType = EventType.ENCRYPTED, contentMap = sendToDeviceMap, - transactionId = localId + transactionId = txnId ) ) cryptoStore.updateGossipingRequestState( From 0d408264e0ef02ad5d7bfa7738b6c313d17f3502 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 21 Jul 2021 11:46:11 +0200 Subject: [PATCH 03/10] Bad copy paste --- .../sdk/api/session/crypto/verification/VerificationService.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/VerificationService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/VerificationService.kt index 54a1e896ae..f4b7e43f63 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/VerificationService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/VerificationService.kt @@ -52,7 +52,7 @@ interface VerificationService { transactionId: String?): String? /** - * Request a key verification from another user using toDevice events. + * Request a key verification from another user using event in a room. */ fun requestKeyVerificationInDMs(methods: List, otherUserId: String, From a2180ec69560db6dee221bc011b91d9b452e99a3 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 21 Jul 2021 12:01:20 +0200 Subject: [PATCH 04/10] Create RequestIdHelper.createUniqueRequestId() for code clarity --- .../crypto/OutgoingGossipingRequestManager.kt | 4 ++-- .../crypto/store/db/RealmCryptoStore.kt | 6 ++--- .../internal/crypto/util/RequestIdHelper.kt | 23 +++++++++++++++++++ 3 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/util/RequestIdHelper.kt diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OutgoingGossipingRequestManager.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OutgoingGossipingRequestManager.kt index 6005fae854..ccdb5ab137 100755 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OutgoingGossipingRequestManager.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/OutgoingGossipingRequestManager.kt @@ -16,7 +16,6 @@ package org.matrix.android.sdk.internal.crypto -import org.matrix.android.sdk.api.session.events.model.LocalEcho import org.matrix.android.sdk.internal.crypto.model.rest.RoomKeyRequestBody import org.matrix.android.sdk.internal.crypto.store.IMXCryptoStore import org.matrix.android.sdk.internal.di.SessionId @@ -27,6 +26,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.delay import kotlinx.coroutines.launch import org.matrix.android.sdk.internal.crypto.tasks.createUniqueTxnId +import org.matrix.android.sdk.internal.crypto.util.RequestIdHelper import timber.log.Timber import javax.inject.Inject @@ -156,7 +156,7 @@ internal class OutgoingGossipingRequestManager @Inject constructor( if (resend) { val reSendParams = SendGossipRequestWorker.Params( sessionId = sessionId, - keyShareRequest = request.copy(requestId = LocalEcho.createLocalEchoId()), + keyShareRequest = request.copy(requestId = RequestIdHelper.createUniqueRequestId()), txnId = createUniqueTxnId() ) val reSendWorkRequest = gossipingWorkManager.createWork(WorkerParamsFactory.toData(reSendParams), true) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStore.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStore.kt index 9ae93d61eb..cb0ae45295 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStore.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/RealmCryptoStore.kt @@ -27,7 +27,6 @@ import io.realm.Sort import io.realm.kotlin.where import org.matrix.android.sdk.api.session.crypto.crosssigning.MXCrossSigningInfo import org.matrix.android.sdk.api.session.events.model.Event -import org.matrix.android.sdk.api.session.events.model.LocalEcho import org.matrix.android.sdk.api.session.room.send.SendState import org.matrix.android.sdk.api.util.Optional import org.matrix.android.sdk.api.util.toOptional @@ -89,6 +88,7 @@ import org.matrix.android.sdk.internal.crypto.store.db.query.delete import org.matrix.android.sdk.internal.crypto.store.db.query.get import org.matrix.android.sdk.internal.crypto.store.db.query.getById import org.matrix.android.sdk.internal.crypto.store.db.query.getOrCreate +import org.matrix.android.sdk.internal.crypto.util.RequestIdHelper import org.matrix.android.sdk.internal.database.mapper.ContentMapper import org.matrix.android.sdk.internal.database.tools.RealmDebugTools import org.matrix.android.sdk.internal.di.CryptoDatabase @@ -1109,7 +1109,7 @@ internal class RealmCryptoStore @Inject constructor( if (existing == null) { request = realm.createObject(OutgoingGossipingRequestEntity::class.java).apply { - this.requestId = LocalEcho.createLocalEchoId() + this.requestId = RequestIdHelper.createUniqueRequestId() this.setRecipients(recipients) this.requestState = OutgoingGossipingRequestState.UNSENT this.type = GossipRequestType.KEY @@ -1139,7 +1139,7 @@ internal class RealmCryptoStore @Inject constructor( this.type = GossipRequestType.SECRET setRecipients(recipients) this.requestState = OutgoingGossipingRequestState.UNSENT - this.requestId = LocalEcho.createLocalEchoId() + this.requestId = RequestIdHelper.createUniqueRequestId() this.requestedInfoStr = secretName }.toOutgoingGossipingRequest() as? OutgoingSecretRequest } else { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/util/RequestIdHelper.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/util/RequestIdHelper.kt new file mode 100644 index 0000000000..9b3cb1942a --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/util/RequestIdHelper.kt @@ -0,0 +1,23 @@ +/* + * Copyright (c) 2021 The Matrix.org Foundation C.I.C. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.matrix.android.sdk.internal.crypto.util + +import java.util.UUID + +internal object RequestIdHelper { + fun createUniqueRequestId() = "req_" + UUID.randomUUID().toString() +} From bb617ffaa7d0d19fe5891d7af5efb3d73e39289f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 21 Jul 2021 13:59:19 +0200 Subject: [PATCH 05/10] Update matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CancelGossipRequestWorker.kt Co-authored-by: poljar --- .../android/sdk/internal/crypto/CancelGossipRequestWorker.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CancelGossipRequestWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CancelGossipRequestWorker.kt index 178bb4233d..26c0403b3f 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CancelGossipRequestWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CancelGossipRequestWorker.kt @@ -43,7 +43,7 @@ internal class CancelGossipRequestWorker(context: Context, override val sessionId: String, val requestId: String, val recipients: Map>, - // The txnId for the sendToDevice request. Nullable for compatibility reason, but MUST always be provided + // The txnId for the sendToDevice request. Nullable for compatibility reasons, but MUST always be provided // to use the same value if this worker is retried. val txnId: String? = null, override val lastFailureMessage: String? = null From bf1ce17972a5050859e35325883542bf3ccbb4f2 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 21 Jul 2021 13:59:32 +0200 Subject: [PATCH 06/10] Update matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipRequestWorker.kt Co-authored-by: poljar --- .../android/sdk/internal/crypto/SendGossipRequestWorker.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipRequestWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipRequestWorker.kt index cff4002610..bee18d4c25 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipRequestWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipRequestWorker.kt @@ -46,7 +46,7 @@ internal class SendGossipRequestWorker(context: Context, override val sessionId: String, val keyShareRequest: OutgoingRoomKeyRequest? = null, val secretShareRequest: OutgoingSecretRequest? = null, - // The txnId for the sendToDevice request. Nullable for compatibility reason, but MUST always be provided + // The txnId for the sendToDevice request. Nullable for compatibility reasons, but MUST always be provided // to use the same value if this worker is retried. val txnId: String? = null, override val lastFailureMessage: String? = null From eded4eacd7d871c4942a0946ee49b71bd52c3743 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 21 Jul 2021 13:59:40 +0200 Subject: [PATCH 07/10] Update matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipWorker.kt Co-authored-by: poljar --- .../org/matrix/android/sdk/internal/crypto/SendGossipWorker.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipWorker.kt index e6531e72ac..8c49a6f47f 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipWorker.kt @@ -48,7 +48,7 @@ internal class SendGossipWorker(context: Context, val requestUserId: String?, val requestDeviceId: String?, val requestId: String?, - // The txnId for the sendToDevice request. Nullable for compatibility reason, but MUST always be provided + // The txnId for the sendToDevice request. Nullable for compatibility reasons, but MUST always be provided // to use the same value if this worker is retried. val txnId: String? = null, override val lastFailureMessage: String? = null From ab6e0767bbc9879a92d835c81e5756b9088cbc1e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 21 Jul 2021 14:05:51 +0200 Subject: [PATCH 08/10] Update matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/VerificationService.kt Co-authored-by: poljar --- .../sdk/api/session/crypto/verification/VerificationService.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/VerificationService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/VerificationService.kt index f4b7e43f63..b9d0c0ad2c 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/VerificationService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/crypto/verification/VerificationService.kt @@ -52,7 +52,7 @@ interface VerificationService { transactionId: String?): String? /** - * Request a key verification from another user using event in a room. + * Request key verification with another user via room events (instead of the to-device API) */ fun requestKeyVerificationInDMs(methods: List, otherUserId: String, From 4ead39038c06ca1ed120e7bcf8da57f8e8d9e1f5 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 21 Jul 2021 14:06:47 +0200 Subject: [PATCH 09/10] Code review --- .../android/sdk/internal/crypto/tasks/SendToDeviceTask.kt | 4 +++- .../android/sdk/internal/crypto/util/RequestIdHelper.kt | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendToDeviceTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendToDeviceTask.kt index 4b60d54238..c6af9b0cd1 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendToDeviceTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendToDeviceTask.kt @@ -46,7 +46,9 @@ internal class DefaultSendToDeviceTask @Inject constructor( messages = params.contentMap.map ) - // Create a unique txnId first, to use the same value if the request is retried + // If params.transactionId is not provided, we create a unique txnId. + // It's important to do that outside the requestBlock parameter of executeRequest() + // to use the same value if the request is retried val txnId = params.transactionId ?: createUniqueTxnId() return executeRequest( diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/util/RequestIdHelper.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/util/RequestIdHelper.kt index 9b3cb1942a..3106d5820c 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/util/RequestIdHelper.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/util/RequestIdHelper.kt @@ -19,5 +19,5 @@ package org.matrix.android.sdk.internal.crypto.util import java.util.UUID internal object RequestIdHelper { - fun createUniqueRequestId() = "req_" + UUID.randomUUID().toString() + fun createUniqueRequestId() = UUID.randomUUID().toString() } From 08ea3c0888a70709bf25e81d5d3325f64b005cb6 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 21 Jul 2021 14:40:07 +0200 Subject: [PATCH 10/10] More useful comment --- .../android/sdk/internal/crypto/CancelGossipRequestWorker.kt | 4 +++- .../android/sdk/internal/crypto/SendGossipRequestWorker.kt | 4 +++- .../matrix/android/sdk/internal/crypto/SendGossipWorker.kt | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CancelGossipRequestWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CancelGossipRequestWorker.kt index 26c0403b3f..0ec020bc46 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CancelGossipRequestWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/CancelGossipRequestWorker.kt @@ -70,7 +70,9 @@ internal class CancelGossipRequestWorker(context: Context, } override suspend fun doSafeWork(params: Params): Result { - // (temporary code) + // params.txnId should be provided in all cases now. But Params can be deserialized by + // the WorkManager from data serialized in a previous version of the application, so without the txnId field. + // So if not present, we create a txnId val txnId = params.txnId ?: createUniqueTxnId() val contentMap = MXUsersDevicesMap() val toDeviceContent = ShareRequestCancellation( diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipRequestWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipRequestWorker.kt index bee18d4c25..2e26720abb 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipRequestWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipRequestWorker.kt @@ -61,7 +61,9 @@ internal class SendGossipRequestWorker(context: Context, } override suspend fun doSafeWork(params: Params): Result { - // (temporary code) + // params.txnId should be provided in all cases now. But Params can be deserialized by + // the WorkManager from data serialized in a previous version of the application, so without the txnId field. + // So if not present, we create a txnId val txnId = params.txnId ?: createUniqueTxnId() val contentMap = MXUsersDevicesMap() val eventType: String diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipWorker.kt index 8c49a6f47f..c5c6d26f79 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SendGossipWorker.kt @@ -65,7 +65,9 @@ internal class SendGossipWorker(context: Context, } override suspend fun doSafeWork(params: Params): Result { - // (temporary code) + // params.txnId should be provided in all cases now. But Params can be deserialized by + // the WorkManager from data serialized in a previous version of the application, so without the txnId field. + // So if not present, we create a txnId val txnId = params.txnId ?: createUniqueTxnId() val eventType: String = EventType.SEND_SECRET