Merge pull request #202 from square/jwilson.1015.deep

Limit to 31 levels of nested structure.

moshi/src/main/java/com/squareup/moshi/BufferedSinkJsonWriter.java

@@ -57,14 +57,17 @@ final class BufferedSinkJsonWriter extends JsonWriter {
   /** The output data, containing at most one top-level array or object. */
   private final BufferedSink sink;
 
-  private int[] stack = new int[32];
+  // The nesting stack. Using a manual array rather than an ArrayList saves 20%. This stack permits
+  // up to 32 levels of nesting including the top-level document. Deeper nesting is prone to trigger
+  // StackOverflowErrors.
+  private final int[] stack = new int[32];
   private int stackSize = 0;
   {
     push(EMPTY_DOCUMENT);
   }
 
-  private String[] pathNames = new String[32];
+  private final String[] pathNames = new String[32];
-  private int[] pathIndices = new int[32];
+  private final int[] pathIndices = new int[32];
 
   /**
    * A string containing a full set of spaces for a single level of
@@ -143,8 +146,8 @@ final class BufferedSinkJsonWriter extends JsonWriter {
    */
   private JsonWriter open(int empty, String openBracket) throws IOException {
     beforeValue();
-    pathIndices[stackSize] = 0;
     push(empty);
+    pathIndices[stackSize - 1] = 0;
     sink.writeUtf8(openBracket);
     return this;
   }
@@ -175,9 +178,7 @@ final class BufferedSinkJsonWriter extends JsonWriter {
 
   private void push(int newTop) {
     if (stackSize == stack.length) {
-      int[] newStack = new int[stackSize * 2];
+      throw new JsonDataException("Nesting too deep at " + getPath() + ": circular reference?");
-      System.arraycopy(stack, 0, newStack, 0, stackSize);
-      stack = newStack;
     }
     stack[stackSize++] = newTop;
   }

moshi/src/main/java/com/squareup/moshi/BufferedSourceJsonReader.java

@@ -92,17 +92,17 @@ final class BufferedSourceJsonReader extends JsonReader {
    */
   private String peekedString;
 
-  /*
+  // The nesting stack. Using a manual array rather than an ArrayList saves 20%. This stack permits
-   * The nesting stack. Using a manual array rather than an ArrayList saves 20%.
+  // up to 32 levels of nesting including the top-level document. Deeper nesting is prone to trigger
-   */
+  // StackOverflowErrors.
-  private int[] stack = new int[32];
+  private final int[] stack = new int[32];
   private int stackSize = 0;
   {
     stack[stackSize++] = JsonScope.EMPTY_DOCUMENT;
   }
 
-  private String[] pathNames = new String[32];
+  private final String[] pathNames = new String[32];
-  private int[] pathIndices = new int[32];
+  private final int[] pathIndices = new int[32];
 
   BufferedSourceJsonReader(BufferedSource source) {
     if (source == null) {
@@ -901,15 +901,7 @@ final class BufferedSourceJsonReader extends JsonReader {
 
   private void push(int newTop) {
     if (stackSize == stack.length) {
-      int[] newStack = new int[stackSize * 2];
+      throw new JsonDataException("Nesting too deep at " + getPath());
-      int[] newPathIndices = new int[stackSize * 2];
-      String[] newPathNames = new String[stackSize * 2];
-      System.arraycopy(stack, 0, newStack, 0, stackSize);
-      System.arraycopy(pathIndices, 0, newPathIndices, 0, stackSize);
-      System.arraycopy(pathNames, 0, newPathNames, 0, stackSize);
-      stack = newStack;
-      pathIndices = newPathIndices;
-      pathNames = newPathNames;
     }
     stack[stackSize++] = newTop;
   }

moshi/src/main/java/com/squareup/moshi/JsonDataException.java

@@ -22,6 +22,10 @@ package com.squareup.moshi;
  *
  * <p>Exceptions of this type should be fixed by either changing the application code to accept
  * the unexpected JSON, or by changing the JSON to conform to the application's expectations.
+ *
+ * <p>This exception may also be triggered if a document's nesting exceeds 31 levels. This depth is
+ * sufficient for all practical applications, but shallow enough to avoid uglier failures like
+ * {@link StackOverflowError}.
  */
 public final class JsonDataException extends RuntimeException {
   public JsonDataException() {

moshi/src/main/java/com/squareup/moshi/MapJsonAdapter.java

@@ -50,7 +50,7 @@ final class MapJsonAdapter<K, V> extends JsonAdapter<Map<K, V>> {
     writer.beginObject();
     for (Map.Entry<K, V> entry : map.entrySet()) {
       if (entry.getKey() == null) {
-        throw new JsonDataException("Map key is null at path " + writer.getPath());
+        throw new JsonDataException("Map key is null at " + writer.getPath());
       }
       writer.promoteNameToValue();
       keyAdapter.toJson(writer, entry.getKey());

moshi/src/test/java/com/squareup/moshi/BufferedSinkJsonWriterTest.java

@@ -419,31 +419,62 @@ public final class BufferedSinkJsonWriterTest {
   @Test public void deepNestingArrays() throws IOException {
     Buffer buffer = new Buffer();
     JsonWriter jsonWriter = JsonWriter.of(buffer);
-    for (int i = 0; i < 20; i++) {
+    for (int i = 0; i < 31; i++) {
       jsonWriter.beginArray();
     }
-    for (int i = 0; i < 20; i++) {
+    for (int i = 0; i < 31; i++) {
       jsonWriter.endArray();
     }
-    assertThat(buffer.readUtf8()).isEqualTo("[[[[[[[[[[[[[[[[[[[[]]]]]]]]]]]]]]]]]]]]");
+    assertThat(buffer.readUtf8())
+        .isEqualTo("[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]");
+  }
+
+  @Test public void tooDeepNestingArrays() throws IOException {
+    Buffer buffer = new Buffer();
+    JsonWriter jsonWriter = JsonWriter.of(buffer);
+    for (int i = 0; i < 31; i++) {
+      jsonWriter.beginArray();
+    }
+    try {
+      jsonWriter.beginArray();
+      fail();
+    } catch (JsonDataException expected) {
+      assertThat(expected).hasMessage("Nesting too deep at $[0][0][0][0][0][0][0][0][0][0][0][0][0]"
+          + "[0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0]: circular reference?");
+    }
   }
 
   @Test public void deepNestingObjects() throws IOException {
     Buffer buffer = new Buffer();
     JsonWriter jsonWriter = JsonWriter.of(buffer);
+    for (int i = 0; i < 31; i++) {
       jsonWriter.beginObject();
-    for (int i = 0; i < 20; i++) {
       jsonWriter.name("a");
+    }
+    jsonWriter.value(true);
+    for (int i = 0; i < 31; i++) {
+      jsonWriter.endObject();
+    }
+    assertThat(buffer.readUtf8()).isEqualTo(""
+        + "{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":"
+        + "{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":"
+        + "{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":true}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}");
+  }
+
+  @Test public void tooDeepNestingObjects() throws IOException {
+    Buffer buffer = new Buffer();
+    JsonWriter jsonWriter = JsonWriter.of(buffer);
+    for (int i = 0; i < 31; i++) {
       jsonWriter.beginObject();
+      jsonWriter.name("a");
     }
-    for (int i = 0; i < 20; i++) {
+    try {
-      jsonWriter.endObject();
+      jsonWriter.beginObject();
+      fail();
+    } catch (JsonDataException expected) {
+      assertThat(expected).hasMessage("Nesting too deep at $.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a."
+          + "a.a.a.a.a.a.a.a.a.a.a.a: circular reference?");
     }
-    jsonWriter.endObject();
-    assertThat(buffer.readUtf8()).isEqualTo(
-        "{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":"
-        + "{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{\"a\":{"
-        + "}}}}}}}}}}}}}}}}}}}}}");
   }
 
   @Test public void repeatedName() throws IOException {

moshi/src/test/java/com/squareup/moshi/BufferedSourceJsonReaderTest.java

@@ -1498,43 +1498,77 @@ public final class BufferedSourceJsonReaderTest {
   }
 
   @Test public void deeplyNestedArrays() throws IOException {
-    // this is nested 40 levels deep; Gson is tuned for nesting is 30 levels deep or fewer
+    JsonReader reader = newReader("[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]");
-    JsonReader reader = newReader(
+    for (int i = 0; i < 31; i++) {
-        "[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]");
-    for (int i = 0; i < 40; i++) {
       reader.beginArray();
     }
-    assertThat(reader.getPath()).isEqualTo(
+    assertThat(reader.getPath()).isEqualTo("$[0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0]"
-        "$[0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0]"
+        + "[0][0][0][0][0][0][0][0][0][0][0][0][0]");
-        + "[0][0][0][0][0][0][0][0][0][0][0][0][0][0]");
+    for (int i = 0; i < 31; i++) {
-    for (int i = 0; i < 40; i++) {
       reader.endArray();
     }
     assertThat(reader.peek()).isEqualTo(JsonReader.Token.END_DOCUMENT);
   }
 
+  @Test public void tooDeeplyNestedArrays() throws IOException {
+    JsonReader reader = newReader(
+        "[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]");
+    for (int i = 0; i < 31; i++) {
+      reader.beginArray();
+    }
+    try {
+      reader.beginArray();
+      fail();
+    } catch (JsonDataException expected) {
+      assertThat(expected).hasMessage("Nesting too deep at $[0][0][0][0][0][0][0][0][0][0][0][0][0]"
+          + "[0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0]");
+    }
+  }
+
   @Test public void deeplyNestedObjects() throws IOException {
-    // Build a JSON document structured like {"a":{"a":{"a":{"a":true}}}}, but 40 levels deep
+    // Build a JSON document structured like {"a":{"a":{"a":{"a":true}}}}, but 31 levels deep.
     String array = "{\"a\":%s}";
     String json = "true";
-    for (int i = 0; i < 40; i++) {
+    for (int i = 0; i < 31; i++) {
       json = String.format(array, json);
     }
 
     JsonReader reader = newReader(json);
-    for (int i = 0; i < 40; i++) {
+    for (int i = 0; i < 31; i++) {
       reader.beginObject();
       assertThat(reader.nextName()).isEqualTo("a");
     }
-    assertThat(reader.getPath()).isEqualTo("$.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a"
+    assertThat(reader.getPath())
-        + ".a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a");
+        .isEqualTo("$.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a");
     assertThat(reader.nextBoolean()).isTrue();
-    for (int i = 0; i < 40; i++) {
+    for (int i = 0; i < 31; i++) {
       reader.endObject();
     }
     assertThat(reader.peek()).isEqualTo(JsonReader.Token.END_DOCUMENT);
   }
 
+  @Test public void tooDeeplyNestedObjects() throws IOException {
+    // Build a JSON document structured like {"a":{"a":{"a":{"a":true}}}}, but 31 levels deep.
+    String array = "{\"a\":%s}";
+    String json = "true";
+    for (int i = 0; i < 32; i++) {
+      json = String.format(array, json);
+    }
+
+    JsonReader reader = newReader(json);
+    for (int i = 0; i < 31; i++) {
+      reader.beginObject();
+      assertThat(reader.nextName()).isEqualTo("a");
+    }
+    try {
+      reader.beginObject();
+      fail();
+    } catch (JsonDataException expected) {
+      assertThat(expected).hasMessage(
+          "Nesting too deep at $.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a");
+    }
+  }
+
   // http://code.google.com/p/google-gson/issues/detail?id=409
   @Test public void stringEndingInSlash() throws IOException {
     JsonReader reader = newReader("/");

moshi/src/test/java/com/squareup/moshi/MapJsonAdapterTest.java

@@ -23,7 +23,6 @@ import java.util.LinkedHashMap;
 import java.util.Map;
 import okio.Buffer;
 import org.assertj.core.data.MapEntry;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import static com.squareup.moshi.TestUtil.newReader;
@@ -57,7 +56,7 @@ public final class MapJsonAdapterTest {
       toJson(String.class, Boolean.class, map);
       fail();
     } catch (JsonDataException expected) {
-      assertThat(expected).hasMessage("Map key is null at path $.");
+      assertThat(expected).hasMessage("Map key is null at $.");
     }
   }
 

moshi/src/test/java/com/squareup/moshi/MoshiTest.java

@@ -23,13 +23,17 @@ import java.lang.annotation.Retention;
 import java.lang.reflect.Field;
 import java.lang.reflect.Type;
 import java.util.ArrayDeque;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Set;
 import javax.crypto.KeyGenerator;
+import org.junit.Assert;
 import org.junit.Test;
 
 import static com.squareup.moshi.TestUtil.newReader;
@@ -816,6 +820,47 @@ public final class MoshiTest {
     }
   }
 
+  @Test public void referenceCyclesOnArrays() throws Exception {
+    Moshi moshi = new Moshi.Builder().build();
+    Map<String, Object> map = new LinkedHashMap<>();
+    map.put("a", map);
+    try {
+      moshi.adapter(Object.class).toJson(map);
+      fail();
+    } catch (JsonDataException expected) {
+      assertThat(expected).hasMessage("Nesting too deep at $.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a."
+          + "a.a.a.a.a.a.a.a.a.a.a.a: circular reference?");
+    }
+  }
+
+  @Test public void referenceCyclesOnObjects() throws Exception {
+    Moshi moshi = new Moshi.Builder().build();
+    List<Object> list = new ArrayList<>();
+    list.add(list);
+    try {
+      moshi.adapter(Object.class).toJson(list);
+      fail();
+    } catch (JsonDataException expected) {
+      assertThat(expected).hasMessage("Nesting too deep at $[0][0][0][0][0][0][0][0][0][0][0][0][0]"
+          + "[0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0]: circular reference?");
+    }
+  }
+
+  @Test public void referenceCyclesOnMixedTypes() throws Exception {
+    Moshi moshi = new Moshi.Builder().build();
+    List<Object> list = new ArrayList<>();
+    Map<String, Object> map = new LinkedHashMap<>();
+    list.add(map);
+    map.put("a", list);
+    try {
+      moshi.adapter(Object.class).toJson(list);
+      fail();
+    } catch (JsonDataException expected) {
+      assertThat(expected).hasMessage("Nesting too deep at $[0].a[0].a[0].a[0].a[0].a[0].a[0].a[0]."
+          + "a[0].a[0].a[0].a[0].a[0].a[0].a[0].a[0]: circular reference?");
+    }
+  }
+
   static class Pizza {
     final int diameter;
     final boolean extraCheese;