Skip to content

Commit

Permalink
fix(dotnet): AnonymousObject fails runtime type checks (#3709)
Browse files Browse the repository at this point in the history
The `AnonymousObject` class is used when a value is passed from JS to
.NET through a union or `any` typed return point, and there is not
sufficient runtime type information on the value to decisively identify
its dynamic type. The `AnonymousObject` class will be converted by the
jsii runtime to any interface type implicitly (even if that is
technically not correct), and it must hence be allowed through runtime
type-checks for interfaces. Essentially, this is a blind spot of the
runtime type checks, and should not cause valid code to break.



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
RomainMuller committed Aug 18, 2022
1 parent 6b8e75d commit e7fadc0
Show file tree
Hide file tree
Showing 15 changed files with 1,216 additions and 185 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@

using System;
using System.Collections.Generic;
using Amazon.JSII.Runtime.Deputy;
using Amazon.JSII.Tests.CalculatorNamespace;
using Amazon.JSII.Tests.CalculatorNamespace.Anonymous;
using Xunit;

#pragma warning disable CS0612
Expand All @@ -13,17 +15,17 @@ public sealed class TypeCheckingTests : IClassFixture<ServiceContainerFixture>,
const string Prefix = nameof(TypeCheckingTests) + ".";

private readonly IDisposable _serviceContainerFixture;

public TypeCheckingTests(ServiceContainerFixture serviceContainerFixture)
{
_serviceContainerFixture = serviceContainerFixture;
}

void IDisposable.Dispose()
{
_serviceContainerFixture.Dispose();
}

[Fact(DisplayName = Prefix + nameof(Constructor))]
public void Constructor()
{
Expand All @@ -39,7 +41,7 @@ public void Constructor()
);
Assert.Equal("Expected argument unionProperty[0][\"bad\"] to be one of: Amazon.JSII.Tests.CalculatorNamespace.IStructA, Amazon.JSII.Tests.CalculatorNamespace.IStructB; received System.String (Parameter 'unionProperty')", exception.Message);
}

[Fact(DisplayName = Prefix + nameof(Setter))]
public void Setter()
{
Expand All @@ -55,13 +57,21 @@ public void Setter()
});
Assert.Equal("Expected value[0][\"bad\"] to be one of: Amazon.JSII.Tests.CalculatorNamespace.IStructA, Amazon.JSII.Tests.CalculatorNamespace.IStructB; received System.String (Parameter 'value')", exception.Message);
}

[Fact(DisplayName = Prefix + nameof(StaticMethod))]
public void StaticMethod()
{
var exception = Assert.Throws<System.ArgumentException>(() =>
StructUnionConsumer.IsStructA("Not a StructA"));
Assert.Equal("Expected argument struct to be one of: Amazon.JSII.Tests.CalculatorNamespace.IStructA, Amazon.JSII.Tests.CalculatorNamespace.IStructB; received System.String (Parameter 'struct')", exception.Message);
}

[Fact(DisplayName = Prefix + nameof(AnonymousObjectIsValid))]
public void AnonymousObjectIsValid()
{
var anonymousObject = UseOptions.Provide("A");
Assert.IsType<AnonymousObject>(anonymousObject);
Assert.Equal("A", UseOptions.Consume(anonymousObject));
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
namespace Amazon.JSII.Runtime.Deputy
{
internal sealed class AnonymousObject : DeputyBase
public sealed class AnonymousObject : DeputyBase
{
internal AnonymousObject(ByRefValue byRefValue) : base(byRefValue)
{
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,10 @@ def test_setter_to_union(self):
def test_nested_struct(self):
# None of these should throw...
NestingClass.NestedStruct(name="Queen B")

def test_anonymous_object(self):
struct = jsii_calc.StructUnionConsumer.provide_struct("A")
assert jsii_calc.StructUnionConsumer.is_struct_a(struct)

iface = jsii_calc.anonymous.UseOptions.provide("A")
assert jsii_calc.anonymous.UseOptions.consume(iface) == "A"
47 changes: 47 additions & 0 deletions packages/jsii-calc/lib/anonymous/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
export interface IOptionA {
doSomething(): string;
}

export interface IOptionB {
doSomethingElse(): string;
}

export class UseOptions {
public static provide(which: 'A' | 'B'): IOptionA | IOptionB {
switch (which) {
case 'A':
return new OptionA();
case 'B':
return new OptionB();
default:
throw new Error(`Unexpected option: ${which as any}`);
}
}

public static privideAsAny(which: 'A' | 'B'): any {
return this.provide(which);
}

public static consume(option: IOptionA | IOptionB): string {
if (option instanceof OptionA) {
return option.doSomething();
} else if (option instanceof OptionB) {
return option.doSomethingElse();
}
throw new Error(`Unexpected option: ${option as any}`);
}

private constructor() {}
}

class OptionA implements IOptionA {
public doSomething(): string {
return 'A';
}
}

class OptionB implements IOptionB {
public doSomethingElse(): string {
return 'B';
}
}
14 changes: 14 additions & 0 deletions packages/jsii-calc/lib/compliance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2358,6 +2358,20 @@ export class StructUnionConsumer {
}
}

public static provideStruct(which: 'A' | 'B'): StructA | StructB {
switch (which) {
case 'A':
return { requiredString: 'required', optionalNumber: 1337 };
case 'B':
return {
requiredString: 'required',
optionalStructA: this.provideStruct('A'),
};
default:
throw new Error(`Illegal value for which: ${which as any}`);
}
}

private constructor() {}
}

Expand Down
2 changes: 2 additions & 0 deletions packages/jsii-calc/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,5 @@ export * as module2700 from './module2700';

export * as cdk16625 from './cdk16625';
export * as jsii3656 from './jsii3656';

export * as anonymous from './anonymous';
Loading

0 comments on commit e7fadc0

Please sign in to comment.