Skip to content

Commit a8096b7

Browse files
RomainMullermergify[bot]
authored andcommitted
fix(java): remove Jackson confusion with certain patterns (#987)
* fix(java): remove Jackson confusion with certain patterns Upon deserializing (`ObjectMapper#treeToValue`), Jackson will first validate the `valueType` for whether ti can be deserialized with the "standard" bean deserializer. This will bail out if there are multiple ambiguous setters for a given property (they are ambiguous if they have parameters of non-trivial types, such as `List`, some class, ...). In order to remove the problem, this changes the deserialization path so that instead of asking for deserialization into the needed instance type directly, `JsiiObject` will be requested instead when the declared type is a sub-class of that. Since such types are passed by-reference, the custom de-serializer modifier will correctly determine the "right" class to use (the declared type, or a subtype of it), and return this... But Jackson will not get the chance to be confused or unhappy about that type's structure. This was the root cause of aws/aws-cdk#4080 * add test specifically on structs
1 parent 347ddf2 commit a8096b7

17 files changed

Lines changed: 767 additions & 6 deletions

File tree

packages/jsii-calc/lib/compliance.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2266,3 +2266,25 @@ export class Demonstrate982 {
22662266

22672267
public constructor() { }
22682268
}
2269+
2270+
/**
2271+
* This tries to confuse Jackson by having overloaded property setters.
2272+
*
2273+
* @see https://github.com/aws/aws-cdk/issues/4080
2274+
*/
2275+
export class ConfusingToJackson {
2276+
public static makeInstance(): ConfusingToJackson {
2277+
return new ConfusingToJackson();
2278+
}
2279+
2280+
public static makeStructInstance(): ConfusingToJacksonStruct {
2281+
return {};
2282+
}
2283+
2284+
public unionProperty?: Array<IFriendly | AbstractClass> | IFriendly;
2285+
2286+
private constructor() { }
2287+
}
2288+
export interface ConfusingToJacksonStruct {
2289+
readonly unionProperty?: Array<IFriendly | AbstractClass> | IFriendly;
2290+
}

packages/jsii-calc/test/assembly.jsii

Lines changed: 144 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2219,6 +2219,149 @@
22192219
}
22202220
]
22212221
},
2222+
"jsii-calc.ConfusingToJackson": {
2223+
"assembly": "jsii-calc",
2224+
"docs": {
2225+
"see": "https://github.com/aws/aws-cdk/issues/4080",
2226+
"stability": "experimental",
2227+
"summary": "This tries to confuse Jackson by having overloaded property setters."
2228+
},
2229+
"fqn": "jsii-calc.ConfusingToJackson",
2230+
"kind": "class",
2231+
"locationInModule": {
2232+
"filename": "lib/compliance.ts",
2233+
"line": 2275
2234+
},
2235+
"methods": [
2236+
{
2237+
"docs": {
2238+
"stability": "experimental"
2239+
},
2240+
"locationInModule": {
2241+
"filename": "lib/compliance.ts",
2242+
"line": 2276
2243+
},
2244+
"name": "makeInstance",
2245+
"returns": {
2246+
"type": {
2247+
"fqn": "jsii-calc.ConfusingToJackson"
2248+
}
2249+
},
2250+
"static": true
2251+
},
2252+
{
2253+
"docs": {
2254+
"stability": "experimental"
2255+
},
2256+
"locationInModule": {
2257+
"filename": "lib/compliance.ts",
2258+
"line": 2280
2259+
},
2260+
"name": "makeStructInstance",
2261+
"returns": {
2262+
"type": {
2263+
"fqn": "jsii-calc.ConfusingToJacksonStruct"
2264+
}
2265+
},
2266+
"static": true
2267+
}
2268+
],
2269+
"name": "ConfusingToJackson",
2270+
"properties": [
2271+
{
2272+
"docs": {
2273+
"stability": "experimental"
2274+
},
2275+
"locationInModule": {
2276+
"filename": "lib/compliance.ts",
2277+
"line": 2284
2278+
},
2279+
"name": "unionProperty",
2280+
"optional": true,
2281+
"type": {
2282+
"union": {
2283+
"types": [
2284+
{
2285+
"fqn": "@scope/jsii-calc-lib.IFriendly"
2286+
},
2287+
{
2288+
"collection": {
2289+
"elementtype": {
2290+
"union": {
2291+
"types": [
2292+
{
2293+
"fqn": "@scope/jsii-calc-lib.IFriendly"
2294+
},
2295+
{
2296+
"fqn": "jsii-calc.AbstractClass"
2297+
}
2298+
]
2299+
}
2300+
},
2301+
"kind": "array"
2302+
}
2303+
}
2304+
]
2305+
}
2306+
}
2307+
}
2308+
]
2309+
},
2310+
"jsii-calc.ConfusingToJacksonStruct": {
2311+
"assembly": "jsii-calc",
2312+
"datatype": true,
2313+
"docs": {
2314+
"stability": "experimental"
2315+
},
2316+
"fqn": "jsii-calc.ConfusingToJacksonStruct",
2317+
"kind": "interface",
2318+
"locationInModule": {
2319+
"filename": "lib/compliance.ts",
2320+
"line": 2288
2321+
},
2322+
"name": "ConfusingToJacksonStruct",
2323+
"properties": [
2324+
{
2325+
"abstract": true,
2326+
"docs": {
2327+
"stability": "experimental"
2328+
},
2329+
"immutable": true,
2330+
"locationInModule": {
2331+
"filename": "lib/compliance.ts",
2332+
"line": 2289
2333+
},
2334+
"name": "unionProperty",
2335+
"optional": true,
2336+
"type": {
2337+
"union": {
2338+
"types": [
2339+
{
2340+
"fqn": "@scope/jsii-calc-lib.IFriendly"
2341+
},
2342+
{
2343+
"collection": {
2344+
"elementtype": {
2345+
"union": {
2346+
"types": [
2347+
{
2348+
"fqn": "@scope/jsii-calc-lib.IFriendly"
2349+
},
2350+
{
2351+
"fqn": "jsii-calc.AbstractClass"
2352+
}
2353+
]
2354+
}
2355+
},
2356+
"kind": "array"
2357+
}
2358+
}
2359+
]
2360+
}
2361+
}
2362+
}
2363+
]
2364+
},
22222365
"jsii-calc.ConstructorPassesThisOut": {
22232366
"assembly": "jsii-calc",
22242367
"docs": {
@@ -11128,5 +11271,5 @@
1112811271
}
1112911272
},
1113011273
"version": "0.20.5",
11131-
"fingerprint": "/MRTbTnRC1UWxsPIrca+9Yo1IBKsEueT75P22pQoV1o="
11274+
"fingerprint": "vKeZwjjLC1e84B6FiOQCH8JDNoINrldYfdEWYKqCFSQ="
1113211275
}

packages/jsii-dotnet-runtime-test/test/Amazon.JSII.Runtime.IntegrationTests/ComplianceTests.cs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1034,7 +1034,7 @@ public void VariadicCallbacksAreHandledCorrectly()
10341034
}
10351035

10361036
[Fact(DisplayName = Prefix + nameof(ReturnSubclassThatImplementsInterface976))]
1037-
public void ReturnSubclassThatImplementsInterface976()
1037+
public void ReturnSubclassThatImplementsInterface976()
10381038
{
10391039
var obj = SomeTypeJsii976.ReturnReturn();
10401040
Assert.Equal(obj.Foo, 333);
@@ -1265,5 +1265,17 @@ public void StructsCanBeDowncastedToParentType()
12651265
Assert.NotNull(Demonstrate982.TakeThis());
12661266
Assert.NotNull(Demonstrate982.TakeThisToo());
12671267
}
1268+
1269+
[Fact(DisplayName = Prefix + nameof(CanObtainReferenceWithOverloadedSetters))]
1270+
public void CanObtainReferenceWithOverloadedSetters()
1271+
{
1272+
Assert.NotNull(ConfusingToJackson.MakeInstance());
1273+
}
1274+
1275+
[Fact(DisplayName = Prefix + nameof(CanObtainStructReferenceWithOverloadedSetters))]
1276+
public void CanObtainStructReferenceWithOverloadedSetters()
1277+
{
1278+
Assert.NotNull(ConfusingToJackson.MakeStructInstance());
1279+
}
12681280
}
12691281
}

packages/jsii-java-runtime-test/project/src/test/java/software/amazon/jsii/testing/ComplianceTest.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1553,4 +1553,14 @@ public java.lang.Number next() {
15531553
return next;
15541554
}
15551555
}
1556+
1557+
@Test
1558+
public void canObtainReferenceWithOverloadedSetter() {
1559+
assertNotNull(ConfusingToJackson.makeInstance());
1560+
}
1561+
1562+
@Test
1563+
public void canObtainStructReferenceWithOverloadedSetter() {
1564+
assertNotNull(ConfusingToJackson.makeStructInstance());
1565+
}
15561566
}

packages/jsii-java-runtime/project/src/main/java/software/amazon/jsii/JsiiObjectMapper.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,22 @@ public static <T> T treeToValue(final JsonNode tree, final Class<T> valueType) {
5050
return null;
5151
}
5252
try {
53-
final T result = INSTANCE.treeToValue(tree, valueType);
53+
// If the needed type is a sub-class of JsiiObject, we'll be receiving it by-reference, so we can ask Jackson to
54+
// de-serialize a JsiiObject instead of the actual type; and we'll still get the correct instance type. This
55+
// avoids running into problems because of Jackson not liking the structure of a particular class (it will
56+
// validate that before attempting any deserialization operation, and I don't know how to mute this behavior).
57+
final Class<?> deserType = JsiiObject.class.isAssignableFrom(valueType)
58+
? JsiiObject.class
59+
: valueType;
60+
final Object result = INSTANCE.treeToValue(tree, deserType);
5461
if (result != null && valueType.isInterface() && result instanceof JsiiObject) {
5562
// The result type does not implement the interface, returning the proxy instead!
5663
if (!valueType.isAssignableFrom(result.getClass()) && valueType.isAnnotationPresent(Jsii.Proxy.class)) {
5764
final Jsii.Proxy proxyAnnotation = valueType.getAnnotation(Jsii.Proxy.class);
5865
return (T)((JsiiObject) result).asInterfaceProxy(proxyAnnotation.value());
5966
}
6067
}
61-
return result;
68+
return (T)result;
6269
} catch (final JsonProcessingException jpe) {
6370
throw new JsiiException(jpe);
6471
}

0 commit comments

Comments
 (0)