Skip to content

fix: keep untrusted deep nesting from aborting the process - #1249

Merged
lodyai[bot] merged 3 commits into
mainfrom
fix/untrusted-import-stack-depth
Nov 3, 2026
Merged

lodyai[bot] merged 3 commits into
mainfrom
fix/untrusted-import-stack-depth

Conversation

@zxch3n

@zxch3n zxch3n commented Nov 3, 2026

Copy link
Copy Markdown
Member

Summary

Small untrusted updates or snapshots could abort the reading process (stack overflow, or a panic under the state lock followed by panic in a destructor during cleanup). None of these can be caught with catch_unwind. This PR removes the remaining per-level recursion on those paths:

  • LoroValue equality is iterative (values_eq, with Arc::ptr_eq short-circuit). Adjacent rich-text spans with equal, separately allocated deep marks overflowed in to_delta().
  • Container depth is a NonZeroU32 instead of NonZeroU16. At 65,535 levels the + 1 wrapped to None (and panicked in debug), so ContainerWrapper::new panicked during import. ArenaContainers::get_depth now walks the chain with a loop.
  • Deep-value reads (get_container_deep_value{,_with_id}) go through the new iterative state/deep_value.rs. Import is not capped, because a fast snapshot loads lazily and import never walks its chain, so readers must be depth-safe themselves.
  • Map/List/MovableList snapshot export writes values with the existing iterative postcard writer (encode_map/encode_list), byte-identical to the old serde output.
  • Mergeable container names: ancestor registration is a loop, and names whose mergeable ancestors' names exceed 1 MiB in total (MAX_MERGEABLE_ANCESTOR_NAME_BYTES) are no longer mergeable. Registration is quadratic in the name, so a 40 KB update took 5 s and 400 MB. Markers for such children stay inert, and ensure_mergeable_* / apply_diff remap return ArgErr. This caps nesting at about 1000 levels.

Rationale is in context/deep-value-stack-safety.md and context/mergeable-containers.md.

Validation

  • New crates/loro/tests/untrusted_depth.rs (6 tests, 1 MiB stack). On the unfixed base, all 6 fail: 4 stack overflows, 1 u16 overflow panic in arena.rs, and 1 assertion failure. With the fix, all 6 pass.
  • cargo test --features=test_utils,jsonpath -p loro -p loro-internal -p loro-common: 1673 passed, 0 failed.
  • Byte-layout tests for encode_map/encode_list, and a loro-common test that marker activation agrees with is_mergeable at the budget boundary.
  • cargo clippy --all-features -- -Dwarnings currently fails on main because of 3 lints in untouched files (loro.rs:1848, utils/query_by_len.rs). This PR adds no new clippy warnings.

Not in this PR

  • Container IDs longer than ~64 KB (even a plain root name) produce snapshots that fail to import with Invalid compression type. This is a separate KV-store limit.
  • Building a long chain locally with insert_container is still quadratic. Importing the same chain is fast.

🤖 Generated with Claude Code

lody-ai and others added 3 commits November 3, 2026 15:41
Remove per-level recursion reachable from untrusted updates and snapshots:
LoroValue equality, arena container depth (now NonZeroU32, iterative), deep
value reads, Map/List/MovableList snapshot value writers, and mergeable
ancestor registration. Bound mergeable names by their summed ancestor name
bytes (1 MiB), since registration is quadratic in the name.

Rationale: context/deep-value-stack-safety.md, context/mergeable-containers.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#1244 changed try_travel_ancestors callbacks to ControlFlow<(), bool>, while
#1222 added a caller that still returned Continue(()), so main stopped
compiling. Continue(true) keeps walking into the node's ancestors, as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

WASM Size Report

  • Original size: 3493.48 KB
  • Gzipped size: 1171.20 KB
  • Brotli size: 820.40 KB

@lodyai
lodyai Bot merged commit ea1d383 into main Nov 3, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants