Skip to content

ignore unknown messages#287

Merged
kiftio merged 1 commit into
mainfrom
dk/ignore-unknown-messages
Jun 18, 2026
Merged

ignore unknown messages#287
kiftio merged 1 commit into
mainfrom
dk/ignore-unknown-messages

Conversation

@kiftio

@kiftio kiftio commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

What changes are you making?

PR 1 -> parity with web r.e. an allowlist of events

Previously, the SDK responded to certain unsupported ECP methods (e.g. ec.auth, ec.payment.credential_request) with an explicit JSON-RPC -32601 "method not supported" error, and forwarded any unknown methods to the consumer CheckoutCommunicationClient. This approach exposed internal protocol details and allowed arbitrary unrecognized methods to reach the client.

This PR replaces that behavior with an explicit allow-list of supported protocol methods on both Android and iOS:

  • Android: Replaces UNSUPPORTED_METHODS (which sent explicit error responses) with SUPPORTED_CLIENT_METHODS, an allow-list aligned with the web component's CHECKOUT_PROTOCOL_MESSAGES. Methods outside this list — including ep.*, ec.auth, and any unknown methods — are now silently ignored rather than forwarded or rejected with an error.
  • iOS: Adds a supportedProtocolMethods allow-list in CheckoutWebView. Incoming messages are parsed and their method checked against this list before any further processing or client delegation occurs. Messages with unsupported methods are dropped silently.
  • The CheckoutCommunicationClient documentation is updated on both platforms to clarify that only supported ECP messages are forwarded to the client.

How to test

  1. Send an ECP message with an unsupported method (e.g. ec.auth, ec.payment.credential_request, ep.cart.ready, or a custom unknown method) to the bridge on both Android and iOS.
  2. Verify no JSON-RPC error response is sent back to the web page.
  3. Verify the consumer CheckoutCommunicationClient does not receive the message.
  4. Send a supported ECP message (e.g. ec.window.open_request, ec.messages.change) and verify it is still correctly delegated to the client and any response is forwarded back to the web page.
  5. Run the updated unit tests on both platforms to confirm all cases pass.

Before you merge

Important

  • I've added tests to support my implementation
  • I have read and agree with the Contribution Guidelines
  • I have read and agree with the Code of Conduct
  • I've updated the relevant platform README (platforms/swift/README.md and/or platforms/android/README.md)

Releasing a new Swift version?
  • 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)
Releasing a new Android version?
  • 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

Tip

See the Contributing documentation for the full release process per platform.

kiftio commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

@kiftio kiftio force-pushed the dk/ignore-unknown-messages branch from 7156a26 to b2500fb Compare June 16, 2026 11:51
}
assertThat(js).contains("\"error\"")
assertThat(js).contains("-32601")
fun `ec auth is silently ignored and not delegated to client`() {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bringing this back in PR 3, but across all platforms

@kiftio kiftio force-pushed the dk/ignore-unknown-messages branch from b2500fb to b61158a Compare June 16, 2026 12:24
@kiftio kiftio marked this pull request as ready for review June 16, 2026 12:28
@kiftio kiftio requested a review from a team as a code owner June 16, 2026 12:28
@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown

React Native — Coverage Report

Lines Statements Branches Functions
Coverage: 92%
91.66% (319/348) 87.86% (181/206) 100% (82/82)

@kiftio kiftio mentioned this pull request Jun 16, 2026
11 tasks
@kiftio kiftio force-pushed the dk/ignore-unknown-messages branch from b61158a to 4b0d021 Compare June 16, 2026 12:48
extension CheckoutWebView {
fileprivate static let readyMethod = "ec.ready"

fileprivate static let supportedProtocolMethods: Set<String> = [

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CheckoutProtocol is an enum of the events we support
Should we add CaseIterable to it so we can get all supported cases with CheckoutProtocol.allCases

CheckoutProtocol.windowOpen.method
]

fileprivate static func supportedProtocolMethod(_ body: String) -> String? {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feels like this would make sense to exist in the protocol side too

Comment on lines +207 to +233
extension CheckoutWebView {
fileprivate static let readyMethod = "ec.ready"

fileprivate static let supportedProtocolMethods: Set<String> = [
readyMethod,
CheckoutProtocol.start.method,
CheckoutProtocol.complete.method,
CheckoutProtocol.error.method,
CheckoutProtocol.lineItemsChange.method,
CheckoutProtocol.messagesChange.method,
CheckoutProtocol.totalsChange.method,
CheckoutProtocol.windowOpen.method
]

fileprivate static func supportedProtocolMethod(_ body: String) -> String? {
guard
let object = try? JSONSerialization.jsonObject(with: Data(body.utf8)) as? [String: Any],
object["jsonrpc"] as? String == "2.0",
let method = object["method"] as? String,
supportedProtocolMethods.contains(method)
else {
return nil
}

return method
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the protocol side, we're already only binding to a limited set of events →

public enum CheckoutProtocol {
public static let specVersion = "2026-04-08"
public static let defaultDelegations: [String] = ["window.open"]
static let buyerChange = NotificationDescriptor<Checkout>(method: "ec.buyer.change")
public static let complete = NotificationDescriptor<Checkout>(method: "ec.complete")
public static let error = NotificationDescriptor<ErrorResponse>(method: "ec.error")
public static let lineItemsChange = NotificationDescriptor<Checkout>(
method: "ec.line_items.change"
)
public static let messagesChange = NotificationDescriptor<Checkout>(
method: "ec.messages.change"
)
public static let start = NotificationDescriptor<Checkout>(method: "ec.start")
public static let totalsChange = NotificationDescriptor<Checkout>(method: "ec.totals.change")
}

So this feels duplicative, but, saying that, if we were to extend the protocol to expose it as separate package we would need to bind the relevant ones here.

What do you think about keeping this logic in the protocol until this is needed?

@kiftio kiftio force-pushed the dk/ignore-unknown-messages branch from 4b0d021 to e711cb0 Compare June 18, 2026 09:06
@kiftio kiftio merged commit 86e9f2e into main Jun 18, 2026
26 checks passed
@kiftio kiftio deleted the dk/ignore-unknown-messages branch June 18, 2026 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants