From 98e3b8bf525e8b3a603cee8c452013a6f4bc0a3d Mon Sep 17 00:00:00 2001
From: Arturo Mejia <arturomejiamarmol@gmail.com>
Date: Tue, 24 May 2022 17:45:23 -0400
Subject: [PATCH] For #25374: Skip action validation for control messages

---
 .../gleanplumb/NimbusMessagingStorage.kt      | 30 +++++++++++--------
 .../gleanplumb/NimbusMessagingStorageTest.kt  | 22 ++++++++++----
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/app/src/main/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorage.kt b/app/src/main/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorage.kt
index d2458d882..18614178f 100644
--- a/app/src/main/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorage.kt
+++ b/app/src/main/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorage.kt
@@ -64,7 +64,7 @@ class NimbusMessagingStorage(
         val storageMetadata = metadataStorage.getMetadata()
 
         return nimbusMessages.mapNotNull { (key, value) ->
-            val action = sanitizeAction(key, value.action, nimbusActions) ?: return@mapNotNull null
+            val action = sanitizeAction(key, value.action, nimbusActions, value.isControl) ?: return@mapNotNull null
             Message(
                 id = key,
                 data = value,
@@ -137,20 +137,26 @@ class NimbusMessagingStorage(
     internal fun sanitizeAction(
         messageId: String,
         unsafeAction: String,
-        nimbusActions: Map<String, String>
+        nimbusActions: Map<String, String>,
+        isControl: Boolean
     ): String? {
-        return if (unsafeAction.startsWith("http")) {
-            unsafeAction
-        } else {
-            val safeAction = nimbusActions[unsafeAction]
-            if (safeAction.isNullOrBlank() || safeAction.isEmpty()) {
-                if (!malFormedMap.containsKey(unsafeAction)) {
-                    reportMalformedMessage(messageId)
+
+        return when {
+            unsafeAction.startsWith("http") -> {
+                unsafeAction
+            }
+            isControl -> "CONTROL_ACTION"
+            else -> {
+                val safeAction = nimbusActions[unsafeAction]
+                if (safeAction.isNullOrBlank() || safeAction.isEmpty()) {
+                    if (!malFormedMap.containsKey(unsafeAction)) {
+                        reportMalformedMessage(messageId)
+                    }
+                    malFormedMap[unsafeAction] = messageId
+                    return null
                 }
-                malFormedMap[unsafeAction] = messageId
-                return null
+                safeAction
             }
-            safeAction
         }
     }
 
diff --git a/app/src/test/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorageTest.kt b/app/src/test/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorageTest.kt
index b5b0b3e3d..c9da3042d 100644
--- a/app/src/test/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorageTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorageTest.kt
@@ -237,9 +237,9 @@ class NimbusMessagingStorageTest {
     fun `GIVEN a malformed action WHEN calling sanitizeAction THEN return null`() {
         val actionsMap = mapOf("action-1" to "action-1-url")
 
-        val notFoundAction = storage.sanitizeAction("messageId", "no-found-action", actionsMap)
-        val emptyAction = storage.sanitizeAction("messageId", "", actionsMap)
-        val blankAction = storage.sanitizeAction("messageId", " ", actionsMap)
+        val notFoundAction = storage.sanitizeAction("messageId", "no-found-action", actionsMap, false)
+        val emptyAction = storage.sanitizeAction("messageId", "", actionsMap, false)
+        val blankAction = storage.sanitizeAction("messageId", " ", actionsMap, false)
 
         assertNull(notFoundAction)
         assertNull(emptyAction)
@@ -253,7 +253,7 @@ class NimbusMessagingStorageTest {
 
         storage.malFormedMap["malformed-action"] = "messageId"
 
-        val action = storage.sanitizeAction("messageId", "malformed-action", actionsMap)
+        val action = storage.sanitizeAction("messageId", "malformed-action", actionsMap, false)
 
         assertNull(action)
         assertFalse(malformedWasReported)
@@ -263,7 +263,7 @@ class NimbusMessagingStorageTest {
     fun `GIVEN a non-previously stored malformed action WHEN calling sanitizeAction THEN return null and report malFormed`() {
         val actionsMap = mapOf("action-1" to "action-1-url")
 
-        val action = storage.sanitizeAction("messageId", "malformed-action", actionsMap)
+        val action = storage.sanitizeAction("messageId", "malformed-action", actionsMap, false)
 
         assertNull(action)
         assertTrue(storage.malFormedMap.containsKey("malformed-action"))
@@ -282,11 +282,21 @@ class NimbusMessagingStorageTest {
     fun `GIVEN a valid action WHEN calling sanitizeAction THEN return the action`() {
         val actionsMap = mapOf("action-1" to "action-1-url")
 
-        val validAction = storage.sanitizeAction("messageId", "action-1", actionsMap)
+        val validAction = storage.sanitizeAction("messageId", "action-1", actionsMap, false)
 
         assertEquals("action-1-url", validAction)
     }
 
+    @Test
+    fun `GIVEN a valid action for control message WHEN calling sanitizeAction THEN return a empty action`() {
+        val actionsMap = mapOf("action-1" to "action-1-url")
+
+        val validAction = storage.sanitizeAction("messageId", "", actionsMap, true)
+
+        assertEquals("CONTROL_ACTION", validAction)
+        assertFalse(malformedWasReported)
+    }
+
     @Test
     fun `GIVEN a trigger action WHEN calling sanitizeTriggers THEN return null`() {
         val triggersMap = mapOf("trigger-1" to "trigger-1-expression")