diff --git a/changelog.d/7832-iterative-json-parse.md b/changelog.d/7832-iterative-json-parse.md new file mode 100644 index 0000000000..71b4655d84 --- /dev/null +++ b/changelog.d/7832-iterative-json-parse.md @@ -0,0 +1,4 @@ +**`JSON.parse` now handles deeply nested documents without exhausting a worker +thread's native stack.** Inputs beyond the recursive fast path use strict tape +validation and heap-backed iterative materialization, with a separate +500,000-level resource limit. diff --git a/crates/perry-runtime/src/json/mod.rs b/crates/perry-runtime/src/json/mod.rs index 654f36ff5c..221cd33e89 100644 --- a/crates/perry-runtime/src/json/mod.rs +++ b/crates/perry-runtime/src/json/mod.rs @@ -824,7 +824,7 @@ mod tests { } } - /// #7792 — deeply nested input must throw, not take the process out. + /// #7792 / #7817 — deeply nested input must not take the process out. /// /// Both parsers that read the document recurse once per nesting level, so /// a deep enough document exhausted the stack: SIGSEGV, exit 139, no @@ -833,7 +833,17 @@ mod tests { /// document is unusual. mod nesting_depth { use super::*; - use crate::json::parser::{nesting_depth_exceeds, MAX_NESTING_DEPTH}; + use crate::json::parser::{ + nesting_depth_exceeds, MAX_ITERATIVE_NESTING_DEPTH, MAX_RECURSIVE_NESTING_DEPTH, + }; + + fn nested_arrays(depth: usize, leaf: u8) -> Vec { + let mut input = Vec::with_capacity(depth * 2 + 1); + input.extend(std::iter::repeat_n(b'[', depth)); + input.push(leaf); + input.extend(std::iter::repeat_n(b']', depth)); + input + } #[test] fn the_scan_counts_only_structural_brackets() { @@ -855,28 +865,79 @@ mod tests { assert!(!nesting_depth_exceeds(b"", 0)); } - /// The limit is the point of the change, so pin the boundary itself: - /// one level under passes, one level over is refused. + /// Pin the parser handoff boundary: both sides produce a value. #[test] - fn parse_refuses_input_past_the_limit_and_accepts_input_under_it() { - let ok_depth = MAX_NESTING_DEPTH - 1; - let mut ok = vec![b'['; ok_depth]; - ok.extend(std::iter::repeat(b']').take(ok_depth)); + fn parse_switches_to_the_iterative_path_past_the_recursive_threshold() { + let ok_depth = MAX_RECURSIVE_NESTING_DEPTH - 1; + let ok = nested_arrays(ok_depth, b'0'); let text = js_string_from_bytes(ok.as_ptr(), ok.len() as u32); assert!( unsafe { js_json_parse_result(text) }.is_ok(), - "input inside the limit must still parse" + "input below the handoff must parse" ); - let deep_depth = MAX_NESTING_DEPTH + 1; - let mut deep = vec![b'['; deep_depth]; - deep.extend(std::iter::repeat(b']').take(deep_depth)); + let deep_depth = MAX_RECURSIVE_NESTING_DEPTH + 1; + let deep = nested_arrays(deep_depth, b'0'); let text = js_string_from_bytes(deep.as_ptr(), deep.len() as u32); assert!( - unsafe { js_json_parse_result(text) }.is_err(), - "input past the limit must be refused rather than descended into" + unsafe { js_json_parse_result(text) }.is_ok(), + "input above the handoff must parse through the heap-stack path" + ); + } + + #[test] + fn parses_three_hundred_thousand_levels_on_a_small_worker_stack() { + const DEPTH: usize = 300_000; + std::thread::Builder::new() + .name("json-deep-worker".into()) + .stack_size(2 * 1024 * 1024) + .spawn(|| { + let input = nested_arrays(DEPTH, b'7'); + let text = js_string_from_bytes(input.as_ptr(), input.len() as u32); + let mut value = unsafe { js_json_parse_result(text) } + .expect("deep JSON must parse on a worker-sized stack"); + + for level in 0..DEPTH { + assert!(value.is_pointer(), "level {level} must be an array"); + let array = (value.bits() & POINTER_MASK) as *const crate::ArrayHeader; + assert_eq!(unsafe { (*array).length }, 1, "level {level}"); + value = crate::array::js_array_get(array, 0); + } + assert_eq!(f64::from_bits(value.bits()), 7.0); + }) + .expect("worker thread starts") + .join() + .expect("worker parse does not panic"); + } + + #[test] + fn rejects_nesting_beyond_the_iterative_resource_budget() { + let input = nested_arrays(MAX_ITERATIVE_NESTING_DEPTH + 1, b'0'); + let text = js_string_from_bytes(input.as_ptr(), input.len() as u32); + let error = unsafe { js_json_parse_result(text) } + .expect_err("the iterative path must keep a finite resource budget"); + let error = (error.to_bits() & POINTER_MASK) as *const crate::error::ErrorHeader; + assert_eq!( + unsafe { (*error).error_kind }, + crate::error::ERROR_KIND_RANGE_ERROR ); } + + #[test] + fn iterative_path_still_rejects_malformed_json() { + let depth = MAX_RECURSIVE_NESTING_DEPTH + 1; + let mut trailing = nested_arrays(depth, b'0'); + trailing.push(b'x'); + let mut invalid_number = Vec::with_capacity(depth * 2 + 2); + invalid_number.extend(std::iter::repeat_n(b'[', depth)); + invalid_number.extend_from_slice(b"01"); + invalid_number.extend(std::iter::repeat_n(b']', depth)); + + for input in [trailing, invalid_number] { + let text = js_string_from_bytes(input.as_ptr(), input.len() as u32); + assert!(unsafe { js_json_parse_result(text) }.is_err()); + } + } } #[test] diff --git a/crates/perry-runtime/src/json/parse_api.rs b/crates/perry-runtime/src/json/parse_api.rs index ffa0aa0df2..5f3683172b 100644 --- a/crates/perry-runtime/src/json/parse_api.rs +++ b/crates/perry-runtime/src/json/parse_api.rs @@ -69,8 +69,6 @@ fn syntax_error_value(message: &str) -> f64 { f64::from_bits(JSValue::pointer(err as *const u8).bits()) } -/// A catchable `RangeError`, for the one JSON failure that is about size -/// rather than shape: input nested deeper than the parser can descend. fn range_error_value(message: &str) -> f64 { let msg_ptr = js_string_from_bytes(message.as_ptr(), message.len() as u32); let err = crate::error::js_rangeerror_new(msg_ptr); @@ -85,21 +83,32 @@ fn throw_range_error(message: &str) -> ! { crate::exception::js_throw(range_error_value(message)) } -/// The one depth check, called by every entry that is about to descend. +/// Select the heap-stack parser before recursive validation or materialization +/// gets close to the smallest worker-thread stack. /// /// `js_json_parse` and `js_json_parse_result` are separate implementations of /// the same flow, and the typed-array path is a third. Sharing the decision is /// what keeps them from drifting — the first version of this fix guarded only /// one of the three and appeared to do nothing at all, because the entry point /// codegen actually calls was one of the other two. -fn nesting_is_too_deep(bytes: &[u8]) -> bool { - crate::json::parser::nesting_depth_exceeds(bytes, crate::json::parser::MAX_NESTING_DEPTH) +fn requires_iterative_parse(bytes: &[u8]) -> bool { + crate::json::parser::nesting_depth_exceeds( + bytes, + crate::json::parser::MAX_RECURSIVE_NESTING_DEPTH, + ) } -fn too_deep_message() -> String { +fn exceeds_iterative_budget(bytes: &[u8]) -> bool { + crate::json::parser::nesting_depth_exceeds( + bytes, + crate::json::parser::MAX_ITERATIVE_NESTING_DEPTH, + ) +} + +fn iterative_budget_message() -> String { format!( - "JSON.parse: input nested deeper than {} levels", - crate::json::parser::MAX_NESTING_DEPTH + "JSON.parse: input exceeds the {}-level iterative nesting budget", + crate::json::parser::MAX_ITERATIVE_NESTING_DEPTH ) } @@ -118,6 +127,53 @@ fn is_json_null_literal(bytes: &[u8]) -> bool { &bytes[start..end] == b"null" } +/// Parse a deeply nested document through the flat tape representation. Tape +/// construction validates syntax with an explicit heap stack; materialization +/// likewise keeps pending containers on the heap. This path runs only beyond +/// the recursive fast path's safe depth, so ordinary JSON keeps its existing +/// allocation and shape-specialization behavior. +unsafe fn try_parse_deep_iterative( + text_ptr: *const StringHeader, + len: usize, + bytes: &[u8], +) -> Option { + let text_root = parse_root_push(JSValue::string_ptr(text_ptr as *mut StringHeader)); + let result = crate::json_tape::with_built_tape(bytes, |tape_entries| { + crate::gc::gc_collect_pending_suppressed_parse(); + crate::gc::gc_check_trigger(); + crate::gc::gc_suppress(); + + let bytes = { + let moved = parse_root_get(text_root); + let hdr = moved.as_string_ptr(); + let data_ptr = (hdr as *const u8).add(std::mem::size_of::()); + std::slice::from_raw_parts(data_ptr, len) + }; + let result = crate::json_tape::materialize_iterative(tape_entries, bytes); + if let Some(value) = result { + parse_root_push(value); + } + + crate::gc::gc_unsuppress(); + crate::gc::gc_bump_malloc_trigger(); + crate::gc::gc_schedule_parse_boundary_collection_if_pressure(); + result + }) + .flatten(); + parse_root_restore(text_root); + + PARSE_KEY_CACHE.with(|cell| { + let cache = cell.borrow(); + if cache.len() > 4096 { + drop(cache); + cell.borrow_mut().clear(); + clear_parse_key_ring(); + } + }); + + result +} + /// Non-throwing JSON parse entry for APIs that must reject a Promise rather than /// synchronously throwing through `JSON.parse`'s FFI boundary. /// @@ -136,12 +192,12 @@ pub unsafe fn js_json_parse_result(text_ptr: *const StringHeader) -> Result JSValue if len == 0 { throw_syntax_error("Unexpected end of JSON input"); } - // #7792: depth first, ahead of the validation pass, for the same reason as - // the `_result` twin above. This is the entry codegen emits, so a guard - // that covered only the twin covered nothing a compiled program can reach. - if nesting_is_too_deep(bytes) { - throw_range_error(&too_deep_message()); + if requires_iterative_parse(bytes) { + if exceeds_iterative_budget(bytes) { + throw_range_error(&iterative_budget_message()); + } + return match try_parse_deep_iterative(text_ptr, len, bytes) { + Some(value) => value, + None => throw_syntax_error("JSON parse error: malformed deep document"), + }; } // Keep serde_json's strict syntax validation, but discard tokens as they // are read instead of allocating an intermediate `serde_json::Value` @@ -559,10 +618,9 @@ pub unsafe extern "C" fn js_json_parse_typed_array( let data_ptr = (text_ptr as *const u8).add(std::mem::size_of::()); let bytes = std::slice::from_raw_parts(data_ptr, len); - // #7792: this path builds its own parser, so it needs its own guard. Hand - // deep input to the generic entry rather than repeating the error here, so - // both report it identically. - if nesting_is_too_deep(bytes) { + // Deep input uses the generic entry's heap-stack fallback. The shape fast + // path is deliberately retained for ordinary payloads only. + if requires_iterative_parse(bytes) { return js_json_parse(text_ptr); } diff --git a/crates/perry-runtime/src/json/parser.rs b/crates/perry-runtime/src/json/parser.rs index e8f1dda18b..7936d8e635 100644 --- a/crates/perry-runtime/src/json/parser.rs +++ b/crates/perry-runtime/src/json/parser.rs @@ -56,30 +56,20 @@ pub(crate) struct ObjectShapeHint { pub(crate) field_count: u32, } -/// The deepest `[`/`{` nesting `JSON.parse` will accept. +/// The deepest `[`/`{` nesting handled by the recursive fast path. /// -/// Both parsers that see the input recurse once per level — the `serde_json` -/// validation pass and Perry's own value parser — so a deep enough document -/// exhausts the stack and takes the whole process out with SIGSEGV, no -/// diagnostic and no output, on input that is very often attacker-supplied -/// (#7792). Measured on a default 8 MB main-thread stack the crash lands -/// between 20,000 and 40,000 levels — but that is the most generous stack in -/// the process, and it is the wrong one to size against. Perry parses JSON on -/// `perry/thread` workers and tokio workers too, and a 2 MiB thread stack -/// overflows well before 10,000 levels: a first attempt at this limit picked -/// 10,000 off the main-thread measurement, and the unit test below promptly -/// crashed the test harness at 9,999. +/// Both the `serde_json` validation pass and Perry's direct value parser recurse +/// once per container, so their cutoff is sized for Perry's smallest worker +/// stack rather than the main thread's larger stack (#7792). /// -/// So the limit is sized for the SMALLEST stack in the process, not the -/// largest, and 1,000 is the same depth Python's parser has settled on. Real -/// documents do not come close: JSON nested past a hundred levels is already -/// unusual, and past a thousand is a machine talking to itself. -/// -/// This is a deliberate parity gap. Node parses far deeper than this because -/// V8's parser is iterative and does not consume stack per level; matching it -/// means making this parser iterative too, which is the follow-up. Until then -/// a catchable error beats a SIGSEGV on untrusted input. -pub(crate) const MAX_NESTING_DEPTH: usize = 1_000; +/// Deeper documents switch to the flat-tape parser and iterative materializer, +/// so this is a native-stack safety threshold rather than an input limit. +pub(crate) const MAX_RECURSIVE_NESTING_DEPTH: usize = 1_000; + +/// Heap-stack safety ceiling. This remains well above Node-parity cases such as +/// #7817's 300,000-level document, while bounding the tape, pending-frame stack, +/// and runtime-container amplification for unusually deep input. +pub(crate) const MAX_ITERATIVE_NESTING_DEPTH: usize = 500_000; /// Does `bytes` nest deeper than `limit`? /// diff --git a/crates/perry-runtime/src/json_tape.rs b/crates/perry-runtime/src/json_tape.rs index ac3415224d..5ea9813249 100644 --- a/crates/perry-runtime/src/json_tape.rs +++ b/crates/perry-runtime/src/json_tape.rs @@ -32,6 +32,9 @@ use crate::value::JSValue; use std::cell::Cell; +mod iterative; +pub(crate) use iterative::materialize_iterative; + /// One tape entry. Kind + byte offset + (for container kinds) a /// parent/sibling pointer that lets materialization skip over /// already-traversed subtrees. @@ -190,10 +193,8 @@ fn build_tape_into(bytes: &[u8], entries: &mut Vec, stack: &mut Vec bool { debug_assert_eq!(bytes[*pos], b'"'); @@ -209,7 +210,21 @@ fn build_tape_into(bytes: &[u8], entries: &mut Vec, stack: &mut Vec= bytes.len() { return false; } - *pos += 1; + match bytes[*pos] { + b'"' | b'\\' | b'/' | b'b' | b'f' | b'n' | b'r' | b't' => *pos += 1, + b'u' => { + *pos += 1; + if *pos + 4 > bytes.len() + || !bytes[*pos..*pos + 4].iter().all(u8::is_ascii_hexdigit) + { + return false; + } + *pos += 4; + } + _ => return false, + } + } else if c < 0x20 { + return false; } else { *pos += 1; } @@ -217,30 +232,46 @@ fn build_tape_into(bytes: &[u8], entries: &mut Vec, stack: &mut Vec bool { if *pos < bytes.len() && bytes[*pos] == b'-' { *pos += 1; } - while *pos < bytes.len() && bytes[*pos].is_ascii_digit() { - *pos += 1; + match bytes.get(*pos) { + Some(b'0') => *pos += 1, + Some(b'1'..=b'9') => { + *pos += 1; + while *pos < bytes.len() && bytes[*pos].is_ascii_digit() { + *pos += 1; + } + } + _ => return false, } if *pos < bytes.len() && bytes[*pos] == b'.' { *pos += 1; + let fraction_start = *pos; while *pos < bytes.len() && bytes[*pos].is_ascii_digit() { *pos += 1; } + if *pos == fraction_start { + return false; + } } if *pos < bytes.len() && (bytes[*pos] == b'e' || bytes[*pos] == b'E') { *pos += 1; if *pos < bytes.len() && (bytes[*pos] == b'+' || bytes[*pos] == b'-') { *pos += 1; } + let exponent_start = *pos; while *pos < bytes.len() && bytes[*pos].is_ascii_digit() { *pos += 1; } + if *pos == exponent_start { + return false; + } } + true } // Driver: expecting-value state. After emitting a value, the @@ -378,7 +409,9 @@ fn build_tape_into(bytes: &[u8], entries: &mut Vec, stack: &mut Vec { - skip_number(bytes, &mut pos); + if !skip_number(bytes, &mut pos) { + return false; + } entries.push(TapeEntry { offset: tok_off, kind: KIND_NUMBER, @@ -453,9 +486,10 @@ fn build_tape_into(bytes: &[u8], entries: &mut Vec, stack: &mut Vec), + Object { + keys: Vec<*mut crate::StringHeader>, + values: Vec, + }, +} + +impl BuildFrame { + fn push_value(&mut self, value: JSValue) -> bool { + match self { + Self::Array(values) => { + values.push(value); + true + } + Self::Object { keys, values } if values.len() < keys.len() => { + values.push(value); + true + } + Self::Object { .. } => false, + } + } +} + +unsafe fn finish_frame(frame: BuildFrame) -> Option { + match frame { + BuildFrame::Array(values) => { + let capacity = u32::try_from(values.len()).ok()?; + let mut array = crate::array::js_array_alloc(capacity); + for value in values { + array = crate::array::js_array_push(array, value); + } + Some(JSValue::object_ptr(array as *mut u8)) + } + BuildFrame::Object { keys, values } => { + if keys.len() != values.len() { + return None; + } + let object = crate::object::js_object_alloc(0, 0); + for (key, value) in keys.into_iter().zip(values) { + crate::object::js_object_set_field_by_name( + object, + key, + f64::from_bits(value.bits()), + ); + } + Some(JSValue::object_ptr(object as *mut u8)) + } + } +} + +fn attach_value(frames: &mut [BuildFrame], root: &mut Option, value: JSValue) -> bool { + if let Some(parent) = frames.last_mut() { + parent.push_value(value) + } else if root.is_none() { + *root = Some(value); + true + } else { + false + } +} + +/// Materialize a validated tape without consuming one native stack frame per +/// JSON container. Runtime GC must be suppressed by the caller: partially +/// built values live in this function's heap-backed work stack until their +/// parent container is complete. +pub(crate) unsafe fn materialize_iterative(tape: &[TapeEntry], bytes: &[u8]) -> Option { + let source = TapeSource::Borrowed { tape, bytes }; + let mut frames = Vec::new(); + let mut root = None; + + for entry in tape.iter().copied() { + match entry.kind { + KIND_OBJ_START => frames.push(BuildFrame::Object { + keys: Vec::new(), + values: Vec::new(), + }), + KIND_ARR_START => frames.push(BuildFrame::Array(Vec::new())), + KIND_KEY => { + let Some(BuildFrame::Object { keys, values }) = frames.last_mut() else { + return None; + }; + if keys.len() != values.len() { + return None; + } + let key = decode_key_to_interned_string(&source, entry.offset as usize); + if key.is_null() { + return None; + } + keys.push(key); + } + KIND_STRING => { + let value = materialize_string_value(&source, entry.offset as usize); + if !attach_value(&mut frames, &mut root, value) { + return None; + } + } + KIND_NUMBER => { + let value = materialize_number(&source, entry.offset as usize); + if !attach_value(&mut frames, &mut root, value) { + return None; + } + } + KIND_TRUE | KIND_FALSE | KIND_NULL => { + let value = match entry.kind { + KIND_TRUE => JSValue::bool(true), + KIND_FALSE => JSValue::bool(false), + _ => JSValue::null(), + }; + if !attach_value(&mut frames, &mut root, value) { + return None; + } + } + KIND_OBJ_END | KIND_ARR_END => { + let frame = frames.pop()?; + if !matches!( + (&frame, entry.kind), + (BuildFrame::Object { .. }, KIND_OBJ_END) + | (BuildFrame::Array(_), KIND_ARR_END) + ) { + return None; + } + let value = finish_frame(frame)?; + if !attach_value(&mut frames, &mut root, value) { + return None; + } + } + _ => return None, + } + } + + if frames.is_empty() { + root + } else { + None + } +} diff --git a/crates/perry-runtime/src/json_tape_tests.rs b/crates/perry-runtime/src/json_tape_tests.rs index 67bf149c90..833a4c4c14 100644 --- a/crates/perry-runtime/src/json_tape_tests.rs +++ b/crates/perry-runtime/src/json_tape_tests.rs @@ -97,6 +97,12 @@ fn tape_malformed_returns_none() { assert!(build_tape(b"[").is_none(), "unclosed array"); assert!(build_tape(b"{a:1}").is_none(), "unquoted key"); assert!(build_tape(b"{\"a\"}").is_none(), "missing colon"); + assert!(build_tape(b"0 trailing").is_none(), "trailing token"); + assert!(build_tape(b"01").is_none(), "leading zero"); + assert!(build_tape(b"1.").is_none(), "empty fraction"); + assert!(build_tape(b"1e+").is_none(), "empty exponent"); + assert!(build_tape(br#""\q""#).is_none(), "invalid escape"); + assert!(build_tape(b"\"line\nfeed\"").is_none(), "raw control byte"); assert!(build_tape(b"").is_none(), "empty input"); } @@ -109,6 +115,27 @@ fn tape_top_level_scalars() { assert_eq!(build_tape(b"null").unwrap().entries.len(), 1); } +#[test] +fn iterative_materializer_preserves_nested_objects_arrays_and_duplicate_keys() { + let input = br#"{"a":[1,true,"x"],"a":{"b":2}}"#; + let tape = build_tape(input).expect("valid tape"); + let saved_roots = crate::json::parse_root_save_len(); + crate::gc::gc_suppress(); + let value = unsafe { materialize_iterative(&tape.entries, input) }.expect("materializes"); + crate::json::parse_root_push(value); + crate::gc::gc_unsuppress(); + + let object = (value.bits() & crate::value::POINTER_MASK) as *const crate::ObjectHeader; + let key_a = crate::string::js_string_from_bytes(b"a".as_ptr(), 1); + let key_b = crate::string::js_string_from_bytes(b"b".as_ptr(), 1); + let nested = crate::object::js_object_get_field_by_name(object, key_a); + let nested = (nested.bits() & crate::value::POINTER_MASK) as *const crate::ObjectHeader; + let b = crate::object::js_object_get_field_by_name(nested, key_b); + assert_eq!(f64::from_bits(b.bits()), 2.0); + + crate::json::parse_root_restore(saved_roots); +} + /// `TapeEntry` is 12 bytes (u32 + u8 + padding + u32). Keeping /// this compact matters for tape-size parity with parse output: /// a 1 MB JSON blob with ~20k tokens should build a ~240 KB tape,