From f8b33f4e8575d54dc8a0ebdc634c7769f009d0c1 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Sat, 25 Jul 2020 16:53:24 +0200 Subject: [PATCH 01/12] Add comments regarding multiple bounds of wildcard --- .../com/google/gson/internal/$Gson$Types.java | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 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 adea605f59..9f74967813 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 @@ -149,7 +149,10 @@ public static Class getRawType(Type type) { return Object.class; } else if (type instanceof WildcardType) { - return getRawType(((WildcardType) type).getUpperBounds()[0]); + Type[] bounds = ((WildcardType) type).getUpperBounds(); + // Currently the JLS only permits one bound for wildcards so using first bound is safe + assert bounds.length == 1; + return getRawType(bounds[0]); } else { String className = type == null ? "null" : type.getClass().getName(); @@ -277,7 +280,10 @@ static Type getGenericSupertype(Type context, Class rawType, Class toResol static Type getSupertype(Type context, Class contextRawType, Class supertype) { if (context instanceof WildcardType) { // wildcards are useless for resolving supertypes. As the upper bound has the same raw type, use it instead - context = ((WildcardType)context).getUpperBounds()[0]; + Type[] bounds = ((WildcardType)context).getUpperBounds(); + // Currently the JLS only permits one bound for wildcards so using first bound is safe + assert bounds.length == 1; + context = bounds[0]; } checkArgument(supertype.isAssignableFrom(contextRawType)); return resolve(context, contextRawType, @@ -334,11 +340,11 @@ 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 HashSet>()); } private static Type resolve(Type context, Class contextRawType, Type toResolve, - Collection visitedTypeVariables) { + Collection> visitedTypeVariables) { // this implementation is made a little more complicated in an attempt to avoid object-creation while (true) { if (toResolve instanceof TypeVariable) { @@ -550,8 +556,9 @@ public Type getGenericComponentType() { /** * The WildcardType interface supports multiple upper bounds and multiple - * lower bounds. We only support what the Java 6 language needs - at most one - * bound. If a lower bound is set, the upper bound must be Object.class. + * lower bounds. However, the Java Language Specification only permits at most + * one bounds so we are limiting it to that. + * If a lower bound is set, the upper bound must be Object.class. */ private static final class WildcardTypeImpl implements WildcardType, Serializable { private final Type upperBound; From 9faa6c3ddb9e1b8cd126619df75223e1e44736e8 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Sat, 25 Jul 2020 17:15:41 +0200 Subject: [PATCH 02/12] Remove WildcardType check in getCollectionElementType The returned Type is never a wildcard due to the changes made to getSupertype by commit b1fb9ca9a1bea5440bc6a5b506ccf67236b08243. --- gson/src/main/java/com/google/gson/internal/$Gson$Types.java | 3 --- 1 file changed, 3 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 9f74967813..1e4b3044d6 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 @@ -307,9 +307,6 @@ public static Type getArrayComponentType(Type array) { public static Type getCollectionElementType(Type context, Class contextRawType) { Type collectionType = getSupertype(context, contextRawType, Collection.class); - if (collectionType instanceof WildcardType) { - collectionType = ((WildcardType)collectionType).getUpperBounds()[0]; - } if (collectionType instanceof ParameterizedType) { return ((ParameterizedType) collectionType).getActualTypeArguments()[0]; } From 074ec4828bf4da673307c065a91cc1c56e9f9489 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Sun, 26 Jul 2020 12:54:36 +0200 Subject: [PATCH 03/12] Remove redundant getRawType call from MapTypeAdapterFactory getRawType(TypeToken.getType()) is the same as calling TypeToken.getRawType(). --- .../com/google/gson/internal/bind/MapTypeAdapterFactory.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gson/src/main/java/com/google/gson/internal/bind/MapTypeAdapterFactory.java b/gson/src/main/java/com/google/gson/internal/bind/MapTypeAdapterFactory.java index 5a34a5d5fb..3767ba9a57 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/MapTypeAdapterFactory.java +++ b/gson/src/main/java/com/google/gson/internal/bind/MapTypeAdapterFactory.java @@ -120,8 +120,7 @@ public MapTypeAdapterFactory(ConstructorConstructor constructorConstructor, return null; } - Class rawTypeOfSrc = $Gson$Types.getRawType(type); - Type[] keyAndValueTypes = $Gson$Types.getMapKeyAndValueTypes(type, rawTypeOfSrc); + Type[] keyAndValueTypes = $Gson$Types.getMapKeyAndValueTypes(type, rawType); TypeAdapter keyAdapter = getKeyAdapter(gson, keyAndValueTypes[0]); TypeAdapter valueAdapter = gson.getAdapter(TypeToken.get(keyAndValueTypes[1])); ObjectConstructor constructor = constructorConstructor.get(typeToken); From a264348f07ba2ffb2dee1b4e02d6f3589c769c1a Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Wed, 29 Jul 2020 17:26:12 +0200 Subject: [PATCH 04/12] Make TypeToken members private --- .../main/java/com/google/gson/reflect/TypeToken.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/gson/src/main/java/com/google/gson/reflect/TypeToken.java b/gson/src/main/java/com/google/gson/reflect/TypeToken.java index 3fb8af2bcf..7d4de7174c 100644 --- a/gson/src/main/java/com/google/gson/reflect/TypeToken.java +++ b/gson/src/main/java/com/google/gson/reflect/TypeToken.java @@ -45,9 +45,9 @@ * @author Jesse Wilson */ public class TypeToken { - final Class rawType; - final Type type; - final int hashCode; + private final Class rawType; + private final Type type; + private final int hashCode; /** * Constructs a new type literal. Derives represented class from type @@ -68,7 +68,7 @@ protected TypeToken() { * Unsafe. Constructs a type literal manually. */ @SuppressWarnings("unchecked") - TypeToken(Type type) { + private TypeToken(Type type) { this.type = $Gson$Types.canonicalize($Gson$Preconditions.checkNotNull(type)); this.rawType = (Class) $Gson$Types.getRawType(this.type); this.hashCode = this.type.hashCode(); @@ -78,7 +78,7 @@ protected TypeToken() { * Returns the type from super class's type parameter in {@link $Gson$Types#canonicalize * canonical form}. */ - static Type getSuperclassTypeParameter(Class subclass) { + private static Type getSuperclassTypeParameter(Class subclass) { Type superclass = subclass.getGenericSuperclass(); if (superclass instanceof Class) { throw new RuntimeException("Missing type parameter."); From 0e6959326c90381de278b5f8594b0694a37b65f6 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Wed, 29 Jul 2020 17:42:15 +0200 Subject: [PATCH 05/12] Remove incorrect statement about TypeToken wildcards It is possible to use wildcards as part of the type argument, e.g.: `new TypeToken>() {}` --- gson/src/main/java/com/google/gson/reflect/TypeToken.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/gson/src/main/java/com/google/gson/reflect/TypeToken.java b/gson/src/main/java/com/google/gson/reflect/TypeToken.java index 7d4de7174c..755dfbb3d8 100644 --- a/gson/src/main/java/com/google/gson/reflect/TypeToken.java +++ b/gson/src/main/java/com/google/gson/reflect/TypeToken.java @@ -37,9 +37,6 @@ *

* {@code TypeToken> list = new TypeToken>() {};} * - *

This syntax cannot be used to create type literals that have wildcard - * parameters, such as {@code Class} or {@code List}. - * * @author Bob Lee * @author Sven Mawson * @author Jesse Wilson From 02b750009a3df60421e0a68827acbf2a67461662 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Wed, 29 Jul 2020 18:12:32 +0200 Subject: [PATCH 06/12] Only allow direct subclasses of TypeToken Previously subclasses of subclasses (...) of TypeToken were allowed which can behave incorrectly when retrieving the type argument, e.g.: class SubTypeToken extends TypeToken {} new SubTypeToken() {}.getType() This returned `String` despite the class extending TypeToken. --- .../com/google/gson/reflect/TypeToken.java | 21 ++++++----- .../google/gson/reflect/TypeTokenTest.java | 35 +++++++++++++++++-- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/gson/src/main/java/com/google/gson/reflect/TypeToken.java b/gson/src/main/java/com/google/gson/reflect/TypeToken.java index 755dfbb3d8..50a4384488 100644 --- a/gson/src/main/java/com/google/gson/reflect/TypeToken.java +++ b/gson/src/main/java/com/google/gson/reflect/TypeToken.java @@ -56,7 +56,7 @@ public class TypeToken { */ @SuppressWarnings("unchecked") protected TypeToken() { - this.type = getSuperclassTypeParameter(getClass()); + this.type = getTypeTokenTypeArgument(); this.rawType = (Class) $Gson$Types.getRawType(type); this.hashCode = type.hashCode(); } @@ -72,16 +72,21 @@ private TypeToken(Type type) { } /** - * Returns the type from super class's type parameter in {@link $Gson$Types#canonicalize + * Verifies that {@code this} is an instance of a direct subclass of TypeToken and + * returns the type argument for {@code T} in {@link $Gson$Types#canonicalize * canonical form}. */ - private static Type getSuperclassTypeParameter(Class subclass) { - Type superclass = subclass.getGenericSuperclass(); - if (superclass instanceof Class) { - throw new RuntimeException("Missing type parameter."); + private Type getTypeTokenTypeArgument() { + Type superclass = getClass().getGenericSuperclass(); + if (superclass instanceof ParameterizedType) { + ParameterizedType parameterized = (ParameterizedType) superclass; + if (parameterized.getRawType() == TypeToken.class) { + return $Gson$Types.canonicalize(parameterized.getActualTypeArguments()[0]); + } } - ParameterizedType parameterized = (ParameterizedType) superclass; - return $Gson$Types.canonicalize(parameterized.getActualTypeArguments()[0]); + + // User created subclass of subclass of TypeToken + throw new IllegalStateException("Must only create direct subclasses of TypeToken"); } /** diff --git a/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java b/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java index 40572716bf..0e6c8b7fb6 100644 --- a/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java +++ b/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java @@ -27,9 +27,8 @@ /** * @author Jesse Wilson */ -@SuppressWarnings({"deprecation"}) public final class TypeTokenTest extends TestCase { - + // These fields are accessed using reflection by the tests below List listOfInteger = null; List listOfNumber = null; List listOfString = null; @@ -37,6 +36,7 @@ public final class TypeTokenTest extends TestCase { List> listOfSetOfString = null; List> listOfSetOfUnknown = null; + @SuppressWarnings({"deprecation"}) public void testIsAssignableFromRawTypes() { assertTrue(TypeToken.get(Object.class).isAssignableFrom(String.class)); assertFalse(TypeToken.get(String.class).isAssignableFrom(Object.class)); @@ -44,6 +44,7 @@ public void testIsAssignableFromRawTypes() { assertFalse(TypeToken.get(ArrayList.class).isAssignableFrom(RandomAccess.class)); } + @SuppressWarnings({"deprecation"}) public void testIsAssignableFromWithTypeParameters() throws Exception { Type a = getClass().getDeclaredField("listOfInteger").getGenericType(); Type b = getClass().getDeclaredField("listOfNumber").getGenericType(); @@ -56,6 +57,7 @@ public void testIsAssignableFromWithTypeParameters() throws Exception { assertFalse(TypeToken.get(b).isAssignableFrom(a)); } + @SuppressWarnings({"deprecation"}) public void testIsAssignableFromWithBasicWildcards() throws Exception { Type a = getClass().getDeclaredField("listOfString").getGenericType(); Type b = getClass().getDeclaredField("listOfUnknown").getGenericType(); @@ -69,6 +71,7 @@ public void testIsAssignableFromWithBasicWildcards() throws Exception { // assertTrue(TypeToken.get(b).isAssignableFrom(a)); } + @SuppressWarnings({"deprecation"}) public void testIsAssignableFromWithNestedWildcards() throws Exception { Type a = getClass().getDeclaredField("listOfSetOfString").getGenericType(); Type b = getClass().getDeclaredField("listOfSetOfUnknown").getGenericType(); @@ -102,4 +105,32 @@ public void testParameterizedFactory() { Type listOfListOfString = TypeToken.getParameterized(List.class, listOfString).getType(); assertEquals(expectedListOfListOfListOfString, TypeToken.getParameterized(List.class, listOfListOfString)); } + + /** + * User must only create direct subclasses of TypeToken, but not subclasses + * of subclasses (...) of TypeToken. + */ + public void testTypeTokenSubSubClass() { + class SubTypeToken extends TypeToken {} + class SubSubTypeToken1 extends SubTypeToken {} + class SubSubTypeToken2 extends SubTypeToken {} + + try { + new SubTypeToken() {}; + fail(); + } catch (IllegalStateException expected) { + } + + try { + new SubSubTypeToken1(); + fail(); + } catch (IllegalStateException expected) { + } + + try { + new SubSubTypeToken2(); + fail(); + } catch (IllegalStateException expected) { + } + } } From 85159c4b3bc3f4f59457eb65405940db697fd69a Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Wed, 29 Jul 2020 18:38:46 +0200 Subject: [PATCH 07/12] Throw exception when TypeToken captures type variable Due to type erasure the runtime type argument for a type variable is not available. Therefore there is no point in capturing a type variable and it might even give a false sense of type-safety. --- .../com/google/gson/reflect/TypeToken.java | 32 ++++++++++++++- .../google/gson/reflect/TypeTokenTest.java | 41 +++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/gson/src/main/java/com/google/gson/reflect/TypeToken.java b/gson/src/main/java/com/google/gson/reflect/TypeToken.java index 50a4384488..d82759fd44 100644 --- a/gson/src/main/java/com/google/gson/reflect/TypeToken.java +++ b/gson/src/main/java/com/google/gson/reflect/TypeToken.java @@ -22,6 +22,7 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; +import java.lang.reflect.WildcardType; import java.util.HashMap; import java.util.Map; @@ -53,6 +54,13 @@ public class TypeToken { *

Clients create an empty anonymous subclass. Doing so embeds the type * parameter in the anonymous class's type hierarchy so we can reconstitute it * at runtime despite erasure. + * + *

Because {@code TypeToken} is mainly intended for usage with Gson + * (and not other libraries) using a type variable as part of the type + * argument for {@code TypeToken} is not allowed. Due to type erasure the + * runtime type of a type variable is not available to Gson and therefore + * it cannot provide the functionality the user might expect, which would + * give a false sense of type-safety. */ @SuppressWarnings("unchecked") protected TypeToken() { @@ -81,7 +89,9 @@ private Type getTypeTokenTypeArgument() { if (superclass instanceof ParameterizedType) { ParameterizedType parameterized = (ParameterizedType) superclass; if (parameterized.getRawType() == TypeToken.class) { - return $Gson$Types.canonicalize(parameterized.getActualTypeArguments()[0]); + Type typeArgument = $Gson$Types.canonicalize(parameterized.getActualTypeArguments()[0]); + verifyNoTypeVariable(typeArgument); + return typeArgument; } } @@ -89,6 +99,26 @@ private Type getTypeTokenTypeArgument() { throw new IllegalStateException("Must only create direct subclasses of TypeToken"); } + private static void verifyNoTypeVariable(Type type) { + if (type instanceof TypeVariable) { + throw new IllegalArgumentException("TypeToken type argument must not contain a type variable"); + } else if (type instanceof GenericArrayType) { + verifyNoTypeVariable(((GenericArrayType) type).getGenericComponentType()); + } else if (type instanceof ParameterizedType) { + for (Type typeArgument : ((ParameterizedType) type).getActualTypeArguments()) { + verifyNoTypeVariable(typeArgument); + } + } else if (type instanceof WildcardType) { + WildcardType wildcardType = (WildcardType) type; + for (Type bound : wildcardType.getLowerBounds()) { + verifyNoTypeVariable(bound); + } + for (Type bound : wildcardType.getUpperBounds()) { + verifyNoTypeVariable(bound); + } + } + } + /** * Returns the raw (non-generic) type for this type. */ diff --git a/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java b/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java index 0e6c8b7fb6..18e0024dbf 100644 --- a/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java +++ b/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java @@ -133,4 +133,45 @@ class SubSubTypeToken2 extends SubTypeToken {} } catch (IllegalStateException expected) { } } + + /** + * TypeToken type argument must not contain a type variable because, due to + * type erasure, at runtime only the bound of the type variable is available + * which is likely not what the user wanted. + * + *

Note that type variables are allowed for the methods calling {@code TypeToken(Type)} + * because there the return type is {@code TypeToken} which does not give + * a false sense of type-safety. + */ + public void testTypeTokenTypeVariable() { + try { + new TypeToken() {}; + fail(); + } catch (IllegalArgumentException expected) { + } + + try { + new TypeToken>() {}; + fail(); + } catch (IllegalArgumentException expected) { + } + + try { + new TypeToken>() {}; + fail(); + } catch (IllegalArgumentException expected) { + } + + try { + new TypeToken>() {}; + fail(); + } catch (IllegalArgumentException expected) { + } + + try { + new TypeToken>() {}; + fail(); + } catch (IllegalArgumentException expected) { + } + } } From 11c1d0579ec924b454678d3562261e336ac08c6a Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Wed, 29 Jul 2020 18:44:26 +0200 Subject: [PATCH 08/12] Make $Gson$Types members private --- .../java/com/google/gson/internal/$Gson$Types.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 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 1e4b3044d6..df4e7d0ed2 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 @@ -37,7 +37,7 @@ * @author Jesse Wilson */ public final class $Gson$Types { - static final Type[] EMPTY_TYPE_ARRAY = new Type[] {}; + private static final Type[] EMPTY_TYPE_ARRAY = new Type[] {}; private $Gson$Types() { throw new UnsupportedOperationException(); @@ -161,7 +161,7 @@ public static Class getRawType(Type type) { } } - static boolean equal(Object a, Object b) { + private static boolean equal(Object a, Object b) { return a == b || (a != null && a.equals(b)); } @@ -223,7 +223,7 @@ public static boolean equals(Type a, Type b) { } } - static int hashCodeOrZero(Object o) { + private static int hashCodeOrZero(Object o) { return o != null ? o.hashCode() : 0; } @@ -236,7 +236,7 @@ public static String typeToString(Type type) { * IntegerSet}, the result for when supertype is {@code Set.class} is {@code Set} and the * result when the supertype is {@code Collection.class} is {@code Collection}. */ - static Type getGenericSupertype(Type context, Class rawType, Class toResolve) { + private static Type getGenericSupertype(Type context, Class rawType, Class toResolve) { if (toResolve == rawType) { return context; } @@ -277,7 +277,7 @@ static Type getGenericSupertype(Type context, Class rawType, Class toResol * * @param supertype a superclass of, or interface implemented by, this. */ - static Type getSupertype(Type context, Class contextRawType, Class supertype) { + private static Type getSupertype(Type context, Class contextRawType, Class supertype) { if (context instanceof WildcardType) { // wildcards are useless for resolving supertypes. As the upper bound has the same raw type, use it instead Type[] bounds = ((WildcardType)context).getUpperBounds(); @@ -419,7 +419,7 @@ private static Type resolve(Type context, Class contextRawType, Type toResolv } } - static Type resolveTypeVariable(Type context, Class contextRawType, TypeVariable unknown) { + private static Type resolveTypeVariable(Type context, Class contextRawType, TypeVariable unknown) { Class declaredByRaw = declaringClassOf(unknown); // we can't reduce this further @@ -456,7 +456,7 @@ private static Class declaringClassOf(TypeVariable typeVariable) { : null; } - static void checkNotPrimitive(Type type) { + private static void checkNotPrimitive(Type type) { checkArgument(!(type instanceof Class) || !((Class) type).isPrimitive()); } From f853dbbb31f44e8aaf3dea40cdaf38e53c84a73a Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Wed, 29 Jul 2020 19:09:44 +0200 Subject: [PATCH 09/12] Rename $Gson$Types.getGenericSupertype parameter Rename the method parameter to match the documentation of the method and to be similar to getSupertype(...). --- .../com/google/gson/internal/$Gson$Types.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 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 df4e7d0ed2..8f82d2ac2c 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 @@ -236,19 +236,19 @@ public static String typeToString(Type type) { * IntegerSet}, the result for when supertype is {@code Set.class} is {@code Set} and the * result when the supertype is {@code Collection.class} is {@code Collection}. */ - private static Type getGenericSupertype(Type context, Class rawType, Class toResolve) { - if (toResolve == rawType) { + private static Type getGenericSupertype(Type context, Class rawType, Class supertype) { + if (supertype == rawType) { return context; } // we skip searching through interfaces if unknown is an interface - if (toResolve.isInterface()) { + if (supertype.isInterface()) { Class[] interfaces = rawType.getInterfaces(); for (int i = 0, length = interfaces.length; i < length; i++) { - if (interfaces[i] == toResolve) { + if (interfaces[i] == supertype) { return rawType.getGenericInterfaces()[i]; - } else if (toResolve.isAssignableFrom(interfaces[i])) { - return getGenericSupertype(rawType.getGenericInterfaces()[i], interfaces[i], toResolve); + } else if (supertype.isAssignableFrom(interfaces[i])) { + return getGenericSupertype(rawType.getGenericInterfaces()[i], interfaces[i], supertype); } } } @@ -257,17 +257,17 @@ private static Type getGenericSupertype(Type context, Class rawType, Class if (!rawType.isInterface()) { while (rawType != Object.class) { Class rawSupertype = rawType.getSuperclass(); - if (rawSupertype == toResolve) { + if (rawSupertype == supertype) { return rawType.getGenericSuperclass(); - } else if (toResolve.isAssignableFrom(rawSupertype)) { - return getGenericSupertype(rawType.getGenericSuperclass(), rawSupertype, toResolve); + } else if (supertype.isAssignableFrom(rawSupertype)) { + return getGenericSupertype(rawType.getGenericSuperclass(), rawSupertype, supertype); } rawType = rawSupertype; } } // we can't resolve this further - return toResolve; + return supertype; } /** From 443607809e6576cce5f0b0e0433b9171d389c674 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Sun, 6 Feb 2022 18:49:38 +0100 Subject: [PATCH 10/12] Improve tests and handle raw TypeToken supertype better --- .../com/google/gson/reflect/TypeToken.java | 9 +++- .../google/gson/reflect/TypeTokenTest.java | 47 +++++++++++++++++-- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/gson/src/main/java/com/google/gson/reflect/TypeToken.java b/gson/src/main/java/com/google/gson/reflect/TypeToken.java index d82759fd44..f8e75a18ac 100644 --- a/gson/src/main/java/com/google/gson/reflect/TypeToken.java +++ b/gson/src/main/java/com/google/gson/reflect/TypeToken.java @@ -94,6 +94,11 @@ private Type getTypeTokenTypeArgument() { return typeArgument; } } + // Check for raw TypeToken as superclass + else if (superclass == TypeToken.class) { + throw new IllegalStateException("TypeToken must be created with a type argument: new TypeToken<...>() {}; " + + "When using code shrinkers (ProGuard, R8, ...) make sure that generic signatures are preserved."); + } // User created subclass of subclass of TypeToken throw new IllegalStateException("Must only create direct subclasses of TypeToken"); @@ -101,7 +106,9 @@ private Type getTypeTokenTypeArgument() { private static void verifyNoTypeVariable(Type type) { if (type instanceof TypeVariable) { - throw new IllegalArgumentException("TypeToken type argument must not contain a type variable"); + TypeVariable typeVariable = (TypeVariable) type; + throw new IllegalArgumentException("TypeToken type argument must not contain a type variable; captured type variable " + + typeVariable.getName() + " declared by " + typeVariable.getGenericDeclaration()); } else if (type instanceof GenericArrayType) { verifyNoTypeVariable(((GenericArrayType) type).getGenericComponentType()); } else if (type instanceof ParameterizedType) { diff --git a/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java b/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java index 18e0024dbf..0d82d283bb 100644 --- a/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java +++ b/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java @@ -106,6 +106,15 @@ public void testParameterizedFactory() { assertEquals(expectedListOfListOfListOfString, TypeToken.getParameterized(List.class, listOfListOfString)); } + private static class CustomTypeToken extends TypeToken { + } + + public void testTypeTokenNonAnonymousSubclass() { + TypeToken typeToken = new CustomTypeToken(); + assertEquals(String.class, typeToken.getRawType()); + assertEquals(String.class, typeToken.getType()); + } + /** * User must only create direct subclasses of TypeToken, but not subclasses * of subclasses (...) of TypeToken. @@ -119,18 +128,21 @@ class SubSubTypeToken2 extends SubTypeToken {} new SubTypeToken() {}; fail(); } catch (IllegalStateException expected) { + assertEquals("Must only create direct subclasses of TypeToken", expected.getMessage()); } try { new SubSubTypeToken1(); fail(); } catch (IllegalStateException expected) { + assertEquals("Must only create direct subclasses of TypeToken", expected.getMessage()); } try { new SubSubTypeToken2(); fail(); } catch (IllegalStateException expected) { + assertEquals("Must only create direct subclasses of TypeToken", expected.getMessage()); } } @@ -148,30 +160,57 @@ public void testTypeTokenTypeVariable() { new TypeToken() {}; fail(); } catch (IllegalArgumentException expected) { + assertEquals("TypeToken type argument must not contain a type variable; captured type variable T " + + "declared by public void com.google.gson.reflect.TypeTokenTest.testTypeTokenTypeVariable()", + expected.getMessage()); } try { - new TypeToken>() {}; + new TypeToken>>() {}; fail(); } catch (IllegalArgumentException expected) { + assertEquals("TypeToken type argument must not contain a type variable; captured type variable T " + + "declared by public void com.google.gson.reflect.TypeTokenTest.testTypeTokenTypeVariable()", + expected.getMessage()); } try { - new TypeToken>() {}; + new TypeToken>>() {}; fail(); } catch (IllegalArgumentException expected) { + assertEquals("TypeToken type argument must not contain a type variable; captured type variable T " + + "declared by public void com.google.gson.reflect.TypeTokenTest.testTypeTokenTypeVariable()", + expected.getMessage()); } try { - new TypeToken>() {}; + new TypeToken>>() {}; fail(); } catch (IllegalArgumentException expected) { + assertEquals("TypeToken type argument must not contain a type variable; captured type variable T " + + "declared by public void com.google.gson.reflect.TypeTokenTest.testTypeTokenTypeVariable()", + expected.getMessage()); } try { - new TypeToken>() {}; + new TypeToken[]>() {}; fail(); } catch (IllegalArgumentException expected) { + assertEquals("TypeToken type argument must not contain a type variable; captured type variable T " + + "declared by public void com.google.gson.reflect.TypeTokenTest.testTypeTokenTypeVariable()", + expected.getMessage()); + } + } + + @SuppressWarnings("rawtypes") + public void testTypeTokenRaw() { + try { + new TypeToken() {}; + fail(); + } catch (IllegalStateException expected) { + assertEquals("TypeToken must be created with a type argument: new TypeToken<...>() {}; " + + "When using code shrinkers (ProGuard, R8, ...) make sure that generic signatures are preserved.", + expected.getMessage()); } } } From 446bf92196e611f40b95757f0a7c76a30b307c3d Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Sun, 6 Feb 2022 19:10:20 +0100 Subject: [PATCH 11/12] Make some $Gson$Types members package-private again to prevent synthetic accessors --- .../java/com/google/gson/internal/$Gson$Types.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 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 00c49671b2..48be0204f0 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 @@ -42,7 +42,7 @@ * @author Jesse Wilson */ public final class $Gson$Types { - private static final Type[] EMPTY_TYPE_ARRAY = new Type[] {}; + static final Type[] EMPTY_TYPE_ARRAY = new Type[] {}; private $Gson$Types() { throw new UnsupportedOperationException(); @@ -228,10 +228,6 @@ public static boolean equals(Type a, Type b) { } } - private static int hashCodeOrZero(Object o) { - return o != null ? o.hashCode() : 0; - } - public static String typeToString(Type type) { return type instanceof Class ? ((Class) type).getName() : type.toString(); } @@ -480,7 +476,7 @@ private static Class declaringClassOf(TypeVariable typeVariable) { : null; } - private static void checkNotPrimitive(Type type) { + static void checkNotPrimitive(Type type) { checkArgument(!(type instanceof Class) || !((Class) type).isPrimitive()); } @@ -525,6 +521,10 @@ public Type getOwnerType() { && $Gson$Types.equals(this, (ParameterizedType) other); } + private static int hashCodeOrZero(Object o) { + return o != null ? o.hashCode() : 0; + } + @Override public int hashCode() { return Arrays.hashCode(typeArguments) ^ rawType.hashCode() From cba030776434f79f03b84de5a7e7bd50461927cb Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Wed, 16 Feb 2022 00:58:27 +0100 Subject: [PATCH 12/12] Remove TypeToken check for type variable As mentioned in review comments, there are cases during serialization where usage of the type variable is not so problematic (but still not ideal). --- .../com/google/gson/reflect/TypeToken.java | 40 +++---------- .../google/gson/reflect/TypeTokenTest.java | 56 ------------------- 2 files changed, 7 insertions(+), 89 deletions(-) diff --git a/gson/src/main/java/com/google/gson/reflect/TypeToken.java b/gson/src/main/java/com/google/gson/reflect/TypeToken.java index f8e75a18ac..dea636da88 100644 --- a/gson/src/main/java/com/google/gson/reflect/TypeToken.java +++ b/gson/src/main/java/com/google/gson/reflect/TypeToken.java @@ -22,7 +22,6 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; -import java.lang.reflect.WildcardType; import java.util.HashMap; import java.util.Map; @@ -38,6 +37,12 @@ *

* {@code TypeToken> list = new TypeToken>() {};} * + *

Capturing a type variable as type argument of a {@code TypeToken} should + * be avoided. Due to type erasure the runtime type of a type variable is not + * available to Gson and therefore it cannot provide the functionality one + * might expect, which gives a false sense of type-safety at compilation time + * and can lead to an unexpected {@code ClassCastException} at runtime. + * * @author Bob Lee * @author Sven Mawson * @author Jesse Wilson @@ -54,13 +59,6 @@ public class TypeToken { *

Clients create an empty anonymous subclass. Doing so embeds the type * parameter in the anonymous class's type hierarchy so we can reconstitute it * at runtime despite erasure. - * - *

Because {@code TypeToken} is mainly intended for usage with Gson - * (and not other libraries) using a type variable as part of the type - * argument for {@code TypeToken} is not allowed. Due to type erasure the - * runtime type of a type variable is not available to Gson and therefore - * it cannot provide the functionality the user might expect, which would - * give a false sense of type-safety. */ @SuppressWarnings("unchecked") protected TypeToken() { @@ -89,9 +87,7 @@ private Type getTypeTokenTypeArgument() { if (superclass instanceof ParameterizedType) { ParameterizedType parameterized = (ParameterizedType) superclass; if (parameterized.getRawType() == TypeToken.class) { - Type typeArgument = $Gson$Types.canonicalize(parameterized.getActualTypeArguments()[0]); - verifyNoTypeVariable(typeArgument); - return typeArgument; + return $Gson$Types.canonicalize(parameterized.getActualTypeArguments()[0]); } } // Check for raw TypeToken as superclass @@ -104,28 +100,6 @@ else if (superclass == TypeToken.class) { throw new IllegalStateException("Must only create direct subclasses of TypeToken"); } - private static void verifyNoTypeVariable(Type type) { - if (type instanceof TypeVariable) { - TypeVariable typeVariable = (TypeVariable) type; - throw new IllegalArgumentException("TypeToken type argument must not contain a type variable; captured type variable " - + typeVariable.getName() + " declared by " + typeVariable.getGenericDeclaration()); - } else if (type instanceof GenericArrayType) { - verifyNoTypeVariable(((GenericArrayType) type).getGenericComponentType()); - } else if (type instanceof ParameterizedType) { - for (Type typeArgument : ((ParameterizedType) type).getActualTypeArguments()) { - verifyNoTypeVariable(typeArgument); - } - } else if (type instanceof WildcardType) { - WildcardType wildcardType = (WildcardType) type; - for (Type bound : wildcardType.getLowerBounds()) { - verifyNoTypeVariable(bound); - } - for (Type bound : wildcardType.getUpperBounds()) { - verifyNoTypeVariable(bound); - } - } - } - /** * Returns the raw (non-generic) type for this type. */ diff --git a/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java b/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java index 0d82d283bb..1cdd7361af 100644 --- a/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java +++ b/gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java @@ -146,62 +146,6 @@ class SubSubTypeToken2 extends SubTypeToken {} } } - /** - * TypeToken type argument must not contain a type variable because, due to - * type erasure, at runtime only the bound of the type variable is available - * which is likely not what the user wanted. - * - *

Note that type variables are allowed for the methods calling {@code TypeToken(Type)} - * because there the return type is {@code TypeToken} which does not give - * a false sense of type-safety. - */ - public void testTypeTokenTypeVariable() { - try { - new TypeToken() {}; - fail(); - } catch (IllegalArgumentException expected) { - assertEquals("TypeToken type argument must not contain a type variable; captured type variable T " - + "declared by public void com.google.gson.reflect.TypeTokenTest.testTypeTokenTypeVariable()", - expected.getMessage()); - } - - try { - new TypeToken>>() {}; - fail(); - } catch (IllegalArgumentException expected) { - assertEquals("TypeToken type argument must not contain a type variable; captured type variable T " - + "declared by public void com.google.gson.reflect.TypeTokenTest.testTypeTokenTypeVariable()", - expected.getMessage()); - } - - try { - new TypeToken>>() {}; - fail(); - } catch (IllegalArgumentException expected) { - assertEquals("TypeToken type argument must not contain a type variable; captured type variable T " - + "declared by public void com.google.gson.reflect.TypeTokenTest.testTypeTokenTypeVariable()", - expected.getMessage()); - } - - try { - new TypeToken>>() {}; - fail(); - } catch (IllegalArgumentException expected) { - assertEquals("TypeToken type argument must not contain a type variable; captured type variable T " - + "declared by public void com.google.gson.reflect.TypeTokenTest.testTypeTokenTypeVariable()", - expected.getMessage()); - } - - try { - new TypeToken[]>() {}; - fail(); - } catch (IllegalArgumentException expected) { - assertEquals("TypeToken type argument must not contain a type variable; captured type variable T " - + "declared by public void com.google.gson.reflect.TypeTokenTest.testTypeTokenTypeVariable()", - expected.getMessage()); - } - } - @SuppressWarnings("rawtypes") public void testTypeTokenRaw() { try {