Add nesting limit for JsonReader (#2588)

* 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.

* Ignore nesting limit for `JsonTreeReader`

See comment in JsonTreeReaderTest for the rationale

* Fix formatting

* Add concrete example to nesting limit Javadoc

* Add comment about location being slightly incorrect

Author: Marcono1234

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