JSON.stringify: give the DynamicBuffer fast path a depth limit so cyclic values fail fast

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);
+    }
+}

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<CharType, bufferMode>::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<CharType, bufferMode>::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<CharType, bufferMode>::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<CharType, bufferMode>::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]] {