From 3e16fe81695896bb20d21b52085bbd9de7fb640a Mon Sep 17 00:00:00 2001 From: tiagocandido Date: Mon, 28 Sep 2026 16:58:57 +0200 Subject: [PATCH] Handle undefined optional protocol fields --- .../tests/protocol.test.ts | 24 +++ platforms/web/src/checkout-protocol.test.ts | 49 +++++ protocol/languages/typescript/src/index.d.ts | 1 + protocol/languages/typescript/src/index.ts | 4 + .../src/protocol_codec_runtime.d.ts | 7 + .../typescript/src/protocol_codec_runtime.ts | 61 ++++-- .../typescript/test/codec-runtime.test.ts | 178 +++++++++++++++++- 7 files changed, 311 insertions(+), 13 deletions(-) diff --git a/platforms/react-native/modules/@shopify/checkout-kit-react-native/tests/protocol.test.ts b/platforms/react-native/modules/@shopify/checkout-kit-react-native/tests/protocol.test.ts index 52851f4e6..4ec6e5d0b 100644 --- a/platforms/react-native/modules/@shopify/checkout-kit-react-native/tests/protocol.test.ts +++ b/platforms/react-native/modules/@shopify/checkout-kit-react-native/tests/protocol.test.ts @@ -64,6 +64,30 @@ describe('CheckoutProtocol', () => { ).toBe('checkout-123'); }); + it('treats undefined optional checkout fields as absent but rejects undefined required fields', () => { + const checkoutEnvelope = { + id: 'checkout-123', + currency: 'USD', + status: 'incomplete', + line_items: [], + totals: [], + links: [], + ucp: {version: '2026-04-08'}, + order: undefined, + fulfillment: undefined, + }; + + const decoded = decodeProtocolPayload(CheckoutProtocol.start, checkoutEnvelope); + expect(decoded).not.toHaveProperty('order'); + expect(decoded).not.toHaveProperty('fulfillment'); + expect(() => + decodeProtocolPayload(CheckoutProtocol.start, { + ...checkoutEnvelope, + totals: undefined, + }), + ).toThrow('Invalid Checkout.totals'); + }); + it.each(checkoutPayloadMethods)( 'converts %s checkout schema fields to camelCase while preserving dynamic map keys', method => { diff --git a/platforms/web/src/checkout-protocol.test.ts b/platforms/web/src/checkout-protocol.test.ts index 5da94edee..0870114a1 100644 --- a/platforms/web/src/checkout-protocol.test.ts +++ b/platforms/web/src/checkout-protocol.test.ts @@ -228,6 +228,55 @@ describe("", () => { expect(onStartSpy).toHaveBeenCalledOnce(); }); + it("delivers a structured-cloned checkout with undefined optional fields", async () => { + const { checkout, mockCheckoutWindow } = openPopupCheckout(); + const onStartSpy = vi.fn(); + const listenForEvent = waitForEvent(checkout, "start", onStartSpy); + const payload = structuredClone( + makeCheckoutPayload({ order: undefined, fulfillment: undefined }), + ); + + expect(Object.hasOwn(payload.checkout, "order")).toBe(true); + simulateProtocolMessageEvent(checkout, "ec.start", payload, { + source: mockCheckoutWindow, + }); + await listenForEvent; + + expect(onStartSpy).toHaveBeenCalledOnce(); + const event = onStartSpy.mock.calls[0]![0] as CustomEvent; + expect(event.detail.checkout).not.toHaveProperty("order"); + expect(event.detail.checkout).not.toHaveProperty("fulfillment"); + expect(checkout.checkout).toStrictEqual(event.detail.checkout); + }); + + it("drops an invalid present order and records a decode error", async () => { + const telemetrySpy = vi.spyOn(mockTelemetry(), "recordProtocolDecodeError"); + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const { checkout, mockCheckoutWindow } = openPopupCheckout(); + const onStartSpy = vi.fn(); + checkout.addEventListener("start", onStartSpy); + + simulateProtocolMessageEvent( + checkout, + "ec.start", + makeCheckoutPayload({ + order: { + id: undefined, + permalink_url: "https://example.test/orders/order-1", + }, + }), + { source: mockCheckoutWindow }, + ); + await flushProtocolDispatch(); + + expect(onStartSpy).not.toHaveBeenCalled(); + expect(telemetrySpy).toHaveBeenCalledWith({ method: "ec.start", failureType: "params" }); + expect(consoleErrorSpy).toHaveBeenCalledWith( + ": dropped ec.start: failed to decode payload", + "Invalid Checkout.order.id", + ); + }); + it("measures navigation from before the checkout window opens", async () => { let now = 100; vi.spyOn(performance, "now").mockImplementation(() => now); diff --git a/protocol/languages/typescript/src/index.d.ts b/protocol/languages/typescript/src/index.d.ts index e0a43388a..303978c3c 100644 --- a/protocol/languages/typescript/src/index.d.ts +++ b/protocol/languages/typescript/src/index.d.ts @@ -6,3 +6,4 @@ export { checkoutProtocolCatalog, checkoutProtocolCatalogPayloadDecoders, checko export { Client, type DecodeErrorContext } from './client'; export { windowOpenSuccess, windowOpenRejected } from './window_open'; export { EmbeddedCheckoutProtocol } from './embedded_checkout_protocol'; +export { ProtocolValidationError, type ProtocolValidationReason, } from './protocol_codec_runtime'; diff --git a/protocol/languages/typescript/src/index.ts b/protocol/languages/typescript/src/index.ts index 1f8993847..9b8d3f592 100644 --- a/protocol/languages/typescript/src/index.ts +++ b/protocol/languages/typescript/src/index.ts @@ -36,3 +36,7 @@ export { export {Client, type DecodeErrorContext} from './client'; export {windowOpenSuccess, windowOpenRejected} from './window_open'; export {EmbeddedCheckoutProtocol} from './embedded_checkout_protocol'; +export { + ProtocolValidationError, + type ProtocolValidationReason, +} from './protocol_codec_runtime'; diff --git a/protocol/languages/typescript/src/protocol_codec_runtime.d.ts b/protocol/languages/typescript/src/protocol_codec_runtime.d.ts index 438a3638c..b974ac8ac 100644 --- a/protocol/languages/typescript/src/protocol_codec_runtime.d.ts +++ b/protocol/languages/typescript/src/protocol_codec_runtime.d.ts @@ -1,4 +1,11 @@ type JSONRecord = Record; +export type ProtocolValidationReason = 'missing_required' | 'invalid_type'; +/** A schema path and reason that can be reported without exposing payload values. */ +export declare class ProtocolValidationError extends TypeError { + readonly modelPath: string; + readonly reason: ProtocolValidationReason; + constructor(modelPath: string, reason: ProtocolValidationReason); +} export declare function decodeProtocolObject(value: unknown, modelName: string): JSONRecord; export declare function encodeProtocolObject(value: unknown, modelName: string): unknown; export {}; diff --git a/protocol/languages/typescript/src/protocol_codec_runtime.ts b/protocol/languages/typescript/src/protocol_codec_runtime.ts index 7e13c8a2d..940336c98 100644 --- a/protocol/languages/typescript/src/protocol_codec_runtime.ts +++ b/protocol/languages/typescript/src/protocol_codec_runtime.ts @@ -4,6 +4,28 @@ import type {RenameChild, RenameEntry} from './generated/ProtocolRenameMap'; type JSONRecord = Record; type Direction = 'decode' | 'encode'; +export type ProtocolValidationReason = 'missing_required' | 'invalid_type'; + +/** A schema path and reason that can be reported without exposing payload values. */ +export class ProtocolValidationError extends TypeError { + readonly modelPath: string; + readonly reason: ProtocolValidationReason; + + constructor(modelPath: string, reason: ProtocolValidationReason) { + // The generated decoders supply model names and the checks below supply + // schema fields. Keep even direct calls with arbitrary names safe to log. + const safePath = /^[A-Za-z][A-Za-z0-9]*(?:\.[A-Za-z][A-Za-z0-9_]*)*$/.test( + modelPath, + ) + ? modelPath + : 'ProtocolObject'; + super(`Invalid ${safePath}`); + this.name = 'ProtocolValidationError'; + this.modelPath = safePath; + this.reason = reason; + } +} + const REQUIRED_FIELDS: Record = { Checkout: ['currency', 'id', 'line_items', 'links', 'status', 'totals', 'ucp'], ErrorResponse: ['messages', 'ucp'], @@ -16,7 +38,7 @@ const REQUIRED_STRING_FIELDS: Record = { WindowOpenRequest: ['url'], }; -const NESTED_REQUIRED_FIELDS: Record = { +const NESTED_REQUIRED_STRING_FIELDS: Record = { order: ['id', 'permalink_url'], ucp: ['version'], }; @@ -49,7 +71,7 @@ function walkObject( direction === 'decode' && modelName === 'FulfillmentOption' ? normalizeLegacyFulfillmentOptionDescription(value) : value; - if (!entries || !isObjectRecord(input)) { + if (!isObjectRecord(input) || (!entries && direction === 'encode')) { return input; } @@ -57,12 +79,18 @@ function walkObject( const targetIndex = direction === 'decode' ? 1 : 0; const entryBySource = new Map(); - for (const entry of entries) { + for (const entry of entries ?? []) { entryBySource.set(entry[sourceIndex], entry); } const output: JSONRecord = {}; for (const [key, item] of Object.entries(input)) { + // Structured clone preserves undefined-valued own properties whereas JSON + // omits them. Only normalize objects being walked as protocol models; an + // unknown extension value is passed through without changing its contents. + if (direction === 'decode' && item === undefined) { + continue; + } const entry = entryBySource.get(key); if (entry) { output[entry[targetIndex]] = walkChild(item, entry[2], direction); @@ -141,19 +169,23 @@ function isObjectRecord(value: unknown): value is JSONRecord { function requireObject(value: unknown, label: string): JSONRecord { if (!isObjectRecord(value)) { - throw new TypeError(`Invalid ${label}`); + throw new ProtocolValidationError(label, 'invalid_type'); } return value; } +function hasOwnField(value: JSONRecord, field: string): boolean { + return Object.prototype.hasOwnProperty.call(value, field); +} + function requireFields( value: JSONRecord, fields: readonly string[], label: string, ): void { for (const field of fields) { - if (!(field in value)) { - throw new TypeError(`Invalid ${label}`); + if (!hasOwnField(value, field) || value[field] === undefined) { + throw new ProtocolValidationError(`${label}.${field}`, 'missing_required'); } } } @@ -164,18 +196,25 @@ function requireStringFields( label: string, ): void { for (const field of fields) { - if (field in value && typeof value[field] !== 'string') { - throw new TypeError(`Invalid ${label}`); + if ( + hasOwnField(value, field) && + value[field] !== undefined && + typeof value[field] !== 'string' + ) { + throw new ProtocolValidationError(`${label}.${field}`, 'invalid_type'); } } } function requireNestedFields(value: JSONRecord, label: string): void { - for (const [field, requiredFields] of Object.entries(NESTED_REQUIRED_FIELDS)) { - if (!(field in value)) { + for (const [field, requiredStringFields] of Object.entries( + NESTED_REQUIRED_STRING_FIELDS, + )) { + if (!hasOwnField(value, field) || value[field] === undefined) { continue; } const nested = requireObject(value[field], `${label}.${field}`); - requireFields(nested, requiredFields, `${label}.${field}`); + requireFields(nested, requiredStringFields, `${label}.${field}`); + requireStringFields(nested, requiredStringFields, `${label}.${field}`); } } diff --git a/protocol/languages/typescript/test/codec-runtime.test.ts b/protocol/languages/typescript/test/codec-runtime.test.ts index 284018567..c008d0fbc 100644 --- a/protocol/languages/typescript/test/codec-runtime.test.ts +++ b/protocol/languages/typescript/test/codec-runtime.test.ts @@ -4,6 +4,8 @@ import {EmbeddedCheckoutProtocol} from '../src/embedded_checkout_protocol'; import { decodeProtocolObject, encodeProtocolObject, + ProtocolValidationError, + type ProtocolValidationReason, } from '../src/protocol_codec_runtime'; const wire = { @@ -74,7 +76,179 @@ test('renames fields inside map values', () => { }); test('throws when a required string field is not a string', () => { - expect(() => decodeProtocolObject({...wire, currency: 123}, 'Checkout')).toThrow( - 'Invalid Checkout', + expectValidationError( + () => decodeProtocolObject({...wire, currency: 123}, 'Checkout'), + 'Checkout.currency', + 'invalid_type', ); }); + +test('treats structured-cloned undefined properties as absent without changing extension payloads', () => { + const checkout = structuredClone({ + ...wire, + buyer: {email: 'buyer@example.test', first_name: undefined}, + fulfillment: undefined, + order: undefined, + x_partner_data: {nested_key: 'value', optional_key: undefined}, + }); + const omitted = { + ...wire, + buyer: {email: 'buyer@example.test'}, + x_partner_data: {nested_key: 'value', optional_key: undefined}, + }; + + expect(Object.hasOwn(checkout, 'order')).toBe(true); + expect(decodeProtocolObject(checkout, 'Checkout')).toStrictEqual( + decodeProtocolObject(omitted, 'Checkout'), + ); + const decoded = decodeProtocolObject(checkout, 'Checkout'); + expect(decoded).not.toHaveProperty('order'); + expect(decoded).not.toHaveProperty('fulfillment'); + expect(decoded.buyer).not.toHaveProperty('firstName'); + expect(decoded.x_partner_data).toBe(checkout.x_partner_data); + expect(checkout.order).toBeUndefined(); + expect(Object.hasOwn(checkout.buyer, 'first_name')).toBe(true); +}); + +test('decodes direct objects and JSON-serialized delivery the same way', () => { + const checkout = structuredClone({ + ...wire, + order: undefined, + fulfillment: undefined, + }); + const serializedCheckout = JSON.parse(JSON.stringify(checkout)); + + expect(decodeProtocolObject(checkout, 'Checkout')).toStrictEqual( + decodeProtocolObject(serializedCheckout, 'Checkout'), + ); +}); + +test('preserves schema-valid null instead of treating it as absent', () => { + const decoded = decodeProtocolObject( + { + ...wire, + fulfillment: { + available_methods: [ + {line_item_ids: [], type: 'shipping', fulfillable_on: null}, + ], + methods: undefined, + }, + }, + 'Checkout', + ); + + expect(decoded.fulfillment).toStrictEqual({ + availableMethods: [{lineItemIds: [], type: 'shipping', fulfillableOn: null}], + }); +}); + +test.each(['currency', 'totals', 'ucp'])( + 'rejects an undefined required Checkout.%s', + field => { + expectValidationError( + () => decodeProtocolObject({...wire, [field]: undefined}, 'Checkout'), + `Checkout.${field}`, + 'missing_required', + ); + }, +); + +test('rejects undefined and malformed required fields inside a present order', () => { + const permalink_url = 'https://example.test/orders/order-1'; + + expectValidationError( + () => decodeProtocolObject( + {...wire, order: {id: undefined, permalink_url}}, + 'Checkout', + ), + 'Checkout.order.id', + 'missing_required', + ); + expectValidationError( + () => decodeProtocolObject( + {...wire, order: {id: 'order-1', permalink_url: undefined}}, + 'Checkout', + ), + 'Checkout.order.permalink_url', + 'missing_required', + ); + expectValidationError( + () => decodeProtocolObject({...wire, order: 123}, 'Checkout'), + 'Checkout.order', + 'invalid_type', + ); +}); + +test('requires the version of a present ucp object', () => { + expectValidationError( + () => decodeProtocolObject({...wire, ucp: {version: undefined}}, 'Checkout'), + 'Checkout.ucp.version', + 'missing_required', + ); +}); + +test.each([ + [ + 'order.id', + {order: {id: 123, permalink_url: 'https://example.test/orders/order-1'}}, + ], + ['order.permalink_url', {order: {id: 'order-1', permalink_url: null}}], + ['ucp.version', {ucp: {version: {value: '2026-04-08'}}}], +])('rejects non-string Checkout.%s', (field, nested) => { + expectValidationError( + () => decodeProtocolObject({...wire, ...nested}, 'Checkout'), + `Checkout.${field}`, + 'invalid_type', + ); +}); + +test('does not accept inherited required fields', () => { + const inheritedCheckout = Object.create(wire) as Record; + expectValidationError( + () => decodeProtocolObject(inheritedCheckout, 'Checkout'), + 'Checkout.currency', + 'missing_required', + ); + + const inheritedOrder = Object.create({id: 'order-1'}) as Record< + string, + unknown + >; + inheritedOrder.permalink_url = 'https://example.test/orders/order-1'; + expectValidationError( + () => decodeProtocolObject({...wire, order: inheritedOrder}, 'Checkout'), + 'Checkout.order.id', + 'missing_required', + ); +}); + +test('does not include a value in its validation error', () => { + const malformedValue = {private_url: 'https://example.test/private/order-123'}; + expectValidationError( + () => decodeProtocolObject({...wire, currency: malformedValue}, 'Checkout'), + 'Checkout.currency', + 'invalid_type', + ); + expect(new ProtocolValidationError('Checkout.raw\nsecret', 'invalid_type')).toMatchObject({ + modelPath: 'ProtocolObject', + message: 'Invalid ProtocolObject', + }); +}); + +function expectValidationError( + decode: () => unknown, + modelPath: string, + reason: ProtocolValidationReason, +) { + try { + decode(); + throw new Error('Expected protocol validation to fail'); + } catch (error) { + expect(error).toBeInstanceOf(ProtocolValidationError); + expect(error).toMatchObject({ + modelPath, + reason, + message: `Invalid ${modelPath}`, + }); + } +}