Fix an infinite loop in RPC deserialization due to type variable cycles.
Also, add RPCTypeCheck to a test suite so it actually runs, and fix a test
that now receives AssertionError instead of ArrayIndexOutOfBounds.
Fixes issue 7779.
Based on a patch by James@wetheinter.net
Change-Id: I88c2774622d22b982346d1ffe5cf12f5d3c96d92
Review-Link: https://gwt-review.googlesource.com/#/c/1410/
Review by: rluble@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@11467 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/server/rpc/impl/SerializabilityUtil.java b/user/src/com/google/gwt/user/server/rpc/impl/SerializabilityUtil.java
index 7eb246b..c62f0ec 100644
--- a/user/src/com/google/gwt/user/server/rpc/impl/SerializabilityUtil.java
+++ b/user/src/com/google/gwt/user/server/rpc/impl/SerializabilityUtil.java
@@ -219,25 +219,50 @@
}
/**
- * Return the concrete type that a generic type maps to, if known.
+ * Resolve type variables to concrete types if possible. Otherwise, just return
+ * the type variable.
*
- * @param genericType The generic type to resolve.
+ * @param unresolved The type to resolve
* @param resolvedTypes A map of generic types to actual types.
* @return The actual type, which may be of any subclass of Type.
*/
- public static Type findActualType(Type genericType,
+ public static Type findActualType(Type unresolved,
DequeMap<TypeVariable<?>, Type> resolvedTypes) {
- Type result = genericType;
- // Look for things that TypeVariables are mapped to, but stop if mapped
- // to itself. We map a TypeVariable to itself when we wish to explicitly
- // mark it as unmapped.
- while (result instanceof TypeVariable<?> &&
- resolvedTypes.get((TypeVariable<?>) result) != result &&
- resolvedTypes.get((TypeVariable<?>) result) != null) {
- result = resolvedTypes.get((TypeVariable<?>) result);
+
+ // Handle simple cases quickly.
+ if (!(unresolved instanceof TypeVariable<?>)) {
+ return unresolved;
+ }
+ TypeVariable<?> var = (TypeVariable<?>) unresolved;
+ Type target = resolvedTypes.get(var);
+ if (target == null || target == var) {
+ return var;
+ }
+ if (!(target instanceof TypeVariable<?>)) {
+ return target;
}
- return result;
+ // Type variables that point to other type variables might form a cycle, which
+ // means they're all equivalent. Keep track of visited type variables to detect this.
+ Set<TypeVariable<?>> seen = new HashSet<TypeVariable<?>>();
+ seen.add(var);
+ var = (TypeVariable<?>) target;
+ seen.add(var);
+
+ while (true) {
+ target = resolvedTypes.get(var);
+ if (target == null || target == var) {
+ return var;
+ }
+ if (!(target instanceof TypeVariable<?>)) {
+ return target;
+ }
+ var = (TypeVariable<?>) target;
+ if (!seen.add(var)) {
+ // Cycle detected; returning an arbitrary var in the cycle.
+ return var;
+ }
+ }
}
/**
diff --git a/user/test/com/google/gwt/user/RpcSuiteNoBrowser.java b/user/test/com/google/gwt/user/RpcSuiteNoBrowser.java
index a048816..5756bcc 100644
--- a/user/test/com/google/gwt/user/RpcSuiteNoBrowser.java
+++ b/user/test/com/google/gwt/user/RpcSuiteNoBrowser.java
@@ -27,6 +27,7 @@
import com.google.gwt.user.server.rpc.RPCRequestTest;
import com.google.gwt.user.server.rpc.RPCServletUtilsTest;
import com.google.gwt.user.server.rpc.RPCTest;
+import com.google.gwt.user.server.rpc.RPCTypeCheckTest;
import com.google.gwt.user.server.rpc.RemoteServiceServletTest;
import com.google.gwt.user.server.rpc.SerializationPolicyLoaderTest;
import com.google.gwt.user.server.rpc.impl.LegacySerializationPolicyTest;
@@ -57,6 +58,7 @@
suite.addTestSuite(SerializableTypeOracleBuilderTest.class);
suite.addTestSuite(TypeHierarchyUtilsTest.class);
suite.addTestSuite(RPCTest.class);
+ suite.addTestSuite(RPCTypeCheckTest.class);
suite.addTestSuite(RemoteServiceServletTest.class);
suite.addTestSuite(LegacySerializationPolicyTest.class);
suite.addTestSuite(StandardSerializationPolicyTest.class);
diff --git a/user/test/com/google/gwt/user/server/rpc/RPCTypeCheckTest.java b/user/test/com/google/gwt/user/server/rpc/RPCTypeCheckTest.java
index 76f155b..d6fc11d 100644
--- a/user/test/com/google/gwt/user/server/rpc/RPCTypeCheckTest.java
+++ b/user/test/com/google/gwt/user/server/rpc/RPCTypeCheckTest.java
@@ -23,6 +23,7 @@
import com.google.gwt.user.client.rpc.SerializedTypeViolationException;
import com.google.gwt.user.client.rpc.TestSetFactory.ReverseSorter;
import com.google.gwt.user.server.rpc.RPCTypeCheckCollectionsTest.TestHashSet;
+import com.google.gwt.user.server.rpc.testcases.TypeVariableCycle;
import junit.framework.TestCase;
@@ -2458,6 +2459,23 @@
}
/**
+ * Test for <a href="https://code.google.com/p/google-web-toolkit/issues/detail?id=7779">7779</a>.
+ */
+ public void testTypeVariableCycle() throws Exception {
+
+ // Build an RPC request that calls dereference(hello)
+ RPCTypeCheckFactory builder = new RPCTypeCheckFactory(TypeVariableCycle.class, "dereference");
+ builder.write(TypeVariableCycle.HELLO);
+ String request = builder.toString();
+
+ // Make sure we can decode it.
+ RPCRequest decoded = RPC.decodeRequest(request);
+ Object deserializedArg = decoded.getParameters()[0];
+ assertEquals(TypeVariableCycle.PtrPtr.class, deserializedArg.getClass());
+ assertEquals("hello", ((TypeVariableCycle.PtrPtr) deserializedArg).get());
+ }
+
+ /**
* This checks that HashMap correctly reports that it is an incorrect type.
*/
public void testHashMapSpoofingClass() {
@@ -2760,6 +2778,7 @@
* arguments of a primitive value type with another primitive type.
*/
public void testValueSpoofing() {
+ boolean returned = false;
try {
// When an int appears in place of a string, the result will be the
// int value indexing the string table, which will result in
@@ -2767,10 +2786,14 @@
// an incorrect string if the integer value is within range of the string
// table.
RPC.decodeRequest(generateIntSpoofingString());
- fail("Expected ArrayIndexOutOfBoundsException from testValueSpoofing (1)");
- } catch (ArrayIndexOutOfBoundsException e) {
+ returned = true;
+ } catch (AssertionError e) {
// Expected
}
+ if (returned) {
+ fail("RPC.decodeRequest should have thrown.");
+ }
+
try {
// When a string pretends to be an int, it simply results in an incorrect
// integer value.
diff --git a/user/test/com/google/gwt/user/server/rpc/testcases/TypeVariableCycle.java b/user/test/com/google/gwt/user/server/rpc/testcases/TypeVariableCycle.java
new file mode 100644
index 0000000..190f129
--- /dev/null
+++ b/user/test/com/google/gwt/user/server/rpc/testcases/TypeVariableCycle.java
@@ -0,0 +1,113 @@
+/*
+ * Copyright 2013 Google Inc.
+ *
+ * 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.google.gwt.user.server.rpc.testcases;
+
+import com.google.gwt.user.client.rpc.IsSerializable;
+import com.google.gwt.user.client.rpc.RemoteService;
+
+/**
+ * An example of how to create a cycle using type variable inference. To determine
+ * the expected type of the HELLO.ptr field, the type inference in
+ * {@link com.google.gwt.user.server.rpc.impl.SerializabilityUtil}
+ * starts with PtrPtr.ptr and deduces the following:
+ *
+ * <ul>
+ * <li>C = B (by inheritance)
+ * <li>B = A (by inheritance)
+ * <li>A = C (because the raw type of ptr is BasePtr<A> and we are substituting C into it)
+ * </pre>
+ *
+ * <p>If we put these deductions into a map then there will be a cycle in the map.
+ * The type inferencer needs to detect the cycle and conclude that there is no constraint
+ * on any of these type variables, so HELLO.ptr.target could be any serializable
+ * type.</p>
+ *
+ * <p>TODO(skybrian): it's unclear whether this should really be a cycle. The 'A' in the second
+ * deduction annotates the the PtrPtr object (HELLO) and the 'A' in the last deduction is for the
+ * SimplePtr object (HELLO.ptr) so perhaps they are two type variables of the same name in different
+ * scopes? Currently the algorithm in SerializabilityUtil doesn't have scopes so this might
+ * just be a false alias. It doesn't affect the conclusion for this example, though.</p>
+ */
+public class TypeVariableCycle implements RemoteService {
+
+ /**
+ * A value we that we want to send via GWT-RPC.
+ */
+ public static final PtrPtr<String> HELLO =
+ new PtrPtr<String>(new SimplePtr<String>("hello"));
+
+ /**
+ * The RPC method that we will call.
+ */
+ @SuppressWarnings("unused")
+ public static <X> X dereference(BasePtr<X> any) {
+ return any.get();
+ }
+
+ /**
+ * The base of a convoluted class hierarchy of pointer types.
+ */
+ public static abstract class BasePtr<A> implements IsSerializable {
+ abstract A get();
+ }
+
+ /**
+ * An unneeded intermediate class to make a size-3 cycle. (Intermediate
+ * classes could be added to make a cycle of any size.)
+ */
+ public abstract static class NonSimplePtr<B> extends BasePtr<B> {
+ }
+
+ /**
+ * A pointer to a pointer.
+ */
+ public static class PtrPtr<C> extends NonSimplePtr<C> {
+ public BasePtr<C> ptr;
+
+ @SuppressWarnings("unused")
+ PtrPtr() {
+ }
+
+ PtrPtr(BasePtr<C> ptr) {
+ this.ptr = ptr;
+ }
+
+ @Override
+ public C get() {
+ return ptr.get();
+ }
+ }
+
+ /**
+ * A trivial implementation of BasePtr.
+ */
+ public static class SimplePtr<D> extends BasePtr<D> {
+ public D target;
+
+ @SuppressWarnings("unused")
+ SimplePtr() {
+ }
+
+ SimplePtr(D target) {
+ this.target = target;
+ }
+
+ @Override
+ D get() {
+ return target;
+ }
+ }
+}
