diff --git a/JSTests/stress/json-stringify-cyclic-depth-limit.js b/JSTests/stress/json-stringify-cyclic-depth-limit.js new file mode 100644 index 000000000000..14000a4e2836 --- /dev/null +++ b/JSTests/stress/json-stringify-cyclic-depth-limit.js @@ -0,0 +1,89 @@ +// The DynamicBuffer FastStringifier bails to the general Stringifier past a +// depth limit, so a cyclic value is rejected after bounded work instead of +// growing the buffer toward the 2GB string length limit. This test checks the +// behavior around that limit: cycles still throw TypeError, and values nested +// deeper than the limit still stringify correctly through the general path. + +function shouldThrowTypeError(fn) { + let threw = null; + try { + fn(); + } catch (e) { + threw = e; + } + if (!(threw instanceof TypeError)) + throw new Error("expected TypeError, got " + threw); +} + +function makeCyclicObject(keys) { + const o = {}; + for (let i = 0; i < keys; i++) + o["k" + i] = "y".repeat(50); + o.self = o; + return o; +} + +// A payload large enough that the StaticBuffer attempt fails with BufferFull +// and the DynamicBuffer attempt runs. +const bigPayload = "y".repeat(10 * 1024); + +// Cyclic values throw, flat and with a gap, through objects and arrays, for +// direct and indirect cycles. +for (const space of [undefined, 2, "\t"]) { + for (const keys of [0, 8]) + shouldThrowTypeError(() => JSON.stringify(makeCyclicObject(keys), null, space)); + + shouldThrowTypeError(() => JSON.stringify([makeCyclicObject(1)], null, space)); + + const selfArray = [1]; + selfArray.push(selfArray); + shouldThrowTypeError(() => JSON.stringify(selfArray, null, space)); +} + +// Cycles that carry a large payload per revolution. +for (const space of [undefined, 2]) { + const a = { payload: bigPayload }; + const b = { a }; + a.b = b; + shouldThrowTypeError(() => JSON.stringify(a, null, space)); + + const direct = { payload: bigPayload }; + direct.self = direct; + shouldThrowTypeError(() => JSON.stringify(direct, null, space)); +} + +// Acyclic values nested deeper than any reasonable fast path depth limit +// round-trip correctly, flat and with a gap, for objects and arrays, with +// 8-bit and 16-bit content. +for (const space of [undefined, 1]) { + for (const leafValue of ["eight-bit", "sixte\u00e9n-bit \u2603"]) { + const depth = 1000; + + let root = {}; + let node = root; + for (let i = 0; i < depth; i++) + node = node.x = {}; + node.leaf = leafValue; + let parsed = JSON.parse(JSON.stringify(root, null, space)); + let p = parsed; + for (let i = 0; i < depth; i++) + p = p.x; + if (p.leaf !== leafValue) + throw new Error("deep object round-trip broken for space=" + space); + + let arrayRoot = []; + let arrayNode = arrayRoot; + for (let i = 0; i < depth; i++) { + const next = []; + arrayNode.push(next); + arrayNode = next; + } + arrayNode.push(leafValue); + parsed = JSON.parse(JSON.stringify(arrayRoot, null, space)); + p = parsed; + for (let i = 0; i < depth; i++) + p = p[p.length - 1]; + if (p[0] !== leafValue) + throw new Error("deep array round-trip broken for space=" + space); + } +} diff --git a/Source/JavaScriptCore/runtime/JSONObject.cpp b/Source/JavaScriptCore/runtime/JSONObject.cpp index e2ede9fae907..11caf8599398 100644 --- a/Source/JavaScriptCore/runtime/JSONObject.cpp +++ b/Source/JavaScriptCore/runtime/JSONObject.cpp @@ -682,6 +682,12 @@ bool Stringifier::Holder::appendNextProperty(Stringifier& stringifier, StringBui // it counts on hitting the buffer size limit to catch those things. If it fails, // since there is no side effect, the full general purpose Stringifier can be used // and the only cost of the fast stringifying attempt is the time wasted. +// +// In DynamicBuffer mode the buffer limit is the 2GB string length limit, so the +// buffer limit alone is far too expensive as a cycle check: a cyclic value with a +// gap reaches it only after writing (and repeatedly reallocating) gigabytes. A +// depth limit bounds that wasted work. Values nested deeper than the limit take +// the general Stringifier, whose holder stack detects cycles immediately. enum class BufferMode : uint8_t { StaticBuffer, @@ -695,6 +701,7 @@ enum class FailureReason : uint8_t { Found16BitEarly, Found16BitLate, StackOverflow, + DepthLimit, Unknown, }; @@ -707,6 +714,15 @@ class FastStringifier { static constexpr unsigned staticBufferSize = bufferMode == BufferMode::StaticBuffer ? 8192 : 8; static constexpr unsigned dynamicBufferInlineCapacity = bufferMode == BufferMode::StaticBuffer ? 0 : 1024; + // DynamicBuffer mode only: values nested deeper than this bail to the general + // Stringifier. Keeps a cyclic value from filling the buffer up to the string + // length limit before the general Stringifier gets to throw for the cycle. + static constexpr unsigned maximumDepth = 512; + + // m_depth drives the indentation when there is a gap, and the maximumDepth + // check in DynamicBuffer mode. + static constexpr bool trackDepthWithoutGap = bufferMode == BufferMode::DynamicBuffer; + static constexpr bool useShortCopyTier = bufferMode == BufferMode::DynamicBuffer; private: @@ -1351,6 +1367,10 @@ void FastStringifier::append(JSValue value) recordFailure(FailureReason::StackOverflow, "stack overflow"_s); return; } + if (m_depth >= maximumDepth) [[unlikely]] { + recordFailure(FailureReason::DepthLimit, "depth limit"_s); + return; + } } if (value.isNull()) { @@ -1525,7 +1545,7 @@ void FastStringifier::append(JSValue value) recordFailure("object has non-reified static properties"_s); return; } - if constexpr (hasGap == HasGap::Yes) + if constexpr (hasGap == HasGap::Yes || trackDepthWithoutGap) ++m_depth; const unsigned newLineAndIndent = hasGap == HasGap::Yes ? newLineAndIndentSize() : 0; structure.forEachProperty(m_vm, [&](const auto& entry) -> bool { @@ -1654,7 +1674,7 @@ void FastStringifier::append(JSValue value) }); if (haveFailure()) [[unlikely]] return; - if constexpr (hasGap == HasGap::Yes) + if constexpr (hasGap == HasGap::Yes || trackDepthWithoutGap) --m_depth; bool needNewLine = hasGap == HasGap::Yes && buffer()[m_length - 1] != '{'; if (!hasRemainingCapacity(needNewLine ? 1 + newLineAndIndentSize() : 1)) [[unlikely]] { @@ -1701,11 +1721,11 @@ void FastStringifier::append(JSValue value) return; } buffer()[m_length++] = '['; - if constexpr (hasGap == HasGap::Yes) + if constexpr (hasGap == HasGap::Yes || trackDepthWithoutGap) ++m_depth; auto closeArray = [&] { - if constexpr (hasGap == HasGap::Yes) + if constexpr (hasGap == HasGap::Yes || trackDepthWithoutGap) --m_depth; bool needNewLine = hasGap == HasGap::Yes && buffer()[m_length - 1] != '['; if (!hasRemainingCapacity(needNewLine ? 1 + newLineAndIndentSize() : 1)) [[unlikely]] {