From ff74818a4c097f12662f7b3c7120f5f99cde3672 Mon Sep 17 00:00:00 2001 From: Daniel Kift Date: Thu, 18 Jun 2026 14:57:53 +0100 Subject: [PATCH] Use typed Android JSON-RPC params --- .../shopify/checkoutkit/CheckoutProtocol.kt | 92 ++++++++++++++----- .../checkoutkit/EmbeddedCheckoutProtocol.kt | 42 +++------ .../checkoutkit/CheckoutProtocolTest.kt | 78 +++++++++++++++- .../EmbeddedCheckoutProtocolTest.kt | 83 +++++++++++++++-- 4 files changed, 237 insertions(+), 58 deletions(-) diff --git a/platforms/android/lib/src/main/java/com/shopify/checkoutkit/CheckoutProtocol.kt b/platforms/android/lib/src/main/java/com/shopify/checkoutkit/CheckoutProtocol.kt index fe8c0a72..95c22bfa 100644 --- a/platforms/android/lib/src/main/java/com/shopify/checkoutkit/CheckoutProtocol.kt +++ b/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 = NotificationDescriptor( method = "ec.error", decode = { params -> - (params as? JsonObject)?.get("error")?.let { - try { - json.decodeFromJsonElement(it) - } catch (e: SerializationException) { - log.d(BaseWebView.ECP_LOG_TAG, "Failed to decode ec.error params: $e raw=$it") - null - } + try { + json.decodeFromJsonElement(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 = 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(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(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(it) - } catch (e: SerializationException) { - log.d(BaseWebView.ECP_LOG_TAG, "Failed to decode $method checkout payload: $e raw=$it") - null - } + try { + json.decodeFromJsonElement(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(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(message) + return json.decodeFromJsonElement(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 = 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) diff --git a/platforms/android/lib/src/main/java/com/shopify/checkoutkit/EmbeddedCheckoutProtocol.kt b/platforms/android/lib/src/main/java/com/shopify/checkoutkit/EmbeddedCheckoutProtocol.kt index 2e23785f..5920335f 100644 --- a/platforms/android/lib/src/main/java/com/shopify/checkoutkit/EmbeddedCheckoutProtocol.kt +++ b/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 = 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 { - 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 = 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 } -} diff --git a/platforms/android/lib/src/test/java/com/shopify/checkoutkit/CheckoutProtocolTest.kt b/platforms/android/lib/src/test/java/com/shopify/checkoutkit/CheckoutProtocolTest.kt index e7eba55f..e838aa1f 100644 --- a/platforms/android/lib/src/test/java/com/shopify/checkoutkit/CheckoutProtocolTest.kt +++ b/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() 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", diff --git a/platforms/android/lib/src/test/java/com/shopify/checkoutkit/EmbeddedCheckoutProtocolTest.kt b/platforms/android/lib/src/test/java/com/shopify/checkoutkit/EmbeddedCheckoutProtocolTest.kt index 5736f252..ed840281 100644 --- a/platforms/android/lib/src/test/java/com/shopify/checkoutkit/EmbeddedCheckoutProtocolTest.kt +++ b/platforms/android/lib/src/test/java/com/shopify/checkoutkit/EmbeddedCheckoutProtocolTest.kt @@ -76,6 +76,24 @@ class EmbeddedCheckoutProtocolTest { assertThat(js).contains("\"id\":7") } + @Test + fun `ec ready ACK echoes null request id`() { + val js = captureEvaluatedJs { + ecp.postMessage("""{"jsonrpc":"2.0","method":"ec.ready","id":null,"params":{"delegate":[]}}""") + } + assertThat(js).contains("\"id\":null") + } + + @Test + fun `ec ready without request id sends no response`() { + assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"ec.ready","params":{"delegate":[]}}""") + } + + @Test + fun `ec ready with fractional request id sends no response`() { + assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"ec.ready","id":1.5,"params":{"delegate":[]}}""") + } + @Test fun `ec ready response dispatches via window EmbeddedCheckoutProtocol`() { val js = captureEvaluatedJs { @@ -108,15 +126,15 @@ class EmbeddedCheckoutProtocolTest { } @Test - fun `ec ready ignores null and non-string delegate values`() { + fun `ec ready with non-string delegate values sends parse error`() { val js = captureEvaluatedJs { ecp.postMessage( """{"jsonrpc":"2.0","method":"ec.ready","id":"r2","params":{"delegate":["window.open",null,{}]}}""" ) } - assertThat(js).contains("\"delegate\":[\"window.open\"]") - assertThat(js).contains("\"status\":\"success\"") - assertThat(js).doesNotContain("\"error\"") + assertThat(js).contains("\"error\"") + assertThat(js).contains("-32700") + assertThat(js).contains(""""id":"r2"""") } @Test @@ -297,11 +315,22 @@ class EmbeddedCheckoutProtocolTest { assertThat(js).contains("-32602") } + @Test + fun `window open emits invalid params when params url is null`() { + val js = captureEvaluatedJs { + ecp.postMessage( + """{"jsonrpc":"2.0","method":"ec.window.open_request","id":"10","params":{"url":null}}""" + ) + } + assertThat(js).contains("\"error\"") + assertThat(js).contains("-32602") + } + @Test fun `window open emits invalid params when params url is not a string`() { val js = captureEvaluatedJs { ecp.postMessage( - """{"jsonrpc":"2.0","method":"ec.window.open_request","id":"10","params":{"url":{}}}""" + """{"jsonrpc":"2.0","method":"ec.window.open_request","id":"11","params":{"url":{}}}""" ) } assertThat(js).contains("\"error\"") @@ -311,12 +340,38 @@ class EmbeddedCheckoutProtocolTest { @Test fun `window open emits invalid params when params is not an object`() { val js = captureEvaluatedJs { - ecp.postMessage("""{"jsonrpc":"2.0","method":"ec.window.open_request","id":"11","params":[]}""") + ecp.postMessage("""{"jsonrpc":"2.0","method":"ec.window.open_request","id":"12","params":[]}""") } assertThat(js).contains("\"error\"") assertThat(js).contains("-32602") } + @Test + fun `window open response echoes null request id`() { + val merchantClient = CheckoutProtocol.Client() + .on(CheckoutProtocol.windowOpen) { WindowOpenResult.Success } + ecp.setClient(merchantClient) + + val js = captureEvaluatedJs { + ecp.postMessage(windowOpenRequest(id = "null", url = "https://example.com")) + } + + assertThat(js).contains("\"id\":null") + assertThat(js).contains("\"status\":\"success\"") + } + + @Test + fun `window open without request id sends no response`() { + assertIgnoredByBridge( + """{"jsonrpc":"2.0","method":"ec.window.open_request","params":{"url":"https://example.com"}}""" + ) + } + + @Test + fun `window open with fractional request id sends no response`() { + assertIgnoredByBridge(windowOpenRequest(id = "1.5", url = "https://example.com")) + } + // endregion // region ec.start @@ -562,10 +617,22 @@ class EmbeddedCheckoutProtocolTest { assertThat(js).contains(""""id":"11"""") } + @Test + fun `unknown request with null id returns method not found`() { + val js = captureEvaluatedJs { + ecp.postMessage("""{"jsonrpc":"2.0","method":"unknownMethod","id":null}""") + } + + assertThat(js).contains("\"error\"") + assertThat(js).contains("-32601") + assertThat(js).contains("Method not found") + assertThat(js).contains("\"id\":null") + } + @Test fun `unknown request with invalid id sends no response`() { assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"unknownMethod","id":{},"params":{}}""") - assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"unknownMethod","id":null,"params":{}}""") + assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"unknownMethod","id":1.5,"params":{}}""") assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"unknownMethod","id":true,"params":{}}""") } @@ -598,6 +665,7 @@ class EmbeddedCheckoutProtocolTest { } assertThat(js).contains("\"error\"") assertThat(js).contains("-32700") + assertThat(js).contains(""""id":"13"""") } @Test @@ -607,6 +675,7 @@ class EmbeddedCheckoutProtocolTest { } assertThat(js).contains("\"error\"") assertThat(js).contains("-32700") + assertThat(js).contains(""""id":"14"""") } // endregion