diff --git a/moshi/src/main/java/com/squareup/moshi/Types.java b/moshi/src/main/java/com/squareup/moshi/Types.java index a9bca93..40829e3 100644 --- a/moshi/src/main/java/com/squareup/moshi/Types.java +++ b/moshi/src/main/java/com/squareup/moshi/Types.java @@ -139,7 +139,13 @@ public final class Types { * ? extends Object}. */ public static WildcardType subtypeOf(Type bound) { - return new WildcardTypeImpl(new Type[] { bound }, EMPTY_TYPE_ARRAY); + Type[] upperBounds; + if (bound instanceof WildcardType) { + upperBounds = ((WildcardType) bound).getUpperBounds(); + } else { + upperBounds = new Type[] { bound }; + } + return new WildcardTypeImpl(upperBounds, EMPTY_TYPE_ARRAY); } /** @@ -147,7 +153,13 @@ public final class Types { * bound} is {@code String.class}, this returns {@code ? super String}. */ public static WildcardType supertypeOf(Type bound) { - return new WildcardTypeImpl(new Type[] { Object.class }, new Type[] { bound }); + Type[] lowerBounds; + if (bound instanceof WildcardType) { + lowerBounds = ((WildcardType) bound).getLowerBounds(); + } else { + lowerBounds = new Type[] { bound }; + } + return new WildcardTypeImpl(new Type[] { Object.class }, lowerBounds); } public static Class getRawType(Type type) { diff --git a/moshi/src/main/java/com/squareup/moshi/internal/Util.java b/moshi/src/main/java/com/squareup/moshi/internal/Util.java index 1c49869..2480744 100644 --- a/moshi/src/main/java/com/squareup/moshi/internal/Util.java +++ b/moshi/src/main/java/com/squareup/moshi/internal/Util.java @@ -33,7 +33,9 @@ import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; import java.lang.reflect.WildcardType; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.NoSuchElementException; import java.util.Set; @@ -169,17 +171,29 @@ public final class Util { } public static Type resolve(Type context, Class contextRawType, Type toResolve) { + return resolve(context, contextRawType, toResolve, new HashSet()); + } + + private static Type resolve(Type context, Class contextRawType, Type toResolve, + Collection visitedTypeVariables) { // This implementation is made a little more complicated in an attempt to avoid object-creation. while (true) { if (toResolve instanceof TypeVariable) { TypeVariable typeVariable = (TypeVariable) toResolve; + if (visitedTypeVariables.contains(typeVariable)) { + // cannot reduce due to infinite recursion + return toResolve; + } else { + visitedTypeVariables.add(typeVariable); + } toResolve = resolveTypeVariable(context, contextRawType, typeVariable); if (toResolve == typeVariable) return toResolve; } else if (toResolve instanceof Class && ((Class) toResolve).isArray()) { Class original = (Class) toResolve; Type componentType = original.getComponentType(); - Type newComponentType = resolve(context, contextRawType, componentType); + Type newComponentType = resolve(context, contextRawType, componentType, + visitedTypeVariables); return componentType == newComponentType ? original : arrayOf(newComponentType); @@ -187,7 +201,8 @@ public final class Util { } else if (toResolve instanceof GenericArrayType) { GenericArrayType original = (GenericArrayType) toResolve; Type componentType = original.getGenericComponentType(); - Type newComponentType = resolve(context, contextRawType, componentType); + Type newComponentType = resolve(context, contextRawType, componentType, + visitedTypeVariables); return componentType == newComponentType ? original : arrayOf(newComponentType); @@ -195,12 +210,13 @@ public final class Util { } else if (toResolve instanceof ParameterizedType) { ParameterizedType original = (ParameterizedType) toResolve; Type ownerType = original.getOwnerType(); - Type newOwnerType = resolve(context, contextRawType, ownerType); + Type newOwnerType = resolve(context, contextRawType, ownerType, visitedTypeVariables); boolean changed = newOwnerType != ownerType; Type[] args = original.getActualTypeArguments(); for (int t = 0, length = args.length; t < length; t++) { - Type resolvedTypeArgument = resolve(context, contextRawType, args[t]); + Type resolvedTypeArgument = resolve(context, contextRawType, args[t], + visitedTypeVariables); if (resolvedTypeArgument != args[t]) { if (!changed) { args = args.clone(); @@ -220,12 +236,14 @@ public final class Util { Type[] originalUpperBound = original.getUpperBounds(); if (originalLowerBound.length == 1) { - Type lowerBound = resolve(context, contextRawType, originalLowerBound[0]); + Type lowerBound = resolve(context, contextRawType, originalLowerBound[0], + visitedTypeVariables); if (lowerBound != originalLowerBound[0]) { return supertypeOf(lowerBound); } } else if (originalUpperBound.length == 1) { - Type upperBound = resolve(context, contextRawType, originalUpperBound[0]); + Type upperBound = resolve(context, contextRawType, originalUpperBound[0], + visitedTypeVariables); if (upperBound != originalUpperBound[0]) { return subtypeOf(upperBound); } diff --git a/moshi/src/test/java/com/squareup/moshi/RecursiveTypesResolveTest.java b/moshi/src/test/java/com/squareup/moshi/RecursiveTypesResolveTest.java new file mode 100644 index 0000000..21f7d09 --- /dev/null +++ b/moshi/src/test/java/com/squareup/moshi/RecursiveTypesResolveTest.java @@ -0,0 +1,77 @@ +/* + * Copyright (C) 2017 Gson Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.squareup.moshi; + +import com.squareup.moshi.internal.Util; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + +/** + * Test fixes for infinite recursion on {@link Util#resolve(java.lang.reflect.Type, Class, + * java.lang.reflect.Type)}, described at Issue #440 + * and similar issues. + *

+ * These tests originally caused {@link StackOverflowError} because of infinite recursion on attempts to + * resolve generics on types, with an intermediate types like 'Foo2<? extends ? super ? extends ... ? extends A>' + *

+ * Adapted from https://github.com/google/gson/commit/a300148003e3a067875b1444e8268b6e0f0e0e02 in + * service of https://github.com/square/moshi/issues/338. + */ +public final class RecursiveTypesResolveTest { + + private static class Foo1 { + public Foo2 foo2; + } + + private static class Foo2 { + public Foo1 foo1; + } + + /** + * Test simplest case of recursion. + */ + @Test public void recursiveResolveSimple() { + JsonAdapter adapter = new Moshi.Builder().build().adapter(Foo1.class); + assertNotNull(adapter); + } + + // + // Tests belows check the behaviour of the methods changed for the fix + // + + @Test public void doubleSupertype() { + assertEquals(Types.supertypeOf(Number.class), + Types.supertypeOf(Types.supertypeOf(Number.class))); + } + + @Test public void doubleSubtype() { + assertEquals(Types.subtypeOf(Number.class), + Types.subtypeOf(Types.subtypeOf(Number.class))); + } + + @Test public void superSubtype() { + assertEquals(Types.subtypeOf(Object.class), + Types.supertypeOf(Types.subtypeOf(Number.class))); + } + + @Test public void subSupertype() { + assertEquals(Types.subtypeOf(Object.class), + Types.subtypeOf(Types.supertypeOf(Number.class))); + } +} diff --git a/moshi/src/test/java/com/squareup/moshi/TypesTest.java b/moshi/src/test/java/com/squareup/moshi/TypesTest.java index b2d4920..e1419ba 100644 --- a/moshi/src/test/java/com/squareup/moshi/TypesTest.java +++ b/moshi/src/test/java/com/squareup/moshi/TypesTest.java @@ -321,6 +321,40 @@ public final class TypesTest { } } + // + // Regression tests for https://github.com/square/moshi/issues/338 + // + // Adapted from https://github.com/google/gson/pull/1128 + // + + private static final class RecursiveTypeVars { + RecursiveTypeVars superType; + } + + @Test public void recursiveTypeVariablesResolve() { + JsonAdapter> adapter = new Moshi.Builder().build().adapter(Types + .newParameterizedTypeWithOwner(TypesTest.class, RecursiveTypeVars.class, String.class)); + assertThat(adapter).isNotNull(); + } + + @Test public void recursiveTypeVariablesResolve1() { + JsonAdapter adapter = new Moshi.Builder().build().adapter(TestType.class); + assertThat(adapter).isNotNull(); + } + + @Test public void recursiveTypeVariablesResolve2() { + JsonAdapter adapter = new Moshi.Builder().build().adapter(TestType2.class); + assertThat(adapter).isNotNull(); + } + + private static class TestType { + TestType superType; + } + + private static class TestType2 { + TestType2 superReversedType; + } + @JsonClass(generateAdapter = false) static class TestJsonClass {