Skip to content

Commit

Permalink
Remove limitation on the JSON nesting. If the JSON is way too nested,…
Browse files Browse the repository at this point in the history
… an OutOfMemory exception will happen (#5161)
  • Loading branch information
martinbonnin authored Aug 7, 2023
1 parent 645d061 commit 3972d9c
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@
package com.apollographql.apollo3.api.json

import com.apollographql.apollo3.api.Upload
import com.apollographql.apollo3.api.json.BufferedSourceJsonReader.Companion.MAX_STACK_SIZE
import com.apollographql.apollo3.api.json.BufferedSourceJsonReader.Companion.INITIAL_STACK_SIZE
import com.apollographql.apollo3.api.json.internal.JsonScope
import com.apollographql.apollo3.exception.JsonDataException
import okio.BufferedSink
import okio.IOException
import kotlin.jvm.JvmOverloads
Expand All @@ -39,12 +38,10 @@ class BufferedSinkJsonWriter @JvmOverloads constructor(
private val sink: BufferedSink,
private val indent: String? = null,
) : JsonWriter {
// The nesting stack. Using a manual array rather than an ArrayList saves 20%. This stack permits up to MAX_STACK_SIZE levels of nesting including
// the top-level document. Deeper nesting is prone to trigger StackOverflowErrors.
private var stackSize = 0
private val scopes = IntArray(MAX_STACK_SIZE)
private val pathNames = arrayOfNulls<String>(MAX_STACK_SIZE)
private val pathIndices = IntArray(MAX_STACK_SIZE)
private var scopes = IntArray(INITIAL_STACK_SIZE)
private var pathNames = arrayOfNulls<String>(INITIAL_STACK_SIZE)
private var pathIndices = IntArray(INITIAL_STACK_SIZE)

/** The name/value separator; either ":" or ": ". */
private val separator: String
Expand Down Expand Up @@ -256,7 +253,9 @@ class BufferedSinkJsonWriter @JvmOverloads constructor(

private fun pushScope(newTop: Int) {
if (stackSize == scopes.size) {
throw JsonDataException("Nesting too deep at $path: circular reference?")
scopes = scopes.copyOf(scopes.size * 2)
pathNames = pathNames.copyOf(pathNames.size * 2)
pathIndices = pathIndices.copyOf(pathIndices.size * 2)
}
scopes[stackSize++] = newTop
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,14 @@ class BufferedSourceJsonReader(private val source: BufferedSource) : JsonReader
*/
private var peekedString: String? = null

/**
* The nesting stack. Using a manual array rather than an ArrayList saves 20%.
* This stack permits up to MAX_STACK_SIZE levels of nesting including the top-level document.
* Deeper nesting is prone to trigger StackOverflowErrors.
*/
private val stack = IntArray(MAX_STACK_SIZE).apply {
private var stack = IntArray(INITIAL_STACK_SIZE).apply {
this[0] = JsonScope.EMPTY_DOCUMENT
}
private var stackSize = 1
private val pathNames = arrayOfNulls<String>(MAX_STACK_SIZE)
private val pathIndices = IntArray(MAX_STACK_SIZE)
private var pathNames = arrayOfNulls<String>(INITIAL_STACK_SIZE)
private var pathIndices = IntArray(INITIAL_STACK_SIZE)

private val indexStack = IntArray(MAX_STACK_SIZE).apply {
private var indexStack = IntArray(INITIAL_STACK_SIZE).apply {
this[0] = 0
}
private var indexStackSize = 1
Expand Down Expand Up @@ -746,7 +741,12 @@ class BufferedSourceJsonReader(private val source: BufferedSource) : JsonReader
}

private fun push(newTop: Int) {
if (stackSize == stack.size) throw JsonDataException("Nesting too deep at " + getPath())
if (stackSize == stack.size) {
stack = stack.copyOf(stack.size * 2)
pathNames = pathNames.copyOf(pathNames.size * 2)
pathIndices = pathIndices.copyOf(pathIndices.size * 2)
indexStack = indexStack.copyOf(indexStack.size * 2)
}
stack[stackSize++] = newTop
}

Expand Down Expand Up @@ -888,6 +888,6 @@ class BufferedSourceJsonReader(private val source: BufferedSource) : JsonReader
private const val NUMBER_CHAR_EXP_SIGN = 6
private const val NUMBER_CHAR_EXP_DIGIT = 7

internal const val MAX_STACK_SIZE = 256
internal const val INITIAL_STACK_SIZE = 64
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.apollographql.apollo3.api.json

import com.apollographql.apollo3.api.json.BufferedSourceJsonReader.Companion.MAX_STACK_SIZE
import com.apollographql.apollo3.api.json.BufferedSourceJsonReader.Companion.INITIAL_STACK_SIZE
import com.apollographql.apollo3.api.json.MapJsonReader.Companion.buffer
import com.apollographql.apollo3.api.json.internal.toDoubleExact
import com.apollographql.apollo3.api.json.internal.toIntExact
Expand Down Expand Up @@ -50,14 +50,14 @@ constructor(
* - a String representing the current key to be read in a Map
* - null if peekedToken is BEGIN_OBJECT
*/
private val path = arrayOfNulls<Any>(MAX_STACK_SIZE)
private var path = arrayOfNulls<Any>(INITIAL_STACK_SIZE)

/**
* The current object memorized in case we need to rewind
*/
private var containerStack = arrayOfNulls<Map<String, Any?>>(MAX_STACK_SIZE)
private val iteratorStack = arrayOfNulls<Iterator<*>>(MAX_STACK_SIZE)
private val nameIndexStack = IntArray(MAX_STACK_SIZE)
private var containerStack = arrayOfNulls<Map<String, Any?>>(INITIAL_STACK_SIZE)
private var iteratorStack = arrayOfNulls<Iterator<*>>(INITIAL_STACK_SIZE)
private var nameIndexStack = IntArray(INITIAL_STACK_SIZE)

private var stackSize = 0

Expand Down Expand Up @@ -113,17 +113,24 @@ constructor(
}
}

private fun increaseStack() {
if (stackSize == path.size) {
path = path.copyOf(path.size * 2)
containerStack = containerStack.copyOf(containerStack.size * 2)
nameIndexStack = nameIndexStack.copyOf(nameIndexStack.size * 2)
iteratorStack = iteratorStack.copyOf(iteratorStack.size * 2)
}
stackSize++
}

override fun beginArray() = apply {
if (peek() != JsonReader.Token.BEGIN_ARRAY) {
throw JsonDataException("Expected BEGIN_ARRAY but was ${peek()} at path ${getPathAsString()}")
}

val currentValue = peekedData as List<Any?>

check(stackSize < MAX_STACK_SIZE) {
"Nesting too deep"
}
stackSize++
increaseStack()

path[stackSize - 1] = -1
iteratorStack[stackSize - 1] = currentValue.iterator()
Expand All @@ -145,10 +152,8 @@ constructor(
throw JsonDataException("Expected BEGIN_OBJECT but was ${peek()} at path ${getPathAsString()}")
}

check(stackSize < MAX_STACK_SIZE) {
"Nesting too deep"
}
stackSize++
increaseStack()

@Suppress("UNCHECKED_CAST")
containerStack[stackSize - 1] = peekedData as Map<String, Any?>

Expand Down
36 changes: 36 additions & 0 deletions libraries/apollo-api/src/commonTest/kotlin/test/JsonTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@ package test
import com.apollographql.apollo3.api.AnyAdapter
import com.apollographql.apollo3.api.CustomScalarAdapters
import com.apollographql.apollo3.api.LongAdapter
import com.apollographql.apollo3.api.json.MapJsonReader
import com.apollographql.apollo3.api.json.MapJsonWriter
import com.apollographql.apollo3.api.json.buildJsonString
import com.apollographql.apollo3.api.json.jsonReader
import com.apollographql.apollo3.api.json.readAny
import okio.Buffer
import kotlin.test.Test
import kotlin.test.assertEquals

Expand All @@ -27,4 +31,36 @@ class JsonTest {
}
assertEquals("9223372036854775807", json)
}

@Test
fun canReadAndWriteVeryDeeplyNestedJsonSource() {
val json = buildJsonString {
val nesting = 1025
repeat(nesting) {
beginObject()
name("child")
}
value("yooooo")
repeat(nesting) {
endObject()
}
}

Buffer().writeUtf8(json).jsonReader().readAny()
}

@Test
fun canReadVeryDeeplyNestedJsonMap() {
val root = mutableMapOf<String, Any>()
var map = root
val nesting = 1025

repeat(nesting) {
val newMap = mutableMapOf<String, Any>()
map.put("child", newMap)
map = newMap
}

MapJsonReader(root).readAny()
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.apollographql.apollo3.api.json

import com.apollographql.apollo3.api.json.BufferedSourceJsonReader.Companion.MAX_STACK_SIZE
import com.apollographql.apollo3.api.json.BufferedSourceJsonReader.Companion.INITIAL_STACK_SIZE
import com.apollographql.apollo3.api.json.MapJsonReader.Companion.buffer
import com.apollographql.apollo3.api.json.internal.toDoubleExact
import com.apollographql.apollo3.api.json.internal.toIntExact
Expand Down Expand Up @@ -74,14 +74,14 @@ constructor(
* - a String representing the current key to be read in a Map
* - null if peekedToken is BEGIN_OBJECT
*/
private val path = arrayOfNulls<Any>(MAX_STACK_SIZE)
private var path = arrayOfNulls<Any>(INITIAL_STACK_SIZE)

/**
* The current object memorized in case we need to rewind
*/
private var containerStack = arrayOfNulls<Any?>(MAX_STACK_SIZE)
private val iteratorStack = arrayOfNulls<IteratorWrapper>(MAX_STACK_SIZE)
private val nameIndexStack = IntArray(MAX_STACK_SIZE)
private var containerStack = arrayOfNulls<Any?>(INITIAL_STACK_SIZE)
private var iteratorStack = arrayOfNulls<IteratorWrapper>(INITIAL_STACK_SIZE)
private var nameIndexStack = IntArray(INITIAL_STACK_SIZE)

private var stackSize = 0

Expand Down Expand Up @@ -143,17 +143,25 @@ constructor(
}
}

private fun increaseStack() {
if (stackSize == path.size) {
path = path.copyOf(path.size * 2)
containerStack = containerStack.copyOf(containerStack.size * 2)
nameIndexStack = nameIndexStack.copyOf(nameIndexStack.size * 2)
iteratorStack = iteratorStack.copyOf(iteratorStack.size * 2)
}
stackSize++
}


override fun beginArray() = apply {
if (peek() != JsonReader.Token.BEGIN_ARRAY) {
throw JsonDataException("Expected BEGIN_ARRAY but was ${peek()} at path ${getPathAsString()}")
}

val currentValue = peekedData as Array<*>

check(stackSize < MAX_STACK_SIZE) {
"Nesting too deep"
}
stackSize++

increaseStack()

path[stackSize - 1] = -1
iteratorStack[stackSize - 1] = IteratorWrapper.StandardIterator(currentValue.iterator())
Expand All @@ -175,10 +183,7 @@ constructor(
throw JsonDataException("Expected BEGIN_OBJECT but was ${peek()} at path ${getPathAsString()}")
}

check(stackSize < MAX_STACK_SIZE) {
"Nesting too deep"
}
stackSize++
increaseStack()
containerStack[stackSize - 1] = peekedData.asDynamic()

rewind()
Expand Down

0 comments on commit 3972d9c

Please sign in to comment.