Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public <T> ObjectConstructor<T> get(TypeToken<T> typeToken, boolean allowUnsafe)
return defaultConstructor;
}

ObjectConstructor<T> defaultImplementation = newDefaultImplementationConstructor(type, rawType);
ObjectConstructor<T> defaultImplementation = newDefaultImplementationConstructor(rawType);
if (defaultImplementation != null) {
return defaultImplementation;
}
Expand Down Expand Up @@ -286,7 +286,7 @@ private static <T> ObjectConstructor<T> newDefaultConstructor(

/** Constructors for common interface types like Map and List and their subtypes. */
private static <T> ObjectConstructor<T> newDefaultImplementationConstructor(
Type type, Class<? super T> rawType) {
Class<? super T> rawType) {

/*
* IMPORTANT: Must only create instances for classes with public no-args constructor.
Expand All @@ -304,7 +304,7 @@ private static <T> ObjectConstructor<T> newDefaultImplementationConstructor(

if (Map.class.isAssignableFrom(rawType)) {
@SuppressWarnings("unchecked")
ObjectConstructor<T> constructor = (ObjectConstructor<T>) newMapConstructor(type, rawType);
ObjectConstructor<T> constructor = (ObjectConstructor<T>) newMapConstructor(rawType);
return constructor;
}

Expand Down Expand Up @@ -336,32 +336,9 @@ else if (rawType.isAssignableFrom(ArrayDeque.class)) {
return null;
}

private static boolean hasStringKeyType(Type mapType) {
// If mapType is not parameterized, assume it might have String as key type
if (!(mapType instanceof ParameterizedType)) {
return true;
}

Type[] typeArguments = ((ParameterizedType) mapType).getActualTypeArguments();
if (typeArguments.length == 0) {
return false;
}
return GsonTypes.getRawType(typeArguments[0]) == String.class;
}

private static ObjectConstructor<? extends Map<?, Object>> newMapConstructor(
Type type, Class<?> rawType) {
private static ObjectConstructor<? extends Map<?, Object>> newMapConstructor(Class<?> rawType) {
// First try Map implementation
/*
* Legacy special casing for Map<String, ...> to avoid DoS from colliding String hashCode
* values for older JDKs; use own LinkedTreeMap<String, Object> instead
*/
if (rawType.isAssignableFrom(LinkedTreeMap.class) && hasStringKeyType(type)) {
// Must use lambda instead of method reference (`LinkedTreeMap::new`) here, otherwise this
// causes an exception when Gson is used by a custom system class loader, see
// https://github.com/google/gson/pull/2864#issuecomment-3528623716
return () -> new LinkedTreeMap<>();
} else if (rawType.isAssignableFrom(LinkedHashMap.class)) {
if (rawType.isAssignableFrom(LinkedHashMap.class)) {
return LinkedHashMap::new;
}
// Then try SortedMap / NavigableMap implementation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import com.google.gson.ToNumberStrategy;
import com.google.gson.TypeAdapter;
import com.google.gson.TypeAdapterFactory;
import com.google.gson.internal.LinkedTreeMap;
import com.google.gson.reflect.TypeToken;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonToken;
Expand All @@ -30,6 +29,7 @@
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Deque;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -81,7 +81,7 @@ private Object tryBeginNesting(JsonReader in, JsonToken peeked) throws IOExcepti
return new ArrayList<>();
case BEGIN_OBJECT:
in.beginObject();
return new LinkedTreeMap<>();
return new LinkedHashMap<>();
default:
return null;
}
Expand Down
14 changes: 8 additions & 6 deletions gson/src/test/java/com/google/gson/functional/MapTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,14 @@ public void testMapDeserializationWithNullKey() {
assertThat(map.get(null)).isNull();
}

@Test
public void testMapDeserializationWithNullKeyAndListOfEntries() {
Type typeOfMap = new TypeToken<Map<String, Integer>>() {}.getType();
Map<String, Integer> map = gson.fromJson("[[null,123]]", typeOfMap);
assertThat(map).hasSize(1);
assertThat(map.get(null)).isEqualTo(123);
}

@Test
public void testMapSerializationWithIntegerKeys() {
Map<Integer, String> map = new LinkedHashMap<>();
Expand Down Expand Up @@ -220,12 +228,6 @@ public void testMapStringKeyDeserialization() {
Type typeOfMap = new TypeToken<Map<String, Integer>>() {}.getType();
Map<?, ?> map = gson.fromJson("{\"a\":1}", typeOfMap);

assertWithMessage(
"Map<String, ...> should use LinkedTreeMap to protect against DoS in older JDK"
+ " versions")
.that(map)
.isInstanceOf(LinkedTreeMap.class);

Map<?, ?> expectedMap = Collections.singletonMap("a", 1);
assertThat(map).isEqualTo(expectedMap);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,15 +158,12 @@ public void testCustomCollectionInterfaceCreation() {

@Test
public void testStringMapCreation() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename this test method as well to make it more generic?

Suggested change
public void testStringMapCreation() {
public void testMapCreation() {

Previously the test was specific to how Map<String, ...> was handled, but now with all the comments removed the test method name might be a bit misleading.

// When creating raw Map should use Gson's LinkedTreeMap, assuming keys could be String
Object actual = constructorConstructor.get(TypeToken.get(Map.class)).construct();
assertThat(actual).isInstanceOf(LinkedTreeMap.class);
assertThat(actual).isInstanceOf(LinkedHashMap.class);

// When creating a `Map<String, ...>` should use Gson's LinkedTreeMap
actual = constructorConstructor.get(new TypeToken<Map<String, Integer>>() {}).construct();
assertThat(actual).isInstanceOf(LinkedTreeMap.class);
assertThat(actual).isInstanceOf(LinkedHashMap.class);

// But when explicitly requesting a JDK `LinkedHashMap<String, ...>` should use LinkedHashMap
actual =
constructorConstructor.get(new TypeToken<LinkedHashMap<String, Integer>>() {}).construct();
assertThat(actual).isInstanceOf(LinkedHashMap.class);
Expand Down