Skip to content

Commit 6a7f0ff

Browse files
Throw JsonSyntaxException instead of raw exceptions in built-in adapters
Gson.fromJson is documented to throw JsonSyntaxException for malformed input, but four groups of built-in adapters let raw NumberFormatException, IllegalArgumentException or NullPointerException escape to the caller. Callers that guard with catch (JsonParseException) do not catch these. This is the same defect class already fixed for AtomicLongArray in #3038 and currently open for AtomicIntegerArray in #3047 / #3095. In each case the neighbouring target type already behaves correctly. - BitSet with a malformed number - null elements in primitive arrays (int[], double[], char[], ...) - double/float with a malformed number - null elements in collections that reject them (TreeSet, ArrayDeque, PriorityQueue, EnumSet)
1 parent b3f4ca2 commit 6a7f0ff

7 files changed

Lines changed: 138 additions & 5 deletions

File tree

gson/src/main/java/com/google/gson/internal/bind/ArrayTypeAdapter.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.gson.internal.bind;
1818

1919
import com.google.gson.Gson;
20+
import com.google.gson.JsonSyntaxException;
2021
import com.google.gson.TypeAdapter;
2122
import com.google.gson.TypeAdapterFactory;
2223
import com.google.gson.internal.GsonTypes;
@@ -69,17 +70,27 @@ public Object read(JsonReader in) throws IOException {
6970
return null;
7071
}
7172

73+
boolean primitiveComponent = componentType.isPrimitive();
7274
ArrayList<E> list = new ArrayList<>();
7375
in.beginArray();
7476
while (in.hasNext()) {
7577
E instance = componentTypeAdapter.read(in);
78+
if (instance == null && primitiveComponent) {
79+
// A primitive array cannot hold null; reject it here so that callers see Gson's documented
80+
// JsonSyntaxException instead of an IllegalArgumentException from Array.set below.
81+
throw new JsonSyntaxException(
82+
"null is not a valid value for a "
83+
+ componentType.getName()
84+
+ " array element; at path "
85+
+ in.getPreviousPath());
86+
}
7687
list.add(instance);
7788
}
7889
in.endArray();
7990

8091
int size = list.size();
8192
// Have to copy primitives one by one to primitive array
82-
if (componentType.isPrimitive()) {
93+
if (primitiveComponent) {
8394
Object array = Array.newInstance(componentType, size);
8495
for (int i = 0; i < size; i++) {
8596
Array.set(array, i, list.get(i));

gson/src/main/java/com/google/gson/internal/bind/CollectionTypeAdapterFactory.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.gson.internal.bind;
1818

1919
import com.google.gson.Gson;
20+
import com.google.gson.JsonSyntaxException;
2021
import com.google.gson.TypeAdapter;
2122
import com.google.gson.TypeAdapterFactory;
2223
import com.google.gson.internal.ConstructorConstructor;
@@ -82,7 +83,24 @@ public Collection<E> read(JsonReader in) throws IOException {
8283
in.beginArray();
8384
while (in.hasNext()) {
8485
E instance = elementTypeAdapter.read(in);
85-
collection.add(instance);
86+
if (instance == null) {
87+
// Most collections accept null, but some (TreeSet, ArrayDeque, PriorityQueue, EnumSet...)
88+
// reject it with a NullPointerException. Surface that as Gson's documented
89+
// JsonSyntaxException. Only the null case is wrapped, so a NullPointerException from
90+
// anywhere else is not masked.
91+
try {
92+
collection.add(null);
93+
} catch (NullPointerException e) {
94+
throw new JsonSyntaxException(
95+
"null is not a valid element for "
96+
+ collection.getClass().getName()
97+
+ "; at path "
98+
+ in.getPreviousPath(),
99+
e);
100+
}
101+
} else {
102+
collection.add(instance);
103+
}
86104
}
87105
in.endArray();
88106
return collection;

gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,14 @@ public BitSet read(JsonReader in) throws IOException {
102102
switch (tokenType) {
103103
case NUMBER:
104104
case STRING:
105-
int intValue = in.nextInt();
105+
int intValue;
106+
try {
107+
intValue = in.nextInt();
108+
} catch (NumberFormatException e) {
109+
// Match the other adapters: malformed numbers must surface as Gson's documented
110+
// JsonSyntaxException, not as a raw NumberFormatException.
111+
throw new JsonSyntaxException(e);
112+
}
106113
if (intValue == 0) {
107114
set = false;
108115
} else if (intValue == 1) {
@@ -467,7 +474,13 @@ public Float read(JsonReader in) throws IOException {
467474
in.nextNull();
468475
return null;
469476
}
470-
return (float) in.nextDouble();
477+
try {
478+
return (float) in.nextDouble();
479+
} catch (NumberFormatException e) {
480+
// Consistent with BYTE, SHORT, INTEGER, LONG, BIG_DECIMAL and BIG_INTEGER above: a
481+
// malformed number must surface as Gson's documented JsonSyntaxException.
482+
throw new JsonSyntaxException(e);
483+
}
471484
}
472485

473486
@Override
@@ -500,7 +513,13 @@ public Double read(JsonReader in) throws IOException {
500513
in.nextNull();
501514
return null;
502515
}
503-
return in.nextDouble();
516+
try {
517+
return in.nextDouble();
518+
} catch (NumberFormatException e) {
519+
// Consistent with BYTE, SHORT, INTEGER, LONG, BIG_DECIMAL and BIG_INTEGER above: a
520+
// malformed number must surface as Gson's documented JsonSyntaxException.
521+
throw new JsonSyntaxException(e);
522+
}
504523
}
505524

506525
@Override

gson/src/test/java/com/google/gson/functional/ArrayTest.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.google.gson.Gson;
2323
import com.google.gson.GsonBuilder;
2424
import com.google.gson.JsonParseException;
25+
import com.google.gson.JsonSyntaxException;
2526
import com.google.gson.common.TestTypes.BagOfPrimitives;
2627
import com.google.gson.common.TestTypes.ClassWithObjects;
2728
import com.google.gson.reflect.TypeToken;
@@ -294,4 +295,29 @@ public void testArrayElementsAreArrays() {
294295
assertThat(new Gson().toJson(stringArrays))
295296
.isEqualTo("[[\"test1\",\"test2\"],[\"test3\",\"test4\"]]");
296297
}
298+
299+
@Test
300+
public void testPrimitiveArrayWithNullElement() {
301+
// A primitive array cannot hold null, so this must be reported as malformed input rather than
302+
// as the IllegalArgumentException that Array.set would throw.
303+
JsonSyntaxException e =
304+
assertThrows(JsonSyntaxException.class, () -> gson.fromJson("[1,null]", int[].class));
305+
assertThat(e)
306+
.hasMessageThat()
307+
.isEqualTo("null is not a valid value for a int array element; at path $[1]");
308+
309+
e = assertThrows(JsonSyntaxException.class, () -> gson.fromJson("[null]", double[].class));
310+
assertThat(e)
311+
.hasMessageThat()
312+
.isEqualTo("null is not a valid value for a double array element; at path $[0]");
313+
314+
e = assertThrows(JsonSyntaxException.class, () -> gson.fromJson("[[null]]", int[][].class));
315+
assertThat(e)
316+
.hasMessageThat()
317+
.isEqualTo("null is not a valid value for a int array element; at path $[0][0]");
318+
319+
// Arrays of reference types still accept null.
320+
assertThat(gson.fromJson("[1,null]", Integer[].class)).asList().containsExactly(1, null);
321+
assertThat(gson.fromJson("[null]", String[].class)).asList().containsExactly((Object) null);
322+
}
297323
}

gson/src/test/java/com/google/gson/functional/CollectionTest.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,12 @@
2727
import com.google.gson.JsonPrimitive;
2828
import com.google.gson.JsonSerializationContext;
2929
import com.google.gson.JsonSerializer;
30+
import com.google.gson.JsonSyntaxException;
3031
import com.google.gson.common.TestTypes.BagOfPrimitives;
3132
import com.google.gson.reflect.TypeToken;
3233
import java.lang.reflect.Type;
3334
import java.util.AbstractCollection;
35+
import java.util.ArrayDeque;
3436
import java.util.ArrayList;
3537
import java.util.Arrays;
3638
import java.util.Collection;
@@ -44,6 +46,7 @@
4446
import java.util.Queue;
4547
import java.util.Set;
4648
import java.util.Stack;
49+
import java.util.TreeSet;
4750
import java.util.Vector;
4851
import org.junit.Before;
4952
import org.junit.Test;
@@ -501,4 +504,29 @@ public void testIssue1107() {
501504
assertThat(small).isNotNull();
502505
assertThat(small.inSmall).isEqualTo("hello");
503506
}
507+
508+
@Test
509+
public void testCollectionRejectingNullElement() {
510+
// TreeSet, ArrayDeque and PriorityQueue throw NullPointerException from add(null); that must be
511+
// reported as malformed input instead of escaping to the caller.
512+
TypeToken<TreeSet<String>> treeSetType = new TypeToken<TreeSet<String>>() {};
513+
TypeToken<ArrayDeque<String>> dequeType = new TypeToken<ArrayDeque<String>>() {};
514+
TypeToken<PriorityQueue<String>> queueType = new TypeToken<PriorityQueue<String>>() {};
515+
TypeToken<List<String>> listType = new TypeToken<List<String>>() {};
516+
517+
JsonSyntaxException e =
518+
assertThrows(JsonSyntaxException.class, () -> gson.fromJson("[null]", treeSetType));
519+
assertThat(e)
520+
.hasMessageThat()
521+
.isEqualTo("null is not a valid element for java.util.TreeSet; at path $[0]");
522+
523+
assertThrows(JsonSyntaxException.class, () -> gson.fromJson("[null]", dequeType));
524+
assertThrows(JsonSyntaxException.class, () -> gson.fromJson("[null]", queueType));
525+
526+
// Collections that do accept null are unaffected.
527+
assertThat(gson.<List<String>>fromJson("[null]", listType)).containsExactly((String) null);
528+
assertThat(gson.<TreeSet<String>>fromJson("[\"b\",\"a\"]", treeSetType))
529+
.containsExactly("a", "b")
530+
.inOrder();
531+
}
504532
}

gson/src/test/java/com/google/gson/functional/DefaultTypeAdaptersTest.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,15 @@ public void testBitSetDeserialization() {
432432
assertThat(exception)
433433
.hasMessageThat()
434434
.isEqualTo("Invalid bitset value 2, expected 0 or 1; at path $[1]");
435+
436+
// A malformed number must not escape as a raw NumberFormatException.
437+
exception =
438+
assertThrows(JsonSyntaxException.class, () -> gson.fromJson("[\"0AA\"]", BitSet.class));
439+
assertThat(exception)
440+
.hasMessageThat()
441+
.isEqualTo(
442+
"java.lang.NumberFormatException: Expected an int but was 0AA at line 1 column 7"
443+
+ " path $[0]");
435444
}
436445

437446
@Test

gson/src/test/java/com/google/gson/functional/PrimitiveTest.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -969,4 +969,26 @@ public void testStringsAsBooleans() {
969969
List<Boolean> deserialized = gson.fromJson(json, new TypeToken<List<Boolean>>() {});
970970
assertThat(deserialized).isEqualTo(Arrays.asList(true, false, true, false, false));
971971
}
972+
973+
@Test
974+
public void testDoubleAndFloatDeserializationMalformed() {
975+
// Consistent with byte, short, int, long, BigDecimal and BigInteger, which already wrap the
976+
// NumberFormatException from JsonReader into a JsonSyntaxException.
977+
JsonSyntaxException e =
978+
assertThrows(JsonSyntaxException.class, () -> gson.fromJson("\"0AA\"", double.class));
979+
assertThat(e)
980+
.hasMessageThat()
981+
.isEqualTo(
982+
"java.lang.NumberFormatException: Expected a double but was 0AA at line 1 column 6"
983+
+ " path $");
984+
985+
assertThrows(JsonSyntaxException.class, () -> gson.fromJson("\"0AA\"", float.class));
986+
assertThrows(JsonSyntaxException.class, () -> gson.fromJson("\"0AA\"", Double.class));
987+
988+
// Well-formed values keep working, including the special ones.
989+
assertThat(gson.fromJson("1.5", double.class)).isEqualTo(1.5);
990+
assertThat(gson.fromJson("\"1.5\"", double.class)).isEqualTo(1.5);
991+
assertThat(gson.fromJson("1e3", double.class)).isEqualTo(1000.0);
992+
assertThat(gson.fromJson("NaN", double.class)).isNaN();
993+
}
972994
}

0 commit comments

Comments
 (0)