fix: updated json adapter usage to safe parsing

customerio · Oct 10, 2022 · f72280b · f72280b
1 parent 406319e
commit f72280b
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 32 deletions.
diff --git a/sdk/src/main/java/io/customer/sdk/queue/QueueRunner.kt b/sdk/src/main/java/io/customer/sdk/queue/QueueRunner.kt
@@ -13,6 +13,7 @@ import io.customer.sdk.queue.type.QueueTask
 import io.customer.sdk.queue.type.QueueTaskType
 import io.customer.sdk.util.JsonAdapter
 import io.customer.sdk.util.Logger
+import java.io.IOException
 
 interface QueueRunner {
     suspend fun runTask(task: QueueTask): QueueRunTaskResult
@@ -24,57 +25,63 @@ internal class QueueRunnerImpl(
     private val logger: Logger
 ) : QueueRunner {
     override suspend fun runTask(task: QueueTask): QueueRunTaskResult {
-        return when (valueOfOrNull<QueueTaskType>(task.type)) {
+        val taskResult = when (valueOfOrNull<QueueTaskType>(task.type)) {
             QueueTaskType.IdentifyProfile -> identifyProfile(task)
             QueueTaskType.TrackEvent -> trackEvent(task)
             QueueTaskType.RegisterDeviceToken -> registerDeviceToken(task)
             QueueTaskType.DeletePushToken -> deleteDeviceToken(task)
             QueueTaskType.TrackPushMetric -> trackPushMetrics(task)
             QueueTaskType.TrackDeliveryEvent -> trackDeliveryEvents(task)
-            null -> {
-                val errorMessage =
-                    "Queue task ${task.type} could not find an enum to map to. Could not run task."
-                logger.error(errorMessage)
-                return Result.failure(RuntimeException(errorMessage))
-            }
+            null -> null
+        }
+        return if (taskResult != null) taskResult
+        else {
+            val errorMessage =
+                "Queue task ${task.type} could not find an enum to map to. Could not run task."
+            logger.error(errorMessage)
+            Result.failure(RuntimeException(errorMessage))
         }
     }
 
-    private suspend fun identifyProfile(task: QueueTask): QueueRunTaskResult {
-        val taskData: IdentifyProfileQueueTaskData = jsonAdapter.fromJson(task.data)
+    private suspend fun identifyProfile(task: QueueTask): QueueRunTaskResult? {
+        val taskData: IdentifyProfileQueueTaskData =
+            jsonAdapter.fromJsonOrNull(task.data) ?: return null
 
         return cioHttpClient.identifyProfile(taskData.identifier, taskData.attributes)
     }
 
-    private suspend fun trackEvent(task: QueueTask): QueueRunTaskResult {
-        val taskData: TrackEventQueueTaskData = jsonAdapter.fromJson(task.data)
+    private suspend fun trackEvent(task: QueueTask): QueueRunTaskResult? {
+        val taskData: TrackEventQueueTaskData = jsonAdapter.fromJsonOrNull(task.data) ?: return null
 
         return cioHttpClient.track(taskData.identifier, taskData.event)
     }
 
-    private suspend fun deleteDeviceToken(task: QueueTask): QueueRunTaskResult {
-        val taskData: DeletePushNotificationQueueTaskData = jsonAdapter.fromJson(task.data)
+    private suspend fun deleteDeviceToken(task: QueueTask): QueueRunTaskResult? {
+        val taskData: DeletePushNotificationQueueTaskData =
+            jsonAdapter.fromJsonOrNull(task.data) ?: return null
 
         return cioHttpClient.deleteDevice(taskData.profileIdentified, taskData.deviceToken)
     }
 
-    private suspend fun registerDeviceToken(task: QueueTask): QueueRunTaskResult {
-        val taskData: RegisterPushNotificationQueueTaskData = jsonAdapter.fromJson(task.data)
+    private suspend fun registerDeviceToken(task: QueueTask): QueueRunTaskResult? {
+        val taskData: RegisterPushNotificationQueueTaskData =
+            jsonAdapter.fromJsonOrNull(task.data) ?: return null
 
         return cioHttpClient.registerDevice(
             taskData.profileIdentified,
             taskData.device
         )
     }
 
-    private suspend fun trackPushMetrics(task: QueueTask): QueueRunTaskResult {
-        val taskData: Metric = jsonAdapter.fromJson(task.data)
+    private suspend fun trackPushMetrics(task: QueueTask): QueueRunTaskResult? {
+        val taskData: Metric = jsonAdapter.fromJsonOrNull(task.data) ?: return null
 
         return cioHttpClient.trackPushMetrics(taskData)
     }
 
-    private suspend fun trackDeliveryEvents(task: QueueTask): QueueRunTaskResult {
-        val taskData: DeliveryEvent = jsonAdapter.fromJson(task.data)
+    @Throws(IOException::class, RuntimeException::class)
+    private suspend fun trackDeliveryEvents(task: QueueTask): QueueRunTaskResult? {
+        val taskData: DeliveryEvent = jsonAdapter.fromJsonOrNull(task.data) ?: return null
 
         return cioHttpClient.trackDeliveryEvents(taskData)
     }

diff --git a/sdk/src/main/java/io/customer/sdk/queue/QueueStorage.kt b/sdk/src/main/java/io/customer/sdk/queue/QueueStorage.kt
@@ -101,7 +101,7 @@ internal class QueueStorageImpl internal constructor(
     @Synchronized
     override fun get(taskStorageId: String): QueueTask? {
         val fileContents = fileStorage.get(FileType.QueueTask(taskStorageId)) ?: return null
-        return jsonAdapter.fromJson(fileContents)
+        return jsonAdapter.fromJsonOrNull(fileContents)
     }
 
     @Synchronized

diff --git a/sdk/src/main/java/io/customer/sdk/util/JsonAdapter.kt b/sdk/src/main/java/io/customer/sdk/util/JsonAdapter.kt
@@ -37,9 +37,16 @@ class JsonAdapter internal constructor(val moshi: Moshi) {
      * ```
      *
      * Moshi handles arrays differently: https://github.com/square/moshi#parse-json-arrays
+     *
+     * Parses data string to single objects. Using this method directly is
+     * discouraged as parsing json incorrectly can lead to crashes on client
+     * apps. Prefer safe parsing instead by using [fromJsonOrNull].
+     *
+     * @see [fromJsonOrNull] for safe parsing
      */
-    inline fun <reified T : Any> fromJson(json: String): T {
-        val json = json.trim()
+    @Throws(Exception::class)
+    inline fun <reified T : Any> fromJson(data: String): T {
+        val json = data.trim()
 
         if (json.isNotEmpty() && json[0] == '[') throw IllegalArgumentException("String is a list. Use `fromJsonList` instead.")
 
@@ -49,18 +56,25 @@ class JsonAdapter internal constructor(val moshi: Moshi) {
     }
 
     /**
-     * Use if you anticipate the json parsing to not work. You don't care about the exception. Else, use [fromJson].
+     * Parses data string to a single objects or null if there is any exception
+     * e.g. malformed json, missing adapter, etc.
      */
-    inline fun <reified T : Any> fromJsonOrNull(json: String): T? {
-        return try {
-            fromJson(json)
-        } catch (e: Exception) {
-            null
-        }
+    inline fun <reified T : Any> fromJsonOrNull(json: String): T? = try {
+        fromJson(json)
+    } catch (ex: Exception) {
+        null
     }
 
-    inline fun <reified T : Any> fromJsonList(json: String): List<T> {
-        val json = json.trim()
+    /**
+     * Parses data string to list of objects. Using this method directly is
+     * discouraged as parsing json incorrectly can lead to crashes on client
+     * apps. Prefer safe parsing instead by using [fromJsonListOrNull].
+     *
+     * @see [fromJsonListOrNull] for safe parsing
+     */
+    @Throws(Exception::class)
+    inline fun <reified T : Any> fromJsonList(data: String): List<T> {
+        val json = data.trim()
 
         if (json.isNotEmpty() && json[0] != '[') throw IllegalArgumentException("String is not a list. Use `fromJson` instead.")
 
@@ -71,7 +85,8 @@ class JsonAdapter internal constructor(val moshi: Moshi) {
     }
 
     /**
-     * Use if you anticipate the json parsing to not work. You don't care about the exception. Else, use [fromJsonList].
+     * Parses data string to list of objects or null if there is any exception
+     * e.g. malformed json, missing adapter, etc.
      */
     inline fun <reified T : Any> fromJsonListOrNull(json: String): List<T>? = try {
         fromJsonList(json)