diff --git a/moshi/src/main/java/com/squareup/moshi/BufferedSinkJsonWriter.java b/moshi/src/main/java/com/squareup/moshi/BufferedSinkJsonWriter.java index e0603e400..e2ac91533 100644 --- a/moshi/src/main/java/com/squareup/moshi/BufferedSinkJsonWriter.java +++ b/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 int[] pathIndices = new int[32]; + private final String[] pathNames = new String[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 @@ private JsonWriter close(int empty, int nonempty, String closeBracket) private void push(int newTop) { if (stackSize == stack.length) { - int[] newStack = new int[stackSize * 2]; - System.arraycopy(stack, 0, newStack, 0, stackSize); - stack = newStack; + throw new JsonDataException("Nesting too deep at " + getPath() + ": circular reference?"); } stack[stackSize++] = newTop; } diff --git a/moshi/src/main/java/com/squareup/moshi/BufferedSourceJsonReader.java b/moshi/src/main/java/com/squareup/moshi/BufferedSourceJsonReader.java index 087fd9151..1e193830b 100644 --- a/moshi/src/main/java/com/squareup/moshi/BufferedSourceJsonReader.java +++ b/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%. - */ - 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; { stack[stackSize++] = JsonScope.EMPTY_DOCUMENT; } - private String[] pathNames = new String[32]; - private int[] pathIndices = new int[32]; + private final String[] pathNames = new String[32]; + private final int[] pathIndices = new int[32]; BufferedSourceJsonReader(BufferedSource source) { if (source == null) { @@ -901,15 +901,7 @@ private void skipUnquotedValue() throws IOException { private void push(int newTop) { if (stackSize == stack.length) { - int[] newStack = new int[stackSize * 2]; - 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; + throw new JsonDataException("Nesting too deep at " + getPath()); } stack[stackSize++] = newTop; } diff --git a/moshi/src/main/java/com/squareup/moshi/JsonDataException.java b/moshi/src/main/java/com/squareup/moshi/JsonDataException.java index a84630555..062626379 100644 --- a/moshi/src/main/java/com/squareup/moshi/JsonDataException.java +++ b/moshi/src/main/java/com/squareup/moshi/JsonDataException.java @@ -22,6 +22,10 @@ * *

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

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() { diff --git a/moshi/src/main/java/com/squareup/moshi/MapJsonAdapter.java b/moshi/src/main/java/com/squareup/moshi/MapJsonAdapter.java index b0bb34db9..889d1f6dc 100644 --- a/moshi/src/main/java/com/squareup/moshi/MapJsonAdapter.java +++ b/moshi/src/main/java/com/squareup/moshi/MapJsonAdapter.java @@ -50,7 +50,7 @@ public MapJsonAdapter(Moshi moshi, Type keyType, Type valueType) { writer.beginObject(); for (Map.Entry 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()); diff --git a/moshi/src/test/java/com/squareup/moshi/BufferedSinkJsonWriterTest.java b/moshi/src/test/java/com/squareup/moshi/BufferedSinkJsonWriterTest.java index d430b2f5e..0cdfec11e 100644 --- a/moshi/src/test/java/com/squareup/moshi/BufferedSinkJsonWriterTest.java +++ b/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); - jsonWriter.beginObject(); - for (int i = 0; i < 20; i++) { - jsonWriter.name("a"); + for (int i = 0; i < 31; i++) { jsonWriter.beginObject(); + jsonWriter.name("a"); } - for (int i = 0; i < 20; i++) { + jsonWriter.value(true); + for (int i = 0; i < 31; i++) { jsonWriter.endObject(); } - 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\":{" - + "}}}}}}}}}}}}}}}}}}}}}"); + 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"); + } + try { + 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?"); + } } @Test public void repeatedName() throws IOException { diff --git a/moshi/src/test/java/com/squareup/moshi/BufferedSourceJsonReaderTest.java b/moshi/src/test/java/com/squareup/moshi/BufferedSourceJsonReaderTest.java index 69000871d..117479579 100644 --- a/moshi/src/test/java/com/squareup/moshi/BufferedSourceJsonReaderTest.java +++ b/moshi/src/test/java/com/squareup/moshi/BufferedSourceJsonReaderTest.java @@ -1498,43 +1498,77 @@ private void testFailWithPosition(String message, String json) throws IOExceptio } @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( - "[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]"); - for (int i = 0; i < 40; i++) { + JsonReader reader = newReader("[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]"); + for (int i = 0; i < 31; i++) { reader.beginArray(); } - 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]"); - for (int i = 0; i < 40; i++) { + 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]"); + for (int i = 0; i < 31; 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" - + ".a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.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.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("/"); diff --git a/moshi/src/test/java/com/squareup/moshi/MapJsonAdapterTest.java b/moshi/src/test/java/com/squareup/moshi/MapJsonAdapterTest.java index 2e382aae6..4c713f6af 100644 --- a/moshi/src/test/java/com/squareup/moshi/MapJsonAdapterTest.java +++ b/moshi/src/test/java/com/squareup/moshi/MapJsonAdapterTest.java @@ -23,7 +23,6 @@ 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 $."); } } diff --git a/moshi/src/test/java/com/squareup/moshi/MoshiTest.java b/moshi/src/test/java/com/squareup/moshi/MoshiTest.java index 0acff82b8..3e176ba76 100644 --- a/moshi/src/test/java/com/squareup/moshi/MoshiTest.java +++ b/moshi/src/test/java/com/squareup/moshi/MoshiTest.java @@ -23,13 +23,17 @@ 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 @@ static class Message { } } + @Test public void referenceCyclesOnArrays() throws Exception { + Moshi moshi = new Moshi.Builder().build(); + Map 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 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 list = new ArrayList<>(); + Map 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;