diff --git a/yarn-project/foundation/src/json-rpc/class_converter.ts b/yarn-project/foundation/src/json-rpc/class_converter.ts index 2be9dab1d5b5..c1e9478380ef 100644 --- a/yarn-project/foundation/src/json-rpc/class_converter.ts +++ b/yarn-project/foundation/src/json-rpc/class_converter.ts @@ -158,7 +158,8 @@ export class ClassConverter { * @returns If it is a registered class. */ isRegisteredClass(obj: any) { - return this.toName.has(obj); + const name = obj.prototype.constructor.name; + return this.toName.has(obj) || this.isRegisteredClassName(name); } /** * Convert a JSON-like object to a class object. @@ -182,10 +183,23 @@ export class ClassConverter { * @returns The class object. */ toJsonObj(classObj: any): JsonEncodedClass | StringEncodedClass { - const result = this.toName.get(classObj.constructor); - assert(result, `Could not find class in lookup.`); - const [type, encoding] = result; + const { type, encoding } = this.lookupObject(classObj); const data = encoding === 'string' ? classObj.toString() : classObj.toJSON(); return { type: type!, data }; } + + /** + * Loads the corresponding type for this class based on constructor first and constructor name if not found. + * Constructor match works in the event of a minifier changing function names, and constructor name match + * works in the event of duplicated instances of node modules being loaded (see #1826). + * @param classObj - Object to lookup in the registered types. + * @returns Registered type name and encoding. + */ + private lookupObject(classObj: any) { + const nameResult = this.toName.get(classObj.constructor); + if (nameResult) return { type: nameResult[0], encoding: nameResult[1] }; + const classResult = this.toClass.get(classObj.constructor.name); + if (classResult) return { type: classObj.constructor.name, encoding: classResult[1] }; + throw new Error(`Could not find class ${classObj.constructor.name} in lookup.`); + } } diff --git a/yarn-project/foundation/src/json-rpc/convert.test.ts b/yarn-project/foundation/src/json-rpc/convert.test.ts index 4927b9c68a08..53b567769046 100644 --- a/yarn-project/foundation/src/json-rpc/convert.test.ts +++ b/yarn-project/foundation/src/json-rpc/convert.test.ts @@ -2,6 +2,8 @@ import { Buffer } from 'buffer'; import { ClassConverter } from './class_converter.js'; import { convertBigintsInObj, convertFromJsonObj, convertToJsonObj } from './convert.js'; +import { ToStringClass as ToStringClassA } from './fixtures/class_a.js'; +import { ToStringClass as ToStringClassB } from './fixtures/class_b.js'; import { TestNote } from './fixtures/test_state.js'; const TEST_BASE64 = 'YmFzZTY0IGRlY29kZXI='; @@ -24,3 +26,52 @@ test('does not convert a string', () => { expect(convertBigintsInObj('hello')).toEqual('hello'); expect(convertBigintsInObj({ msg: 'hello' })).toEqual({ msg: 'hello' }); }); + +test('converts a registered class', () => { + const cc = new ClassConverter({ ToStringClass: ToStringClassA }); + const obj = { content: new ToStringClassA('a', 'b') }; + const serialised = convertToJsonObj(cc, obj); + const deserialised = convertFromJsonObj(cc, serialised) as { content: ToStringClassA }; + expect(deserialised.content).toBeInstanceOf(ToStringClassA); + expect(deserialised.content.x).toEqual('a'); + expect(deserialised.content.y).toEqual('b'); +}); + +test('converts a class by name in the event of duplicate modules being loaded', () => { + expect(ToStringClassA.prototype.constructor.name).toEqual('ToStringClass'); + expect(ToStringClassB.prototype.constructor.name).toEqual('ToStringClass'); + const cc = new ClassConverter({ ToStringClass: ToStringClassA }); + const obj = { content: new ToStringClassB('a', 'b') }; + const serialised = convertToJsonObj(cc, obj); + const deserialised = convertFromJsonObj(cc, serialised) as { content: ToStringClassA }; + expect(deserialised.content).toBeInstanceOf(ToStringClassA); + expect(deserialised.content.x).toEqual('a'); + expect(deserialised.content.y).toEqual('b'); +}); + +test('converts a class by constructor instead of name in the event of minified bundle', () => { + const cc = new ClassConverter({ NotMinifiedToStringClassName: ToStringClassA }); + const obj = { content: new ToStringClassA('a', 'b') }; + const serialised = convertToJsonObj(cc, obj); + const deserialised = convertFromJsonObj(cc, serialised) as { content: ToStringClassA }; + expect(deserialised.content).toBeInstanceOf(ToStringClassA); + expect(deserialised.content.x).toEqual('a'); + expect(deserialised.content.y).toEqual('b'); +}); + +test('converts a plain object', () => { + const obj = { a: 10, b: [20, 30], c: 'foo' }; + const cc = new ClassConverter(); + expect(convertFromJsonObj(cc, convertToJsonObj(cc, obj))).toEqual(obj); +}); + +test('refuses to convert to json an unknown class', () => { + const cc = new ClassConverter(); + expect(() => convertToJsonObj(cc, { content: new ToStringClassA('a', 'b') })).toThrowError(/not registered/); +}); + +test('refuses to convert from json an unknown class', () => { + const cc = new ClassConverter({ ToStringClass: ToStringClassA }); + const serialised = convertToJsonObj(cc, { content: new ToStringClassA('a', 'b') }); + expect(() => convertFromJsonObj(new ClassConverter(), serialised)).toThrowError(/not registered/); +}); diff --git a/yarn-project/foundation/src/json-rpc/convert.ts b/yarn-project/foundation/src/json-rpc/convert.ts index ac6cef5f381a..7ca545ddbd4d 100644 --- a/yarn-project/foundation/src/json-rpc/convert.ts +++ b/yarn-project/foundation/src/json-rpc/convert.ts @@ -3,6 +3,31 @@ import cloneDeepWith from 'lodash.clonedeepwith'; import { ClassConverter } from './class_converter.js'; +/** + * Check prototype chain to determine if an object is 'plain' (not a class instance). + * @param obj - The object to check. + * @returns True if the object is 'plain'. + */ +function isPlainObject(obj: any) { + if (obj === null) { + return false; + } + + let proto = obj; + let counter = 0; + const MAX_PROTOTYPE_CHAIN_LENGTH = 1000; // Adjust as needed + while (Object.getPrototypeOf(proto) !== null) { + if (counter >= MAX_PROTOTYPE_CHAIN_LENGTH) { + // This is a failsafe in case circular prototype chain has been created. It should not be hit + return false; + } + proto = Object.getPrototypeOf(proto); + counter++; + } + + return Object.getPrototypeOf(obj) === proto; +} + /** * Recursively looks through an object for bigints and converts them to object format. * @param obj - The object to convert. @@ -59,8 +84,12 @@ export function convertFromJsonObj(cc: ClassConverter, obj: any): any { } // Is this a convertible type? - if (typeof obj.type === 'string' && cc.isRegisteredClassName(obj.type)) { - return cc.toClassObj(obj); + if (typeof obj.type === 'string') { + if (cc.isRegisteredClassName(obj.type)) { + return cc.toClassObj(obj); + } else { + throw new Error(`Object ${obj.type} not registered for serialisation`); + } } // Is this an array? @@ -118,7 +147,10 @@ export function convertToJsonObj(cc: ClassConverter, obj: any): any { } return newObj; } - + // Throw if this is a non-primitive class that was not registered + if (typeof obj === 'object' && !isPlainObject(obj)) { + throw new Error(`Object ${obj.constructor.name} not registered for serialisation`); + } // Leave alone, assume JSON primitive return obj; } diff --git a/yarn-project/foundation/src/json-rpc/fixtures/class_a.ts b/yarn-project/foundation/src/json-rpc/fixtures/class_a.ts new file mode 100644 index 000000000000..58bb9a0cc193 --- /dev/null +++ b/yarn-project/foundation/src/json-rpc/fixtures/class_a.ts @@ -0,0 +1,15 @@ +/** + * Test class for testing string converter. + */ +export class ToStringClass { + constructor(/** A value */ public readonly x: string, /** Another value */ public readonly y: string) {} + + toString(): string { + return [this.x, this.y].join('-'); + } + + static fromString(value: string) { + const [x, y] = value.split('-'); + return new ToStringClass(x, y); + } +} diff --git a/yarn-project/foundation/src/json-rpc/fixtures/class_b.ts b/yarn-project/foundation/src/json-rpc/fixtures/class_b.ts new file mode 100644 index 000000000000..58bb9a0cc193 --- /dev/null +++ b/yarn-project/foundation/src/json-rpc/fixtures/class_b.ts @@ -0,0 +1,15 @@ +/** + * Test class for testing string converter. + */ +export class ToStringClass { + constructor(/** A value */ public readonly x: string, /** Another value */ public readonly y: string) {} + + toString(): string { + return [this.x, this.y].join('-'); + } + + static fromString(value: string) { + const [x, y] = value.split('-'); + return new ToStringClass(x, y); + } +}