fix(kernel): "any" serialization breaks private type instances (#1347)

When a "private" type (aka a class that is not visible in the `.jsii`
assembly) is passed from JS to Host through an `any`-typed entity, the
value was serialized by-value, passing only the instance's properties,
and omitting any methods.

This changes the behavior of the `SerializationClass.Any` serializer,
so that when an anonymous object is encountered, if it has a dynamic
getter, setter or any method (including a constructor), it is passed
by-reference instead of by-value.

Fixes aws/aws-cdk#6746

docs/specifications/2-type-system.md

@@ -201,8 +201,11 @@ term *primitive* encompasses `boolean`, `string`, and `number`.
 `interface` | `undefined` | :x:         | :x:         | :x:         | [Reference] | [Reference]
 `struct`    | `undefined` | :x:         | :x:         | :x:         | :x:         | [Value]
 `class`     | `undefined` | :x:         | :x:         | :x:         | [Reference] | [Reference]
-`any`       | `undefined` | [Date]      | [Identity]  | [Array]     | [Reference] | [Mapping]
+`any`       | `undefined` | [Date]      | [Identity]  | [Array]     | [Reference] | [Value] or [Reference]
 
+In the case of `object` being passed though `any`, the value may be serialized
+by [Value] only if the value being passed does not have any method or dynamic
+accessor. Otherwise, it must be passed by [Reference] instead.
 
 > :warning: The serialization behavior around `undefined` values is affected by
 > the `optional` attribute of the declared type. As discussed earlier, the `any`

packages/@jsii/kernel/lib/objects.ts

@@ -26,8 +26,7 @@ const JSII_SYMBOL = Symbol.for('__jsii__');
  * information.
  */
 export function jsiiTypeFqn(obj: any): string | undefined {
-  const jsii = obj.constructor[JSII_SYMBOL];
-  return jsii?.fqn;
+  return obj.constructor[JSII_SYMBOL]?.fqn;
 }
 
 /**

packages/@jsii/kernel/lib/serialization.ts

@@ -474,7 +474,7 @@ export const SERIALIZERS: {[k: string]: Serializer} = {
 
       // If this is or should be a reference type, pass or make the reference
       // (Like regular reftype serialization, but without the type derivation to an interface)
-      const jsiiType = jsiiTypeFqn(value);
+      const jsiiType = jsiiTypeFqn(value) ?? (isByReferenceOnly(value) ? EMPTY_OBJECT_FQN : undefined);
       if (jsiiType) { return host.objects.registerObject(value, jsiiType); }
 
       // At this point we have an object that is not of an exported type. Either an object
@@ -737,3 +737,32 @@ function compareSerializationClasses(l: SerializationClass, r: SerializationClas
   ];
   return order.indexOf(l) - order.indexOf(r);
 }
+
+/**
+ * Determines whether `obj` must be passed by-reference or if by-value is acceptable. For example,
+ * objects with methods, or dynamic getters (or setters) should be passed by-reference as a matter
+ * of security. The behavior in non-JS runtimes could otherwise differ from that in pure JS (if
+ * getters are not stable, etc...).
+ *
+ * @param obj the object to be tested.
+ *
+ * @returns true if `obj` must be passed by-reference.
+ */
+function isByReferenceOnly(obj: any): boolean {
+  if (Array.isArray(obj)) { return false; }
+
+  let curr = obj;
+  // Crawl up the prototype chain to look for dynamic properties or methods.
+  do {
+    for (const prop of Object.getOwnPropertyNames(curr)) {
+      const descr = Object.getOwnPropertyDescriptor(curr, prop);
+      if (descr?.get != null || descr?.set != null || typeof descr?.value === 'function') {
+        // Property has a dynamic getter, setter or is a constructor/method, so by-ref required!
+        return true;
+      }
+    }
+    // End when the parent proto is `Object`, which has no parent proto itself.
+  } while (Object.getPrototypeOf(curr = Object.getPrototypeOf(curr)) != null);
+
+  return false;
+}

packages/@jsii/kernel/test/serialization.test.ts

@@ -1,22 +1,61 @@
-import { OptionalValue, PrimitiveType } from '@jsii/spec';
+import { CANONICAL_ANY, OptionalValue, PrimitiveType } from '@jsii/spec';
+import { TOKEN_REF } from '../lib/api';
 import { ObjectTable } from '../lib/objects';
 import { SerializationClass, SerializerHost, SERIALIZERS } from '../lib/serialization';
 
+const TYPE_ANY: OptionalValue = { type: CANONICAL_ANY };
 const TYPE_BOOLEAN: OptionalValue = { type: { primitive: PrimitiveType.Boolean } };
 const TYPE_NUMBER: OptionalValue = { type: { primitive: PrimitiveType.Number } };
 const TYPE_STRING: OptionalValue = { type: { primitive: PrimitiveType.String } };
 const TYPE_VOID = 'void';
 
+const lookupType: SerializerHost['lookupType'] = jest.fn().mockName('host.lookupType');
+const host: SerializerHost = {
+  debug: jest.fn().mockName('host.debug'),
+  findSymbol: jest.fn().mockName('host.findSymbol'),
+  lookupType,
+  objects: new ObjectTable(lookupType),
+  recurse: jest.fn().mockName('host.recurse'),
+};
+
+describe(SerializationClass.Any, () => {
+  const anySerializer = SERIALIZERS[SerializationClass.Any];
+  class PrivateType {
+    private readonly randomValue = Math.random();
+
+    public random() {
+      return this.randomValue;
+    }
+  }
+
+  beforeEach(done => {
+    (host.recurse as jest.Mock<any, any>).mockImplementation((x: any, type: OptionalValue) => {
+      expect(type).toEqual(TYPE_ANY);
+      return anySerializer.serialize(x, type, host);
+    });
+    done();
+  });
+
+  describe(anySerializer.serialize, () => {
+    test('by-value object literal', () => {
+      expect(anySerializer.serialize({ literal: { integer: 1337 } }, TYPE_ANY, host))
+        .toEqual({ literal: { integer: 1337 } });
+    });
+
+    test('non-exported type instance', () => {
+      expect(anySerializer.serialize(new PrivateType, TYPE_ANY, host))
+        .toEqual({ [TOKEN_REF]: 'Object@10000' });
+    });
+
+    test('arrays', () => {
+      expect(anySerializer.serialize([{ literal: { integer: 1337 } }], TYPE_ANY, host))
+        .toEqual([{ literal: { integer: 1337 } }]);
+    });
+  });
+});
+
 describe(SerializationClass.Scalar, () => {
   const scalarSerializer = SERIALIZERS[SerializationClass.Scalar];
-  const lookupType: SerializerHost['lookupType'] = jest.fn().mockName('host.lookupType');
-  const host: SerializerHost = {
-    debug: jest.fn().mockName('host.debug'),
-    findSymbol: jest.fn().mockName('host.findSymbol'),
-    lookupType,
-    objects: new ObjectTable(lookupType),
-    recurse: jest.fn().mockName('host.recurse'),
-  };
 
   describe(scalarSerializer.deserialize, () => {
     describe('void', () => {

packages/@jsii/python-runtime/src/jsii/_reference_map.py

@@ -95,9 +95,9 @@ def resolve(self, kernel, ref):
             return data_type(**python_props)
         elif class_fqn in _enums:
             return _enums[class_fqn]
-        elif class_fqn == "Object" and ref.interfaces is not None:
+        elif class_fqn == "Object":
             # If any one interface is a struct, all of them are guaranteed to be (Kernel invariant)
-            if any(fqn in _data_types for fqn in ref.interfaces):
+            if ref.interfaces is not None and any(fqn in _data_types for fqn in ref.interfaces):
                 # Ugly delayed import here because I can't solve the cyclic
                 # package dependency right now :(.
                 from ._runtime import python_jsii_mapping
@@ -117,9 +117,7 @@ def resolve_id(self, id):
         return self._refs[id]
 
     def build_interface_proxies_for_ref(self, ref):
-        if ref.interfaces is None:
-            raise AssertionError("Attempted to create interface proxies for ObjectRef without interfaces!")
-        ifaces = [_interfaces[fqn] for fqn in ref.interfaces]
+        ifaces = [_interfaces[fqn] for fqn in ref.interfaces or []]
         classes = [iface.__jsii_proxy_class__() for iface in ifaces]
         insts = [klass.__new__(klass) for klass in classes]
         for inst in insts:

packages/@jsii/python-runtime/tests/README.md

@@ -0,0 +1,10 @@
+# Python jsii runtime tests
+## Development Iteration
+
+When iterating on the jsii runtime for Python, the develomer must run
+`yarn build` before making a subsequent attempt at running `pytest` (e.g: via
+`yarn test`). This is because the tests run on the code installed in `.env` and
+this is updated only by `yarn build`.
+
+Note also that stack traces from test failures will point to the `.env` tree,
+so be careful when using IDE linkage to navigate to points of that trace.