diff --git a/gson/src/main/java/com/google/gson/internal/bind/JsonTreeReader.java b/gson/src/main/java/com/google/gson/internal/bind/JsonTreeReader.java index fc23c2d53d..6b5ecc6dd3 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/JsonTreeReader.java +++ b/gson/src/main/java/com/google/gson/internal/bind/JsonTreeReader.java @@ -51,10 +51,15 @@ public void close() { }; private static final Object SENTINEL_CLOSED = new Object(); - /* - * The nesting stack. Using a manual array rather than an ArrayList saves 20%. - */ + /** The nesting stack. Using a manual array rather than an ArrayList saves 20%. */ private Object[] stack = new Object[32]; + + /** + * The used size of {@link #stack}; the value at {@code stackSize - 1} is the value last placed on + * the stack. {@code stackSize} might differ from the nesting depth, because the stack also + * contains temporary additional objects, for example for a JsonArray it contains the JsonArray + * object as well as the corresponding iterator. + */ private int stackSize = 0; /* diff --git a/gson/src/main/java/com/google/gson/stream/JsonReader.java b/gson/src/main/java/com/google/gson/stream/JsonReader.java index d2846f6b32..8b4adcdd9b 100644 --- a/gson/src/main/java/com/google/gson/stream/JsonReader.java +++ b/gson/src/main/java/com/google/gson/stream/JsonReader.java @@ -19,6 +19,7 @@ import com.google.gson.Gson; import com.google.gson.GsonBuilder; import com.google.gson.Strictness; +import com.google.gson.TypeAdapter; import com.google.gson.internal.JsonReaderInternalAccess; import com.google.gson.internal.TroubleshootingGuide; import com.google.gson.internal.bind.JsonTreeReader; @@ -70,6 +71,7 @@ * * * * The default configuration of {@code JsonReader} instances used internally by the {@link Gson} @@ -250,6 +252,10 @@ public class JsonReader implements Closeable { private final Reader in; private Strictness strictness = Strictness.LEGACY_STRICT; + // Default nesting limit is based on + // https://github.com/square/moshi/blob/parent-1.15.0/moshi/src/main/java/com/squareup/moshi/JsonReader.java#L228-L230 + private static final int DEFAULT_NESTING_LIMIT = 255; + private int nestingLimit = DEFAULT_NESTING_LIMIT; static final int BUFFER_SIZE = 1024; @@ -286,10 +292,9 @@ public class JsonReader implements Closeable { */ private String peekedString; - /* - * The nesting stack. Using a manual array rather than an ArrayList saves 20%. - */ + /** The nesting stack. Using a manual array rather than an ArrayList saves 20%. */ private int[] stack = new int[32]; + private int stackSize = 0; { @@ -411,6 +416,41 @@ public final Strictness getStrictness() { return strictness; } + /** + * Sets the nesting limit of this reader. + * + *

The nesting limit defines how many JSON arrays or objects may be open at the same time. For + * example a nesting limit of 0 means no arrays or objects may be opened at all, a nesting limit + * of 1 means one array or object may be open at the same time, and so on. So a nesting limit of 3 + * allows reading the JSON data [{"a":[true]}], but for a nesting limit of 2 it would + * fail at the inner {@code [true]}. + * + *

The nesting limit can help to protect against a {@link StackOverflowError} when recursive + * {@link TypeAdapter} implementations process deeply nested JSON data. + * + *

The default nesting limit is {@value #DEFAULT_NESTING_LIMIT}. + * + * @throws IllegalArgumentException if the nesting limit is negative. + * @since $next-version$ + * @see #getNestingLimit() + */ + public final void setNestingLimit(int limit) { + if (limit < 0) { + throw new IllegalArgumentException("Invalid nesting limit: " + limit); + } + this.nestingLimit = limit; + } + + /** + * Returns the nesting limit of this reader. + * + * @since $next-version$ + * @see #setNestingLimit(int) + */ + public final int getNestingLimit() { + return nestingLimit; + } + /** * Consumes the next token from the JSON stream and asserts that it is the beginning of a new * array. @@ -1408,7 +1448,13 @@ public void skipValue() throws IOException { pathIndices[stackSize - 1]++; } - private void push(int newTop) { + private void push(int newTop) throws MalformedJsonException { + // - 1 because stack contains as first element either EMPTY_DOCUMENT or NONEMPTY_DOCUMENT + if (stackSize - 1 >= nestingLimit) { + throw new MalformedJsonException( + "Nesting limit " + nestingLimit + " reached" + locationString()); + } + if (stackSize == stack.length) { int newLength = stackSize * 2; stack = Arrays.copyOf(stack, newLength); diff --git a/gson/src/test/java/com/google/gson/JsonParserTest.java b/gson/src/test/java/com/google/gson/JsonParserTest.java index 6bff950389..47192c797b 100644 --- a/gson/src/test/java/com/google/gson/JsonParserTest.java +++ b/gson/src/test/java/com/google/gson/JsonParserTest.java @@ -94,23 +94,17 @@ public void testParseMixedArray() { assertThat(array.get(2).getAsString()).isEqualTo("stringValue"); } - private static String repeat(String s, int times) { - StringBuilder stringBuilder = new StringBuilder(s.length() * times); - for (int i = 0; i < times; i++) { - stringBuilder.append(s); - } - return stringBuilder.toString(); - } - /** Deeply nested JSON arrays should not cause {@link StackOverflowError} */ @Test public void testParseDeeplyNestedArrays() throws IOException { int times = 10000; // [[[ ... ]]] - String json = repeat("[", times) + repeat("]", times); + String json = "[".repeat(times) + "]".repeat(times); + JsonReader jsonReader = new JsonReader(new StringReader(json)); + jsonReader.setNestingLimit(Integer.MAX_VALUE); int actualTimes = 0; - JsonArray current = JsonParser.parseString(json).getAsJsonArray(); + JsonArray current = JsonParser.parseReader(jsonReader).getAsJsonArray(); while (true) { actualTimes++; if (current.isEmpty()) { @@ -127,10 +121,12 @@ public void testParseDeeplyNestedArrays() throws IOException { public void testParseDeeplyNestedObjects() throws IOException { int times = 10000; // {"a":{"a": ... {"a":null} ... }} - String json = repeat("{\"a\":", times) + "null" + repeat("}", times); + String json = "{\"a\":".repeat(times) + "null" + "}".repeat(times); + JsonReader jsonReader = new JsonReader(new StringReader(json)); + jsonReader.setNestingLimit(Integer.MAX_VALUE); int actualTimes = 0; - JsonObject current = JsonParser.parseString(json).getAsJsonObject(); + JsonObject current = JsonParser.parseReader(jsonReader).getAsJsonObject(); while (true) { assertThat(current.size()).isEqualTo(1); actualTimes++; diff --git a/gson/src/test/java/com/google/gson/ObjectTypeAdapterTest.java b/gson/src/test/java/com/google/gson/ObjectTypeAdapterTest.java index 274de8fad5..840a83d71b 100644 --- a/gson/src/test/java/com/google/gson/ObjectTypeAdapterTest.java +++ b/gson/src/test/java/com/google/gson/ObjectTypeAdapterTest.java @@ -18,7 +18,9 @@ import static com.google.common.truth.Truth.assertThat; +import com.google.gson.stream.JsonReader; import java.io.IOException; +import java.io.StringReader; import java.util.Arrays; import java.util.Collections; import java.util.LinkedHashMap; @@ -64,24 +66,18 @@ public void testSerializeObject() { assertThat(adapter.toJson(new Object())).isEqualTo("{}"); } - private static String repeat(String s, int times) { - StringBuilder stringBuilder = new StringBuilder(s.length() * times); - for (int i = 0; i < times; i++) { - stringBuilder.append(s); - } - return stringBuilder.toString(); - } - /** Deeply nested JSON arrays should not cause {@link StackOverflowError} */ @SuppressWarnings("unchecked") @Test public void testDeserializeDeeplyNestedArrays() throws IOException { int times = 10000; // [[[ ... ]]] - String json = repeat("[", times) + repeat("]", times); + String json = "[".repeat(times) + "]".repeat(times); + JsonReader jsonReader = new JsonReader(new StringReader(json)); + jsonReader.setNestingLimit(Integer.MAX_VALUE); int actualTimes = 0; - List> current = (List>) adapter.fromJson(json); + List> current = (List>) adapter.read(jsonReader); while (true) { actualTimes++; if (current.isEmpty()) { @@ -99,10 +95,12 @@ public void testDeserializeDeeplyNestedArrays() throws IOException { public void testDeserializeDeeplyNestedObjects() throws IOException { int times = 10000; // {"a":{"a": ... {"a":null} ... }} - String json = repeat("{\"a\":", times) + "null" + repeat("}", times); + String json = "{\"a\":".repeat(times) + "null" + "}".repeat(times); + JsonReader jsonReader = new JsonReader(new StringReader(json)); + jsonReader.setNestingLimit(Integer.MAX_VALUE); int actualTimes = 0; - Map> current = (Map>) adapter.fromJson(json); + Map> current = (Map>) adapter.read(jsonReader); while (current != null) { assertThat(current).hasSize(1); actualTimes++; diff --git a/gson/src/test/java/com/google/gson/functional/ObjectTest.java b/gson/src/test/java/com/google/gson/functional/ObjectTest.java index 39e2fb2416..9da8ab9869 100644 --- a/gson/src/test/java/com/google/gson/functional/ObjectTest.java +++ b/gson/src/test/java/com/google/gson/functional/ObjectTest.java @@ -33,6 +33,7 @@ import com.google.gson.JsonPrimitive; import com.google.gson.JsonSerializationContext; import com.google.gson.JsonSerializer; +import com.google.gson.JsonSyntaxException; import com.google.gson.common.TestTypes.ArrayOfObjects; import com.google.gson.common.TestTypes.BagOfPrimitiveWrappers; import com.google.gson.common.TestTypes.BagOfPrimitives; @@ -43,6 +44,7 @@ import com.google.gson.common.TestTypes.Nested; import com.google.gson.common.TestTypes.PrimitiveArray; import com.google.gson.reflect.TypeToken; +import com.google.gson.stream.MalformedJsonException; import java.io.EOFException; import java.lang.reflect.Type; import java.util.ArrayList; @@ -767,4 +769,29 @@ public ClassWithThrowingConstructor() { throw thrownException; } } + + @Test + public void testDeeplyNested() { + int defaultLimit = 255; + // json = {"r":{"r": ... {"r":null} ... }} + String json = "{\"r\":".repeat(defaultLimit) + "null" + "}".repeat(defaultLimit); + RecursiveClass deserialized = gson.fromJson(json, RecursiveClass.class); + assertThat(deserialized).isNotNull(); + assertThat(deserialized.r).isNotNull(); + + // json = {"r":{"r": ... {"r":null} ... }} + String json2 = "{\"r\":".repeat(defaultLimit + 1) + "null" + "}".repeat(defaultLimit + 1); + JsonSyntaxException e = + assertThrows(JsonSyntaxException.class, () -> gson.fromJson(json2, RecursiveClass.class)); + assertThat(e).hasCauseThat().isInstanceOf(MalformedJsonException.class); + assertThat(e) + .hasCauseThat() + .hasMessageThat() + .isEqualTo( + "Nesting limit 255 reached at line 1 column 1277 path $" + ".r".repeat(defaultLimit)); + } + + private static class RecursiveClass { + RecursiveClass r; + } } diff --git a/gson/src/test/java/com/google/gson/internal/bind/JsonTreeReaderTest.java b/gson/src/test/java/com/google/gson/internal/bind/JsonTreeReaderTest.java index 04b0d87132..42d4649683 100644 --- a/gson/src/test/java/com/google/gson/internal/bind/JsonTreeReaderTest.java +++ b/gson/src/test/java/com/google/gson/internal/bind/JsonTreeReaderTest.java @@ -137,6 +137,51 @@ public JsonElement deepCopy() { "Custom JsonElement subclass " + CustomSubclass.class.getName() + " is not supported"); } + /** + * {@link JsonTreeReader} ignores nesting limit because: + * + *

+ */ + @Test + public void testNestingLimitIgnored() throws IOException { + int limit = 10; + JsonArray json = new JsonArray(); + JsonArray current = json; + // This adds additional `limit` nested arrays, so in total there are `limit + 1` arrays + for (int i = 0; i < limit; i++) { + JsonArray nested = new JsonArray(); + current.add(nested); + current = nested; + } + + JsonTreeReader reader = new JsonTreeReader(json); + reader.setNestingLimit(limit); + assertThat(reader.getNestingLimit()).isEqualTo(limit); + + for (int i = 0; i < limit; i++) { + reader.beginArray(); + } + // Does not throw exception; limit is ignored + reader.beginArray(); + + reader.endArray(); + for (int i = 0; i < limit; i++) { + reader.endArray(); + } + assertThat(reader.peek()).isEqualTo(JsonToken.END_DOCUMENT); + reader.close(); + } + /** * {@link JsonTreeReader} effectively replaces the complete reading logic of {@link JsonReader} to * read from a {@link JsonElement} instead of a {@link Reader}. Therefore all relevant methods of @@ -149,7 +194,9 @@ public void testOverrides() { "setLenient(boolean)", "isLenient()", "setStrictness(com.google.gson.Strictness)", - "getStrictness()"); + "getStrictness()", + "setNestingLimit(int)", + "getNestingLimit()"); MoreAsserts.assertOverridesMethods(JsonReader.class, JsonTreeReader.class, ignoredMethods); } } diff --git a/gson/src/test/java/com/google/gson/stream/JsonReaderTest.java b/gson/src/test/java/com/google/gson/stream/JsonReaderTest.java index 3e62d33ac2..d889542521 100644 --- a/gson/src/test/java/com/google/gson/stream/JsonReaderTest.java +++ b/gson/src/test/java/com/google/gson/stream/JsonReaderTest.java @@ -1751,6 +1751,83 @@ public void testDeeplyNestedObjects() throws IOException { assertThat(reader.peek()).isEqualTo(JsonToken.END_DOCUMENT); } + @Test + public void testNestingLimitDefault() throws IOException { + int defaultLimit = 255; + String json = repeat('[', defaultLimit + 1); + JsonReader reader = new JsonReader(reader(json)); + assertThat(reader.getNestingLimit()).isEqualTo(defaultLimit); + + for (int i = 0; i < defaultLimit; i++) { + reader.beginArray(); + } + MalformedJsonException e = + assertThrows(MalformedJsonException.class, () -> reader.beginArray()); + assertThat(e) + .hasMessageThat() + .isEqualTo( + "Nesting limit 255 reached at line 1 column 257 path $" + "[0]".repeat(defaultLimit)); + } + + // Note: The column number reported in the expected exception messages is slightly off and points + // behind instead of directly at the '[' or '{' + @Test + public void testNestingLimit() throws IOException { + JsonReader reader = new JsonReader(reader("[{\"a\":1}]")); + reader.setNestingLimit(2); + assertThat(reader.getNestingLimit()).isEqualTo(2); + reader.beginArray(); + reader.beginObject(); + assertThat(reader.nextName()).isEqualTo("a"); + assertThat(reader.nextInt()).isEqualTo(1); + reader.endObject(); + reader.endArray(); + + JsonReader reader2 = new JsonReader(reader("[{\"a\":[]}]")); + reader2.setNestingLimit(2); + reader2.beginArray(); + reader2.beginObject(); + assertThat(reader2.nextName()).isEqualTo("a"); + MalformedJsonException e = + assertThrows(MalformedJsonException.class, () -> reader2.beginArray()); + assertThat(e) + .hasMessageThat() + .isEqualTo("Nesting limit 2 reached at line 1 column 8 path $[0].a"); + + JsonReader reader3 = new JsonReader(reader("[]")); + reader3.setNestingLimit(0); + e = assertThrows(MalformedJsonException.class, () -> reader3.beginArray()); + assertThat(e).hasMessageThat().isEqualTo("Nesting limit 0 reached at line 1 column 2 path $"); + + JsonReader reader4 = new JsonReader(reader("[]")); + reader4.setNestingLimit(0); + // Currently also checked when skipping values + e = assertThrows(MalformedJsonException.class, () -> reader4.skipValue()); + assertThat(e).hasMessageThat().isEqualTo("Nesting limit 0 reached at line 1 column 2 path $"); + + JsonReader reader5 = new JsonReader(reader("1")); + reader5.setNestingLimit(0); + // Reading value other than array or object should be allowed + assertThat(reader5.nextInt()).isEqualTo(1); + + // Test multiple top-level arrays + JsonReader reader6 = new JsonReader(reader("[] [[]]")); + reader6.setStrictness(Strictness.LENIENT); + reader6.setNestingLimit(1); + reader6.beginArray(); + reader6.endArray(); + reader6.beginArray(); + e = assertThrows(MalformedJsonException.class, () -> reader6.beginArray()); + assertThat(e) + .hasMessageThat() + .isEqualTo("Nesting limit 1 reached at line 1 column 6 path $[0]"); + + JsonReader reader7 = new JsonReader(reader("[]")); + IllegalArgumentException argException = + assertThrows(IllegalArgumentException.class, () -> reader7.setNestingLimit(-1)); + assertThat(argException).hasMessageThat().isEqualTo("Invalid nesting limit: -1"); + } + // http://code.google.com/p/google-gson/issues/detail?id=409 @Test public void testStringEndingInSlash() {