Repository navigation
fix: keep untrusted deep nesting from aborting the process - #1249
Merged
Merged
Conversation
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>
Contributor
WASM Size Report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 withcatch_unwind. This PR removes the remaining per-level recursion on those paths:LoroValueequality is iterative (values_eq, withArc::ptr_eqshort-circuit). Adjacent rich-text spans with equal, separately allocated deep marks overflowed into_delta().NonZeroU32instead ofNonZeroU16. At 65,535 levels the+ 1wrapped toNone(and panicked in debug), soContainerWrapper::newpanicked during import.ArenaContainers::get_depthnow walks the chain with a loop.get_container_deep_value{,_with_id}) go through the new iterativestate/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.encode_map/encode_list), byte-identical to the old serde output.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, andensure_mergeable_*/apply_diffremap returnArgErr. This caps nesting at about 1000 levels.Rationale is in
context/deep-value-stack-safety.mdandcontext/mergeable-containers.md.Validation
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 inarena.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.encode_map/encode_list, and a loro-common test that marker activation agrees withis_mergeableat the budget boundary.cargo clippy --all-features -- -Dwarningscurrently 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
Invalid compression type. This is a separate KV-store limit.insert_containeris still quadratic. Importing the same chain is fast.🤖 Generated with Claude Code