Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ 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
import kotlinx.serialization.json.putJsonObject
Expand Down Expand Up @@ -55,15 +56,22 @@ internal class EmbeddedCheckoutProtocol(
@JavascriptInterface
fun postMessage(message: String) {
try {
val request = decoder.decodeFromString<EcpRequest>(message)
val requestObject = decoder.decodeFromString<JsonObject>(message)
val request = decoder.decodeFromJsonElement<EcpRequest>(requestObject)
val method = CheckoutProtocol.supportedProtocolMethod(request)
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.start.method -> handleStart(message)
CheckoutProtocol.complete.method -> handleComplete(message)
null -> log.d(LOG_TAG, "Ignoring unsupported ECP method: ${request.method}.")
null -> {
log.d(LOG_TAG, "Ignoring unsupported ECP method: ${request.method}.")
if (requestId != null) {
sendError(requestId, CODE_METHOD_NOT_FOUND, "Method not found")
}
}
else -> handleClientMessage(method, message)
}
} catch (e: SerializationException) {
Expand Down Expand Up @@ -236,5 +244,11 @@ internal class EmbeddedCheckoutProtocol(
private val KIT_SUPPORTED_DELEGATIONS = setOf("window.open")

private const val CODE_PARSE_ERROR = -32700
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 }
}
Original file line number Diff line number Diff line change
Expand Up @@ -150,42 +150,58 @@ class EmbeddedCheckoutProtocolTest {

// endregion

// region unsupported methods — silently ignored
// region unsupported methods

@Test
fun `ec auth is silently ignored and not delegated to client`() {
assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"ec.auth","id":"1","params":{"type":"oauth"}}""")
fun `ec auth request returns method not found and is not delegated to client`() {
assertMethodNotFoundByBridge(
"""{"jsonrpc":"2.0","method":"ec.auth","id":"1","params":{"type":"oauth"}}""",
""""id":"1"""",
)
}

@Test
fun `ec payment instruments change request is silently ignored and not delegated to client`() {
assertIgnoredByBridge(
"""{"jsonrpc":"2.0","method":"ec.payment.instruments_change_request","id":"2","params":{}}"""
fun `ec payment instruments change request returns method not found and is not delegated to client`() {
assertMethodNotFoundByBridge(
"""{"jsonrpc":"2.0","method":"ec.payment.instruments_change_request","id":"2","params":{}}""",
""""id":"2"""",
)
}

@Test
fun `ec payment credential request is silently ignored and not delegated to client`() {
assertIgnoredByBridge(
"""{"jsonrpc":"2.0","method":"ec.payment.credential_request","id":"3","params":{}}"""
fun `ec payment credential request returns method not found and is not delegated to client`() {
assertMethodNotFoundByBridge(
"""{"jsonrpc":"2.0","method":"ec.payment.credential_request","id":"3","params":{}}""",
""""id":"3"""",
)
}

@Test
fun `ec fulfillment address change request is silently ignored and not delegated to client`() {
assertIgnoredByBridge(
"""{"jsonrpc":"2.0","method":"ec.fulfillment.address_change_request","id":"4","params":{}}"""
fun `ec fulfillment address change request returns method not found and is not delegated to client`() {
assertMethodNotFoundByBridge(
"""{"jsonrpc":"2.0","method":"ec.fulfillment.address_change_request","id":"4","params":{}}""",
""""id":"4"""",
)
}

@Test
fun `ec buyer change is silently ignored and not delegated to client`() {
fun `ep cart request returns method not found and is not delegated to client`() {
assertMethodNotFoundByBridge(
"""{"jsonrpc":"2.0","method":"ep.cart.ready","id":"5","params":{}}""",
""""id":"5"""",
)
}

@Test
fun `ec buyer change notification is ignored and not delegated to client`() {
assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"ec.buyer.change","params":{"checkout":{}}}""")
}

@Test
fun `ep cart methods fall through as unsupported and are not delegated to client`() {
assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"ep.cart.ready","id":"5","params":{}}""")
fun `unsupported notifications are ignored and not delegated to client`() {
assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"ec.auth","params":{"type":"oauth"}}""")
assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"ec.buyer.change","params":{"checkout":{}}}""")
assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"ep.cart.ready","params":{}}""")
}

// endregion
Expand Down Expand Up @@ -496,9 +512,9 @@ class EmbeddedCheckoutProtocolTest {
// region client delegation — requests

@Test
fun `unknown method is silently ignored and not delegated to client`() {
fun `unknown request returns method not found and is not delegated to client`() {
val rawMessage = """{"jsonrpc":"2.0","method":"customMethod","id":"8","params":{}}"""
assertIgnoredByBridge(rawMessage)
assertMethodNotFoundByBridge(rawMessage, """"id":"8"""")
}

@Test
Expand Down Expand Up @@ -530,11 +546,27 @@ class EmbeddedCheckoutProtocolTest {
}

@Test
fun `unknown method with no client sends nothing to checkout`() {
ecp.postMessage("""{"jsonrpc":"2.0","method":"unknownMethod","id":"11"}""")
shadowOf(Looper.getMainLooper()).runToEndOfTasks()
fun `unknown notification sends nothing to checkout`() {
assertIgnoredByBridge("""{"jsonrpc":"2.0","method":"unknownMethod"}""")
}

verify(viewSpy, never()).evaluateJavascript(any(), any())
@Test
fun `unknown request with no client returns method not found`() {
val js = captureEvaluatedJs {
ecp.postMessage("""{"jsonrpc":"2.0","method":"unknownMethod","id":"11"}""")
}

assertThat(js).contains("\"error\"")
assertThat(js).contains("-32601")
assertThat(js).contains("Method not found")
assertThat(js).contains(""""id":"11"""")
}

@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":true,"params":{}}""")
}

// endregion
Expand Down Expand Up @@ -600,6 +632,21 @@ class EmbeddedCheckoutProtocolTest {
verify(client, never()).process(any())
}

private fun assertMethodNotFoundByBridge(rawMessage: String, expectedId: String) {
val client = mock<CheckoutCommunicationClient>()
ecp.setClient(client)

val js = captureEvaluatedJs {
ecp.postMessage(rawMessage)
}

assertThat(js).contains("\"error\"")
assertThat(js).contains("-32601")
assertThat(js).contains("Method not found")
assertThat(js).contains(expectedId)
verify(client, never()).process(any())
}

private fun ecErrorMessage(severity: String): String {
val messages =
"""[{"type":"error","code":"session_failed","content":"Session failed","severity":"$severity"}]"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,11 @@
}

guard let method = CheckoutProtocol.supportedProtocolMethod(body) else {
if let response = CheckoutProtocol.methodNotFoundResponse(forUnsupportedProtocolRequest: body) {
Task { @MainActor in
await checkoutBridge.sendResponse(self, messageBody: response)
}
}
return
}

Expand All @@ -209,7 +214,7 @@
}

extension CheckoutWebView: WKNavigationDelegate {
func webView(_: WKWebView, decidePolicyFor action: WKNavigationAction, decisionHandler: @escaping (WKNavigationActionPolicy) -> Void) {

Check warning on line 217 in platforms/swift/Sources/ShopifyCheckoutKit/CheckoutWebView.swift

View workflow job for this annotation

GitHub Actions / Breaking Changes / Swift

instance method 'webView(_:decidePolicyFor:decisionHandler:)' nearly matches optional requirement 'webView(_:decidePolicyFor:decisionHandler:)' of protocol 'WKNavigationDelegate'

Check warning on line 217 in platforms/swift/Sources/ShopifyCheckoutKit/CheckoutWebView.swift

View workflow job for this annotation

GitHub Actions / Breaking Changes / Swift

instance method 'webView(_:decidePolicyFor:decisionHandler:)' nearly matches optional requirement 'webView(_:decidePolicyFor:decisionHandler:)' of protocol 'WKNavigationDelegate'

Check warning on line 217 in platforms/swift/Sources/ShopifyCheckoutKit/CheckoutWebView.swift

View workflow job for this annotation

GitHub Actions / Breaking Changes / Swift

instance method 'webView(_:decidePolicyFor:decisionHandler:)' nearly matches optional requirement 'webView(_:decidePolicyFor:decisionHandler:)' of protocol 'WKNavigationDelegate'

Check warning on line 217 in platforms/swift/Sources/ShopifyCheckoutKit/CheckoutWebView.swift

View workflow job for this annotation

GitHub Actions / Swift / package-tests / Run Package Tests

instance method 'webView(_:decidePolicyFor:decisionHandler:)' nearly matches optional requirement 'webView(_:decidePolicyFor:decisionHandler:)' of protocol 'WKNavigationDelegate'

Check warning on line 217 in platforms/swift/Sources/ShopifyCheckoutKit/CheckoutWebView.swift

View workflow job for this annotation

GitHub Actions / Swift / package-tests / Run Package Tests

instance method 'webView(_:decidePolicyFor:decisionHandler:)' nearly matches optional requirement 'webView(_:decidePolicyFor:decisionHandler:)' of protocol 'WKNavigationDelegate'
// Handle rare cases where the url is nil
guard let url = action.request.url else {
decisionHandler(.allow)
Expand All @@ -235,7 +240,7 @@
decisionHandler(.allow)
}

func webView(_: WKWebView, decidePolicyFor navigationResponse: WKNavigationResponse, decisionHandler: @escaping (WKNavigationResponsePolicy) -> Void) {

Check warning on line 243 in platforms/swift/Sources/ShopifyCheckoutKit/CheckoutWebView.swift

View workflow job for this annotation

GitHub Actions / Breaking Changes / Swift

instance method 'webView(_:decidePolicyFor:decisionHandler:)' nearly matches optional requirement 'webView(_:decidePolicyFor:decisionHandler:)' of protocol 'WKNavigationDelegate'

Check warning on line 243 in platforms/swift/Sources/ShopifyCheckoutKit/CheckoutWebView.swift

View workflow job for this annotation

GitHub Actions / Breaking Changes / Swift

instance method 'webView(_:decidePolicyFor:decisionHandler:)' nearly matches optional requirement 'webView(_:decidePolicyFor:decisionHandler:)' of protocol 'WKNavigationDelegate'

Check warning on line 243 in platforms/swift/Sources/ShopifyCheckoutKit/CheckoutWebView.swift

View workflow job for this annotation

GitHub Actions / Breaking Changes / Swift

instance method 'webView(_:decidePolicyFor:decisionHandler:)' nearly matches optional requirement 'webView(_:decidePolicyFor:decisionHandler:)' of protocol 'WKNavigationDelegate'

Check warning on line 243 in platforms/swift/Sources/ShopifyCheckoutKit/CheckoutWebView.swift

View workflow job for this annotation

GitHub Actions / Swift / package-tests / Run Package Tests

instance method 'webView(_:decidePolicyFor:decisionHandler:)' nearly matches optional requirement 'webView(_:decidePolicyFor:decisionHandler:)' of protocol 'WKNavigationDelegate'

Check warning on line 243 in platforms/swift/Sources/ShopifyCheckoutKit/CheckoutWebView.swift

View workflow job for this annotation

GitHub Actions / Swift / package-tests / Run Package Tests

instance method 'webView(_:decidePolicyFor:decisionHandler:)' nearly matches optional requirement 'webView(_:decidePolicyFor:decisionHandler:)' of protocol 'WKNavigationDelegate'
if let response = navigationResponse.response as? HTTPURLResponse {
decisionHandler(handleResponse(response))
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,16 +350,48 @@ class CheckoutWebViewTests: XCTestCase {
}

@MainActor
func testUnsupportedProtocolMethodsDoNotInvokeClient() async {
func testUnsupportedProtocolRequestsReturnMethodNotFoundAndDoNotInvokeClient() async throws {
let client = RecordingBridgeClient(response: #"{"jsonrpc":"2.0","id":"raw","result":{}}"#)
view.client = client
let responseSent = expectation(description: "method-not-found responses sent")
responseSent.expectedFulfillmentCount = 3
MockCheckoutBridge.sendResponseExpectation = responseSent
let messages = [
(#"{"jsonrpc":"2.0","method":"ec.payment.credential_request","id":"unsupported","params":{}}"#, "unsupported"),
(#"{"jsonrpc":"2.0","method":"ep.cart.ready","id":"ep","params":{}}"#, "ep"),
(#"{"jsonrpc":"2.0","method":"customMethod","id":"custom","params":{}}"#, "custom")
]

for (body, _) in messages {
view.userContentController(WKUserContentController(), didReceive: MockScriptMessage(body: body))
}

await fulfillment(of: [responseSent], timeout: 5.0)
XCTAssertEqual(MockCheckoutBridge.sendResponseCount, messages.count)
let parsedResponses = try MockCheckoutBridge.responseBodies.map { response -> [String: Any] in
try XCTUnwrap(try JSONSerialization.jsonObject(with: Data(response.utf8)) as? [String: Any])
}
XCTAssertEqual(parsedResponses.map { $0["id"] as? String }, messages.map { $0.1 })
for parsed in parsedResponses {
let error = try XCTUnwrap(parsed["error"] as? [String: Any])
XCTAssertEqual(error["code"] as? Int, -32601)
XCTAssertEqual(error["message"] as? String, "Method not found")
}
let receivedMessages = await client.messages()
XCTAssertEqual(receivedMessages, [])
}

@MainActor
func testUnsupportedProtocolNotificationsDoNotInvokeClient() async {
let client = RecordingBridgeClient(response: #"{"jsonrpc":"2.0","id":"raw","result":{}}"#)
view.client = client
let notSent = expectation(description: "sendResponse must not fire")
notSent.isInverted = true
MockCheckoutBridge.sendResponseExpectation = notSent
let messages = [
#"{"jsonrpc":"2.0","method":"ec.payment.credential_request","id":"unsupported","params":{}}"#,
#"{"jsonrpc":"2.0","method":"ep.cart.ready","id":"ep","params":{}}"#,
#"{"jsonrpc":"2.0","method":"customMethod","id":"custom","params":{}}"#
#"{"jsonrpc":"2.0","method":"ec.payment.credential_request","params":{}}"#,
#"{"jsonrpc":"2.0","method":"ep.cart.ready","params":{}}"#,
#"{"jsonrpc":"2.0","method":"customMethod","params":{}}"#
]

for body in messages {
Expand Down Expand Up @@ -594,19 +626,22 @@ class MockCheckoutBridge: CheckoutBridgeProtocol {
static var sendResponseCalled = false
static var sendResponseCount = 0
static var lastResponseBody: String?
static var responseBodies: [String] = []
static var sendResponseExpectation: XCTestExpectation?

static func reset() {
sendResponseCalled = false
sendResponseCount = 0
lastResponseBody = nil
responseBodies = []
sendResponseExpectation = nil
}

@MainActor static func sendResponse(_: WKWebView, messageBody: String) async -> Bool {
sendResponseCalled = true
sendResponseCount += 1
lastResponseBody = messageBody
responseBodies.append(messageBody)
sendResponseExpectation?.fulfill()
return true
}
Expand Down
Loading
Loading