Skip to content

Commit e5be60a

Browse files
kiftiokiftio
authored
catch more specific errors (#189)
### What changes are you making? Replaces broad `catch (e: Exception)` blocks with specific exception types (`SerializationException`, `NoSuchElementException`, `SecurityException`) across the Android platform to avoid silently swallowing unexpected exceptions. - `CheckoutBridge`: Catches `SerializationException` and `NoSuchElementException` separately, extracting the error notification logic into a shared `notifyDecodeFailure` helper. - `CheckoutProtocol`: Narrows exception handling in `error`, `windowOpen`, and checkout notification decoders, and refactors `Client.process` to extract `decodeRequest` with a scoped `SerializationException` catch. Replaces `runCatching` with an explicit `try/catch` in `Typed.dispatch`. - `EmbeddedCheckoutProtocol`: Replaces unsafe `jsonObject`, `jsonArray`, and `jsonPrimitive` extension property accesses (which throw `IllegalArgumentException`) with safe `as?` casts that throw `SerializationException` on invalid structure, ensuring malformed `ec.ready` params are handled as parse errors. - `ExternalUriLauncher`: Narrows the fallback catch to `SecurityException` only. - `CheckoutErrorDecoder` and `CheckoutCompletedEventDecoder`: Replace `catch (e: Exception)` with `SerializationException` (and `NoSuchElementException` where applicable). A test is added to verify that an empty error payload body triggers `onCheckoutViewFailedWithError`, and two new tests confirm that non-object `ec.ready` params and a non-array `delegate` field both produce a parse error response. A new `ExternalUriLauncherTest` verifies that a `SecurityException` from `startActivity` results in a `Rejected` result. ### How to test 1. Run the Android unit test suite: `./gradlew :lib:test` 2. Verify the new tests pass: - `CheckoutBridgeTest` – `should call onCheckoutViewFailedWithError if error payload is empty` - `EmbeddedCheckoutProtocolTest` – `ec ready with non-object params sends parse error` and `ec ready with non-array delegate sends parse error` - `ExternalUriLauncherTest` – `launch rejects when startActivity throws security exception` 3. Confirm no regressions in existing checkout flow tests. --- ### Before you merge > [!IMPORTANT] > > - [ ] I've added tests to support my implementation > - [ ] I have read and agree with the [Contribution Guidelines](./CONTRIBUTING.md) > - [ ] I have read and agree with the [Code of Conduct](./CODE_OF_CONDUCT.md) > - [ ] I've updated the relevant platform README (`platforms/swift/README.md` and/or `platforms/android/README.md`) --- <details> <summary>Releasing a new Swift version?</summary> - [ ] I have bumped the version in `ShopifyCheckoutKit.podspec` - [ ] I have bumped the version in `platforms/swift/Sources/ShopifyCheckoutKit/ShopifyCheckoutKit.swift` - [ ] I have updated `platforms/swift/CHANGELOG.md` - [ ] I have updated the SwiftPM/CocoaPods version snippets in `platforms/swift/README.md` (major version only) </details> <details> <summary>Releasing a new Android version?</summary> - [ ] I have bumped the `versionName` in `platforms/android/lib/build.gradle` - [ ] I have updated `platforms/android/CHANGELOG.md` - [ ] I have updated the Gradle/Maven version snippets in `platforms/android/README.md` </details> > [!TIP] > See the [Contributing documentation](./CONTRIBUTING.md) for the full release process per platform.
1 parent fd1f598 commit e5be60a

5 files changed

Lines changed: 121 additions & 24 deletions

File tree

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

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,17 @@ import android.os.Looper
55
import androidx.core.net.toUri
66
import com.shopify.checkoutkit.ShopifyCheckoutKit.log
77
import kotlinx.serialization.Serializable
8+
import kotlinx.serialization.SerializationException
89
import kotlinx.serialization.json.Json
910
import kotlinx.serialization.json.JsonElement
1011
import kotlinx.serialization.json.JsonNull
1112
import kotlinx.serialization.json.JsonObject
13+
import kotlinx.serialization.json.JsonPrimitive
1214
import kotlinx.serialization.json.buildJsonObject
1315
import kotlinx.serialization.json.contentOrNull
1416
import kotlinx.serialization.json.decodeFromJsonElement
1517
import kotlinx.serialization.json.encodeToJsonElement
1618
import kotlinx.serialization.json.jsonObject
17-
import kotlinx.serialization.json.jsonPrimitive
1819
import kotlinx.serialization.json.put
1920
import kotlinx.serialization.json.putJsonObject
2021
import java.util.concurrent.CountDownLatch
@@ -48,10 +49,10 @@ public object CheckoutProtocol {
4849
public val error: NotificationDescriptor<ErrorResponse> = NotificationDescriptor(
4950
method = "ec.error",
5051
decode = { params ->
51-
params?.jsonObject?.get("error")?.let {
52+
(params as? JsonObject)?.get("error")?.let {
5253
try {
5354
json.decodeFromJsonElement<ErrorResponse>(it)
54-
} catch (e: Exception) {
55+
} catch (e: SerializationException) {
5556
log.d(BaseWebView.ECP_LOG_TAG, "Failed to decode ec.error params: $e raw=$it")
5657
null
5758
}
@@ -65,7 +66,7 @@ public object CheckoutProtocol {
6566
public val windowOpen: DelegationDescriptor<WindowOpenRequest, WindowOpenResult> = DelegationDescriptor(
6667
method = "ec.window.open_request",
6768
decode = { params ->
68-
params?.jsonObject?.get("url")?.jsonPrimitive?.contentOrNull
69+
((params as? JsonObject)?.get("url") as? JsonPrimitive)?.contentOrNull
6970
?.takeIf { it.isNotBlank() }
7071
?.let { runCatching { it.toUri() }.getOrNull() }
7172
?.let(::WindowOpenRequest)
@@ -77,10 +78,10 @@ public object CheckoutProtocol {
7778
NotificationDescriptor(
7879
method = method,
7980
decode = { params ->
80-
params?.jsonObject?.get("checkout")?.let {
81+
(params as? JsonObject)?.get("checkout")?.let {
8182
try {
8283
json.decodeFromJsonElement<Checkout>(it)
83-
} catch (e: Exception) {
84+
} catch (e: SerializationException) {
8485
log.d(BaseWebView.ECP_LOG_TAG, "Failed to decode $method checkout payload: $e raw=$it")
8586
null
8687
}
@@ -157,16 +158,19 @@ public object CheckoutProtocol {
157158
): Client = Client(handlers, delegations + (descriptor.method to Delegation.Typed(descriptor, handler)))
158159

159160
/** Called by [EmbeddedCheckoutProtocol] for every delegated EC message. */
160-
override fun process(message: String): String? {
161-
return try {
162-
val request = json.decodeFromString<EcpRequest>(message)
163-
delegations[request.method]?.let { return it.dispatch(request) }
164-
dispatchNotification(request)
165-
null
166-
} catch (e: Exception) {
167-
log.d(LOG_TAG, "Error processing ECP message in typed client: $e")
168-
null
161+
override fun process(message: String): String? =
162+
decodeRequest(message)?.let { request ->
163+
delegations[request.method]?.dispatch(request) ?: run {
164+
dispatchNotification(request)
165+
null
166+
}
169167
}
168+
169+
private fun decodeRequest(message: String): EcpRequest? = try {
170+
json.decodeFromString<EcpRequest>(message)
171+
} catch (e: SerializationException) {
172+
log.d(LOG_TAG, "Error processing ECP message in typed client: $e")
173+
null
170174
}
171175

172176
/**
@@ -206,7 +210,9 @@ public object CheckoutProtocol {
206210
private val handler: (P) -> R,
207211
) : Delegation() {
208212
override fun dispatch(request: EcpRequest): String {
209-
val payload = runCatching { descriptor.decode(request.params) }.getOrElse { e ->
213+
val payload = try {
214+
descriptor.decode(request.params)
215+
} catch (e: SerializationException) {
210216
log.d(LOG_TAG, "Decode failed for ${request.method}: $e")
211217
null
212218
} ?: return jsonRpcError(

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

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,15 @@ import android.content.Context
44
import android.webkit.JavascriptInterface
55
import com.shopify.checkoutkit.ShopifyCheckoutKit.log
66
import kotlinx.serialization.Serializable
7+
import kotlinx.serialization.SerializationException
78
import kotlinx.serialization.json.Json
9+
import kotlinx.serialization.json.JsonArray
810
import kotlinx.serialization.json.JsonElement
911
import kotlinx.serialization.json.JsonObject
12+
import kotlinx.serialization.json.JsonPrimitive
1013
import kotlinx.serialization.json.add
1114
import kotlinx.serialization.json.buildJsonObject
1215
import kotlinx.serialization.json.contentOrNull
13-
import kotlinx.serialization.json.jsonArray
14-
import kotlinx.serialization.json.jsonObject
15-
import kotlinx.serialization.json.jsonPrimitive
1616
import kotlinx.serialization.json.put
1717
import kotlinx.serialization.json.putJsonArray
1818
import kotlinx.serialization.json.putJsonObject
@@ -53,16 +53,14 @@ internal class EmbeddedCheckoutProtocol(
5353
request.method == METHOD_COMPLETE -> handleComplete(message)
5454
else -> handleClientMessage(request.method, message)
5555
}
56-
} catch (e: Exception) {
56+
} catch (e: SerializationException) {
5757
log.d(LOG_TAG, "Failed to decode ECP message: $e raw=$message")
5858
sendError(null, CODE_PARSE_ERROR, "Parse error")
5959
}
6060
}
6161

6262
private fun handleReady(request: EcpRequest) {
63-
val checkoutAcceptedDelegations = request.params?.jsonObject?.get("delegate")?.jsonArray
64-
?.mapNotNull { it.jsonPrimitive.contentOrNull }
65-
?: emptyList()
63+
val checkoutAcceptedDelegations = checkoutAcceptedDelegations(request.params)
6664
val negotiatedDelegations = checkoutAcceptedDelegations.filter { it in KIT_SUPPORTED_DELEGATIONS }
6765
log.d(
6866
LOG_TAG,
@@ -74,6 +72,20 @@ internal class EmbeddedCheckoutProtocol(
7472
sendResult(request.id, ucpReadyResult(negotiatedDelegations))
7573
}
7674

75+
private fun checkoutAcceptedDelegations(params: JsonElement?): List<String> = when (params) {
76+
null -> emptyList()
77+
!is JsonObject -> throw SerializationException("$METHOD_READY params must be an object")
78+
else -> params["delegate"]?.let(::delegationStrings) ?: emptyList()
79+
}
80+
81+
private fun delegationStrings(delegate: JsonElement): List<String> {
82+
val delegateArray = delegate as? JsonArray ?: throw SerializationException("$METHOD_READY delegate must be an array")
83+
return delegateArray.mapNotNull(::delegationStringOrNull)
84+
}
85+
86+
private fun delegationStringOrNull(delegate: JsonElement): String? =
87+
(delegate as? JsonPrimitive)?.contentOrNull
88+
7789
private fun ucpReadyResult(negotiatedDelegations: List<String>): String =
7890
decoder.encodeToString(
7991
JsonObject.serializer(),

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ internal object ExternalUriLauncher {
2626
Result.Launched
2727
} catch (e: ActivityNotFoundException) {
2828
Result.Rejected(reason = e.message ?: "No activity resolves $uri")
29-
} catch (e: Exception) {
29+
} catch (e: SecurityException) {
3030
Result.Rejected(reason = e.message)
3131
}
3232
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,18 @@ class EmbeddedCheckoutProtocolTest {
103103
assertThat(js).doesNotContain("payment.credential")
104104
}
105105

106+
@Test
107+
fun `ec ready ignores null and non-string delegate values`() {
108+
val js = captureEvaluatedJs {
109+
ecp.postMessage(
110+
"""{"jsonrpc":"2.0","method":"ec.ready","id":"r2","params":{"delegate":["window.open",null,{}]}}"""
111+
)
112+
}
113+
assertThat(js).contains("\"delegate\":[\"window.open\"]")
114+
assertThat(js).contains("\"status\":\"success\"")
115+
assertThat(js).doesNotContain("\"error\"")
116+
}
117+
106118
@Test
107119
fun `ec ready omits delegate field when no supported delegations requested`() {
108120
val js = captureEvaluatedJs {
@@ -449,6 +461,24 @@ class EmbeddedCheckoutProtocolTest {
449461
assertThat(js).contains("-32700")
450462
}
451463

464+
@Test
465+
fun `ec ready with non-object params sends parse error`() {
466+
val js = captureEvaluatedJs {
467+
ecp.postMessage("""{"jsonrpc":"2.0","method":"ec.ready","id":"13","params":[]}""")
468+
}
469+
assertThat(js).contains("\"error\"")
470+
assertThat(js).contains("-32700")
471+
}
472+
473+
@Test
474+
fun `ec ready with non-array delegate sends parse error`() {
475+
val js = captureEvaluatedJs {
476+
ecp.postMessage("""{"jsonrpc":"2.0","method":"ec.ready","id":"14","params":{"delegate":{}}}""")
477+
}
478+
assertThat(js).contains("\"error\"")
479+
assertThat(js).contains("-32700")
480+
}
481+
452482
// endregion
453483

454484
// region helpers

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* MIT License
3+
*
4+
* Copyright 2023-present, Shopify Inc.
5+
*
6+
* Permission is hereby granted, free of charge, to any person obtaining a copy
7+
* of this software and associated documentation files (the "Software"), to deal
8+
* in the Software without restriction, including without limitation the rights
9+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
10+
* copies of the Software, and to permit persons to whom the Software is
11+
* furnished to do so, subject to the following conditions:
12+
*
13+
* The above copyright notice and this permission notice shall be included in all
14+
* copies or substantial portions of the Software.
15+
*
16+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
22+
*/
23+
package com.shopify.checkoutkit
24+
25+
import android.content.Context
26+
import android.content.Intent
27+
import android.net.Uri
28+
import org.assertj.core.api.Assertions.assertThat
29+
import org.junit.Test
30+
import org.junit.runner.RunWith
31+
import org.mockito.kotlin.any
32+
import org.mockito.kotlin.doThrow
33+
import org.mockito.kotlin.mock
34+
import org.mockito.kotlin.whenever
35+
import org.robolectric.RobolectricTestRunner
36+
37+
@RunWith(RobolectricTestRunner::class)
38+
class ExternalUriLauncherTest {
39+
40+
@Test
41+
fun `launch rejects when startActivity throws security exception`() {
42+
val context = mock<Context>()
43+
doThrow(SecurityException("blocked")).whenever(context).startActivity(any<Intent>())
44+
45+
val result = ExternalUriLauncher.launch(context, Uri.parse("https://example.com"))
46+
47+
assertThat(result).isEqualTo(ExternalUriLauncher.Result.Rejected(reason = "blocked"))
48+
}
49+
}

0 commit comments

Comments
 (0)