Use typed Android JSON-RPC params

platforms/android/lib/src/main/java/com/shopify/checkoutkit/CheckoutProtocol.kt

@@ -12,7 +12,6 @@ import kotlinx.serialization.json.JsonNull
 import kotlinx.serialization.json.JsonObject
 import kotlinx.serialization.json.JsonPrimitive
 import kotlinx.serialization.json.buildJsonObject
-import kotlinx.serialization.json.contentOrNull
 import kotlinx.serialization.json.decodeFromJsonElement
 import kotlinx.serialization.json.encodeToJsonElement
 import kotlinx.serialization.json.jsonObject
@@ -49,13 +48,11 @@ public object CheckoutProtocol {
     public val error: NotificationDescriptor<ErrorResponse> = NotificationDescriptor(
         method = "ec.error",
         decode = { params ->
-            (params as? JsonObject)?.get("error")?.let {
-                try {
-                    json.decodeFromJsonElement<ErrorResponse>(it)
-                } catch (e: SerializationException) {
-                    log.d(BaseWebView.ECP_LOG_TAG, "Failed to decode ec.error params: $e  raw=$it")
-                    null
-                }
+            try {
+                json.decodeFromJsonElement<ErrorParams>(params ?: JsonNull).error
+            } catch (e: SerializationException) {
+                log.d(BaseWebView.ECP_LOG_TAG, "Failed to decode ec.error params: $e  raw=$params")
+                null
             }
         }
     )
@@ -66,10 +63,15 @@ public object CheckoutProtocol {
     public val windowOpen: DelegationDescriptor<WindowOpenRequest, WindowOpenResult> = DelegationDescriptor(
         method = "ec.window.open_request",
         decode = { params ->
-            ((params as? JsonObject)?.get("url") as? JsonPrimitive)?.contentOrNull
-                ?.takeIf { it.isNotBlank() }
-                ?.let { runCatching { it.toUri() }.getOrNull() }
-                ?.let(::WindowOpenRequest)
+            try {
+                json.decodeFromJsonElement<WindowOpenParams>(params ?: JsonNull).url
+                    .takeIf { it.isNotBlank() }
+                    ?.let { runCatching { it.toUri() }.getOrNull() }
+                    ?.let(::WindowOpenRequest)
+            } catch (e: SerializationException) {
+                log.d(BaseWebView.ECP_LOG_TAG, "Failed to decode ${windowOpen.method} params: $e  raw=$params")
+                null
+            }
         },
         encode = { result -> encodeWindowOpenResult(result) },
     )
@@ -90,11 +92,13 @@ public object CheckoutProtocol {
 
     internal fun supportedProtocolMethod(request: EcpRequest): String? =
         request.method.takeIf {
-            request.jsonrpc == "2.0" && request.method in supportedProtocolMethods
+            request.jsonrpc == "2.0" &&
+                request.method in supportedProtocolMethods &&
+                request.hasValidJsonRpcRequestId()
         }
 
     private fun decodeProtocolRequest(message: String): EcpRequest? = try {
-        json.decodeFromString<EcpRequest>(message)
+        decodeEcpRequest(message)
     } catch (_: SerializationException) {
         null
     }
@@ -103,13 +107,11 @@ public object CheckoutProtocol {
         NotificationDescriptor(
             method = method,
             decode = { params ->
-                (params as? JsonObject)?.get("checkout")?.let {
-                    try {
-                        json.decodeFromJsonElement<Checkout>(it)
-                    } catch (e: SerializationException) {
-                        log.d(BaseWebView.ECP_LOG_TAG, "Failed to decode $method checkout payload: $e  raw=$it")
-                        null
-                    }
+                try {
+                    json.decodeFromJsonElement<CheckoutParams>(params ?: JsonNull).checkout
+                } catch (e: SerializationException) {
+                    log.d(BaseWebView.ECP_LOG_TAG, "Failed to decode $method checkout params: $e  raw=$params")
+                    null
                 }
             }
         )
@@ -185,14 +187,18 @@ public object CheckoutProtocol {
         /** Called by [EmbeddedCheckoutProtocol] for every delegated EC message. */
         override fun process(message: String): String? =
             decodeRequest(message)?.let { request ->
-                delegations[request.method]?.dispatch(request) ?: run {
+                val delegation = delegations[request.method]
+                if (delegation != null) {
+                    jsonRpcRequestId(request.id)?.let { delegation.dispatch(request) }
+                } else {
                     dispatchNotification(request)
                     null
                 }
             }
 
         private fun decodeRequest(message: String): EcpRequest? = try {
-            json.decodeFromString<EcpRequest>(message)
+            decodeEcpRequest(message)
+                .takeIf { it.hasValidJsonRpcRequestId() }
         } catch (e: SerializationException) {
             log.d(LOG_TAG, "Error processing ECP message in typed client: $e")
             null
@@ -257,6 +263,11 @@ public object CheckoutProtocol {
     private const val LOG_TAG = BaseWebView.ECP_LOG_TAG
     private const val CODE_INVALID_PARAMS = -32602
 
+    private fun decodeEcpRequest(message: String): EcpRequest {
+        val requestObject = json.decodeFromString<JsonObject>(message)
+        return json.decodeFromJsonElement<EcpRequest>(requestObject).copy(id = requestObject["id"])
+    }
+
     private fun jsonRpcResult(id: JsonElement?, result: JsonElement): String =
         json.encodeToString(
             JsonObject.serializer(),
@@ -293,6 +304,21 @@ public object CheckoutProtocol {
     }
 }
 
+internal fun EcpRequest.hasValidJsonRpcRequestId(): Boolean =
+    id == null || jsonRpcRequestId(id) != null
+
+internal fun jsonRpcRequestId(id: JsonElement?): JsonElement? =
+    when (id) {
+        JsonNull -> JsonNull
+        is JsonPrimitive -> id.takeIf {
+            it.isString ||
+                (!it.isString && JSON_RPC_INTEGER.matches(it.content) && it.content.toLongOrNull() != null)
+        }
+        else -> null
+    }
+
+private val JSON_RPC_INTEGER = Regex("-?(0|[1-9]\\d*)")
+
 /**
  * Describes a typed EC notification handler binding.
  *
@@ -325,6 +351,26 @@ internal data class EcpRequest(
     val params: JsonElement? = null,
 )
 
+@Serializable
+internal data class ReadyParams(
+    val delegate: List<String> = emptyList(),
+)
+
+@Serializable
+internal data class CheckoutParams(
+    val checkout: Checkout,
+)
+
+@Serializable
+internal data class ErrorParams(
+    val error: ErrorResponse,
+)
+
+@Serializable
+internal data class WindowOpenParams(
+    val url: String,
+)
+
 /** Payload delivered with the [CheckoutProtocol.windowOpen] delegation. */
 @ConsistentCopyVisibility
 public data class WindowOpenRequest internal constructor(public val url: Uri)

platforms/android/lib/src/main/java/com/shopify/checkoutkit/EmbeddedCheckoutProtocol.kt

@@ -4,13 +4,10 @@ import android.webkit.JavascriptInterface
 import com.shopify.checkoutkit.ShopifyCheckoutKit.log
 import kotlinx.serialization.SerializationException
 import kotlinx.serialization.json.Json
-import kotlinx.serialization.json.JsonArray
 import kotlinx.serialization.json.JsonElement
 import kotlinx.serialization.json.JsonObject
-import kotlinx.serialization.json.JsonPrimitive
 import kotlinx.serialization.json.add
 import kotlinx.serialization.json.buildJsonObject
-import kotlinx.serialization.json.contentOrNull
 import kotlinx.serialization.json.decodeFromJsonElement
 import kotlinx.serialization.json.put
 import kotlinx.serialization.json.putJsonArray
@@ -62,8 +59,8 @@ internal class EmbeddedCheckoutProtocol(
             val requestId = jsonRpcRequestId(requestObject["id"])
             log.d(LOG_TAG, "Received bridge message: method=${request.method} id=${request.id}")
             when (method) {
-                CheckoutProtocol.READY_METHOD -> handleReady(request)
-                CheckoutProtocol.windowOpen.method -> handleWindowOpenRequest(message)
+                CheckoutProtocol.READY_METHOD -> requestId?.let { handleReady(request, it) }
+                CheckoutProtocol.windowOpen.method -> requestId?.let { handleWindowOpenRequest(message) }
                 CheckoutProtocol.start.method -> handleStart(message)
                 CheckoutProtocol.complete.method -> handleComplete(message)
                 null -> {
@@ -80,8 +77,11 @@ internal class EmbeddedCheckoutProtocol(
         }
     }
 
-    private fun handleReady(request: EcpRequest) {
-        val checkoutAcceptedDelegations = checkoutAcceptedDelegations(request.params)
+    private fun handleReady(request: EcpRequest, requestId: JsonElement) {
+        val checkoutAcceptedDelegations = readyParams(request.params)?.delegate ?: run {
+            sendError(requestId, CODE_PARSE_ERROR, "Parse error")
+            return
+        }
         val negotiatedDelegations = checkoutAcceptedDelegations.filter { it in KIT_SUPPORTED_DELEGATIONS }
         log.d(
             LOG_TAG,
@@ -91,23 +91,16 @@ internal class EmbeddedCheckoutProtocol(
                 "checkoutKitSupportedDelegations=$KIT_SUPPORTED_DELEGATIONS " +
                 "negotiatedDelegations=$negotiatedDelegations"
         )
-        sendResult(request.id, ucpReadyResult(negotiatedDelegations))
-    }
-
-    private fun checkoutAcceptedDelegations(params: JsonElement?): List<String> = when (params) {
-        null -> emptyList()
-        !is JsonObject -> throw SerializationException("${CheckoutProtocol.READY_METHOD} params must be an object")
-        else -> params["delegate"]?.let(::delegationStrings) ?: emptyList()
-    }
-
-    private fun delegationStrings(delegate: JsonElement): List<String> {
-        val delegateArray = delegate as? JsonArray
-            ?: throw SerializationException("${CheckoutProtocol.READY_METHOD} delegate must be an array")
-        return delegateArray.mapNotNull(::delegationStringOrNull)
+        sendResult(requestId, ucpReadyResult(negotiatedDelegations))
     }
 
-    private fun delegationStringOrNull(delegate: JsonElement): String? =
-        (delegate as? JsonPrimitive)?.contentOrNull
+    private fun readyParams(params: JsonElement?): ReadyParams? =
+        try {
+            decoder.decodeFromJsonElement(params ?: JsonObject(emptyMap()))
+        } catch (e: SerializationException) {
+            log.d(LOG_TAG, "Failed to decode ${CheckoutProtocol.READY_METHOD} params: $e  raw=$params")
+            null
+        }
 
     private fun ucpReadyResult(negotiatedDelegations: List<String>): String =
         decoder.encodeToString(
@@ -247,8 +240,3 @@ internal class EmbeddedCheckoutProtocol(
         private const val CODE_METHOD_NOT_FOUND = -32601
     }
 }
-
-private fun jsonRpcRequestId(id: JsonElement?): JsonElement? {
-    val primitive = id as? JsonPrimitive ?: return null
-    return primitive.takeIf { it.isString || it.contentOrNull?.toDoubleOrNull() != null }
-}

platforms/android/lib/src/test/java/com/shopify/checkoutkit/CheckoutProtocolTest.kt

@@ -79,6 +79,25 @@ class CheckoutProtocolTest {
         assertThat(CheckoutProtocol.supportedProtocolMethod("not json")).isNull()
     }
 
+    @Test
+    fun `supported protocol method rejects invalid request ids`() {
+        assertThat(
+            CheckoutProtocol.supportedProtocolMethod(
+                """{"jsonrpc":"2.0","method":"ec.start","id":true,"params":{"checkout":{}}}"""
+            )
+        ).isNull()
+        assertThat(
+            CheckoutProtocol.supportedProtocolMethod(
+                """{"jsonrpc":"2.0","method":"ec.start","id":{},"params":{"checkout":{}}}"""
+            )
+        ).isNull()
+        assertThat(
+            CheckoutProtocol.supportedProtocolMethod(
+                """{"jsonrpc":"2.0","method":"ec.start","id":1.5,"params":{"checkout":{}}}"""
+            )
+        ).isNull()
+    }
+
     // endregion
 
     // region process — notification dispatch
@@ -144,6 +163,60 @@ class CheckoutProtocolTest {
         assertThat(client.process("not valid json {{{")).isNull()
     }
 
+    @Test
+    fun `process preserves null id for registered delegations`() {
+        val client = CheckoutProtocol.Client()
+            .on(CheckoutProtocol.windowOpen) { WindowOpenResult.Success }
+
+        val response = client.process(windowOpenMessage(id = "null"))
+
+        assertThat(response).contains("\"id\":null")
+        assertThat(response).contains("\"status\":\"success\"")
+    }
+
+    @Test
+    fun `process preserves integer id for registered delegations`() {
+        val client = CheckoutProtocol.Client()
+            .on(CheckoutProtocol.windowOpen) { WindowOpenResult.Success }
+
+        val response = client.process(windowOpenMessage(id = "7"))
+
+        assertThat(response).contains("\"id\":7")
+        assertThat(response).contains("\"status\":\"success\"")
+    }
+
+    @Test
+    fun `process ignores registered delegations with fractional id`() {
+        var handled = false
+        val client = CheckoutProtocol.Client()
+            .on(CheckoutProtocol.windowOpen) {
+                handled = true
+                WindowOpenResult.Success
+            }
+
+        val response = client.process(windowOpenMessage(id = "1.5"))
+
+        assertThat(response).isNull()
+        assertThat(handled).isFalse()
+    }
+
+    @Test
+    fun `process ignores registered delegations without id`() {
+        var handled = false
+        val client = CheckoutProtocol.Client()
+            .on(CheckoutProtocol.windowOpen) {
+                handled = true
+                WindowOpenResult.Success
+            }
+
+        val response = client.process(
+            """{"jsonrpc":"2.0","method":"ec.window.open_request","params":{"url":"https://example.com"}}"""
+        )
+
+        assertThat(response).isNull()
+        assertThat(handled).isFalse()
+    }
+
     // endregion
 
     // region process — message without checkout in params
@@ -300,7 +373,7 @@ class CheckoutProtocolTest {
     }
 
     @Test
-    fun `process does not dispatch when params has no checkout field`() {
+    fun `process does not dispatch when checkout params are invalid`() {
         val received = mutableListOf<Checkout>()
         val client = CheckoutProtocol.Client()
             .on(CheckoutProtocol.start) { checkout -> received.add(checkout) }
@@ -351,6 +424,9 @@ class CheckoutProtocolTest {
     private fun ecCompleteMessage(): String =
         """{"jsonrpc":"2.0","method":"ec.complete","params":{"checkout":${checkoutJson(status = "completed")}}}"""
 
+    private fun windowOpenMessage(id: String, url: String = "https://example.com"): String =
+        """{"jsonrpc":"2.0","method":"ec.window.open_request","id":$id,"params":{"url":"$url"}}"""
+
     private fun checkoutJson(
         id: String = "chk1",
         currency: String = "USD",