Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion crates/monty/src/modules/json/dump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::{
exception_private::{ExcType, RunResult},
heap::{DropWithHeap, HeapData, HeapGuard, HeapId, HeapReadOutput},
intern::StaticStrings,
resource::ResourceTracker,
resource::{ResourceError, ResourceTracker},
sorting::{apply_permutation, sort_indices},
types::{PyTrait, long_int::check_bigint_str_digits_limit, str::allocate_string},
value::Value,
Expand Down Expand Up @@ -367,6 +367,15 @@ fn json_separator_to_string(value: &Value, role: &str, vm: &VM<'_, impl Resource
}
}

/// Maximum nesting depth accepted by `json.dumps()`.
///
/// Mirrors `JSON_RECURSION_LIMIT` on the `json.loads()` side: serialization is
/// mutually recursive across `serialize_value`/`serialize_sequence`/`serialize_dict`
/// and would otherwise overflow the host's native stack on a deep (acyclic)
/// structure that the cycle detector cannot catch. CPython raises a catchable
/// `RecursionError` here; we map the depth-limit `ResourceError` to the same.
const JSON_DUMP_RECURSION_LIMIT: usize = 200;
Comment on lines +370 to +377

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this use a constant rather than respect the ResourceLimit recursion limit?

Separately, it seems like CPython does not, a 20000-deep nested tuple will serialize fine even if sys.setrecursionlimit is set very low.

Seems like CPython uses stack overflow protections for these cases, if I write a million element tuple I eventually get

RecursionError: Stack overflow (used 8144 kB) while encoding a JSON object

... maybe #440?


/// Serializes a Monty value into JSON text.
///
/// The function handles immediate primitives directly and delegates to
Expand All @@ -379,6 +388,13 @@ fn serialize_value(
active_containers: &mut Vec<HeapId>,
vm: &mut VM<'_, impl ResourceTracker>,
) -> RunResult<()> {
if depth > JSON_DUMP_RECURSION_LIMIT {
return Err(ResourceError::Recursion {
limit: JSON_DUMP_RECURSION_LIMIT,
depth,
}
.into());
}
match value {
Value::None => {
out.push_str("null");
Expand Down
45 changes: 45 additions & 0 deletions crates/monty/tests/resource_limits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2039,3 +2039,48 @@ len(x) + len(d) + len(s)
);
assert_eq!(result.unwrap(), MontyObject::Int(300));
}

// === json.dumps resource-limit tests ===

/// Test that serializing a deeply-nested (acyclic) structure raises a catchable
/// `RecursionError` rather than overflowing the host's native stack.
///
/// `json.dumps` is mutually recursive across the value/sequence/dict serializers
/// and runs entirely inside one bytecode instruction, so the per-instruction
/// recursion-depth check never fires. The serializer must enforce its own depth
/// ceiling, mirroring the one applied by `json.loads`.
#[test]
fn json_dumps_deep_nesting_recursion_limit() {
let code = r"
import json
x = []
for _ in range(500):
x = [x]
try:
json.dumps(x)
out = 'no error'
except RecursionError:
out = 'recursion error'
out
";
let ex = MontyRun::new(code.to_owned(), "test.py", vec![]).unwrap();
let result = ex.run_no_limits(vec![]);
assert!(result.is_ok(), "{result:?}");
assert_eq!(result.unwrap(), MontyObject::String("recursion error".to_owned()));
}

/// Test that moderate nesting in `json.dumps` succeeds.
#[test]
fn json_dumps_moderate_nesting_within_limit() {
let code = r"
import json
x = []
for _ in range(50):
x = [x]
json.dumps(x)[:5]
";
let ex = MontyRun::new(code.to_owned(), "test.py", vec![]).unwrap();
let result = ex.run_no_limits(vec![]);
assert!(result.is_ok(), "{result:?}");
assert_eq!(result.unwrap(), MontyObject::String("[[[[[".to_owned()));
}
Loading