Skip to content

Commit

Permalink
fix(kernel): cannot pass decorated structs as "any"
Browse files Browse the repository at this point in the history
When a struct is serialized with a $jsii.struct decoration from a jsii host language and passed into a value typed `any`, the kernel ignored the decoration. This resulted in a malformed object passed to the javascript code.

This fix undecorates the object and continues to deserialize it as `any`.

Fixes aws/aws-cdk#5066
  • Loading branch information
Elad Ben-Israel committed Nov 18, 2019
1 parent 08c4294 commit 684a368
Show file tree
Hide file tree
Showing 16 changed files with 256 additions and 7 deletions.
13 changes: 13 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2331,3 +2331,16 @@ export class ObjectWithPropertyProvider {

private constructor() { }
}

/**
* Make sure structs are un-decorated on the way in.
*
* @see https://github.com/aws/aws-cdk/issues/5066
*/
export class JsonFormatter {
public static stringify(value: any): string {
return JSON.stringify(value, null, 2);
}

private constructor() { }
}
43 changes: 42 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -6826,6 +6826,47 @@
}
]
},
"jsii-calc.JsonFormatter": {
"assembly": "jsii-calc",
"docs": {
"see": "https://github.com/aws/aws-cdk/issues/5066",
"stability": "experimental",
"summary": "Make sure structs are un-decorated on the way in."
},
"fqn": "jsii-calc.JsonFormatter",
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2340
},
"methods": [
{
"docs": {
"stability": "experimental"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2341
},
"name": "stringify",
"parameters": [
{
"name": "value",
"type": {
"primitive": "any"
}
}
],
"returns": {
"type": {
"primitive": "string"
}
},
"static": true
}
],
"name": "JsonFormatter"
},
"jsii-calc.LoadBalancedFargateServiceProps": {
"assembly": "jsii-calc",
"datatype": true,
Expand Down Expand Up @@ -11413,5 +11454,5 @@
}
},
"version": "0.20.6",
"fingerprint": "veHd1Q/376CoMR3O/DjjAiH9aVD/jcwnPEg88barg9I="
"fingerprint": "xXC7/htdFP0Ns+aNf856G/K6PBPRJjPlsg5KtO3bEXY="
}
Original file line number Diff line number Diff line change
Expand Up @@ -1297,5 +1297,18 @@ public void CanUseInterfaceSetters()
obj.Property = "New Value";
Assert.True(obj.WasSet());
}

[Fact(DisplayName = Prefix + nameof(StructsAreUndecoratedOntheWayToKernel))]
public void StructsAreUndecoratedOntheWayToKernel()
{
var json = JsonFormatter.Stringify(new StructB {RequiredString = "Bazinga!", OptionalBoolean = false});
var actual = JObject.Parse(json);

var expected = new JObject();
expected.Add("RequiredString", "Bazinga!");
expected.Add("OptionalBoolean", false);

Assert.Equal(expected, actual);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ public sealed class JsiiByValueAttribute : Attribute
{
public JsiiByValueAttribute(string fqn)
{
Fqn = fqn ?? throw new ArgumentNullException(nameof(fqn));
FullyQualifiedName = fqn ?? throw new ArgumentNullException(nameof(fqn));
}

public string Fqn { get; }
public string FullyQualifiedName { get; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ protected override bool TryConvertClass(Type type, IReferenceMap referenceMap, o
}

var structInfo = new JObject();
structInfo.Add(new JProperty("fqn", byValueAttribute.Fqn));
structInfo.Add(new JProperty("fqn", byValueAttribute.FullyQualifiedName));
structInfo.Add(new JProperty("data", data));

var resultObject = new JObject();
Expand Down Expand Up @@ -353,6 +353,12 @@ TypeReference InferType(IReferenceMap referenceMap, Type type)
return new TypeReference(enumAttribute.FullyQualifiedName);
}

JsiiByValueAttribute structAttribute = type.GetCustomAttribute<JsiiByValueAttribute>();
if (structAttribute != null)
{
return new TypeReference(structAttribute.FullyQualifiedName);
}

if (typeof(string).IsAssignableFrom(type))
{
return new TypeReference(primitive: PrimitiveType.String);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package software.amazon.jsii.testing;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.junit.Test;
Expand Down Expand Up @@ -1580,4 +1581,17 @@ public void canUseInterfaceSetters() {
obj.setProperty("New Value");
assertTrue(obj.wasSet());
}

@Test
public void structsAreUndecoratedOntheWayToKernel() throws IOException {
final ObjectMapper om = new ObjectMapper();
final String json = JsonFormatter.stringify(StructB.builder().requiredString("Bazinga!").optionalBoolean(false).build());
final JsonNode actual = om.readTree(json);

final ObjectNode expected = om.createObjectNode();
expected.put("requiredString", "Bazinga!");
expected.put("optionalBoolean", Boolean.FALSE);

assertEquals(expected, actual);
}
}
11 changes: 10 additions & 1 deletion packages/jsii-kernel/lib/serialization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
*/

import * as spec from 'jsii-spec';
import { isObjRef, isWireDate, isWireEnum, isWireMap, ObjRef, TOKEN_DATE, TOKEN_ENUM, TOKEN_MAP, WireDate, WireEnum } from './api';
import { isObjRef, isWireDate, isWireEnum, isWireMap, ObjRef, TOKEN_DATE, TOKEN_ENUM, TOKEN_MAP, WireDate, WireEnum, isWireStruct, TOKEN_STRUCT } from './api';
import { jsiiTypeFqn, objectReference, ObjectTable } from './objects';
import { api } from '.';

Expand Down Expand Up @@ -520,6 +520,15 @@ export const SERIALIZERS: {[k: string]: Serializer} = {
host.debug('ANY is a Ref');
return host.objects.findObject(value).instance;
}

// if the value has a struct token, it was serialized by a typed jsii
// struct, but since the deserialization target is ANY, all we can do is
// strip the data from $jsii.struct and continue to deserialize as ANY.
if (isWireStruct(value)) {
const { fqn, data } = value[TOKEN_STRUCT];
host.debug(`ANY is a struct of type ${fqn}`);
return SERIALIZERS[SerializationClass.Any].deserialize(data, _type, host);
}

// At this point again, deserialize by-value.
host.debug('ANY is a Map');
Expand Down
7 changes: 7 additions & 0 deletions packages/jsii-kernel/test/kernel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1218,6 +1218,13 @@ defineTest('correctly deserializes struct unions', (sandbox) => {
}
});

defineTest('structs are undecorated on the way to kernel', sandbox => {
const input = { '$jsii.struct': { 'fqn': 'jsii-calc.StructB', 'data': { 'requiredString': 'Bazinga!', 'optionalBoolean': false } } };
const ret = sandbox.sinvoke({ fqn: 'jsii-calc.JsonFormatter', method: 'stringify', args: [input] });
const json = JSON.parse(ret.result);
expect(json).toStrictEqual({ 'requiredString': 'Bazinga!', 'optionalBoolean': false });
});

// =================================================================================================

const testNames: { [name: string]: boolean } = { };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6826,6 +6826,47 @@
}
]
},
"jsii-calc.JsonFormatter": {
"assembly": "jsii-calc",
"docs": {
"see": "https://github.com/aws/aws-cdk/issues/5066",
"stability": "experimental",
"summary": "Make sure structs are un-decorated on the way in."
},
"fqn": "jsii-calc.JsonFormatter",
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2340
},
"methods": [
{
"docs": {
"stability": "experimental"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 2341
},
"name": "stringify",
"parameters": [
{
"name": "value",
"type": {
"primitive": "any"
}
}
],
"returns": {
"type": {
"primitive": "string"
}
},
"static": true
}
],
"name": "JsonFormatter"
},
"jsii-calc.LoadBalancedFargateServiceProps": {
"assembly": "jsii-calc",
"datatype": true,
Expand Down Expand Up @@ -11413,5 +11454,5 @@
}
},
"version": "0.20.6",
"fingerprint": "veHd1Q/376CoMR3O/DjjAiH9aVD/jcwnPEg88barg9I="
"fingerprint": "xXC7/htdFP0Ns+aNf856G/K6PBPRJjPlsg5KtO3bEXY="
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
using Amazon.JSII.Runtime.Deputy;

namespace Amazon.JSII.Tests.CalculatorNamespace
{
/// <summary>Make sure structs are un-decorated on the way in.</summary>
/// <remarks>
/// stability: Experimental
/// see:
/// https://github.com/aws/aws-cdk/issues/5066
/// </remarks>
[JsiiClass(nativeType: typeof(Amazon.JSII.Tests.CalculatorNamespace.JsonFormatter), fullyQualifiedName: "jsii-calc.JsonFormatter")]
public class JsonFormatter : DeputyBase
{
protected JsonFormatter(ByRefValue reference): base(reference)
{
}

protected JsonFormatter(DeputyProps props): base(props)
{
}

/// <remarks>
/// stability: Experimental
/// </remarks>
[JsiiMethod(name: "stringify", returnsJson: "{\"type\":{\"primitive\":\"string\"}}", parametersJson: "[{\"name\":\"value\",\"type\":{\"primitive\":\"any\"}}]")]
public static string Stringify(object @value)
{
return InvokeStaticMethod<string>(typeof(Amazon.JSII.Tests.CalculatorNamespace.JsonFormatter), new System.Type[]{typeof(object)}, new object[]{@value});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ protected Class<?> resolveClass(final String fqn) throws ClassNotFoundException
case "jsii-calc.Jsii487Derived": return software.amazon.jsii.tests.calculator.Jsii487Derived.class;
case "jsii-calc.Jsii496Derived": return software.amazon.jsii.tests.calculator.Jsii496Derived.class;
case "jsii-calc.JsiiAgent": return software.amazon.jsii.tests.calculator.JsiiAgent.class;
case "jsii-calc.JsonFormatter": return software.amazon.jsii.tests.calculator.JsonFormatter.class;
case "jsii-calc.LoadBalancedFargateServiceProps": return software.amazon.jsii.tests.calculator.LoadBalancedFargateServiceProps.class;
case "jsii-calc.Multiply": return software.amazon.jsii.tests.calculator.Multiply.class;
case "jsii-calc.Negate": return software.amazon.jsii.tests.calculator.Negate.class;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package software.amazon.jsii.tests.calculator;

/**
* Make sure structs are un-decorated on the way in.
*
* EXPERIMENTAL
*
* @see https://github.com/aws/aws-cdk/issues/5066
*/
@javax.annotation.Generated(value = "jsii-pacmak")
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
@software.amazon.jsii.Jsii(module = software.amazon.jsii.tests.calculator.$Module.class, fqn = "jsii-calc.JsonFormatter")
public class JsonFormatter extends software.amazon.jsii.JsiiObject {

protected JsonFormatter(final software.amazon.jsii.JsiiObjectRef objRef) {
super(objRef);
}

protected JsonFormatter(final software.amazon.jsii.JsiiObject.InitializationMode initializationMode) {
super(initializationMode);
}

/**
* EXPERIMENTAL
*
* @param value This parameter is required.
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
public static java.lang.String stringify(final java.lang.Object value) {
return software.amazon.jsii.JsiiObject.jsiiStaticCall(software.amazon.jsii.tests.calculator.JsonFormatter.class, "stringify", java.lang.String.class, new Object[] { value });
}
}
Loading

0 comments on commit 684a368

Please sign in to comment.