Add nesting limit for JsonReader

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;
 
  /*

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 @@
  *
  * <ul>
  * <li>{@link #setStrictness(Strictness)}, the default is {@link Strictness#LEGACY_STRICT}
+ * <li>{@link #setNestingLimit(int)}, the default is {@value #DEFAULT_NESTING_LIMIT}
  * </ul>
  *
  * 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.
+ *
+ * <p>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 <code>[{"a":[true]}]</code>, but for a nesting limit of 2 it would
+ * fail at the inner {@code [true]}.
+ *
+ * <p>The nesting limit can help to protect against a {@link StackOverflowError} when recursive
+ * {@link TypeAdapter} implementations process deeply nested JSON data.
+ *
+ * <p>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);

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++;

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<List<?>> current = (List<List<?>>) adapter.fromJson(json);
+ List<List<?>> current = (List<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<String, Map<?, ?>> current = (Map<String, Map<?, ?>>) adapter.fromJson(json);
+ Map<String, Map<?, ?>> current = (Map<String, Map<?, ?>>) adapter.read(jsonReader);
  while (current != null) {
  assertThat(current).hasSize(1);
  actualTimes++;

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;
+ }
 }

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:
+ *
+ * <ul>
+ * <li>It is an internal class and often created implicitly without the user having access to it
+ * (as {@link JsonReader}), so they cannot easily adjust the limit
+ * <li>{@link JsonTreeReader} may be created based on an existing {@link JsonReader}; in that
+ * case it would be necessary to propagate settings to account for a custom nesting limit,
+ * see also related https://github.com/google/gson/pull/2151
+ * <li>Nesting limit as protection against {@link StackOverflowError} is not that relevant for
+ * {@link JsonTreeReader} because a deeply nested {@link JsonElement} tree would first have
+ * to be constructed; and if it is constructed from a regular {@link JsonReader}, then its
+ * nesting limit would already apply
+ * </ul>
+ */
+ @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);
  }
 }