From e2296f42d553e3f299c7dd2f4d1a4efd60007ade Mon Sep 17 00:00:00 2001 From: Mike Cumings Date: Tue, 25 Sep 2018 16:09:48 -0700 Subject: [PATCH 1/2] Fix issue with recursive type variable protections to fix #1390 When a type variable is referenced multiple times it needs to resolve to the same value. Previously, the second attempt would abort resolution early in order to protect against infinite recursion. --- .../com/google/gson/internal/$Gson$Types.java | 53 ++++++++++++------ .../ReusedTypeVariablesFullyResolveTest.java | 54 +++++++++++++++++++ 2 files changed, 92 insertions(+), 15 deletions(-) create mode 100644 gson/src/test/java/com/google/gson/functional/ReusedTypeVariablesFullyResolveTest.java diff --git a/gson/src/main/java/com/google/gson/internal/$Gson$Types.java b/gson/src/main/java/com/google/gson/internal/$Gson$Types.java index adea605f59..42344200d1 100644 --- a/gson/src/main/java/com/google/gson/internal/$Gson$Types.java +++ b/gson/src/main/java/com/google/gson/internal/$Gson$Types.java @@ -25,7 +25,12 @@ import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; import java.lang.reflect.WildcardType; -import java.util.*; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.Properties; import static com.google.gson.internal.$Gson$Preconditions.checkArgument; import static com.google.gson.internal.$Gson$Preconditions.checkNotNull; @@ -334,41 +339,50 @@ public static Type[] getMapKeyAndValueTypes(Type context, Class contextRawTyp } public static Type resolve(Type context, Class contextRawType, Type toResolve) { - return resolve(context, contextRawType, toResolve, new HashSet()); + return resolve(context, contextRawType, toResolve, new HashMap()); } private static Type resolve(Type context, Class contextRawType, Type toResolve, - Collection visitedTypeVariables) { + Map visitedTypeVariables) { // this implementation is made a little more complicated in an attempt to avoid object-creation + TypeVariable resolving = null; while (true) { if (toResolve instanceof TypeVariable) { TypeVariable typeVariable = (TypeVariable) toResolve; - if (visitedTypeVariables.contains(typeVariable)) { + Type previouslyResolved = visitedTypeVariables.get(typeVariable); + if (previouslyResolved != null) { // cannot reduce due to infinite recursion - return toResolve; - } else { - visitedTypeVariables.add(typeVariable); + return (previouslyResolved == Void.TYPE) ? toResolve : previouslyResolved; } + + // Insert a placeholder to mark the fact that we are in the process of resolving this type + visitedTypeVariables.put(typeVariable, Void.TYPE); + if (resolving == null) { + resolving = typeVariable; + } + toResolve = resolveTypeVariable(context, contextRawType, typeVariable); if (toResolve == typeVariable) { - return toResolve; + break; } } else if (toResolve instanceof Class && ((Class) toResolve).isArray()) { Class original = (Class) toResolve; Type componentType = original.getComponentType(); Type newComponentType = resolve(context, contextRawType, componentType, visitedTypeVariables); - return componentType == newComponentType + toResolve = componentType == newComponentType ? original : arrayOf(newComponentType); + break; } else if (toResolve instanceof GenericArrayType) { GenericArrayType original = (GenericArrayType) toResolve; Type componentType = original.getGenericComponentType(); Type newComponentType = resolve(context, contextRawType, componentType, visitedTypeVariables); - return componentType == newComponentType + toResolve = componentType == newComponentType ? original : arrayOf(newComponentType); + break; } else if (toResolve instanceof ParameterizedType) { ParameterizedType original = (ParameterizedType) toResolve; @@ -388,9 +402,10 @@ private static Type resolve(Type context, Class contextRawType, Type toResolv } } - return changed + toResolve = changed ? newParameterizedTypeWithOwner(newOwnerType, original.getRawType(), args) : original; + break; } else if (toResolve instanceof WildcardType) { WildcardType original = (WildcardType) toResolve; @@ -400,20 +415,28 @@ private static Type resolve(Type context, Class contextRawType, Type toResolv if (originalLowerBound.length == 1) { Type lowerBound = resolve(context, contextRawType, originalLowerBound[0], visitedTypeVariables); if (lowerBound != originalLowerBound[0]) { - return supertypeOf(lowerBound); + toResolve = supertypeOf(lowerBound); + break; } } else if (originalUpperBound.length == 1) { Type upperBound = resolve(context, contextRawType, originalUpperBound[0], visitedTypeVariables); if (upperBound != originalUpperBound[0]) { - return subtypeOf(upperBound); + toResolve = subtypeOf(upperBound); + break; } } - return original; + toResolve = original; + break; } else { - return toResolve; + break; } } + // ensure that any in-process resolution gets updated with the final result + if (resolving != null) { + visitedTypeVariables.put(resolving, toResolve); + } + return toResolve; } static Type resolveTypeVariable(Type context, Class contextRawType, TypeVariable unknown) { diff --git a/gson/src/test/java/com/google/gson/functional/ReusedTypeVariablesFullyResolveTest.java b/gson/src/test/java/com/google/gson/functional/ReusedTypeVariablesFullyResolveTest.java new file mode 100644 index 0000000000..e3ddd840e9 --- /dev/null +++ b/gson/src/test/java/com/google/gson/functional/ReusedTypeVariablesFullyResolveTest.java @@ -0,0 +1,54 @@ +package com.google.gson.functional; + +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import org.junit.Before; +import org.junit.Test; + +import java.util.Collection; +import java.util.Iterator; +import java.util.Set; + +import static org.junit.Assert.*; + +/** + * This test covers the scenario described in #1390 where a type variable needs to be used + * by a type definition multiple times. Both type variable references should resolve to the + * same underlying concrete type. + */ +public class ReusedTypeVariablesFullyResolveTest { + + private Gson gson; + + @Before + public void setUp() { + gson = new GsonBuilder().create(); + } + + @SuppressWarnings("ConstantConditions") // The instances were being unmarshaled as Strings instead of TestEnums + @Test + public void testGenericsPreservation() { + TestEnumSetCollection withSet = gson.fromJson("{\"collection\":[\"ONE\",\"THREE\"]}", TestEnumSetCollection.class); + Iterator iterator = withSet.collection.iterator(); + assertNotNull(withSet); + assertNotNull(withSet.collection); + assertEquals(2, withSet.collection.size()); + TestEnum first = iterator.next(); + TestEnum second = iterator.next(); + + assertTrue(first instanceof TestEnum); + assertTrue(second instanceof TestEnum); + } + + enum TestEnum { ONE, TWO, THREE } + + private static class TestEnumSetCollection extends SetCollection {} + + private static class SetCollection extends BaseCollection> {} + + private static class BaseCollection> + { + public C collection; + } + +} From 69f7c4e243c385b318ed63205817347e4bbe379e Mon Sep 17 00:00:00 2001 From: Mike Cumings Date: Wed, 26 Sep 2018 22:38:53 -0700 Subject: [PATCH 2/2] Replace instance equality checks in $Gson$Types#resolve --- .../main/java/com/google/gson/internal/$Gson$Types.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gson/src/main/java/com/google/gson/internal/$Gson$Types.java b/gson/src/main/java/com/google/gson/internal/$Gson$Types.java index 42344200d1..53985bc30a 100644 --- a/gson/src/main/java/com/google/gson/internal/$Gson$Types.java +++ b/gson/src/main/java/com/google/gson/internal/$Gson$Types.java @@ -370,7 +370,7 @@ private static Type resolve(Type context, Class contextRawType, Type toResolv Class original = (Class) toResolve; Type componentType = original.getComponentType(); Type newComponentType = resolve(context, contextRawType, componentType, visitedTypeVariables); - toResolve = componentType == newComponentType + toResolve = equal(componentType, newComponentType) ? original : arrayOf(newComponentType); break; @@ -379,7 +379,7 @@ private static Type resolve(Type context, Class contextRawType, Type toResolv GenericArrayType original = (GenericArrayType) toResolve; Type componentType = original.getGenericComponentType(); Type newComponentType = resolve(context, contextRawType, componentType, visitedTypeVariables); - toResolve = componentType == newComponentType + toResolve = equal(componentType, newComponentType) ? original : arrayOf(newComponentType); break; @@ -388,12 +388,12 @@ private static Type resolve(Type context, Class contextRawType, Type toResolv ParameterizedType original = (ParameterizedType) toResolve; Type ownerType = original.getOwnerType(); Type newOwnerType = resolve(context, contextRawType, ownerType, visitedTypeVariables); - boolean changed = newOwnerType != ownerType; + boolean changed = !equal(newOwnerType, ownerType); Type[] args = original.getActualTypeArguments(); for (int t = 0, length = args.length; t < length; t++) { Type resolvedTypeArgument = resolve(context, contextRawType, args[t], visitedTypeVariables); - if (resolvedTypeArgument != args[t]) { + if (!equal(resolvedTypeArgument, args[t])) { if (!changed) { args = args.clone(); changed = true;