cloudwego / cloudwego/sonic-rs

get_many can cause a process-terminating stack overflow on deeply nested JSON

Open
#232 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
920
Forks
68
Avg merge
2h 5m
Merged PRs (30d)
1

Description

**Describe the bug**

Checked `get_many` can overflow the thread stack when processing a deeply nested but otherwise valid JSON payload.

This can happen even when the requested path is shallow: while searching for a requested field, `get_many` recursively validates and skips preceding, unrelated JSON values. A single deeply nested value can therefore abort the entire Rust process instead of returning a recoverable parsing error.

**To Reproduce**

```rust
use sonic_rs::{get_many, PointerTree};

fn main() {
std::thread::Builder::new()
.name("get_many_repro".into())
.stack_size(1024 * 1024)
.spawn(|| {
let depth = 10_000;
let nested = format!("{}0{}", "[".repeat(depth), "]".repeat(depth));
let json = format!(r#"{{"ignored":{nested},"wanted":1}}"#);

let mut tree = PointerTree::new();
tree.add_path(["wanted"]);

println!("{:?}", get_many(&json, &tree));
})
.unwrap()
.join()
.unwrap();
}
```

A runnable reproduction is also available here:

https://github.com/starcatmeow/sonic-rs/blob/rayli/repro-get-many-stack-overflow/examples/repro_get_many_stack_overflow.rs

Run:

```bash
git clone \
--branch rayli/repro-get-many-stack-overflow \
--single-branch \
https://github.com/starcatmeow/sonic-rs.git \
sonic-rs-get-many-repro

cd sonic-rs-get-many-repro
cargo run --release --example repro_get_many_stack_overflow
```

Observed output:

```text
thread 'get_many_repro' has overflowed its stack
fatal runtime error: stack overflow, aborting
```

The debug build also reproduces the issue:

```bash
cargo run --example repro_get_many_stack_overflow
```

**Expected behavior**

`get_many` should return a parsing or recursion-limit error before exhausting the stack.

A deeply nested, unrelated value should not be able to terminate a process that is only attempting to extract a shallow field.

**sonic-rs version:**

`0.5.9` / current `main`.

**Environment:**

- `aarch64-apple-darwin`
- `rustc 1.96.1`
- Reproduced in both debug and release builds with a 1 MiB worker-thread stack.

**Additional context**

This is particularly important for services processing externally supplied events, since a Rust stack overflow aborts the process and cannot be handled like a normal parsing error.

There appear to be multiple recursive paths involved:

- `get_many_rec` recursively traverses matching `PointerTree` paths.
- Checked `get_many` recursively skips and validates selected or unmatched values.
- Container type-mismatch validation can also recursively skip the actual value.

Only limiting the `PointerTree` traversal would therefore not cover the shallow-path reproduction above.

A similar stack-overflow issue was previously reported for deserialization in #208 and addressed in #213.

I would be happy to prepare a PR to fix this. Before doing so, I would like to understand which approach maintainers would prefer:

1. Add a fixed maximum depth for `get_many`, similar to the existing deserializer recursion limit (for example, 255 levels), and return `RecursionLimitExceeded` once reached.
2. Allow callers to configure the maximum depth explicitly.
3. Provide an optional feature or opt-out mechanism for callers that need to process deeper inputs and can ensure sufficient stack space.

Any guidance on the preferred default and API/feature direction would be appreciated.

Contributor guide

Open the contributing guide

Research direction

Start by running the linked repro example with both debug and release builds. Read get_many and get_many_rec, then trace the checked recursive skipping, validation, and PointerTree traversal paths described in the issue. Done means deeply nested unrelated JSON produces a recoverable parsing or recursion-limit error instead of terminating the process.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.