Limit to 31 levels of nested structure.

With the implicit enclosing document this is 32 levels total. The limit
is arbitrary but sufficient - deeper limits yield code that fails with
StackOverflowExceptions during the depth-first traversal.

We were previously broken in JsonWriter on this - the code we added to
support path building was accessing the top of the stack before the
stack had been resized, causing a crash. Because this crash has existed
forever without much outcry we know the limit is likely sufficient for
our existing users.

Closes: https://github.com/square/moshi/issues/189
This commit is contained in:
jwilson
2016-10-15 13:33:22 -04:00
parent fce8fc6976
commit d8743eeef1
8 changed files with 157 additions and 51 deletions

View File

@@ -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 @@ final class BufferedSinkJsonWriter extends JsonWriter {
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;
}

View File

@@ -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 @@ final class BufferedSourceJsonReader extends JsonReader {
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;
}

View File

@@ -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() {

View File

@@ -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());

View File

@@ -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 {

View File

@@ -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(
"[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]");
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("/");

View File

@@ -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 $.");
}
}

View File

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