cloudwego / cloudwego/sonic-rs

`sonic_rs::Value` parsing bypasses the recursion depth limit fixed in #213

Open
#239 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**

`deserialize_newtype_struct` routes `sonic_rs::Value` targets to `deserialize_value` -> `Value::parse_with_padding`, which recurses through `parser.rs` directly bypassing the `with_depth_limit` guard #213 added to the generic serde `Deserializer` path. `parser.rs` has no depth tracking on `main`. The regression test added by #213 already flags this, in a comment:

```rust
// Use serde_json::Value so we go through the recursive path
// (sonic_rs::Value may use a fast path when index==0).
```

Same root cause as #232 (unguarded `parser.rs` recursion), different call site: plain `from_str` `from_slice::` — arguably the most common entry point in the crate, and still unprotected.

**To Reproduce**

```rust
let depth = 100_000;
let src = format!("{}{}", "[".repeat(depth), "]".repeat(depth));
let _ = sonic_rs::from_str::(&src); // aborts, not Err
```

Run in a thread with a small stack (e.g. 1 MiB) to reproduce at low depth; also aborts on default stacks at larger depth.

**Expected behavior**

`RecursionLimitExceeded`, matching the guarantee #213 already gives typed/`serde_json::Value` deserialization.

**sonic-rs version:** 0.5.8 (also current `main` — `parser.rs` has no depth guard)

**Environment:** aarch64-apple-darwin, debug and release.

**Additional context**

Suggest fixing at the `parser.rs` level (`parse_array`/`parse_object`) so every caller — `Value`, `get_many` (#232), typed deserialize — shares one guard instead of a guard per call site.

Related: #208, #232, #213

Contributor guide

Open the contributing guide

Research direction

Start in parser.rs at parse_array and parse_object, then read the recursion-depth guard added for the generic serde Deserializer in #213. Reproduce the deeply nested sonic_rs::Value case from the issue and run the existing regression test; done means it returns RecursionLimitExceeded instead of aborting, while callers such as from_str and get_many share the protection.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.