Fix duplicate key detection when first value is null (#3006)

andrewstellman · web-flow · commit f4d371d29c04 · 2026-04-05T10:27:39.000-07:00
* Fix duplicate key detection when first value is null

MapTypeAdapterFactory detected duplicate map keys by checking the
return value of Map.put(): `if (replaced != null)`. This missed
duplicates where the first value was null, because Map.put() returns
null (the previous value) which passed the != null check.

For example, {"a":null,"a":1} was silently accepted as {a=1} instead
of throwing JsonSyntaxException("duplicate key: a").

Fix: check Map.containsKey() before put() instead of relying on the
return value of put(). This correctly detects duplicates regardless
of whether the first value is null.

Fixes both the object form and the array-of-entries form
(enableComplexMapKeySerialization).

* Apply spotless formatting
diff --git a/gson/src/main/java/com/google/gson/internal/bind/MapTypeAdapterFactory.java b/gson/src/main/java/com/google/gson/internal/bind/MapTypeAdapterFactory.java
@@ -188,10 +188,10 @@ public Map<K, V> read(JsonReader in) throws IOException {
           in.beginArray(); // entry array
           K key = keyTypeAdapter.read(in);
           V value = valueTypeAdapter.read(in);
-          V replaced = map.put(key, value);
-          if (replaced != null) {
+          if (map.containsKey(key)) {
             throw new JsonSyntaxException("duplicate key: " + key);
           }
+          map.put(key, value);
           in.endArray();
         }
         in.endArray();
@@ -201,10 +201,10 @@ public Map<K, V> read(JsonReader in) throws IOException {
           JsonReaderInternalAccess.INSTANCE.promoteNameToValue(in);
           K key = keyTypeAdapter.read(in);
           V value = valueTypeAdapter.read(in);
-          V replaced = map.put(key, value);
-          if (replaced != null) {
+          if (map.containsKey(key)) {
             throw new JsonSyntaxException("duplicate key: " + key);
           }
+          map.put(key, value);
         }
         in.endObject();
       }
diff --git a/gson/src/test/java/com/google/gson/functional/MapAsArrayTypeAdapterTest.java b/gson/src/test/java/com/google/gson/functional/MapAsArrayTypeAdapterTest.java
@@ -80,6 +80,16 @@ public void testTwoTypesCollapseToOneDeserialize() {
     assertThat(e).hasMessageThat().isEqualTo("duplicate key: 1.0");
   }
 
+  @Test
+  public void testDuplicateKeyWithNullFirstValueArrayForm() {
+    Gson gson = new GsonBuilder().enableComplexMapKeySerialization().create();
+
+    String s = "[[\"a\",null],[\"a\",\"x\"]]";
+    Type type = new TypeToken<Map<String, String>>() {}.getType();
+    var e = assertThrows(JsonSyntaxException.class, () -> gson.fromJson(s, type));
+    assertThat(e).hasMessageThat().isEqualTo("duplicate key: a");
+  }
+
   @Test
   public void testMultipleEnableComplexKeyRegistrationHasNoEffect() {
     Type type = new TypeToken<Map<Point, String>>() {}.getType();
diff --git a/gson/src/test/java/com/google/gson/functional/MapTest.java b/gson/src/test/java/com/google/gson/functional/MapTest.java
@@ -693,6 +693,25 @@ public void testMapDeserializationWithDuplicateKeys() {
     assertThat(e).hasMessageThat().isEqualTo("duplicate key: a");
   }
 
+  @Test
+  public void testMapDeserializationWithDuplicateKeysNullFirstValue() {
+    Type type = new TypeToken<Map<String, Integer>>() {}.getType();
+    // Duplicate key where the first value is null; map.put() returns null (the previous value)
+    // so the old `if (replaced != null)` check failed to detect the duplicate
+    var e =
+        assertThrows(JsonSyntaxException.class, () -> gson.fromJson("{\"a\":null,\"a\":1}", type));
+    assertThat(e).hasMessageThat().isEqualTo("duplicate key: a");
+  }
+
+  @Test
+  public void testMapDeserializationWithDuplicateKeysBothNull() {
+    Type type = new TypeToken<Map<String, Integer>>() {}.getType();
+    var e =
+        assertThrows(
+            JsonSyntaxException.class, () -> gson.fromJson("{\"a\":null,\"a\":null}", type));
+    assertThat(e).hasMessageThat().isEqualTo("duplicate key: a");
+  }
+
   @Test
   public void testSerializeMapOfMaps() {
     Type type = new TypeToken<Map<String, Map<String, String>>>() {}.getType();

Original file line number	Diff line number	Diff line change
`@@ -188,10 +188,10 @@ public Map<K, V> read(JsonReader in) throws IOException {`
`188`	`188`	`in.beginArray(); // entry array`
`189`	`189`	`K key = keyTypeAdapter.read(in);`
`190`	`190`	`V value = valueTypeAdapter.read(in);`
`191`		`- V replaced = map.put(key, value);`
`192`		`- if (replaced != null) {`
	`191`	`+ if (map.containsKey(key)) {`
`193`	`192`	`throw new JsonSyntaxException("duplicate key: " + key);`
`194`	`193`	`}`
	`194`	`+ map.put(key, value);`
`195`	`195`	`in.endArray();`
`196`	`196`	`}`
`197`	`197`	`in.endArray();`
`@@ -201,10 +201,10 @@ public Map<K, V> read(JsonReader in) throws IOException {`
`201`	`201`	`JsonReaderInternalAccess.INSTANCE.promoteNameToValue(in);`
`202`	`202`	`K key = keyTypeAdapter.read(in);`
`203`	`203`	`V value = valueTypeAdapter.read(in);`
`204`		`- V replaced = map.put(key, value);`
`205`		`- if (replaced != null) {`
	`204`	`+ if (map.containsKey(key)) {`
`206`	`205`	`throw new JsonSyntaxException("duplicate key: " + key);`
`207`	`206`	`}`
	`207`	`+ map.put(key, value);`
`208`	`208`	`}`
`209`	`209`	`in.endObject();`
`210`	`210`	`}`