Skip to content
Merged
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
5 changes: 5 additions & 0 deletions .changeset/text-import-out-of-bounds.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"loro-crdt": patch
---

Reject imported Text ops whose positions lie past the end of the text. An insert, delete or mark past the end used to panic inside the document (a `RuntimeError: unreachable` trap in WASM, leaving the doc unusable). `import`, `importBatch` and `importJsonUpdates` now throw a decode error and leave the document unchanged and usable, like out-of-bounds List and MovableList ops (#1160).
13 changes: 11 additions & 2 deletions context/movable-list-op-validation.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Movable List `Move`/`Set` Validation on Import

Verified against code 2026-09-28 (merged with main `6e294c87`).
Verified against code 2026-09-28 (merged with main `6e294c87`). Text bounds and the rollback-scope list verified 2026-09-30.

Imported movable-list `Move { from, to, elem_id }` and `Set { elem_id, value }` ops
come from other peers, so they are external input. Several shapes of them used
Expand Down Expand Up @@ -75,12 +75,21 @@ Tests:
- It checks the composed delta, so an out-of-bounds move that a later op in the
same import cancels (e.g. `move to: 9` followed by `delete pos: 9`) is accepted.
Every path gives the same result for it.
- Text has the same check: `RichtextState::validate_diff` bounds-checks the text
delta in entity-index space (style anchors count). An insert, delete or mark
past the end of the text used to panic in `insert_elem_at_entity_index` with
the locks held (loro-dev/loro#1160).

### Why every such import gets a rollback scope

`ImportChangesPreflight` (`OpLog::preflight_import_changes`) and
`PendingChanges::has_state_apply_rollback_ops` set `needs_state_apply_rollback`
for List, MovableList and Tree ops.
for List, MovableList, Text and Tree ops (`oplog::state_apply_can_reject`). An
import without that scope panics if `validate_diff` rejects its diff, so a
container type whose `validate_diff` can fail must be listed there.
- The scope costs a few small clones and journal entries per import: about
+0.3 µs per single-op Text import (loro-dev/loro#1160 measured 20k imports at
~44 → ~50 ms). Large imports are not measurably slower.
- The preflight inspects the ops of **every** new change, including ones whose
deps are not in the DAG yet. Those deps may be earlier changes of the same import,
which then unlock them during the import.
Expand Down
26 changes: 19 additions & 7 deletions crates/loro-internal/src/oplog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,19 @@ pub(crate) struct ReplayBase {
pub concurrent_containers: Option<FxHashSet<ContainerIdx>>,
}

/// Whether an imported op on a container of type `ty` can be rejected by
/// `ContainerState::validate_diff` (e.g. a sequence position past the end), so
/// the import needs a rollback scope to undo the op log on that error.
pub(crate) fn state_apply_can_reject(ty: ContainerType) -> bool {
matches!(
ty,
ContainerType::List
| ContainerType::MovableList
| ContainerType::Text
| ContainerType::Tree
)
}

impl OpLog {
#[inline]
pub(crate) fn new(visible_op_count: Arc<AtomicUsize>) -> Self {
Expand Down Expand Up @@ -365,12 +378,11 @@ impl OpLog {

// Inspect the ops even when the deps are not in the DAG yet: they may be
// earlier changes of this same import, which then unlock this one.
if change.ops.iter().any(|op| {
matches!(
op.container.get_type(),
ContainerType::List | ContainerType::MovableList | ContainerType::Tree
)
}) {
if change
.ops
.iter()
.any(|op| state_apply_can_reject(op.container.get_type()))
{
ans.needs_state_apply_rollback = true;
}

Expand All @@ -385,7 +397,7 @@ impl OpLog {

// Any newly applied change can unlock pending changes whose ops are not
// visible in `changes`, so include pending in the rollback decision.
// Keep this narrow: text/map-only pending changes cannot return a
// Keep this narrow: map-only pending changes cannot return a
// state-apply error, and forcing rollback there adds lock traffic to
// small sync/import workloads.
//
Expand Down
14 changes: 5 additions & 9 deletions crates/loro-internal/src/oplog/pending_changes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ use crate::{
version::{ImVersionVector, VersionRange},
OpLog, VersionVector,
};
use loro_common::{
ContainerType, Counter, CounterSpan, HasCounterSpan, HasIdSpan, IdSpan, PeerID, ID,
};
use loro_common::{Counter, CounterSpan, HasCounterSpan, HasIdSpan, IdSpan, PeerID, ID};
use rustc_hash::FxHashMap;

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -43,12 +41,10 @@ impl PendingChanges {
self.changes.values().any(|tree| {
tree.values().any(|changes| {
changes.iter().any(|change| {
change.ops.iter().any(|op| {
matches!(
op.container.get_type(),
ContainerType::List | ContainerType::MovableList | ContainerType::Tree
)
})
change
.ops
.iter()
.any(|op| super::state_apply_can_reject(op.container.get_type()))
})
})
})
Expand Down
2 changes: 1 addition & 1 deletion crates/loro-internal/src/state/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ before changing mergeable child behavior.
[../../../../context/shallow-snapshot-style-redaction.md](../../../../context/shallow-snapshot-style-redaction.md)
before changing the richtext snapshot codec or that pass.
`movable_list_state.rs` import validation and the Checkout-mode fallback for
moves of deleted elements:
moves of deleted elements, plus the List/Text `validate_diff` bounds checks:
[../../../../context/movable-list-op-validation.md](../../../../context/movable-list-op-validation.md).
- `mergeable.rs`: logical child edge resolution for mergeable containers.
- `dead_containers_cache.rs`: dead/alive tracking. Map/list deletions are
Expand Down
40 changes: 40 additions & 0 deletions crates/loro-internal/src/state/richtext_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,46 @@ impl ContainerState for RichtextState {
Diff::Text(ans)
}

fn validate_diff(&self, diff: &InternalDiff) -> LoroResult<()> {
let InternalDiff::RichtextRaw(delta) = diff else {
unreachable!()
};

// Text items are addressed by entity index (style anchors included).
let mut cursor = 0usize;
let mut projected = match &self.state {
LazyLoad::Src(s) => s.entity_index,
LazyLoad::Dst(s) => s.len_entity(),
};
for span in delta.iter() {
match span {
loro_delta::DeltaItem::Retain { len, .. } => {
cursor += len;
if cursor > projected {
return Err(LoroError::DecodeError(
format!(
"text diff retains {cursor} items but state only has {projected}"
)
.into_boxed_str(),
));
}
}
loro_delta::DeltaItem::Replace { value, delete, .. } => {
if cursor + delete > projected {
return Err(LoroError::DecodeError(
format!("text diff deletes {delete} at {cursor} but state only has {projected}")
.into_boxed_str(),
));
}
projected = projected - delete + value.rle_len();
cursor += value.rle_len();
}
}
}

Ok(())
}

fn apply_diff(&mut self, diff: InternalDiff, _ctx: DiffApplyContext) -> LoroResult<()> {
self.update_version();
let InternalDiff::RichtextRaw(richtext) = diff else {
Expand Down
99 changes: 99 additions & 0 deletions crates/loro-wasm/tests/text_invalid_ops.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { describe, expect, it } from "vitest";
import { LoroDoc } from "../bundler/index";

// A Text op past the end of the text used to panic inside the doc locks, which
// traps the WASM instance ("RuntimeError: unreachable") (loro-dev/loro#1160). It
// must throw a readable error and leave the doc unchanged and usable.

/** Peer 1 writes "ab" (counters/lamports 0..=1). */
function base(): LoroDoc {
const doc = new LoroDoc();
doc.setPeerId(1);
doc.getText("t").insert(0, "ab");
doc.commit();
return doc;
}

function forged(ops: unknown[]): string {
return JSON.stringify({
schema_version: 1,
start_version: {},
peers: ["1"],
changes: [
{
id: "2@0",
timestamp: 0,
deps: ["1@0"],
lamport: 2,
msg: null,
ops: ops.map((content, i) => ({
container: "cid:root-t:Text",
content,
counter: 2 + i,
})),
},
],
});
}

function expectRejected(doc: LoroDoc, json: string) {
const value = doc.toJSON();
const delta = doc.getText("t").toDelta();
const vv = doc.oplogVersion().toJSON();
let error: unknown;
try {
doc.importJsonUpdates(json);
} catch (e) {
error = e;
}
expect(error).toBeDefined();
expect(String(error)).toMatch(/Decode error/);
expect(String(error)).not.toMatch(/unreachable/);
expect(doc.toJSON()).toEqual(value);
expect(doc.getText("t").toDelta()).toEqual(delta);
expect(doc.oplogVersion().toJSON()).toEqual(vv);

// Still usable.
const text = doc.getText("t");
text.insert(text.length, "!");
doc.commit();
const remote = new LoroDoc();
remote.import(doc.export({ mode: "snapshot" }));
remote.getText("t").insert(0, "r");
remote.commit();
doc.import(remote.export({ mode: "update", from: doc.oplogVersion() }));
expect(doc.toJSON()).toEqual(remote.toJSON());
}

describe("out-of-bounds text ops", () => {
it("rejects an insert past the end of the text", () => {
expectRejected(base(), forged([{ type: "insert", pos: 3, text: "x" }]));
});

it("rejects deletes and marks past the end of the text", () => {
expectRejected(
base(),
forged([{ type: "delete", pos: 1, len: 2, start_id: "1@0" }]),
);
expectRejected(
base(),
forged([
{
type: "mark",
start: 1,
end: 9,
style_key: "bold",
style_value: true,
info: 0,
},
{ type: "mark_end" },
]),
);
});

it("still imports an insert at the end of the text", () => {
const doc = base();
doc.importJsonUpdates(forged([{ type: "insert", pos: 2, text: "x" }]));
expect(doc.getText("t").toString()).toBe("abx");
});
});
Loading
Loading