From db74b343ab34f848e497c7583d1721b0dcb41bde Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Sun, 24 Sep 2023 17:38:18 +0200 Subject: [PATCH 1/5] Add nesting limit for `JsonReader` For now don't expose this as additional GsonBuilder method assuming that the default nesting limit is high enough for most users. Otherwise users can first obtain a JsonReader from `Gson.newJsonReader` and then set a custom nesting limit. --- .../gson/internal/bind/JsonTreeReader.java | 26 ++++++- .../com/google/gson/stream/JsonReader.java | 47 ++++++++++- .../java/com/google/gson/JsonParserTest.java | 20 ++--- .../google/gson/ObjectTypeAdapterTest.java | 22 +++--- .../google/gson/functional/ObjectTest.java | 25 +++++- .../internal/bind/JsonTreeReaderTest.java | 78 ++++++++++++++++++- .../google/gson/stream/JsonReaderTest.java | 66 ++++++++++++++++ 7 files changed, 255 insertions(+), 29 deletions(-) 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 ec8dcea302..c2e0163cbb 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 @@ -48,11 +48,22 @@ public final class JsonTreeReader extends JsonReader { }; private static final Object SENTINEL_CLOSED = new Object(); - /* + /** * 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 {@link #nestingDepth}, 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; + /** + * The current nesting depth (= number of open arrays or objects). + */ + private int nestingDepth = 0; /* * The path members. It corresponds directly to stack: At indices where the @@ -70,8 +81,18 @@ public JsonTreeReader(JsonElement element) { push(element); } + private void increaseNestingDepth() throws MalformedJsonException { + int nestingLimit = getNestingLimit(); + if (nestingDepth >= nestingLimit) { + throw new MalformedJsonException("Nesting limit " + nestingLimit + " reached" + locationString()); + } + + nestingDepth++; + } + @Override public void beginArray() throws IOException { expect(JsonToken.BEGIN_ARRAY); + increaseNestingDepth(); JsonArray array = (JsonArray) peekStack(); push(array.iterator()); pathIndices[stackSize - 1] = 0; @@ -79,6 +100,7 @@ public JsonTreeReader(JsonElement element) { @Override public void endArray() throws IOException { expect(JsonToken.END_ARRAY); + nestingDepth--; popStack(); // empty iterator popStack(); // array if (stackSize > 0) { @@ -88,12 +110,14 @@ public JsonTreeReader(JsonElement element) { @Override public void beginObject() throws IOException { expect(JsonToken.BEGIN_OBJECT); + increaseNestingDepth(); JsonObject object = (JsonObject) peekStack(); push(object.entrySet().iterator()); } @Override public void endObject() throws IOException { expect(JsonToken.END_OBJECT); + nestingDepth--; pathNames[stackSize - 1] = null; // Free the last path name so that it can be garbage collected popStack(); // empty iterator popStack(); // object 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 b85fba8f7c..75d9f3c83b 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 behavior of this reader can be customized with the following methods: * * * The default configuration of {@code JsonReader} instances used internally by @@ -241,6 +243,9 @@ 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; /** @@ -277,7 +282,7 @@ public class JsonReader implements Closeable { */ private String peekedString; - /* + /** * The nesting stack. Using a manual array rather than an ArrayList saves 20%. */ private int[] stack = new int[32]; @@ -400,6 +405,39 @@ public final void setStrictness(Strictness strictness) { 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. 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. @@ -1388,7 +1426,12 @@ 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 7c9f360264..f8d69c55e1 100644 --- a/gson/src/test/java/com/google/gson/JsonParserTest.java +++ b/gson/src/test/java/com/google/gson/JsonParserTest.java @@ -93,23 +93,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()) { @@ -126,10 +120,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 7d36338240..3bcbf0e3e9 100644 --- a/gson/src/test/java/com/google/gson/functional/ObjectTest.java +++ b/gson/src/test/java/com/google/gson/functional/ObjectTest.java @@ -17,6 +17,7 @@ package com.google.gson.functional; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; import com.google.gson.ExclusionStrategy; @@ -30,6 +31,7 @@ import com.google.gson.JsonParseException; 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; @@ -39,8 +41,8 @@ import com.google.gson.common.TestTypes.ClassWithTransientFields; import com.google.gson.common.TestTypes.Nested; import com.google.gson.common.TestTypes.PrimitiveArray; -import com.google.gson.internal.JavaVersion; import com.google.gson.reflect.TypeToken; +import com.google.gson.stream.MalformedJsonException; import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Collection; @@ -691,4 +693,25 @@ 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 a208967f44..5dfa2640b5 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 @@ -16,12 +16,14 @@ package com.google.gson.internal.bind; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; import com.google.gson.JsonArray; import com.google.gson.JsonElement; import com.google.gson.JsonNull; import com.google.gson.JsonObject; +import com.google.gson.JsonPrimitive; import com.google.gson.common.MoreAsserts; import com.google.gson.stream.JsonReader; import com.google.gson.stream.JsonToken; @@ -137,6 +139,76 @@ public JsonElement deepCopy() { } } + @Test + public void testNestingLimitDefault() throws IOException { + int defaultLimit = 255; + JsonArray json = new JsonArray(); + JsonArray current = json; + // This adds additional `defaultLimit` nested arrays, so in total there are `defaultLimit + 1` arrays + for (int i = 0; i < defaultLimit; i++) { + JsonArray nested = new JsonArray(); + current.add(nested); + current = nested; + } + + JsonTreeReader reader = new JsonTreeReader(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 path $" + "[0]".repeat(defaultLimit)); + } + + @Test + public void testNestingLimit() throws IOException { + // json = [{"a": 1}] + JsonArray json = new JsonArray(); + JsonObject jsonObject = new JsonObject(); + jsonObject.addProperty("a", 1); + json.add(jsonObject); + + JsonTreeReader reader = new JsonTreeReader(json); + 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(); + + // json = [{"a": []}] + json = new JsonArray(); + jsonObject = new JsonObject(); + jsonObject.add("a", new JsonArray()); + json.add(jsonObject); + + JsonTreeReader reader2 = new JsonTreeReader(json); + 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 path $[0].a"); + + JsonTreeReader reader3 = new JsonTreeReader(new JsonArray()); + reader3.setNestingLimit(0); + e = assertThrows(MalformedJsonException.class, () -> reader3.beginArray()); + assertThat(e).hasMessageThat().isEqualTo("Nesting limit 0 reached at path $"); + + JsonTreeReader reader4 = new JsonTreeReader(new JsonArray()); + reader4.setNestingLimit(0); + // Currently not checked when skipping values + reader4.skipValue(); + + JsonTreeReader reader5 = new JsonTreeReader(new JsonPrimitive(1)); + reader5.setNestingLimit(0); + // Reading value other than array or object should be allowed + assertThat(reader5.nextInt()).isEqualTo(1); + } + /** * {@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 @@ -144,7 +216,11 @@ public JsonElement deepCopy() { */ @Test public void testOverrides() { - List ignoredMethods = Arrays.asList("setLenient(boolean)", "isLenient()", "setStrictness(com.google.gson.Strictness)", "getStrictness()"); + List ignoredMethods = Arrays.asList( + "setLenient(boolean)", "isLenient()", + "setStrictness(com.google.gson.Strictness)", "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 67e441166f..18b0789ea7 100644 --- a/gson/src/test/java/com/google/gson/stream/JsonReaderTest.java +++ b/gson/src/test/java/com/google/gson/stream/JsonReaderTest.java @@ -1965,6 +1965,72 @@ 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)); + } + + @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() throws IOException { From 51a41c81486e8916d8a050319226fb1628fb1988 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Tue, 3 Oct 2023 13:25:26 +0200 Subject: [PATCH 2/5] Ignore nesting limit for `JsonTreeReader` See comment in JsonTreeReaderTest for the rationale --- .../gson/internal/bind/JsonTreeReader.java | 19 +---- .../internal/bind/JsonTreeReaderTest.java | 84 +++++++------------ 2 files changed, 29 insertions(+), 74 deletions(-) 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 c2e0163cbb..8940fa78d6 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 @@ -55,15 +55,11 @@ public final class JsonTreeReader extends JsonReader { /** * 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 {@link #nestingDepth}, because 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; - /** - * The current nesting depth (= number of open arrays or objects). - */ - private int nestingDepth = 0; /* * The path members. It corresponds directly to stack: At indices where the @@ -81,18 +77,8 @@ public JsonTreeReader(JsonElement element) { push(element); } - private void increaseNestingDepth() throws MalformedJsonException { - int nestingLimit = getNestingLimit(); - if (nestingDepth >= nestingLimit) { - throw new MalformedJsonException("Nesting limit " + nestingLimit + " reached" + locationString()); - } - - nestingDepth++; - } - @Override public void beginArray() throws IOException { expect(JsonToken.BEGIN_ARRAY); - increaseNestingDepth(); JsonArray array = (JsonArray) peekStack(); push(array.iterator()); pathIndices[stackSize - 1] = 0; @@ -100,7 +86,6 @@ private void increaseNestingDepth() throws MalformedJsonException { @Override public void endArray() throws IOException { expect(JsonToken.END_ARRAY); - nestingDepth--; popStack(); // empty iterator popStack(); // array if (stackSize > 0) { @@ -110,14 +95,12 @@ private void increaseNestingDepth() throws MalformedJsonException { @Override public void beginObject() throws IOException { expect(JsonToken.BEGIN_OBJECT); - increaseNestingDepth(); JsonObject object = (JsonObject) peekStack(); push(object.entrySet().iterator()); } @Override public void endObject() throws IOException { expect(JsonToken.END_OBJECT); - nestingDepth--; pathNames[stackSize - 1] = null; // Free the last path name so that it can be garbage collected popStack(); // empty iterator popStack(); // object 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 5dfa2640b5..020c6b28a0 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 @@ -16,14 +16,12 @@ package com.google.gson.internal.bind; import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; import com.google.gson.JsonArray; import com.google.gson.JsonElement; import com.google.gson.JsonNull; import com.google.gson.JsonObject; -import com.google.gson.JsonPrimitive; import com.google.gson.common.MoreAsserts; import com.google.gson.stream.JsonReader; import com.google.gson.stream.JsonToken; @@ -139,74 +137,48 @@ public JsonElement deepCopy() { } } + /** + * {@link JsonTreeReader} ignores nesting limit because: + *

+ */ @Test - public void testNestingLimitDefault() throws IOException { - int defaultLimit = 255; + public void testNestingLimitIgnored() throws IOException { + int limit = 10; JsonArray json = new JsonArray(); JsonArray current = json; - // This adds additional `defaultLimit` nested arrays, so in total there are `defaultLimit + 1` arrays - for (int i = 0; i < defaultLimit; i++) { + // 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); - assertThat(reader.getNestingLimit()).isEqualTo(defaultLimit); + reader.setNestingLimit(limit); + assertThat(reader.getNestingLimit()).isEqualTo(limit); - for (int i = 0; i < defaultLimit; i++) { + for (int i = 0; i < limit; i++) { reader.beginArray(); } - MalformedJsonException e = assertThrows(MalformedJsonException.class, () -> reader.beginArray()); - assertThat(e).hasMessageThat().isEqualTo("Nesting limit 255 reached at path $" + "[0]".repeat(defaultLimit)); - } - - @Test - public void testNestingLimit() throws IOException { - // json = [{"a": 1}] - JsonArray json = new JsonArray(); - JsonObject jsonObject = new JsonObject(); - jsonObject.addProperty("a", 1); - json.add(jsonObject); - - JsonTreeReader reader = new JsonTreeReader(json); - reader.setNestingLimit(2); - assertThat(reader.getNestingLimit()).isEqualTo(2); + // Does not throw exception; limit is ignored reader.beginArray(); - reader.beginObject(); - assertThat(reader.nextName()).isEqualTo("a"); - assertThat(reader.nextInt()).isEqualTo(1); - reader.endObject(); - reader.endArray(); - // json = [{"a": []}] - json = new JsonArray(); - jsonObject = new JsonObject(); - jsonObject.add("a", new JsonArray()); - json.add(jsonObject); - - JsonTreeReader reader2 = new JsonTreeReader(json); - 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 path $[0].a"); - - JsonTreeReader reader3 = new JsonTreeReader(new JsonArray()); - reader3.setNestingLimit(0); - e = assertThrows(MalformedJsonException.class, () -> reader3.beginArray()); - assertThat(e).hasMessageThat().isEqualTo("Nesting limit 0 reached at path $"); - - JsonTreeReader reader4 = new JsonTreeReader(new JsonArray()); - reader4.setNestingLimit(0); - // Currently not checked when skipping values - reader4.skipValue(); - - JsonTreeReader reader5 = new JsonTreeReader(new JsonPrimitive(1)); - reader5.setNestingLimit(0); - // Reading value other than array or object should be allowed - assertThat(reader5.nextInt()).isEqualTo(1); + reader.endArray(); + for (int i = 0; i < limit; i++) { + reader.endArray(); + } + assertThat(reader.peek()).isEqualTo(JsonToken.END_DOCUMENT); + reader.close(); } /** From c4d2877d848eb8dd62b04bd568fc305d306beaea Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Sun, 19 Nov 2023 17:39:02 +0100 Subject: [PATCH 3/5] Fix formatting --- .../gson/internal/bind/JsonTreeReader.java | 14 +++++------ .../com/google/gson/stream/JsonReader.java | 21 +++++++++-------- .../google/gson/functional/ObjectTest.java | 10 +++++--- .../internal/bind/JsonTreeReaderTest.java | 19 +++++++-------- .../google/gson/stream/JsonReaderTest.java | 23 +++++++++++++------ 5 files changed, 50 insertions(+), 37 deletions(-) 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 ae3bb40a98..1f2965c74e 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,16 +51,14 @@ 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. + * 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 2f50797a23..44cd2fb186 100644 --- a/gson/src/main/java/com/google/gson/stream/JsonReader.java +++ b/gson/src/main/java/com/google/gson/stream/JsonReader.java @@ -252,7 +252,8 @@ 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 + // 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; @@ -291,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; { @@ -418,11 +418,11 @@ public final Strictness getStrictness() { /** * 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. The nesting limit can help to protect against a {@link StackOverflowError} - * when recursive {@link TypeAdapter} implementations process deeply nested JSON data. + *

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. 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}. * @@ -1430,7 +1430,8 @@ public void skipValue() throws IOException { 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()); + throw new MalformedJsonException( + "Nesting limit " + nestingLimit + " reached" + locationString()); } if (stackSize == stack.length) { 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 90637e6874..eb81179a19 100644 --- a/gson/src/test/java/com/google/gson/functional/ObjectTest.java +++ b/gson/src/test/java/com/google/gson/functional/ObjectTest.java @@ -727,10 +727,14 @@ public void testDeeplyNested() { // 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)); + 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)); + assertThat(e) + .hasCauseThat() + .hasMessageThat() + .isEqualTo( + "Nesting limit 255 reached at line 1 column 1277 path $" + ".r".repeat(defaultLimit)); } private static class RecursiveClass { 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 94a668f8a8..e7fcde8829 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 @@ -144,16 +144,17 @@ public JsonElement deepCopy() { /** * {@link JsonTreeReader} ignores nesting limit because: + * *

*/ @Test 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 bcdda9b8b8..db5bdea438 100644 --- a/gson/src/test/java/com/google/gson/stream/JsonReaderTest.java +++ b/gson/src/test/java/com/google/gson/stream/JsonReaderTest.java @@ -2055,9 +2055,12 @@ public void testNestingLimitDefault() throws IOException { 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)); + MalformedJsonException e = + assertThrows(MalformedJsonException.class, () -> reader.beginArray()); + assertThat(e) + .hasMessageThat() + .isEqualTo( + "Nesting limit 255 reached at line 1 column 257 path $" + "[0]".repeat(defaultLimit)); } @Test @@ -2077,8 +2080,11 @@ public void testNestingLimit() throws IOException { 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"); + 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); @@ -2104,10 +2110,13 @@ public void testNestingLimit() throws IOException { 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]"); + 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)); + IllegalArgumentException argException = + assertThrows(IllegalArgumentException.class, () -> reader7.setNestingLimit(-1)); assertThat(argException).hasMessageThat().isEqualTo("Invalid nesting limit: -1"); } From cb7bdd901faa48e0441167453a8a5a2004266d61 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Sun, 19 Nov 2023 17:45:05 +0100 Subject: [PATCH 4/5] Add concrete example to nesting limit Javadoc --- .../src/main/java/com/google/gson/stream/JsonReader.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 44cd2fb186..a28233133d 100644 --- a/gson/src/main/java/com/google/gson/stream/JsonReader.java +++ b/gson/src/main/java/com/google/gson/stream/JsonReader.java @@ -420,9 +420,12 @@ public final Strictness getStrictness() { * *

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. The nesting limit can - * help to protect against a {@link StackOverflowError} when recursive {@link TypeAdapter} - * implementations process deeply nested JSON data. + * 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}. * From 8e84a9572d6a3d8ccd7ad9ee4a819ca576f50b38 Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Wed, 27 Dec 2023 21:45:50 +0100 Subject: [PATCH 5/5] Add comment about location being slightly incorrect --- gson/src/test/java/com/google/gson/stream/JsonReaderTest.java | 2 ++ 1 file changed, 2 insertions(+) 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 537273bb80..6c6f862bfd 100644 --- a/gson/src/test/java/com/google/gson/stream/JsonReaderTest.java +++ b/gson/src/test/java/com/google/gson/stream/JsonReaderTest.java @@ -2063,6 +2063,8 @@ public void testNestingLimitDefault() throws IOException { "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}]"));