From 4e2e73637a45a0be0f0921a8bfb48a763cb5f61b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 2 Mar 2021 16:14:57 +0100 Subject: [PATCH 1/2] Be robust if Event.type is missing (#2946) --- CHANGES.md | 1 + .../android/sdk/api/session/events/model/Event.kt | 4 ++-- .../android/sdk/api/session/events/model/EventType.kt | 2 ++ .../sdk/internal/crypto/tasks/EncryptEventTask.kt | 2 +- .../sdk/internal/crypto/tasks/SendEventTask.kt | 2 +- .../crypto/tasks/SendVerificationMessageTask.kt | 2 +- .../sdk/internal/database/mapper/EventMapper.kt | 11 ++++++----- .../session/room/membership/LoadRoomMembersTask.kt | 9 +++++++-- .../session/room/relation/SendRelationWorker.kt | 2 +- .../internal/session/room/send/LocalEchoRepository.kt | 8 ++++---- .../sdk/internal/session/sync/RoomSyncHandler.kt | 10 +++++----- .../app/features/devtools/RoomStateListController.kt | 2 +- .../features/notifications/NotifiableEventResolver.kt | 2 +- 13 files changed, 33 insertions(+), 24 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 5125d158ec..146db2c5d8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -11,6 +11,7 @@ Improvements 🙌: Bugfix 🐛: - Try to fix crash about UrlPreview (#2640) + - Be robust if Event.type is missing (#2946) Translations 🗣: - diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/Event.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/Event.kt index 8f9f409b74..a208b15364 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/Event.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/Event.kt @@ -66,7 +66,7 @@ inline fun T.toContent(): Content { */ @JsonClass(generateAdapter = true) data class Event( - @Json(name = "type") val type: String, + @Json(name = "type") val type: String? = null, @Json(name = "event_id") val eventId: String? = null, @Json(name = "content") val content: Content? = null, @Json(name = "prev_content") val prevContent: Content? = null, @@ -135,7 +135,7 @@ data class Event( * @return the event type */ fun getClearType(): String { - return mxDecryptionResult?.payload?.get("type")?.toString() ?: type + return mxDecryptionResult?.payload?.get("type")?.toString() ?: type ?: EventType.MISSING_TYPE } /** diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/EventType.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/EventType.kt index d79117ad87..905e18b8e8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/EventType.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/EventType.kt @@ -20,6 +20,8 @@ package org.matrix.android.sdk.api.session.events.model * Constants defining known event types from Matrix specifications. */ object EventType { + // Used when the type is missing, which should not happen + const val MISSING_TYPE = "org.matrix.android.sdk.missing_type" const val PRESENCE = "m.presence" const val MESSAGE = "m.room.message" diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/EncryptEventTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/EncryptEventTask.kt index 56b267decd..627352f568 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/EncryptEventTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/EncryptEventTask.kt @@ -45,7 +45,7 @@ internal class DefaultEncryptEventTask @Inject constructor( // don't want to wait for any query // if (!params.crypto.isRoomEncrypted(params.roomId)) return params.event val localEvent = params.event - if (localEvent.eventId == null) { + if (localEvent.eventId == null || localEvent.type == null) { throw IllegalArgumentException() } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendEventTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendEventTask.kt index b772bfbce2..1fbc30d6f6 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendEventTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendEventTask.kt @@ -58,7 +58,7 @@ internal class DefaultSendEventTask @Inject constructor( localId, roomId = event.roomId ?: "", content = event.content, - eventType = event.type + eventType = event.type ?: "" ) } localEchoRepository.updateSendState(localId, params.event.roomId, SendState.SENT) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendVerificationMessageTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendVerificationMessageTask.kt index c39dfb1016..ab125135bb 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendVerificationMessageTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendVerificationMessageTask.kt @@ -50,7 +50,7 @@ internal class DefaultSendVerificationMessageTask @Inject constructor( localId, roomId = event.roomId ?: "", content = event.content, - eventType = event.type + eventType = event.type ?: "" ) } localEchoRepository.updateSendState(localId, event.roomId, SendState.SENT) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/EventMapper.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/EventMapper.kt index 66eccdfba0..a4a2fadd21 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/EventMapper.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/EventMapper.kt @@ -19,6 +19,7 @@ package org.matrix.android.sdk.internal.database.mapper import com.squareup.moshi.JsonDataException import org.matrix.android.sdk.api.session.crypto.MXCryptoError 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.UnsignedData import org.matrix.android.sdk.api.session.room.send.SendState import org.matrix.android.sdk.internal.crypto.algorithms.olm.OlmDecryptionResult @@ -29,8 +30,6 @@ import timber.log.Timber internal object EventMapper { fun map(event: Event, roomId: String): EventEntity { - val uds = if (event.unsignedData == null) null - else MoshiProvider.providesMoshi().adapter(UnsignedData::class.java).toJson(event.unsignedData) val eventEntity = EventEntity() // TODO change this as we shouldn't use event everywhere eventEntity.eventId = event.eventId ?: "$$roomId-${System.currentTimeMillis()}-${event.hashCode()}" @@ -39,14 +38,16 @@ internal object EventMapper { eventEntity.prevContent = ContentMapper.map(event.resolvedPrevContent()) eventEntity.isUseless = IsUselessResolver.isUseless(event) eventEntity.stateKey = event.stateKey - eventEntity.type = event.type + eventEntity.type = event.type ?: EventType.MISSING_TYPE eventEntity.sender = event.senderId eventEntity.originServerTs = event.originServerTs eventEntity.redacts = event.redacts eventEntity.age = event.unsignedData?.age ?: event.originServerTs - eventEntity.unsignedData = uds + eventEntity.unsignedData = event.unsignedData?.let { + MoshiProvider.providesMoshi().adapter(UnsignedData::class.java).toJson(it) + } eventEntity.decryptionResultJson = event.mxDecryptionResult?.let { - MoshiProvider.providesMoshi().adapter(OlmDecryptionResult::class.java).toJson(it) + MoshiProvider.providesMoshi().adapter(OlmDecryptionResult::class.java).toJson(it) } eventEntity.decryptionErrorReason = event.mCryptoErrorReason eventEntity.decryptionErrorCode = event.mCryptoError?.name diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/LoadRoomMembersTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/LoadRoomMembersTask.kt index 2be90bf8e3..97cfcdaa44 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/LoadRoomMembersTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/LoadRoomMembersTask.kt @@ -105,12 +105,17 @@ internal class DefaultLoadRoomMembersTask @Inject constructor( ?: realm.createObject(roomId) val now = System.currentTimeMillis() for (roomMemberEvent in response.roomMemberEvents) { - if (roomMemberEvent.eventId == null || roomMemberEvent.stateKey == null) { + if (roomMemberEvent.eventId == null || roomMemberEvent.stateKey == null || roomMemberEvent.type == null) { continue } val ageLocalTs = roomMemberEvent.unsignedData?.age?.let { now - it } val eventEntity = roomMemberEvent.toEntity(roomId, SendState.SYNCED, ageLocalTs).copyToRealmOrIgnore(realm, EventInsertType.PAGINATION) - CurrentStateEventEntity.getOrCreate(realm, roomId, roomMemberEvent.stateKey, roomMemberEvent.type).apply { + CurrentStateEventEntity.getOrCreate( + realm, + roomId, + roomMemberEvent.stateKey, + roomMemberEvent.type + ).apply { eventId = roomMemberEvent.eventId root = eventEntity } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/relation/SendRelationWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/relation/SendRelationWorker.kt index c12597bea0..403aa274fe 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/relation/SendRelationWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/relation/SendRelationWorker.kt @@ -89,7 +89,7 @@ internal class SendRelationWorker(context: Context, params: WorkerParameters) roomId = roomId, parentId = relatedEventId, relationType = relationType, - eventType = localEvent.type, + eventType = localEvent.type!!, content = localEvent.content ) } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/LocalEchoRepository.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/LocalEchoRepository.kt index f742271fa7..1277e81fe3 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/LocalEchoRepository.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/send/LocalEchoRepository.kt @@ -56,10 +56,10 @@ internal class LocalEchoRepository @Inject constructor(@SessionDatabase private fun createLocalEcho(event: Event) { val roomId = event.roomId ?: throw IllegalStateException("You should have set a roomId for your event") - val senderId = event.senderId ?: throw IllegalStateException("You should have set a senderIf for your event") - if (event.eventId == null) { - throw IllegalStateException("You should have set an eventId for your event") - } + val senderId = event.senderId ?: throw IllegalStateException("You should have set a senderId for your event") + event.eventId ?: throw IllegalStateException("You should have set an eventId for your event") + event.type ?: throw IllegalStateException("You should have set a type for your event") + val timelineEventEntity = realmSessionProvider.withRealm { realm -> val eventEntity = event.toEntity(roomId, SendState.UNSENT, System.currentTimeMillis()) val roomMemberHelper = RoomMemberHelper(realm, roomId) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/RoomSyncHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/RoomSyncHandler.kt index 648dd2d88f..9f54d0b86c 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/RoomSyncHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/RoomSyncHandler.kt @@ -198,7 +198,7 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle // State event if (roomSync.state?.events?.isNotEmpty() == true) { for (event in roomSync.state.events) { - if (event.eventId == null || event.stateKey == null) { + if (event.eventId == null || event.stateKey == null || event.type == null) { continue } val ageLocalTs = event.unsignedData?.age?.let { syncLocalTimestampMillis - it } @@ -254,7 +254,7 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle roomEntity.membership = Membership.INVITE if (roomSync.inviteState != null && roomSync.inviteState.events.isNotEmpty()) { roomSync.inviteState.events.forEach { event -> - if (event.stateKey == null) { + if (event.stateKey == null || event.type == null) { return@forEach } val ageLocalTs = event.unsignedData?.age?.let { syncLocalTimestampMillis - it } @@ -281,7 +281,7 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle syncLocalTimestampMillis: Long): RoomEntity { val roomEntity = RoomEntity.where(realm, roomId).findFirst() ?: realm.createObject(roomId) for (event in roomSync.state?.events.orEmpty()) { - if (event.eventId == null || event.stateKey == null) { + if (event.eventId == null || event.stateKey == null || event.type == null) { continue } val ageLocalTs = event.unsignedData?.age?.let { syncLocalTimestampMillis - it } @@ -293,7 +293,7 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle roomMemberEventHandler.handle(realm, roomId, event) } for (event in roomSync.timeline?.events.orEmpty()) { - if (event.eventId == null || event.senderId == null) { + if (event.eventId == null || event.senderId == null || event.type == null) { continue } val ageLocalTs = event.unsignedData?.age?.let { syncLocalTimestampMillis - it } @@ -340,7 +340,7 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle val roomMemberContentsByUser = HashMap() for (event in eventList) { - if (event.eventId == null || event.senderId == null) { + if (event.eventId == null || event.senderId == null || event.type == null) { continue } eventIds.add(event.eventId) diff --git a/vector/src/main/java/im/vector/app/features/devtools/RoomStateListController.kt b/vector/src/main/java/im/vector/app/features/devtools/RoomStateListController.kt index 69070c509b..38138474d9 100644 --- a/vector/src/main/java/im/vector/app/features/devtools/RoomStateListController.kt +++ b/vector/src/main/java/im/vector/app/features/devtools/RoomStateListController.kt @@ -37,7 +37,7 @@ class RoomStateListController @Inject constructor( override fun buildModels(data: RoomDevToolViewState?) { when (data?.displayMode) { RoomDevToolViewState.Mode.StateEventList -> { - val stateEventsGroups = data.stateEvents.invoke().orEmpty().groupBy { it.type } + val stateEventsGroups = data.stateEvents.invoke().orEmpty().groupBy { it.getClearType() } if (stateEventsGroups.isEmpty()) { noResultItem { diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt index c1bb1dde36..a4f617bf5b 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt @@ -68,7 +68,7 @@ class NotifiableEventResolver @Inject constructor(private val stringProvider: St // If the event can be displayed, display it as is Timber.w("NotifiableEventResolver Received an unsupported event matching a bing rule") // TODO Better event text display - val bodyPreview = event.type + val bodyPreview = event.type ?: EventType.MISSING_TYPE return SimpleNotifiableEvent( session.myUserId, From 2d664c423d54e09f3b915ec4dda2ffd0f6cda13c Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 2 Mar 2021 16:21:40 +0100 Subject: [PATCH 2/2] Simplify code --- .../fcm/VectorFirebaseMessagingService.kt | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/vector/src/gplay/java/im/vector/app/gplay/push/fcm/VectorFirebaseMessagingService.kt b/vector/src/gplay/java/im/vector/app/gplay/push/fcm/VectorFirebaseMessagingService.kt index cfd241d4f9..4d2cbecfe4 100755 --- a/vector/src/gplay/java/im/vector/app/gplay/push/fcm/VectorFirebaseMessagingService.kt +++ b/vector/src/gplay/java/im/vector/app/gplay/push/fcm/VectorFirebaseMessagingService.kt @@ -253,25 +253,15 @@ class VectorFirebaseMessagingService : FirebaseMessagingService() { * Try to create an event from the FCM data * * @param data the FCM data - * @return the event + * @return the event or null if required data are missing */ private fun parseEvent(data: Map?): Event? { - // accept only event with room id. - if (null == data || !data.containsKey("room_id") || !data.containsKey("event_id")) { - return null - } - - try { - return Event(eventId = data["event_id"], - senderId = data["sender"], - roomId = data["room_id"], - type = data.getValue("type"), - // TODO content = data.getValue("content"), - originServerTs = System.currentTimeMillis()) - } catch (e: Exception) { - Timber.e(e, "buildEvent fails ") - } - - return null + return Event( + eventId = data?.get("event_id") ?: return null, + senderId = data["sender"], + roomId = data["room_id"] ?: return null, + type = data["type"] ?: return null, + originServerTs = System.currentTimeMillis() + ) } }