Johannes/gson-comments
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Improve TypeToken creation validation (#2072)

Browse Source 
* Add comments regarding multiple bounds of wildcard

* Remove WildcardType check in getCollectionElementType

The returned Type is never a wildcard due to the changes made to getSupertype
by commit b1fb9ca9a1.

* Remove redundant getRawType call from MapTypeAdapterFactory

getRawType(TypeToken.getType()) is the same as calling TypeToken.getRawType().

* Make TypeToken members private

* Remove incorrect statement about TypeToken wildcards

It is possible to use wildcards as part of the type argument, e.g.:
`new TypeToken<List<? extends CharSequence>>() {}`

* 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<T> extends TypeToken<Integer> {}
  new SubTypeToken<String>() {}.getType()

This returned `String` despite the class extending TypeToken<Integer>.

* 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.

* Make $Gson$Types members private

* Rename $Gson$Types.getGenericSupertype parameter

Rename the method parameter to match the documentation of the method and
to be similar to getSupertype(...).

* Improve tests and handle raw TypeToken supertype better

* Make some $Gson$Types members package-private again to prevent synthetic accessors

* 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).
This commit is contained in:
Marcono1234 2022-04-19 17:20:58 +02:00 committed by GitHub
parent feaf8ddc05
commit b1c399fd62
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 110 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
gson/src/main/java/com/google/gson/internal/$Gson$Types.java
View File

@ -154,7 +154,10 @@ public final class $Gson$Types {
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();
@ -163,7 +166,7 @@ public final class $Gson$Types {
}
}
static boolean equal(Object a, Object b) {
private static boolean equal(Object a, Object b) {
return a == b || (a != null && a.equals(b));
}
@ -225,10 +228,6 @@ public final class $Gson$Types {
}
}
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();
}
@ -238,19 +237,19 @@ public final class $Gson$Types {
* IntegerSet}, the result for when supertype is {@code Set.class} is {@code Set<Integer>} and the
* result when the supertype is {@code Collection.class} is {@code Collection<Integer>}.
*/
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);
}
}
}
@ -259,17 +258,17 @@ public final class $Gson$Types {
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;
}
/**
@ -279,10 +278,13 @@ public final class $Gson$Types {
*
* @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
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,
@ -306,9 +308,6 @@ public final class $Gson$Types {
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];
}
@ -440,7 +439,7 @@ public final class $Gson$Types {
return toResolve;
}
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
@ -522,6 +521,10 @@ public final class $Gson$Types {
&& $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()

3
gson/src/main/java/com/google/gson/internal/bind/MapTypeAdapterFactory.java
View File

@ -120,8 +120,7 @@ public final class MapTypeAdapterFactory implements TypeAdapterFactory {
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<T> constructor = constructorConstructor.get(typeToken);

41
gson/src/main/java/com/google/gson/reflect/TypeToken.java
View File

@ -37,17 +37,20 @@ import java.util.Map;
* <p>
* {@code TypeToken<List<String>> list = new TypeToken<List<String>>() {};}
*
* <p>This syntax cannot be used to create type literals that have wildcard
* parameters, such as {@code Class<?>} or {@code List<? extends CharSequence>}.
* <p>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
*/
public class TypeToken<T> {
final Class<? super T> rawType;
final Type type;
final int hashCode;
private final Class<? super T> rawType;
private final Type type;
private final int hashCode;
/**
* Constructs a new type literal. Derives represented class from type
@ -59,7 +62,7 @@ public class TypeToken<T> {
*/
@SuppressWarnings("unchecked")
protected TypeToken() {
this.type = getSuperclassTypeParameter(getClass());
this.type = getTypeTokenTypeArgument();
this.rawType = (Class<? super T>) $Gson$Types.getRawType(type);
this.hashCode = type.hashCode();
}
@ -68,23 +71,33 @@ public class TypeToken<T> {
* 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<? super T>) $Gson$Types.getRawType(this.type);
this.hashCode = this.type.hashCode();
}
/**
* 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}.
*/
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]);
// 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");
}
/**

59
gson/src/test/java/com/google/gson/reflect/TypeTokenTest.java
View File

@ -27,9 +27,8 @@ import junit.framework.TestCase;
/**
* @author Jesse Wilson
*/
@SuppressWarnings({"deprecation"})
public final class TypeTokenTest extends TestCase {
// These fields are accessed using reflection by the tests below
List<Integer> listOfInteger = null;
List<Number> listOfNumber = null;
List<String> listOfString = null;
@ -37,6 +36,7 @@ public final class TypeTokenTest extends TestCase {
List<Set<String>> listOfSetOfString = null;
List<Set<?>> 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 final class TypeTokenTest extends TestCase {
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 final class TypeTokenTest extends TestCase {
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 final class TypeTokenTest extends TestCase {
// 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,56 @@ public final class TypeTokenTest extends TestCase {
Type listOfListOfString = TypeToken.getParameterized(List.class, listOfString).getType();
assertEquals(expectedListOfListOfListOfString, TypeToken.getParameterized(List.class, listOfListOfString));
}
private static class CustomTypeToken extends TypeToken<String> {
}
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.
*/
public void testTypeTokenSubSubClass() {
class SubTypeToken<T> extends TypeToken<String> {}
class SubSubTypeToken1<T> extends SubTypeToken<T> {}
class SubSubTypeToken2 extends SubTypeToken<Integer> {}
try {
new SubTypeToken<Integer>() {};
fail();
} catch (IllegalStateException expected) {
assertEquals("Must only create direct subclasses of TypeToken", expected.getMessage());
}
try {
new SubSubTypeToken1<Integer>();
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());
}
}
@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());
}
}
}