diff --git a/benches/scripts/json/stringify_circular.js b/benches/scripts/json/stringify_circular.js new file mode 100644 index 00000000000..3c986764284 --- /dev/null +++ b/benches/scripts/json/stringify_circular.js @@ -0,0 +1,24 @@ +// Creates a circular structure at depth to measure efficient cycle detection. +// Catches the error to allow the benchmark to run repeatedly. + +function createCircularObject(depth) { + let root = {}; + let cur = root; + for (let i = 0; i < depth; i++) { + cur.next = {}; + cur = cur.next; + } + cur.next = root; // Create the cycle back to the root + return root; +} + +const circularObj = createCircularObject(100); + +function main() { + try { + JSON.stringify(circularObj); + } catch (_) { + // Expected TypeError: cyclic object value + return; + } +} diff --git a/benches/scripts/json/stringify_deep.js b/benches/scripts/json/stringify_deep.js new file mode 100644 index 00000000000..a598911bf0d --- /dev/null +++ b/benches/scripts/json/stringify_deep.js @@ -0,0 +1,19 @@ +// Creates a deep acyclic object to measure linear vs O(1) cycle detection cost. +// Depth of 400 is large enough to show the benefit while avoiding stack overflow. + +function createDeepObject(depth) { + let root = {}; + let cur = root; + for (let i = 0; i < depth; i++) { + cur.value = i; + cur.next = {}; + cur = cur.next; + } + return root; +} + +const deepObj = createDeepObject(400); + +function main() { + return JSON.stringify(deepObj); +} diff --git a/core/engine/src/builtins/json/mod.rs b/core/engine/src/builtins/json/mod.rs index 7b4d9aafed3..cc99f2fc42b 100644 --- a/core/engine/src/builtins/json/mod.rs +++ b/core/engine/src/builtins/json/mod.rs @@ -15,6 +15,8 @@ use std::{borrow::Cow, iter::once}; +use rustc_hash::FxHashSet; + use boa_ast::scope::Scope; use boa_gc::{Finalize, Gc, Trace}; use boa_macros::utf16; @@ -623,13 +625,10 @@ impl Json { args: &[JsValue], context: &mut Context, ) -> JsResult { - // 1. Let stack be a new empty List. - let stack = Vec::new(); - - // 2. Let indent be the empty String. + // 1. Let indent be the empty String. let indent = js_string!(); - // 3. Let PropertyList and ReplacerFunction be undefined. + // 2. Let PropertyList and ReplacerFunction be undefined. let mut property_list = None; let mut replacer_function = None; @@ -747,7 +746,7 @@ impl Json { // 11. Let state be the Record { [[ReplacerFunction]]: ReplacerFunction, [[Stack]]: stack, [[Indent]]: indent, [[Gap]]: gap, [[PropertyList]]: PropertyList }. let mut state = StateRecord { replacer_function, - stack, + stack_set: FxHashSet::default(), indent, gap, property_list, @@ -965,15 +964,12 @@ impl Json { context: &mut Context, ) -> JsResult { // 1. If state.[[Stack]] contains value, throw a TypeError exception because the structure is cyclical. - if state.stack.contains(value) { + if !state.stack_set.insert(value.clone()) { return Err(JsNativeError::typ() .with_message("cyclic object value") .into()); } - // 2. Append value to state.[[Stack]]. - state.stack.push(value.clone()); - // 3. Let stepback be state.[[Indent]]. let stepback = state.indent.clone(); @@ -987,14 +983,32 @@ impl Json { // 6. Else, } else { // a. Let K be ? EnumerableOwnPropertyNames(value, key). - let keys = value.enumerable_own_property_names(PropertyNameKind::Key, context)?; + let keys = value.enumerable_own_property_names(PropertyNameKind::Key, context); + // 11. Remove value from state.[[Stack]] on error. + let keys = match keys { + Ok(k) => k, + Err(e) => { + let removed = state.stack_set.remove(value); + debug_assert!(removed); + return Err(e); + } + }; // Unwrap is safe, because EnumerableOwnPropertyNames with kind "key" only returns string values. - keys.iter() + match keys + .iter() .map(|v| { v.to_string(context) .js_expect("EnumerableOwnPropertyNames only returns strings") }) - .collect::, _>>()? + .collect::, _>>() + { + Ok(k) => k, + Err(e) => { + let removed = state.stack_set.remove(value); + debug_assert!(removed); + return Err(e.into()); + } + } }; // 7. Let partial be a new empty List. @@ -1003,7 +1017,15 @@ impl Json { // 8. For each element P of K, do for p in &k { // a. Let strP be ? SerializeJSONProperty(state, P, value). - let str_p = Self::serialize_json_property(state, p.clone(), value, context)?; + let str_p = Self::serialize_json_property(state, p.clone(), value, context); + let str_p = match str_p { + Ok(v) => v, + Err(e) => { + let removed = state.stack_set.remove(value); + debug_assert!(removed); + return Err(e); + } + }; // b. If strP is not undefined, then if let Some(str_p) = str_p { @@ -1078,12 +1100,13 @@ impl Json { } }; - // 11. Remove the last element of state.[[Stack]]. - state.stack.pop(); - // 12. Set state.[[Indent]] to stepback. state.indent = stepback; + // 11. Remove value from state.[[Stack]]. + let removed = state.stack_set.remove(value); + debug_assert!(removed); + // 13. Return final. Ok(r#final) } @@ -1100,15 +1123,12 @@ impl Json { context: &mut Context, ) -> JsResult { // 1. If state.[[Stack]] contains value, throw a TypeError exception because the structure is cyclical. - if state.stack.contains(value) { + if !state.stack_set.insert(value.clone()) { return Err(JsNativeError::typ() .with_message("cyclic object value") .into()); } - // 2. Append value to state.[[Stack]]. - state.stack.push(value.clone()); - // 3. Let stepback be state.[[Indent]]. let stepback = state.indent.clone(); @@ -1116,7 +1136,14 @@ impl Json { state.indent = js_string!(&state.indent, &state.gap); // 6. Let len be ? LengthOfArrayLike(value). - let len = value.length_of_array_like(context)?; + let len = match value.length_of_array_like(context) { + Ok(n) => n, + Err(e) => { + let removed = state.stack_set.remove(value); + debug_assert!(removed); + return Err(e); + } + }; // 5. Let partial be a new empty List. let mut partial = Vec::with_capacity(len as usize); @@ -1127,7 +1154,15 @@ impl Json { // 8. Repeat, while index < len, while index < len { // a. Let strP be ? SerializeJSONProperty(state, ! ToString(𝔽(index)), value). - let str_p = Self::serialize_json_property(state, index.into(), value, context)?; + let str_p = Self::serialize_json_property(state, index.into(), value, context); + let str_p = match str_p { + Ok(v) => v, + Err(e) => { + let removed = state.stack_set.remove(value); + debug_assert!(removed); + return Err(e); + } + }; // b. If strP is undefined, then if let Some(str_p) = str_p { @@ -1192,12 +1227,13 @@ impl Json { } }; - // 11. Remove the last element of state.[[Stack]]. - state.stack.pop(); - // 12. Set state.[[Indent]] to stepback. state.indent = stepback; + // 11. Remove value from state.[[Stack]]. + let removed = state.stack_set.remove(value); + debug_assert!(removed); + // 13. Return final. Ok(r#final) } @@ -1205,7 +1241,7 @@ impl Json { struct StateRecord { replacer_function: Option, - stack: Vec, + stack_set: FxHashSet, indent: JsString, gap: JsString, property_list: Option>, diff --git a/core/engine/src/builtins/json/tests.rs b/core/engine/src/builtins/json/tests.rs index af3d1f6a8a8..6c58066c1c8 100644 --- a/core/engine/src/builtins/json/tests.rs +++ b/core/engine/src/builtins/json/tests.rs @@ -315,3 +315,30 @@ fn json_parse_with_no_args_throws_syntax_error() { "expected value at line 1 column 1", )]); } + +#[test] +fn json_stringify_cyclic_object_throws_type_error() { + run_test_actions([TestAction::assert_native_error( + "var a = {}; a.a = a; JSON.stringify(a);", + JsNativeErrorKind::Type, + "cyclic object value", + )]); +} + +#[test] +fn json_stringify_cyclic_array_throws_type_error() { + run_test_actions([TestAction::assert_native_error( + "var a = []; a[0] = a; JSON.stringify(a);", + JsNativeErrorKind::Type, + "cyclic object value", + )]); +} + +#[test] +fn json_stringify_cyclic_nested_object_throws_type_error() { + run_test_actions([TestAction::assert_native_error( + "var a = {}; var b = { a }; a.b = b; JSON.stringify(a);", + JsNativeErrorKind::Type, + "cyclic object value", + )]); +}