fix: Disallow unregistered classes in JSON RPC interface and match by name

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.`);
+  }
 }

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,33 @@ 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', () => {
+  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');
+});

yarn-project/foundation/src/json-rpc/convert.ts

@@ -75,7 +75,10 @@ export function convertFromJsonObj(cc: ClassConverter, obj: any): any {
     }
     return newObj;
   }
-
+  // Throw if this is a non-primitive class that was not registered
+  if (typeof obj === 'object' && Object.getPrototypeOf(obj) === Object.getPrototypeOf({})) {
+    throw new Error(`Class ${obj.constructor.name} not registered for deserialisation`);
+  }
   // Leave alone, assume JSON primitive
   return obj;
 }
@@ -118,7 +121,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' && Object.getPrototypeOf(obj) === Object.getPrototypeOf({})) {
+    throw new Error(`Object ${obj.constructor.name} not registered for serialisation`);
+  }
   // Leave alone, assume JSON primitive
   return obj;
 }

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);
+  }
+}

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);
+  }
+}