GWT-RPC: don't inherit type variables for non-parameterized types
For example, suppose we're deserializing a field:
Foo field
If Foo is a simple class or interface with no type parameters,
it shouldn't inherit the values of any type variables from its
environment. (The Foo class may have type variables of its own
but they should be independent.)
As a workaround I just made sure resolvedTypes is empty.
To do the job right probably requires a rewrite and public
API change for server-side custom serializers.
Includes a test.
Bug: issue 7628
Change-Id: Ib674812af6b8eabace42b4185ec5f07e967f1761
Review-Link: https://gwt-review.googlesource.com/#/c/5320/
diff --git a/user/src/com/google/gwt/user/server/rpc/impl/DequeMap.java b/user/src/com/google/gwt/user/server/rpc/impl/DequeMap.java
index f8e6aeb..35402a6 100644
--- a/user/src/com/google/gwt/user/server/rpc/impl/DequeMap.java
+++ b/user/src/com/google/gwt/user/server/rpc/impl/DequeMap.java
@@ -34,17 +34,17 @@
/**
* The default initial capacity for the Deque objects.
*/
- static final int DEFAULT_INITIAL_DEQUE_CAPACITY = 3;
+ private static final int DEFAULT_INITIAL_DEQUE_CAPACITY = 3;
/**
* The default initial capacity for the Deque objects.
*/
- static final int DEFAULT_INITIAL_MAP_CAPACITY = 3;
+ private static final int DEFAULT_INITIAL_MAP_CAPACITY = 3;
/**
* The initial capacity for the Deque objects.
*/
- int dequeCapacity = DEFAULT_INITIAL_DEQUE_CAPACITY;
+ private int dequeCapacity = DEFAULT_INITIAL_DEQUE_CAPACITY;
/**
* The map used to hold data.
@@ -52,7 +52,9 @@
* Note that we do not extend map because we require control over the
* addition and removal of elements from the map.
*/
- HashMap<K, Deque<V>> map = null;
+ private final HashMap<K, ArrayDeque<V>> map;
+
+ private int size = 0;
/**
* Constructs an empty <tt>DequeMap</tt> with the default initial capacities
@@ -71,7 +73,7 @@
* @throws IllegalArgumentException if the initial capacity is negative.
*/
public DequeMap(int initialMapCapacity, int initialDequeCapacity) {
- map = new HashMap<K, Deque<V>>(initialMapCapacity);
+ map = new HashMap<K, ArrayDeque<V>>(initialMapCapacity);
dequeCapacity = initialDequeCapacity;
}
@@ -86,12 +88,16 @@
* @param value the value to push for the key
*/
public void add(K key, V value) {
- Deque<V> deque = map.get(key);
+ ArrayDeque<V> deque = map.get(key);
if (deque == null) {
deque = new ArrayDeque<V>(dequeCapacity);
deque.addFirst(value);
map.put(key, deque);
+ size++;
} else {
+ if (deque.isEmpty()) {
+ size++;
+ }
deque.addFirst(value);
}
}
@@ -123,6 +129,19 @@
if (deque == null) {
return null;
}
- return deque.pollFirst();
+ boolean wasEmpty = deque.isEmpty();
+ V result = deque.pollFirst();
+ if (deque.isEmpty() && !wasEmpty) {
+ assert size > 0;
+ size--;
+ }
+ return result;
+ }
+
+ /**
+ * Returns true if no mappings are defined.
+ */
+ public boolean isEmpty() {
+ return size == 0;
}
}
diff --git a/user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamReader.java b/user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamReader.java
index 6b703e8..8638960 100644
--- a/user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamReader.java
+++ b/user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamReader.java
@@ -578,6 +578,17 @@
return null;
}
+ if (isSimpleClass(expectedType) && !resolvedTypes.isEmpty()) {
+ // Start a new scope for resolving type variables. This is a workaround for false sharing
+ // because the type checker doesn't implement type variables properly.
+ //
+ // For example, if expectedType is Serializable and the actual value is a LinkedHashMap, we
+ // don't want any of LinkedHashMap, HashMap, or Map to inherit any bindings for their K,V
+ // variables from the surrounding content.
+ // Fixes issue 7628.
+ resolvedTypes = new DequeMap<TypeVariable<?>, Type>();
+ }
+
return deserialize(typeSignature, expectedType, resolvedTypes);
}
@@ -1116,6 +1127,17 @@
return null;
}
+ /**
+ * Returns true if the given type is a top-level class or interface with no type parameters.
+ */
+ private boolean isSimpleClass(Type t) {
+ if (!(t instanceof Class)) {
+ return false;
+ }
+ Class cl = (Class) t;
+ return cl.getTypeParameters().length == 0 && cl.getEnclosingClass() == null;
+ }
+
private void validateTypeVersions(Class<?> instanceClass,
SerializedInstanceReference serializedInstRef) throws SerializationException {
String clientTypeSignature = serializedInstRef.getSignature();
diff --git a/user/test/com/google/gwt/user/RpcJreSuite.java b/user/test/com/google/gwt/user/RpcJreSuite.java
index 22d8c35..e21c423 100644
--- a/user/test/com/google/gwt/user/RpcJreSuite.java
+++ b/user/test/com/google/gwt/user/RpcJreSuite.java
@@ -24,6 +24,7 @@
import com.google.gwt.user.server.Base64Test;
import com.google.gwt.user.server.UtilTest;
import com.google.gwt.user.server.rpc.AbstractXsrfProtectedServiceServletTest;
+import com.google.gwt.user.server.rpc.DequeMapTest;
import com.google.gwt.user.server.rpc.RPCRequestTest;
import com.google.gwt.user.server.rpc.RPCServletUtilsTest;
import com.google.gwt.user.server.rpc.RPCTest;
@@ -54,6 +55,7 @@
public static Test suite() {
TestSuite suite = new TestSuite("Non-browser tests for com.google.gwt.user.client.rpc");
suite.addTestSuite(BlacklistTypeFilterTest.class);
+ suite.addTestSuite(DequeMapTest.class);
suite.addTestSuite(SerializationUtilsTest.class);
suite.addTestSuite(SerializableTypeOracleBuilderTest.class);
suite.addTestSuite(TypeHierarchyUtilsTest.class);
diff --git a/user/test/com/google/gwt/user/server/rpc/DequeMapTest.java b/user/test/com/google/gwt/user/server/rpc/DequeMapTest.java
new file mode 100644
index 0000000..6efcb78
--- /dev/null
+++ b/user/test/com/google/gwt/user/server/rpc/DequeMapTest.java
@@ -0,0 +1,55 @@
+/*
+ * 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;
+
+import com.google.gwt.user.server.rpc.impl.DequeMap;
+
+import junit.framework.TestCase;
+
+/**
+ * Verifies {@link DequeMap}.
+ */
+public class DequeMapTest extends TestCase {
+
+ public void testIsEmpty() throws Exception {
+ DequeMap<String, String> m = new DequeMap<String, String>();
+ assertTrue(m.isEmpty());
+ assertEquals(null, m.get("foo"));
+
+ m.add("foo", "first");
+ assertFalse(m.isEmpty());
+ assertEquals("first", m.get("foo"));
+
+ m.add("foo", "second");
+ assertFalse(m.isEmpty());
+ assertEquals("second", m.get("foo"));
+
+ assertEquals(null, m.remove("unknown"));
+ assertFalse(m.isEmpty());
+
+ assertEquals("second", m.remove("foo"));
+ assertFalse(m.isEmpty());
+ assertEquals("first", m.get("foo"));
+
+ assertEquals("first", m.remove("foo"));
+ assertTrue(m.isEmpty());
+ assertEquals(null, m.get("foo"));
+
+ assertEquals(null, m.remove("foo"));
+ assertTrue(m.isEmpty());
+ }
+}
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 d6fc11d..9129bae 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,9 @@
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.SubtypeUsedTwice;
+import com.google.gwt.user.server.rpc.testcases.SubtypeUsedTwice.Arg;
+import com.google.gwt.user.server.rpc.testcases.SubtypeUsedTwice.TypedHandle;
import com.google.gwt.user.server.rpc.testcases.TypeVariableCycle;
import junit.framework.TestCase;
@@ -2475,6 +2478,22 @@
assertEquals("hello", ((TypeVariableCycle.PtrPtr) deserializedArg).get());
}
+ public void testSubtypeUsedTwice() throws Exception {
+
+ // Build an RPC request that calls send(ARG)
+ RPCTypeCheckFactory builder = new RPCTypeCheckFactory(SubtypeUsedTwice.class, "send");
+ builder.write(SubtypeUsedTwice.makeArg(123));
+ String request = builder.toString();
+
+ // Make sure we can decode it.
+ RPCRequest decoded = RPC.decodeRequest(request);
+ Object deserializedArg = decoded.getParameters()[0];
+ assertEquals(Arg.class, deserializedArg.getClass());
+ IsSerializable thing = ((Arg) deserializedArg).handle.thing.any;
+ TypedHandle handle = (TypedHandle) thing;
+ assertEquals(123, handle.thing);
+ }
+
/**
* This checks that HashMap correctly reports that it is an incorrect type.
*/
diff --git a/user/test/com/google/gwt/user/server/rpc/testcases/SubtypeUsedTwice.java b/user/test/com/google/gwt/user/server/rpc/testcases/SubtypeUsedTwice.java
new file mode 100644
index 0000000..fc19ab4
--- /dev/null
+++ b/user/test/com/google/gwt/user/server/rpc/testcases/SubtypeUsedTwice.java
@@ -0,0 +1,75 @@
+/*
+ * 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;
+
+/**
+ * Tests that we can use the type parameter on a subtype two different ways.
+ */
+public class SubtypeUsedTwice implements RemoteService {
+
+ /**
+ * Creates the argument we will use to calling {@link #send}.
+ */
+ public static Arg makeArg(int value) {
+ TypedHandle<Integer> intHolder = new MyTypedHandle<Integer>();
+ intHolder.thing = value;
+
+ Any any = new Any();
+ any.any = intHolder;
+
+ Arg arg = new Arg();
+ arg.handle.thing = any;
+
+ return arg;
+ }
+
+ /**
+ * A dummy RPC call that sends one argument to the server.
+ */
+ public static void send(Arg a) {
+ }
+
+ /**
+ * The RPC call's argument.
+ */
+ public static class Arg implements IsSerializable {
+ public TypedHandle<Any> handle = new MyTypedHandle<Any>();
+ }
+
+ /**
+ * A class that will hold the second instance of MyTypedHandle,
+ * without constraining it.
+ */
+ public static class Any implements IsSerializable {
+ public IsSerializable any;
+ }
+
+ /**
+ * Superclass with a type variable.
+ */
+ public static abstract class TypedHandle<T> implements IsSerializable {
+ public T thing;
+ }
+
+ /**
+ * Subclass with a different type variable.
+ */
+ public static class MyTypedHandle<T2> extends TypedHandle<T2> {
+ }
+}
