Skip to content

Commit

Permalink
fix(.net): occasional incorrect param type cast (#568)
Browse files Browse the repository at this point in the history
When structures (as literal JSON blocks) get passed through a "map"
argument, a `JObject` would be passed to the method despite the declared
type on the .NET code is `IDictionary`, resulting in a crash.

This code forcefully converts `JObject` to `IDictionary<string, objetc>`
in such code paths.

Fixes aws/aws-cdk#3093
  • Loading branch information
RomainMuller authored Jul 1, 2019
1 parent a4de2d8 commit c89d0fa
Show file tree
Hide file tree
Showing 17 changed files with 415 additions and 4 deletions.
15 changes: 15 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1726,3 +1726,18 @@ export enum SingletonIntEnum {
/** Elite! */
SINGLETON_INT = 1337
}

/**
* Verifies proper type handling through dynamic overrides.
*/
export class DataRenderer {
constructor() { }

public render(data: MyFirstStruct = { anumber: 42, astring: 'bazinga!' }): string {
return this.renderMap(data);
}

public renderMap(map: { [key: string]: any }): string {
return JSON.stringify(map, null, 2);
}
}
75 changes: 74 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -2082,6 +2082,79 @@
],
"name": "ConsumersOfThisCrazyTypeSystem"
},
"jsii-calc.DataRenderer": {
"assembly": "jsii-calc",
"docs": {
"stability": "experimental",
"summary": "Verifies proper type handling through dynamic overrides."
},
"fqn": "jsii-calc.DataRenderer",
"initializer": {
"docs": {
"stability": "experimental"
}
},
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1733
},
"methods": [
{
"docs": {
"stability": "experimental"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1736
},
"name": "render",
"parameters": [
{
"name": "data",
"optional": true,
"type": {
"fqn": "@scope/jsii-calc-lib.MyFirstStruct"
}
}
],
"returns": {
"type": {
"primitive": "string"
}
}
},
{
"docs": {
"stability": "experimental"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1740
},
"name": "renderMap",
"parameters": [
{
"name": "map",
"type": {
"collection": {
"elementtype": {
"primitive": "any"
},
"kind": "map"
}
}
}
],
"returns": {
"type": {
"primitive": "string"
}
}
}
],
"name": "DataRenderer"
},
"jsii-calc.DefaultedConstructorArgument": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -8778,5 +8851,5 @@
}
},
"version": "0.13.2",
"fingerprint": "IzBpmwowWT7JOt7LEK9v3wIGmNWcpmqf9O1C927cs8o="
"fingerprint": "iCTmGcYc3DTgEt3Y8PBPaiJNOI7O9tRRGI49RI6rAwk="
}
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,22 @@ public void CorrectlyReturnsFromVoidCallback()
Assert.True(voidCallback.MethodWasCalled);
}

[Fact(DisplayName = Prefix + nameof(CallbacksCorrectlyDeserializeArguments))]
public void CallbacksCorrectlyDeserializeArguments()
{
var obj = new DataRendererSubclass();
Assert.Equal("{\n \"anumber\": 42,\n \"astring\": \"bazinga!\"\n}", obj.Render(null));
}

class DataRendererSubclass : DataRenderer
{
[JsiiMethod("renderMap", returnsJson: "{\"type\":{\"primitive\":\"string\"}}", parametersJson: "[{\"name\":\"map\",\"type\":{\"collection\":{\"kind\":\"map\",\"elementtype\":{\"primitive\":\"any\"}}}}]", isOverride: true)]
public override string RenderMap(IDictionary<string, object> map)
{
return base.RenderMap(map);
}
}

class VoidCallbackImpl : VoidCallback
{
protected override void OverrideMe()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Amazon.JSII.Runtime.Services.Converters;
using Newtonsoft.Json.Linq;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;

Expand Down Expand Up @@ -136,6 +137,22 @@ private static object FromKernel(object obj, IReferenceMap referenceMap)
{
return referenceMap.GetOrCreateNativeReference(new ByRefValue(prop.Value.Value<String>()));
}

/*
* Turning all outstanding JObjects to IDictionary<string, object> (recursively), as the code generator
* will have emitted IDictionary<string, object> for maps of string to <anything>. Not doing so would
* result in an ArgumentError for not being able to convert JObject to IDictionary.
*/
var dict = ((JObject)obj).ToObject<Dictionary<string, object>>();
foreach (var key in dict.Keys)
{
var value = dict[key];
if (value != null && value.GetType() == typeof(JObject))
{
dict[key] = FromKernel(value, referenceMap);
}
}
return dict;
}
return obj;
}
Expand All @@ -145,7 +162,7 @@ internal class CallbackResult : OptionalValue
{
public CallbackResult(IOptionalValue optionalValue, object value)
: this(optionalValue?.Type, optionalValue?.IsOptional ?? false, value) {}

private CallbackResult(TypeReference type, bool isOptional, object value): base(type, isOptional)
{
Value = value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import software.amazon.jsii.tests.calculator.CalculatorProps;
import software.amazon.jsii.tests.calculator.ClassWithPrivateConstructorAndAutomaticProperties;
import software.amazon.jsii.tests.calculator.Constructors;
import software.amazon.jsii.tests.calculator.DataRenderer;
import software.amazon.jsii.tests.calculator.DerivedStruct;
import software.amazon.jsii.tests.calculator.DoNotOverridePrivates;
import software.amazon.jsii.tests.calculator.DoubleTrouble;
Expand Down Expand Up @@ -1037,6 +1038,16 @@ public void variadicMethodCanBeInvoked() {
assertEquals(Arrays.asList(1, 3, 4, 5, 6), result);
}

@Test
public void callbacsCorrectlyDeserializeArguments() {
final DataRenderer renderer = new DataRenderer() {
public final String renderMap(final Map<String, Object> map) {
return super.renderMap(map);
}
};
assertEquals("{\n \"anumber\": 42,\n \"astring\": \"bazinga!\"\n}", renderer.render());
}

static class PartiallyInitializedThisConsumerImpl extends PartiallyInitializedThisConsumer {
@Override
public String consumePartiallyInitializedThis(final ConstructorPassesThisOut obj,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2082,6 +2082,79 @@
],
"name": "ConsumersOfThisCrazyTypeSystem"
},
"jsii-calc.DataRenderer": {
"assembly": "jsii-calc",
"docs": {
"stability": "experimental",
"summary": "Verifies proper type handling through dynamic overrides."
},
"fqn": "jsii-calc.DataRenderer",
"initializer": {
"docs": {
"stability": "experimental"
}
},
"kind": "class",
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1733
},
"methods": [
{
"docs": {
"stability": "experimental"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1736
},
"name": "render",
"parameters": [
{
"name": "data",
"optional": true,
"type": {
"fqn": "@scope/jsii-calc-lib.MyFirstStruct"
}
}
],
"returns": {
"type": {
"primitive": "string"
}
}
},
{
"docs": {
"stability": "experimental"
},
"locationInModule": {
"filename": "lib/compliance.ts",
"line": 1740
},
"name": "renderMap",
"parameters": [
{
"name": "map",
"type": {
"collection": {
"elementtype": {
"primitive": "any"
},
"kind": "map"
}
}
}
],
"returns": {
"type": {
"primitive": "string"
}
}
}
],
"name": "DataRenderer"
},
"jsii-calc.DefaultedConstructorArgument": {
"assembly": "jsii-calc",
"docs": {
Expand Down Expand Up @@ -8778,5 +8851,5 @@
}
},
"version": "0.13.2",
"fingerprint": "IzBpmwowWT7JOt7LEK9v3wIGmNWcpmqf9O1C927cs8o="
"fingerprint": "iCTmGcYc3DTgEt3Y8PBPaiJNOI7O9tRRGI49RI6rAwk="
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
using Amazon.JSII.Runtime.Deputy;
using Amazon.JSII.Tests.CalculatorNamespace.LibNamespace;
using System.Collections.Generic;

namespace Amazon.JSII.Tests.CalculatorNamespace
{
/// <summary>Verifies proper type handling through dynamic overrides.</summary>
/// <remarks>stability: Experimental</remarks>
[JsiiClass(nativeType: typeof(DataRenderer), fullyQualifiedName: "jsii-calc.DataRenderer")]
public class DataRenderer : DeputyBase
{
/// <remarks>stability: Experimental</remarks>
public DataRenderer(): base(new DeputyProps(new object[]{}))
{
}

protected DataRenderer(ByRefValue reference): base(reference)
{
}

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

/// <remarks>stability: Experimental</remarks>
[JsiiMethod(name: "render", returnsJson: "{\"type\":{\"primitive\":\"string\"}}", parametersJson: "[{\"name\":\"data\",\"type\":{\"fqn\":\"@scope/jsii-calc-lib.MyFirstStruct\"},\"optional\":true}]")]
public virtual string Render(IMyFirstStruct data)
{
return InvokeInstanceMethod<string>(new object[]{data});
}

/// <remarks>stability: Experimental</remarks>
[JsiiMethod(name: "renderMap", returnsJson: "{\"type\":{\"primitive\":\"string\"}}", parametersJson: "[{\"name\":\"map\",\"type\":{\"collection\":{\"kind\":\"map\",\"elementtype\":{\"primitive\":\"any\"}}}}]")]
public virtual string RenderMap(IDictionary<string, object> map)
{
return InvokeInstanceMethod<string>(new object[]{map});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ protected Class<?> resolveClass(final String fqn) throws ClassNotFoundException
case "jsii-calc.ConstructorPassesThisOut": return software.amazon.jsii.tests.calculator.ConstructorPassesThisOut.class;
case "jsii-calc.Constructors": return software.amazon.jsii.tests.calculator.Constructors.class;
case "jsii-calc.ConsumersOfThisCrazyTypeSystem": return software.amazon.jsii.tests.calculator.ConsumersOfThisCrazyTypeSystem.class;
case "jsii-calc.DataRenderer": return software.amazon.jsii.tests.calculator.DataRenderer.class;
case "jsii-calc.DefaultedConstructorArgument": return software.amazon.jsii.tests.calculator.DefaultedConstructorArgument.class;
case "jsii-calc.DeprecatedClass": return software.amazon.jsii.tests.calculator.DeprecatedClass.class;
case "jsii-calc.DeprecatedEnum": return software.amazon.jsii.tests.calculator.DeprecatedEnum.class;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package software.amazon.jsii.tests.calculator;

/**
* Verifies proper type handling through dynamic overrides.
*
* EXPERIMENTAL
*/
@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.DataRenderer")
public class DataRenderer extends software.amazon.jsii.JsiiObject {
protected DataRenderer(final software.amazon.jsii.JsiiObject.InitializationMode mode) {
super(mode);
}
/**
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
public DataRenderer() {
super(software.amazon.jsii.JsiiObject.InitializationMode.Jsii);
software.amazon.jsii.JsiiEngine.getInstance().createNewObject(this);
}

/**
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
public java.lang.String render(@javax.annotation.Nullable final software.amazon.jsii.tests.calculator.lib.MyFirstStruct data) {
return this.jsiiCall("render", java.lang.String.class, new Object[] { data });
}

/**
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
public java.lang.String render() {
return this.jsiiCall("render", java.lang.String.class);
}

/**
* EXPERIMENTAL
*/
@software.amazon.jsii.Stability(software.amazon.jsii.Stability.Level.Experimental)
public java.lang.String renderMap(final java.util.Map<java.lang.String, java.lang.Object> map) {
return this.jsiiCall("renderMap", java.lang.String.class, new Object[] { java.util.Objects.requireNonNull(map, "map is required") });
}
}
Loading

0 comments on commit c89d0fa

Please sign in to comment.