PushData must have valid Event and Room ids.

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/firebase/FirebasePushParser.kt

@@ -20,7 +20,7 @@ import io.element.android.libraries.push.impl.push.PushData
 import javax.inject.Inject
 
 class FirebasePushParser @Inject constructor() {
-    fun parse(message: Map<String, String?>): PushData {
+    fun parse(message: Map<String, String?>): PushData? {
         val pushDataFirebase = PushDataFirebase(
             eventId = message["event_id"],
             roomId = message["room_id"],

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/firebase/PushDataFirebase.kt

@@ -16,7 +16,6 @@
 
 package io.element.android.libraries.push.impl.firebase
 
-import io.element.android.libraries.matrix.api.core.MatrixPatterns
 import io.element.android.libraries.matrix.api.core.asEventId
 import io.element.android.libraries.matrix.api.core.asRoomId
 import io.element.android.libraries.push.impl.push.PushData
@@ -41,9 +40,13 @@ data class PushDataFirebase(
     val clientSecret: String?
 )
 
-fun PushDataFirebase.toPushData() = PushData(
+fun PushDataFirebase.toPushData(): PushData? {
-    eventId = eventId?.takeIf { MatrixPatterns.isEventId(it) }?.asEventId(),
+    val safeEventId = eventId?.asEventId() ?: return null
-    roomId = roomId?.takeIf { MatrixPatterns.isRoomId(it) }?.asRoomId(),
+    val safeRoomId = roomId?.asRoomId() ?: return null
-    unread = unread,
+    return PushData(
-    clientSecret = clientSecret,
+        eventId = safeEventId,
-)
+        roomId = safeRoomId,
+        unread = unread,
+        clientSecret = clientSecret,
+    )
+}

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/firebase/VectorFirebaseMessagingService.kt

@@ -53,8 +53,11 @@ class VectorFirebaseMessagingService : FirebaseMessagingService() {
     override fun onMessageReceived(message: RemoteMessage) {
         Timber.tag(loggerTag.value).d("New Firebase message")
         coroutineScope.launch {
-            pushParser.parse(message.data).let {
+            val pushData = pushParser.parse(message.data)
-                pushHandler.handle(it)
+            if (pushData == null) {
+                Timber.tag(loggerTag.value).w("Invalid data received from Firebase")
+            } else {
+                pushHandler.handle(pushData)
             }
         }
     }

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/PushData.kt

@@ -22,14 +22,14 @@ import io.element.android.libraries.matrix.api.core.RoomId
 /**
  * Represent parsed data that the app has received from a Push content.
  *
- * @property eventId The Event ID. If not null, it will not be empty, and will have a valid format.
+ * @property eventId The Event Id.
- * @property roomId The Room ID. If not null, it will not be empty, and will have a valid format.
+ * @property roomId The Room Id.
  * @property unread Number of unread message.
- * @property clientSecret A client secret, used to determine which user should receive the notification.
+ * @property clientSecret data used when the pusher was configured, to be able to determine the session.
  */
 data class PushData(
-        val eventId: EventId?,
+        val eventId: EventId,
-        val roomId: RoomId?,
+        val roomId: RoomId,
         val unread: Int?,
         val clientSecret: String?,
 )

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unifiedpush/PushDataUnifiedPush.kt

@@ -16,7 +16,6 @@
 
 package io.element.android.libraries.push.impl.unifiedpush
 
-import io.element.android.libraries.matrix.api.core.MatrixPatterns
 import io.element.android.libraries.matrix.api.core.asEventId
 import io.element.android.libraries.matrix.api.core.asRoomId
 import io.element.android.libraries.push.impl.push.PushData
@@ -56,9 +55,13 @@ data class PushDataUnifiedPushCounts(
     @SerialName("unread") val unread: Int? = null
 )
 
-fun PushDataUnifiedPush.toPushData() = PushData(
+fun PushDataUnifiedPush.toPushData(): PushData? {
-    eventId = notification?.eventId?.takeIf { MatrixPatterns.isEventId(it) }?.asEventId(),
+    val safeEventId = notification?.eventId?.asEventId() ?: return null
-    roomId = notification?.roomId?.takeIf { MatrixPatterns.isRoomId(it) }?.asRoomId(),
+    val safeRoomId = notification.roomId?.asRoomId() ?: return null
-    unread = notification?.counts?.unread,
+    return PushData(
-    clientSecret = null // TODO EAx check how client secret will be sent through UnifiedPush
+        eventId = safeEventId,
-)
+        roomId = safeRoomId,
+        unread = notification.counts?.unread,
+        clientSecret = null // TODO EAx check how client secret will be sent through UnifiedPush
+    )
+}

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/unifiedpush/VectorUnifiedPushMessagingReceiver.kt

@@ -67,10 +67,11 @@ class VectorUnifiedPushMessagingReceiver : MessagingReceiver() {
     override fun onMessage(context: Context, message: ByteArray, instance: String) {
         Timber.tag(loggerTag.value).d("New message")
         coroutineScope.launch {
-            pushParser.parse(message)?.let {
+            val pushData = pushParser.parse(message)
-                pushHandler.handle(it)
+            if (pushData == null) {
-            } ?: run {
+                Timber.tag(loggerTag.value).w("Invalid data received from UnifiedPush")
-                Timber.tag(loggerTag.value).w("Invalid received data Json format")
+            } else {
+                pushHandler.handle(pushData)
             }
         }
     }

libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/firebase/FirebasePushParserTest.kt

@@ -30,18 +30,11 @@ class FirebasePushParserTest {
         clientSecret = "a-secret"
     )
 
-    private val emptyData = PushData(
-        eventId = null,
-        roomId = null,
-        unread = null,
-        clientSecret = null
-    )
-
     @Test
     fun `test edge cases Firebase`() {
         val pushParser = FirebasePushParser()
         // Empty Json
-        assertThat(pushParser.parse(emptyMap())).isEqualTo(emptyData)
+        assertThat(pushParser.parse(emptyMap())).isNull()
         // Bad Json
         assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("unread", "str"))).isEqualTo(validData.copy(unread = null))
         // Extra data
@@ -57,31 +50,27 @@ class FirebasePushParserTest {
     @Test
     fun `test empty roomId`() {
         val pushParser = FirebasePushParser()
-        val expected = validData.copy(roomId = null)
+        assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", null))).isNull()
-        assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", null))).isEqualTo(expected)
+        assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", ""))).isNull()
-        assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", ""))).isEqualTo(expected)
     }
 
     @Test
     fun `test invalid roomId`() {
         val pushParser = FirebasePushParser()
-        val expected = validData.copy(roomId = null)
+        assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", "aRoomId:domain"))).isNull()
-        assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("room_id", "aRoomId:domain"))).isEqualTo(expected)
     }
 
     @Test
     fun `test empty eventId`() {
         val pushParser = FirebasePushParser()
-        val expected = validData.copy(eventId = null)
+        assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", null))).isNull()
-        assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", null))).isEqualTo(expected)
+        assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", ""))).isNull()
-        assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", ""))).isEqualTo(expected)
     }
 
     @Test
     fun `test invalid eventId`() {
         val pushParser = FirebasePushParser()
-        val expected = validData.copy(eventId = null)
+        assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", "anEventId"))).isNull()
-        assertThat(pushParser.parse(FIREBASE_PUSH_DATA.mutate("event_id", "anEventId"))).isEqualTo(expected)
     }
 
     companion object {

libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/unifiedpush/UnifiedPushParserTest.kt

@@ -31,20 +31,13 @@ class UnifiedPushParserTest {
         clientSecret = null
     )
 
-    private val emptyData = PushData(
-        eventId = null,
-        roomId = null,
-        unread = null,
-        clientSecret = null
-    )
-
     @Test
     fun `test edge cases UnifiedPush`() {
         val pushParser = UnifiedPushParser()
         // Empty string
         assertThat(pushParser.parse("".toByteArray())).isNull()
         // Empty Json
-        assertThat(pushParser.parse("{}".toByteArray())).isEqualTo(emptyData)
+        assertThat(pushParser.parse("{}".toByteArray())).isNull()
         // Bad Json
         assertThat(pushParser.parse("ABC".toByteArray())).isNull()
     }
@@ -58,29 +51,25 @@ class UnifiedPushParserTest {
     @Test
     fun `test empty roomId`() {
         val pushParser = UnifiedPushParser()
-        val expected = validData.copy(roomId = null)
+        assertThat(pushParser.parse(UNIFIED_PUSH_DATA.replace(A_ROOM_ID.value, "").toByteArray())).isNull()
-        assertThat(pushParser.parse(UNIFIED_PUSH_DATA.replace(A_ROOM_ID.value, "").toByteArray())).isEqualTo(expected)
     }
 
     @Test
     fun `test invalid roomId`() {
         val pushParser = UnifiedPushParser()
-        val expected = validData.copy(roomId = null)
+        assertThat(pushParser.parse(UNIFIED_PUSH_DATA.mutate(A_ROOM_ID.value, "aRoomId:domain"))).isNull()
-        assertThat(pushParser.parse(UNIFIED_PUSH_DATA.mutate(A_ROOM_ID.value, "aRoomId:domain"))).isEqualTo(expected)
     }
 
     @Test
     fun `test empty eventId`() {
         val pushParser = UnifiedPushParser()
-        val expected = validData.copy(eventId = null)
+        assertThat(pushParser.parse(UNIFIED_PUSH_DATA.mutate(AN_EVENT_ID.value, ""))).isNull()
-        assertThat(pushParser.parse(UNIFIED_PUSH_DATA.mutate(AN_EVENT_ID.value, ""))).isEqualTo(expected)
     }
 
     @Test
     fun `test invalid eventId`() {
         val pushParser = UnifiedPushParser()
-        val expected = validData.copy(eventId = null)
+        assertThat(pushParser.parse(UNIFIED_PUSH_DATA.mutate(AN_EVENT_ID.value, "anEventId"))).isNull()
-        assertThat(pushParser.parse(UNIFIED_PUSH_DATA.mutate(AN_EVENT_ID.value, "anEventId"))).isEqualTo(expected)
     }
 
     companion object {