Skip to content

Commit 0b3a610

Browse files
kiftiokiftio
committed
respond to unknown requests with error
1 parent e711cb0 commit 0b3a610

9 files changed

Lines changed: 416 additions & 28 deletions

File tree

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import kotlinx.serialization.json.JsonPrimitive
1111
import kotlinx.serialization.json.add
1212
import kotlinx.serialization.json.buildJsonObject
1313
import kotlinx.serialization.json.contentOrNull
14+
import kotlinx.serialization.json.decodeFromJsonElement
1415
import kotlinx.serialization.json.put
1516
import kotlinx.serialization.json.putJsonArray
1617
import kotlinx.serialization.json.putJsonObject
@@ -55,15 +56,22 @@ internal class EmbeddedCheckoutProtocol(
5556
@JavascriptInterface
5657
fun postMessage(message: String) {
5758
try {
58-
val request = decoder.decodeFromString<EcpRequest>(message)
59+
val requestObject = decoder.decodeFromString<JsonObject>(message)
60+
val request = decoder.decodeFromJsonElement<EcpRequest>(requestObject)
5961
val method = CheckoutProtocol.supportedProtocolMethod(request)
62+
val hasRequestId = "id" in requestObject
6063
log.d(LOG_TAG, "Received bridge message: method=${request.method} id=${request.id}")
6164
when (method) {
6265
CheckoutProtocol.READY_METHOD -> handleReady(request)
6366
CheckoutProtocol.windowOpen.method -> handleWindowOpenRequest(message)
6467
CheckoutProtocol.start.method -> handleStart(message)
6568
CheckoutProtocol.complete.method -> handleComplete(message)
66-
null -> log.d(LOG_TAG, "Ignoring unsupported ECP method: ${request.method}.")
69+
null -> {
70+
log.d(LOG_TAG, "Ignoring unsupported ECP method: ${request.method}.")
71+
if (hasRequestId) {
72+
sendError(request.id, CODE_METHOD_NOT_FOUND, "Method not found")
73+
}
74+
}
6775
else -> handleClientMessage(method, message)
6876
}
6977
} catch (e: SerializationException) {
@@ -236,5 +244,6 @@ internal class EmbeddedCheckoutProtocol(
236244
private val KIT_SUPPORTED_DELEGATIONS = setOf("window.open")
237245

238246
private const val CODE_PARSE_ERROR = -32700
247+
private const val CODE_METHOD_NOT_FOUND = -32601
239248
}
240249
}

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

Lines changed: 61 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -150,42 +150,58 @@ class EmbeddedCheckoutProtocolTest {
150150

151151
// endregion
152152

153-
// region unsupported methods — silently ignored
153+
// region unsupported methods
154154

155155
@Test
156-
fun `ec auth is silently ignored and not delegated to client`() {
157-
assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"ec.auth","id":"1","params":{"type":"oauth"}}""")
156+
fun `ec auth request returns method not found and is not delegated to client`() {
157+
assertMethodNotFoundByBridge(
158+
"""{"jsonrpc":"2.0","method":"ec.auth","id":"1","params":{"type":"oauth"}}""",
159+
""""id":"1"""",
160+
)
161+
}
162+
163+
@Test
164+
fun `ec payment instruments change request returns method not found and is not delegated to client`() {
165+
assertMethodNotFoundByBridge(
166+
"""{"jsonrpc":"2.0","method":"ec.payment.instruments_change_request","id":"2","params":{}}""",
167+
""""id":"2"""",
168+
)
158169
}
159170

160171
@Test
161-
fun `ec payment instruments change request is silently ignored and not delegated to client`() {
162-
assertIgnoredByBridge(
163-
"""{"jsonrpc":"2.0","method":"ec.payment.instruments_change_request","id":"2","params":{}}"""
172+
fun `ec payment credential request returns method not found and is not delegated to client`() {
173+
assertMethodNotFoundByBridge(
174+
"""{"jsonrpc":"2.0","method":"ec.payment.credential_request","id":"3","params":{}}""",
175+
""""id":"3"""",
164176
)
165177
}
166178

167179
@Test
168-
fun `ec payment credential request is silently ignored and not delegated to client`() {
169-
assertIgnoredByBridge(
170-
"""{"jsonrpc":"2.0","method":"ec.payment.credential_request","id":"3","params":{}}"""
180+
fun `ec fulfillment address change request returns method not found and is not delegated to client`() {
181+
assertMethodNotFoundByBridge(
182+
"""{"jsonrpc":"2.0","method":"ec.fulfillment.address_change_request","id":"4","params":{}}""",
183+
""""id":"4"""",
171184
)
172185
}
173186

174187
@Test
175-
fun `ec fulfillment address change request is silently ignored and not delegated to client`() {
176-
assertIgnoredByBridge(
177-
"""{"jsonrpc":"2.0","method":"ec.fulfillment.address_change_request","id":"4","params":{}}"""
188+
fun `ep cart request returns method not found and is not delegated to client`() {
189+
assertMethodNotFoundByBridge(
190+
"""{"jsonrpc":"2.0","method":"ep.cart.ready","id":"5","params":{}}""",
191+
""""id":"5"""",
178192
)
179193
}
180194

181195
@Test
182-
fun `ec buyer change is silently ignored and not delegated to client`() {
196+
fun `ec buyer change notification is ignored and not delegated to client`() {
183197
assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"ec.buyer.change","params":{"checkout":{}}}""")
184198
}
185199

186200
@Test
187-
fun `ep cart methods fall through as unsupported and are not delegated to client`() {
188-
assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"ep.cart.ready","id":"5","params":{}}""")
201+
fun `unsupported notifications are ignored and not delegated to client`() {
202+
assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"ec.auth","params":{"type":"oauth"}}""")
203+
assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"ec.buyer.change","params":{"checkout":{}}}""")
204+
assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"ep.cart.ready","params":{}}""")
189205
}
190206

191207
// endregion
@@ -496,9 +512,9 @@ class EmbeddedCheckoutProtocolTest {
496512
// region client delegation — requests
497513

498514
@Test
499-
fun `unknown method is silently ignored and not delegated to client`() {
515+
fun `unknown request returns method not found and is not delegated to client`() {
500516
val rawMessage = """{"jsonrpc":"2.0","method":"customMethod","id":"8","params":{}}"""
501-
assertIgnoredByBridge(rawMessage)
517+
assertMethodNotFoundByBridge(rawMessage, """"id":"8"""")
502518
}
503519

504520
@Test
@@ -530,11 +546,20 @@ class EmbeddedCheckoutProtocolTest {
530546
}
531547

532548
@Test
533-
fun `unknown method with no client sends nothing to checkout`() {
534-
ecp.postMessage("""{"jsonrpc":"2.0","method":"unknownMethod","id":"11"}""")
535-
shadowOf(Looper.getMainLooper()).runToEndOfTasks()
549+
fun `unknown notification sends nothing to checkout`() {
550+
assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"unknownMethod"}""")
551+
}
536552

537-
verify(viewSpy, never()).evaluateJavascript(any(), any())
553+
@Test
554+
fun `unknown request with no client returns method not found`() {
555+
val js = captureEvaluatedJs {
556+
ecp.postMessage("""{"jsonrpc":"2.0","method":"unknownMethod","id":"11"}""")
557+
}
558+
559+
assertThat(js).contains("\"error\"")
560+
assertThat(js).contains("-32601")
561+
assertThat(js).contains("Method not found")
562+
assertThat(js).contains(""""id":"11"""")
538563
}
539564

540565
// endregion
@@ -600,6 +625,21 @@ class EmbeddedCheckoutProtocolTest {
600625
verify(client, never()).process(any())
601626
}
602627

628+
private fun assertMethodNotFoundByBridge(rawMessage: String, expectedId: String) {
629+
val client = mock<CheckoutCommunicationClient>()
630+
ecp.setClient(client)
631+
632+
val js = captureEvaluatedJs {
633+
ecp.postMessage(rawMessage)
634+
}
635+
636+
assertThat(js).contains("\"error\"")
637+
assertThat(js).contains("-32601")
638+
assertThat(js).contains("Method not found")
639+
assertThat(js).contains(expectedId)
640+
verify(client, never()).process(any())
641+
}
642+
603643
private fun ecErrorMessage(severity: String): String {
604644
val messages =
605645
"""[{"type":"error","code":"session_failed","content":"Session failed","severity":"$severity"}]"""

‎platforms/swift/Sources/ShopifyCheckoutKit/CheckoutWebView.swift‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,11 @@ extension CheckoutWebView: WKScriptMessageHandler {
182182
}
183183

184184
guard let method = CheckoutProtocol.supportedProtocolMethod(body) else {
185+
if let response = CheckoutProtocol.methodNotFoundResponse(forUnsupportedProtocolRequest: body) {
186+
Task { @MainActor in
187+
await checkoutBridge.sendResponse(self, messageBody: response)
188+
}
189+
}
185190
return
186191
}
187192

‎platforms/swift/Tests/ShopifyCheckoutKitTests/CheckoutWebViewTests.swift‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -334,16 +334,48 @@ class CheckoutWebViewTests: XCTestCase {
334334
}
335335

336336
@MainActor
337-
func testUnsupportedProtocolMethodsDoNotInvokeClient() async {
337+
func testUnsupportedProtocolRequestsReturnMethodNotFoundAndDoNotInvokeClient() async throws {
338+
let client = RecordingBridgeClient(response: #"{"jsonrpc":"2.0","id":"raw","result":{}}"#)
339+
view.client = client
340+
let responseSent = expectation(description: "method-not-found responses sent")
341+
responseSent.expectedFulfillmentCount = 3
342+
MockCheckoutBridge.sendResponseExpectation = responseSent
343+
let messages = [
344+
(#"{"jsonrpc":"2.0","method":"ec.payment.credential_request","id":"unsupported","params":{}}"#, "unsupported"),
345+
(#"{"jsonrpc":"2.0","method":"ep.cart.ready","id":"ep","params":{}}"#, "ep"),
346+
(#"{"jsonrpc":"2.0","method":"customMethod","id":"custom","params":{}}"#, "custom")
347+
]
348+
349+
for (body, _) in messages {
350+
view.userContentController(WKUserContentController(), didReceive: MockScriptMessage(body: body))
351+
}
352+
353+
await fulfillment(of: [responseSent], timeout: 5.0)
354+
XCTAssertEqual(MockCheckoutBridge.sendResponseCount, messages.count)
355+
let parsedResponses = try MockCheckoutBridge.responseBodies.map { response -> [String: Any] in
356+
try XCTUnwrap(try JSONSerialization.jsonObject(with: Data(response.utf8)) as? [String: Any])
357+
}
358+
XCTAssertEqual(parsedResponses.map { $0["id"] as? String }, messages.map { $0.1 })
359+
for parsed in parsedResponses {
360+
let error = try XCTUnwrap(parsed["error"] as? [String: Any])
361+
XCTAssertEqual(error["code"] as? Int, -32601)
362+
XCTAssertEqual(error["message"] as? String, "Method not found")
363+
}
364+
let receivedMessages = await client.messages()
365+
XCTAssertEqual(receivedMessages, [])
366+
}
367+
368+
@MainActor
369+
func testUnsupportedProtocolNotificationsDoNotInvokeClient() async {
338370
let client = RecordingBridgeClient(response: #"{"jsonrpc":"2.0","id":"raw","result":{}}"#)
339371
view.client = client
340372
let notSent = expectation(description: "sendResponse must not fire")
341373
notSent.isInverted = true
342374
MockCheckoutBridge.sendResponseExpectation = notSent
343375
let messages = [
344-
#"{"jsonrpc":"2.0","method":"ec.payment.credential_request","id":"unsupported","params":{}}"#,
345-
#"{"jsonrpc":"2.0","method":"ep.cart.ready","id":"ep","params":{}}"#,
346-
#"{"jsonrpc":"2.0","method":"customMethod","id":"custom","params":{}}"#
376+
#"{"jsonrpc":"2.0","method":"ec.payment.credential_request","params":{}}"#,
377+
#"{"jsonrpc":"2.0","method":"ep.cart.ready","params":{}}"#,
378+
#"{"jsonrpc":"2.0","method":"customMethod","params":{}}"#
347379
]
348380

349381
for body in messages {
@@ -578,19 +610,22 @@ class MockCheckoutBridge: CheckoutBridgeProtocol {
578610
static var sendResponseCalled = false
579611
static var sendResponseCount = 0
580612
static var lastResponseBody: String?
613+
static var responseBodies: [String] = []
581614
static var sendResponseExpectation: XCTestExpectation?
582615

583616
static func reset() {
584617
sendResponseCalled = false
585618
sendResponseCount = 0
586619
lastResponseBody = nil
620+
responseBodies = []
587621
sendResponseExpectation = nil
588622
}
589623

590624
@MainActor static func sendResponse(_: WKWebView, messageBody: String) async -> Bool {
591625
sendResponseCalled = true
592626
sendResponseCount += 1
593627
lastResponseBody = messageBody
628+
responseBodies.append(messageBody)
594629
sendResponseExpectation?.fulfill()
595630
return true
596631
}

0 commit comments

Comments
 (0)