fix(zero-rust): take JSON nested as deep as zero-cache writes (#205)

The parser rejected JSON nested deeper than 1,500 levels, because cloning,
comparing, and writing a value recursed and a debug build overflowed a
2 MB thread stack by about 2,400 to 3,500 levels. zero-cache writes up to
about 4,100, and JS takes them, so a row with a JSON column nested deeper
than 1,500 made the client reject the poke, disconnect, and fail the same
way after every reconnect.

Clone, PartialEq, and json::write now recurse 128 levels and then go on
over a heap stack, and MAX_DEPTH is 5,000. Dropping fits in a 2 MB debug
stack at that depth without changes. The conformance runner no longer has
a nesting exception: harden-json's 1,502-level case matches JS exactly,
and a rejection for depth now counts as a failure. PARITY.md records why
the two remaining exceptions (lone surrogates, unwritable JSON paths)
stay.

Amp-Thread-ID: https://ampcode.com/threads/T-db04f816-97ed-4bc9-b19e-9ee68611a0ab

Co-authored-by: Amp <amp@ampcode.com>

Author: tmm

.changeset/zero-rust-json-depth.md

@@ -0,0 +1,5 @@
+---
+'zero-rust': patch
+---
+
+Take JSON nested up to 5,000 levels deep, more than zero-cache can write, instead of rejecting it past 1,500: a row with a JSON column nested that deep no longer makes the client disconnect after every reconnect. Cloning, comparing, and writing such a value no longer recurse past a hundred or so levels, so it fits in a 2 MB thread stack even in a debug build.

rust/PARITY.md

@@ -193,10 +193,19 @@ shared zbugs model (`apps/zbugs-model`); a GPUI app is in progress.
 ## Documented differences
 
 Each is counted separately in the conformance report, never as an exact
-match.
-
-| Difference      | Why                                                                                                                                                                                                                                                                                                                         |
-| --------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| Lone surrogate  | A Rust `String` can't hold a lone surrogate, which a JS string can. The parser reads an escaped one (`"\ud800"`) as U+FFFD, as the Swift client does; such a case matches when JS's output, with each such escape read as U+FFFD, is what Rust writes. Real servers send valid UTF-8, so this only shows in fuzzing.        |
-| Nesting         | JSON nested deeper than `json::MAX_DEPTH` (1,500) is rejected. Dropping, comparing, and writing a value recurse, and a debug build overflows a 2 MB thread stack by 3,000 levels. JS has no limit short of its own stack (zero-cache writes about 4,100 levels). The Swift client has the same limit.                       |
-| Unwritable path | A JSON path index is a `u32`, so a program with a negative or fractional index (`json('metadata', -1)`) can't be written in Rust. JS rejects it with "Invalid JSON path segment …"; such a case matches when JS rejects it that way. An index past `MAX_JSON_PATH_INDEX` can be written, and is rejected with JS's message. |
+match. Both come from what Rust's types can hold, not from the client's
+logic, and are kept on purpose.
+
+| Difference      | Why it stays                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                           |
+| --------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| Lone surrogate  | A Rust `String` can't hold a lone surrogate, which a JS string can. The parser reads an escaped one (`"\ud800"`) as U+FFFD, as the Swift client does; such a case matches when JS's output, with each such escape read as U+FFFD, is what Rust writes. Holding one would take a string type of Rust's own (WTF-8) throughout the client and in every app's code. Postgres text and `jsonb` can't store one: only a `json` (not `jsonb`) column, which keeps its text as written, could bring one to the client, and it shows as U+FFFD, as a browser renders it. Nearly all such cases come from the JSON fuzz corpus. |
+| Unwritable path | A JSON path index is a `u32`, so a program with a negative or fractional index (`json('metadata', -1)`) can't be written in Rust. JS rejects it with "Invalid JSON path segment …"; such a case matches when JS rejects it that way. Rust's types rule out at compile time what JS throws for at run time. An index past `MAX_JSON_PATH_INDEX` can be written, and is rejected with JS's message.                                                                                                                                                                                                                      |
+
+JSON nesting is no longer a difference. JSON nested up to `json::MAX_DEPTH`
+(5,000) levels deep, more than the about 4,100 zero-cache can write, is
+parsed, cloned, compared, written, and dropped within a 2 MB thread stack,
+even in a debug build, so the client takes any value the server sends.
+Deeper text is rejected ("JSON nested too deeply"), where JS takes whatever
+fits its stack. The Swift client rejects past 1,500 levels, so a row with a
+JSON column nested deeper than that makes it disconnect after every
+reconnect. Formatting a value with `Debug` still recurses.

rust/conformance/src/suites/json.rs

@@ -10,9 +10,6 @@ use zero_client::json::{self, Value, number};
 pub const LONE_SURROGATE: &str =
     "lone surrogate: a Rust String can't hold one, so `\\ud800` parses as U+FFFD";
 
-/// The exception for JSON nested deeper than `json::MAX_DEPTH`.
-pub const TOO_DEEP: &str = "nesting: JSON deeper than json::MAX_DEPTH is rejected";
-
 pub fn suites() -> Vec<Suite> {
     vec![
         Suite {
@@ -95,7 +92,6 @@ fn parse_and_stringify(c: &Value, _: &Value) -> Result<Outcome, String> {
     let ok = c.bool("ok")?;
     let value = match json::parse(input) {
         Ok(value) => value,
-        Err(e) if ok && e.message == json::TOO_DEEP => return Ok(Outcome::Exception(TOO_DEEP)),
         Err(e) => {
             ensure(!ok, || format!("rejected {}: {e}", quote(input)))?;
             return Ok(Outcome::Pass);

rust/conformance/src/suites/protocol.rs

@@ -186,9 +186,6 @@ fn downstream_text(c: &Value, _: &Value) -> Result<Outcome, String> {
     let result = Downstream::from_text(text);
     match (result, c.bool("ok")?) {
         (Ok(_), true) | (Err(_), false) => Ok(Outcome::Pass),
-        (Err(e), true) if e.message.contains(json::TOO_DEEP) => {
-            Ok(Outcome::Exception(super::json::TOO_DEEP))
-        }
         (Ok(_), false) => Err(format!("accepted {}", quote(text))),
         (Err(e), true) => Err(format!("rejected {}: {e}", quote(text))),
     }

rust/zero-client/src/json/parse.rs

@@ -20,11 +20,12 @@ impl std::error::Error for ParseError {}
 
 /// Nesting deeper than this is rejected with [`TOO_DEEP`].
 ///
-/// Parsing doesn't recurse, but dropping, comparing, and writing a value do,
-/// so the limit keeps them within a 2 MB thread stack (the default for
-/// spawned Rust threads) in a debug build, which overflows one by 3,000
-/// levels. JS has no limit short of its own stack; see `rust/PARITY.md`.
-pub const MAX_DEPTH: usize = 1_500;
+/// It is above the about 4,100 levels zero-cache can write, so the client
+/// takes any value the server sends. JS has no limit short of its own
+/// stack; see `rust/PARITY.md`. Parsing, cloning, comparing, and writing a
+/// value don't recurse that deep, and dropping one fits in a 2 MB thread
+/// stack (the default for spawned Rust threads) even in a debug build.
+pub const MAX_DEPTH: usize = 5_000;
 
 /// The message of the error for JSON nested deeper than [`MAX_DEPTH`].
 pub const TOO_DEEP: &str = "JSON nested too deeply";

rust/zero-client/src/json/stringify.rs

@@ -1,4 +1,6 @@
+use super::value::RECURSION_DEPTH;
 use super::{Value, number};
+use crate::map;
 
 /// Encodes a value as `JSON.stringify` does: the same string escaping and
 /// number formatting, with non-finite numbers written as `null`.
@@ -13,34 +15,99 @@ pub fn stringify(value: &Value) -> String {
 
 /// Appends `value` as JSON text.
 pub fn write(value: &Value, out: &mut String) {
+    write_within(value, out, RECURSION_DEPTH);
+}
+
+/// [`write`], recursing `depth` levels deep.
+fn write_within(value: &Value, out: &mut String, depth: usize) {
     match value {
-        Value::Null => out.push_str("null"),
-        Value::Bool(b) => out.push_str(if *b { "true" } else { "false" }),
-        Value::Number(n) if n.is_finite() => out.push_str(&number::format(*n)),
-        Value::Number(_) => out.push_str("null"),
-        Value::String(s) => write_string(s, out),
-        Value::Array(a) => {
+        Value::Array(a) if depth > 0 => {
             out.push('[');
             for (i, e) in a.iter().enumerate() {
                 if i > 0 {
                     out.push(',');
                 }
-                write(e, out);
+                write_within(e, out, depth - 1);
             }
             out.push(']');
         }
-        Value::Object(o) => {
+        Value::Object(o) if depth > 0 => {
             out.push('{');
             for (i, (k, v)) in o.iter().enumerate() {
                 if i > 0 {
                     out.push(',');
                 }
                 write_string(k, out);
                 out.push(':');
-                write(v, out);
+                write_within(v, out, depth - 1);
             }
             out.push('}');
         }
+        Value::Array(_) | Value::Object(_) => write_iteratively(value, out),
+        scalar => write_scalar(scalar, out),
+    }
+}
+
+fn write_iteratively(value: &Value, out: &mut String) {
+    enum Open<'a> {
+        Array(std::slice::Iter<'a, Value>),
+        Object(map::Iter<'a, Value>),
+    }
+    // The arrays and objects being written, each with whether it has
+    // written an element yet.
+    let mut stack: Vec<(Open, bool)> = Vec::new();
+    let mut next = Some(value);
+    loop {
+        match next.take() {
+            Some(Value::Array(a)) => {
+                out.push('[');
+                stack.push((Open::Array(a.iter()), false));
+            }
+            Some(Value::Object(o)) => {
+                out.push('{');
+                stack.push((Open::Object(o.iter()), false));
+            }
+            Some(scalar) => write_scalar(scalar, out),
+            None => {}
+        }
+        let Some((open, started)) = stack.last_mut() else {
+            return;
+        };
+        let element = match open {
+            Open::Array(rest) => rest.next().map(|v| (None, v)),
+            Open::Object(rest) => rest.next().map(|(k, v)| (Some(k), v)),
+        };
+        match element {
+            Some((key, v)) => {
+                if *started {
+                    out.push(',');
+                }
+                *started = true;
+                if let Some(key) = key {
+                    write_string(key, out);
+                    out.push(':');
+                }
+                next = Some(v);
+            }
+            None => {
+                out.push(if matches!(open, Open::Array(_)) {
+                    ']'
+                } else {
+                    '}'
+                });
+                stack.pop();
+            }
+        }
+    }
+}
+
+fn write_scalar(value: &Value, out: &mut String) {
+    match value {
+        Value::Bool(b) => out.push_str(if *b { "true" } else { "false" }),
+        Value::Number(n) if n.is_finite() => out.push_str(&number::format(*n)),
+        Value::String(s) => write_string(s, out),
+        // Null, the non-finite numbers, and (never passed here) containers.
+        _ => out.push_str("null"),
     }
 }