Fix TypeVariable/WildcardType recursion causing stackoverflows (#948)

* Port Gson Types recursion fix for subtypeOf/supertypeOf

From a300148003

First step in resolving #338

* Port Gson Type resolve() recursion fix for type variables

From https://github.com/google/gson/pull/1128

Resolves #338

* Capture adapter to make checkstyle happy

* Move RecursiveTypesResolveTest out of internal package

* Use moshi convention for tests

* 2 is not 1

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) {

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<TypeVariable>());
+  }
+
+  private static Type resolve(Type context, Class<?> contextRawType, Type toResolve,
+      Collection<TypeVariable> 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);
           }

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 <a href="https://github.com/google/gson/issues/440">Issue #440</a>
+ * and similar issues.
+ * <p>
+ * These tests originally caused {@link StackOverflowError} because of infinite recursion on attempts to
+ * resolve generics on types, with an intermediate types like 'Foo2&lt;? extends ? super ? extends ... ? extends A&gt;'
+ * <p>
+ * 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<A> {
+    public Foo2<? extends A> foo2;
+  }
+
+  private static class Foo2<B> {
+    public Foo1<? super B> foo1;
+  }
+
+  /**
+   * Test simplest case of recursion.
+   */
+  @Test public void recursiveResolveSimple() {
+    JsonAdapter<Foo1> 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)));
+  }
+}

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<T> {
+    RecursiveTypeVars<? super T> superType;
+  }
+
+  @Test public void recursiveTypeVariablesResolve() {
+    JsonAdapter<RecursiveTypeVars<String>> adapter = new Moshi.Builder().build().adapter(Types
+        .newParameterizedTypeWithOwner(TypesTest.class, RecursiveTypeVars.class, String.class));
+    assertThat(adapter).isNotNull();
+  }
+
+  @Test public void recursiveTypeVariablesResolve1() {
+    JsonAdapter<TestType> adapter = new Moshi.Builder().build().adapter(TestType.class);
+    assertThat(adapter).isNotNull();
+  }
+
+  @Test public void recursiveTypeVariablesResolve2() {
+    JsonAdapter<TestType2> adapter = new Moshi.Builder().build().adapter(TestType2.class);
+    assertThat(adapter).isNotNull();
+  }
+
+  private static class TestType<X> {
+    TestType<? super X> superType;
+  }
+
+  private static class TestType2<X, Y> {
+    TestType2<? super Y, ? super X> superReversedType;
+  }
+
   @JsonClass(generateAdapter = false)
   static class TestJsonClass {