diff --git a/.changeset/loro-js-movable-list-invalid-ops.md b/.changeset/loro-js-movable-list-invalid-ops.md new file mode 100644 index 000000000..4939585af --- /dev/null +++ b/.changeset/loro-js-movable-list-invalid-ops.md @@ -0,0 +1,5 @@ +--- +"loro.js": patch +--- + +Importing a MovableList move or set that names an unknown element, an element of another list, an element outside the op's causal history, or (on a shallow document) an element deleted before the shallow root now fails and rolls back the whole import, as in Rust (loro-dev/loro#1125). A move or set of a deleted element is applied like a concurrent one on every import path. diff --git a/context/loro-js-movable-list.md b/context/loro-js-movable-list.md index a367f7635..25a0b776b 100644 --- a/context/loro-js-movable-list.md +++ b/context/loro-js-movable-list.md @@ -8,8 +8,8 @@ reproduce, the loro.js model that implements them, and how the differential suite checks the two against each other. Complexity rules still come from [loro-js-performance.md](loro-js-performance.md). -Status: the model below and atomic imports are implemented. Import validation -("Validation and import atomicity") lands in a follow-up change. +Status: the model below, atomic imports, and import validation are +implemented. ## Rust semantics (the contract) @@ -61,9 +61,9 @@ A MovableList is two CRDTs glued together section 8) lists every alive position in order, dead ones included, plus element IDs and last-set IDs when they differ from the position/element ID. -Invalid `Move`/`Set` handling (Rust #1125, open when this was written) is in -[movable-list-op-validation.md](movable-list-op-validation.md) once merged; see -the "Validation" section below for the loro.js side. +Invalid `Move`/`Set` handling (Rust loro-dev/loro#1125) is in +[movable-list-op-validation.md](movable-list-op-validation.md); see the +"Validation" section below for the loro.js side. ## Why the old loro.js model diverged @@ -211,17 +211,46 @@ Rust's rule for `from_move` inserts that transforms leave behind ## Validation and import atomicity -Atomicity is implemented ([import-batch-atomicity.md](import-batch-atomicity.md), -"loro.js"). Validation is planned. It follows Rust #1125. A `Move`/`Set` whose element is unknown, -lives in another container, is outside the op's causal history, or (on a -shallow doc) was deleted before the root is rejected with an error. So is a -`from`/`to` outside the op-index range of its causal view. A `Move`/`Set` of a -deleted element is applied with the concurrent-op semantics above. -`import`/`importBatch` stage the document state (history indexes, containers, -pending changes, version, subscribers' events, undo stacks) and restore it -when any blob fails, so a rejected import is invisible -([import-batch-atomicity.md](import-batch-atomicity.md) states the Rust -contract). +Validation follows Rust loro-dev/loro#1125 (`validate_movable_list_elem_refs_since`; +[movable-list-op-validation.md](movable-list-op-validation.md)). +`#validateMovableListRefs` in `document.ts` runs on every imported change after +integration: FastUpdates, non-initializing snapshots, and `importBatch` +(except the snapshot that seeds an empty document, which Rust does not +validate op by op either). A `Move`/`Set` whose element is unknown, is not a +`movable-list-insert` into the same list, is outside the op's causal history, +or (on a shallow doc) is older than the root but missing from the root state is +rejected with an error. `MovableListState.applyMove` rejects a `from`/`to` +outside the op-index range of the op's causal view before mutating anything. A +`Move`/`Set` of a deleted element is valid and is applied with the +concurrent-op semantics above, the same on every import path. + +The element lookup maps `(peer, lamport)` to an op ID by binary search over +the peer's indexed changes (lamports grow with counters). While snapshot +history is deferred only the overlay is indexed; a miss there means the element +comes from the snapshot base, and it is accepted without decoding history when +the list's hydrated state holds it and the op has seen the whole base. +Otherwise history is materialized once. The shallow-root element set is built +once per root store and list. + +A rejection rolls back the whole import or batch. `import`/`importBatch` stage +the document state (history indexes, containers, pending changes, version, +subscribers' events, undo stacks) and restore it when any blob fails, so a +rejected import is invisible ([import-batch-atomicity.md](import-batch-atomicity.md), +"loro.js", states both contracts). Rust reports an out-of-range index as +"movable list diff retains N items but state only has M"; loro.js says "out of +range". `loro-js/tests/movable-list-invalid-ops.test.ts` ports +`crates/loro/tests/movable_list_invalid_ops.rs`; run against a WASM build of +`main` with #1125 merged, all of it but the loro.js-only undecodable-snapshot +test passes, error text aside. + +Rust rules loro.js does not copy (edge cases, documented rather than ported): +- Rust rejects any List/MovableList/Text op position at or past 1,073,741,822 + when it decodes ops (`InnerListOp::check_positions`), on every import path. + loro.js rejects such a move only when it applies it, so a detached import + records it until `attach` fails. +- Rust bounds-checks the composed diff of an import, so an out-of-bounds move + that a later op of the same import cancels is accepted. loro.js checks each + op when it applies it and rejects the import. ## Differential suite diff --git a/context/loro-js-performance.md b/context/loro-js-performance.md index 674c3b790..6b32b099c 100644 --- a/context/loro-js-performance.md +++ b/context/loro-js-performance.md @@ -533,7 +533,10 @@ The remaining differences are representation and JavaScript constant factors: concurrently with the snapshot, or names a MovableList element it lacks, rebuilds only that container from its own history (`#prepareSnapshotImport`); so does the first version transition that touches a hydrated MovableList. - Later imports and transitions are incremental again. + Later imports and transitions are incremental again. Import validation of + MovableList moves and sets costs O(log changes) per op and keeps deferred + snapshot history deferred when the target element is in the hydrated state + ([loro-js-movable-list.md](loro-js-movable-list.md), "Validation"). - The million-operation C1.1 concurrent-text trace still exposes a large constant-factor and retained-memory gap. Its local edit phase is about 4x the WASM adapter, and parsing the 6.5 MB snapshot takes 3.62 seconds versus 43 ms. diff --git a/loro-js/src/runtime/document.ts b/loro-js/src/runtime/document.ts index 16967e14e..15f09feab 100644 --- a/loro-js/src/runtime/document.ts +++ b/loro-js/src/runtime/document.ts @@ -720,6 +720,7 @@ export class LoroDoc = Record = Record = Record records)); + const seedChanges = new Set( + snapshotSeed?.records.map(({ change }) => changeKey(change.id)) ?? [], + ); + this.#validateMovableListRefs( + integration.added.filter(({ change }) => !seedChanges.has(changeKey(change.id))), + ); let beforeValues = new Map(); let preparedDiffs = new Map(); if (integration.added.length > 0 && !this.#detached) { @@ -8005,6 +8015,155 @@ export class LoroDoc = Record= opLamport) return false; + let target = this.#idAtLamport(elementId.peer, elementId.lamport); + const deferred = this.#deferredSnapshotHistory; + if (target === undefined && deferred !== undefined) { + // Only the overlay is indexed, so the element comes from the snapshot + // base. Snapshot state only holds elements inserted into that list, all + // of them seen by an op that has seen the whole base. + if ( + this.#snapshotStateHasElement(container, elementId) && + deferred.endVersion + ._codecEntriesUnsorted() + .every(({ peer, counter }) => counter <= (causalVersion.get(peer) ?? 0)) + ) { + return true; + } + this.#materializeDeferredHistory(); + target = this.#idAtLamport(elementId.peer, elementId.lamport); + } + if (target === undefined) return this.#shallowRootHasElement(container, elementId); + const causal = + (target.peer === opId.peer && target.counter < opId.counter) || + target.counter < (causalVersion.get(target.peer) ?? 0); + if (!causal) return false; + const operation = this.#operationAt(target); + if (operation === undefined) return this.#shallowRootHasElement(container, elementId); + return ( + operation.content.type === "movable-list-insert" && + containerIdsEqual(operation.container, container) + ); + } + + /** + * The op ID of `peer`'s op with `lamport`, or undefined when indexed history + * lacks it. Deferred snapshot history is not indexed. + */ + #idAtLamport(peer: bigint, lamport: number): CodecId | undefined { + const records = this.#historyByPeer.get(peer); + if (records === undefined) return undefined; + let low = 0; + let high = records.length; + while (low < high) { + const middle = (low + high) >>> 1; + if (records[middle]!.change.lamport <= lamport) low = middle + 1; + else high = middle; + } + const record = records[low - 1]; + if (record === undefined) return undefined; + const offset = lamport - record.change.lamport; + return offset < changeLength(record.change) + ? { peer, counter: record.change.id.counter + offset } + : undefined; + } + + #snapshotStateHasElement( + id: CodecContainerId, + elementId: { readonly peer: bigint; readonly lamport: number }, + ): boolean { + const container = this.#containers.get(this.#containerKey(id)); + if (!(container instanceof LoroMovableList)) return false; + this._ensureContainerHydrated(container); + return container._state.element(elementId.peer, elementId.lamport) !== undefined; + } + + /** Rust's `shallow_root_has_movable_list_elem`. */ + #shallowRootHasElement( + container: CodecContainerId, + elementId: { readonly peer: bigint; readonly lamport: number }, + ): boolean { + const store = this.#shallowRootStore; + if (store?.kind !== "sstable") return false; + const key = formatContainerId(container); + let elements = shallowRootElements.get(store)?.get(key); + if (elements === undefined) { + elements = new Set(); + const entry = store.containers.find(({ id }) => formatContainerId(id) === key); + const state = entry?.wrapper.state; + if (state?.kind === CodecContainerType.MovableList) { + let listIndex = Number(state.items[0]?.invisibleListItems ?? 0n); + let elementIndex = 0; + for (let index = 1; index < state.items.length; index += 1) { + const item = state.items[index]!; + const listItem = state.listItemIds[listIndex]!; + const id = item.positionIdEqualsElementId + ? { + peerIndex: listItem.peerIndex, + lamport: listItem.counter + listItem.lamportSub, + } + : state.elementIds[elementIndex++]!; + elements.add(`${state.peers[Number(id.peerIndex)]!}:${id.lamport}`); + listIndex += 1 + Number(item.invisibleListItems); + } + } + let byContainer = shallowRootElements.get(store); + if (byContainer === undefined) { + byContainer = new Map(); + shallowRootElements.set(store, byContainer); + } + byContainer.set(key, elements); + } + return elements.has(`${elementId.peer}:${elementId.lamport}`); + } + #operationAt(id: CodecId): DecodedOperation | undefined { const record = this.#recordContaining(id); if (record === undefined) return undefined; @@ -9055,6 +9214,9 @@ function transformListDelta( return output; } +/** Element IDs of each MovableList in a shallow root store, built on first use. */ +const shallowRootElements = new WeakMap>>(); + function hasMaterializedSequenceInsertions( records: readonly HistoryRecord[], containers: ReadonlyMap, diff --git a/loro-js/tests/movable-list-invalid-ops.test.ts b/loro-js/tests/movable-list-invalid-ops.test.ts new file mode 100644 index 000000000..5ae5318ff --- /dev/null +++ b/loro-js/tests/movable-list-invalid-ops.test.ts @@ -0,0 +1,401 @@ +import { describe, expect, test } from "vitest"; + +import { + decodeFastSnapshot, + decodePostcardVersionVector, + decodeSstable, + encodeFastSnapshot, + encodePostcardVersionVector, + encodeSstable, +} from "../src/codec"; +import { LoroDoc, type JsonOpContent, type JsonSchema } from "../src/index"; + +/** + * Port of Rust's crates/loro/tests/movable_list_invalid_ops.rs (loro-dev/loro#1125). + * Forged movable-list moves and sets without a meaning are rejected and leave + * the document untouched and usable; a move or set of a deleted element is + * applied like a concurrent one, identically on every import path. + * See context/loro-js-movable-list.md. + */ + +/** Element IDs below are Rust's `L{lamport}@{peer index}` into the change's `peers`. */ +type ForgedContent = Record; + +/** Peer 1 inserts [a, b, c] (lamports 0..=2), then deletes b (counter 3): [a, c]. */ +function base(): LoroDoc { + const doc = new LoroDoc(); + doc.setPeerId(1); + const list = doc.getMovableList("list"); + list.insert(0, "a"); + list.insert(1, "b"); + list.insert(2, "c"); + doc.commit(); + list.delete(1, 1); + doc.commit(); + return doc; +} + +/** One change by peer 1, causally after `base()`, holding one list op. */ +function forged(content: ForgedContent): JsonSchema { + return { + schema_version: 1, + start_version: {}, + peers: ["1", "2"], + changes: [ + { + id: "4@0", + timestamp: 0, + deps: ["3@0"], + lamport: 4, + msg: null, + ops: [ + { + container: "cid:root-list:MovableList", + counter: 4, + content: content as JsonOpContent, + }, + ], + }, + ], + }; +} + +const moveDeleted = { type: "move", from: 0, to: 1, elem_id: "L1@0" }; + +interface Snapshot { + readonly value: unknown; + readonly version: [string, number][]; +} + +function snapshotOf(doc: LoroDoc): Snapshot { + return { + value: doc.toJSON(), + version: [...doc.oplogVersion().toJSON()].sort(([left], [right]) => + left.localeCompare(right), + ), + }; +} + +/** The document still accepts local edits and valid remote updates. */ +function assertUsable(doc: LoroDoc): void { + const list = doc.getMovableList("list"); + const length = list.length; + list.push("local"); + list.move(0, list.length - 1); + doc.commit(); + expect(list.length).toBe(length + 1); + const remote = new LoroDoc(); + remote.setPeerId(9); + remote.import(doc.export({ mode: "snapshot" })); + remote.getMovableList("list").push("remote"); + remote.commit(); + doc.import(remote.export({ mode: "update", from: doc.oplogVersion() })); + expect(doc.toJSON()).toEqual(remote.toJSON()); +} + +/** `base()` plus an edit by peer 2 that the forged op (deps `3@0`) cannot see. */ +function baseWithConcurrentEdit(): LoroDoc { + const doc = base(); + doc.setPeerId(2); + doc.getMovableList("list").insert(0, "x"); + doc.commit(); + return doc; +} + +function assertRejected(doc: LoroDoc, json: JsonSchema): void { + const before = snapshotOf(doc); + expect(() => doc.importJsonUpdates(json)).toThrow(/causal history|out of range/u); + expect(snapshotOf(doc)).toEqual(before); + assertUsable(doc); +} + +/** Imports `json` onto `base()` through every path; they must agree with a full replay. */ +function assertSameResultOnEveryImportPath(json: JsonSchema, expected: unknown): void { + const carrier = new LoroDoc(); + carrier.import(base().export({ mode: "update" })); + // Detached imports only touch history, which gives us the op as binary. + carrier.detach(); + carrier.importJsonUpdates(json); + const update = carrier.export({ + mode: "updates-in-range", + spans: [{ id: { peer: "1", counter: 4 }, len: 1 }], + }); + const all = carrier.export({ mode: "update" }); + + const replayed = new LoroDoc(); + replayed.import(all); + expect(replayed.toJSON()).toEqual(expected); + + const fromSnapshot = new LoroDoc(); + fromSnapshot.import(base().export({ mode: "snapshot" })); + const detached = base(); + detached.detach(); + const docs = [base(), base(), base(), fromSnapshot, detached]; + docs[0]!.importJsonUpdates(json); + docs[1]!.import(update); + docs[2]!.importBatch([update, update]); + docs[3]!.importJsonUpdates(json); + docs[4]!.import(update); + docs[4]!.attach(); + for (const doc of docs) { + expect(doc.toJSON()).toEqual(replayed.toJSON()); + expect(snapshotOf(doc).version).toEqual(snapshotOf(replayed).version); + assertUsable(doc); + } +} + +describe("movable-list moves and sets from other peers", () => { + test("a move of a deleted element is applied like a concurrent move", () => { + // `from: 0` points at `a` in the op's version, so `a`'s position is consumed + // and `b` comes back at index 1, exactly as for a concurrent move. + assertSameResultOnEveryImportPath(forged(moveDeleted), { list: ["c", "b"] }); + }); + + test("a set of a deleted element is applied like a concurrent set", () => { + assertSameResultOnEveryImportPath( + forged({ type: "set", elem_id: "L1@0", value: "z" }), + { list: ["a", "c"] }, + ); + }); + + test("a move or set of an unknown element is rejected", () => { + const ops = [ + { type: "move", from: 0, to: 1, elem_id: "L99@0" }, + { type: "set", elem_id: "L99@0", value: "z" }, + // Peer 2 has no ops at all in `base()`. + { type: "move", from: 0, to: 1, elem_id: "L0@1" }, + // Lamport 3 is `base()`'s delete op, not an element. + { type: "set", elem_id: "L3@0", value: "z" }, + ]; + for (const op of ops) { + assertRejected(base(), forged(op)); + assertRejected(baseWithConcurrentEdit(), forged(op)); + } + }); + + test("a move of an element from another container is rejected", () => { + const doc = base(); + doc.getMovableList("other").insert(0, "o"); + doc.commit(); + // `other`'s element is `L4@0`; target it from `list` in a change after it. + const json: JsonSchema = { + schema_version: 1, + start_version: {}, + peers: ["1"], + changes: [ + { + id: "5@0", + timestamp: 0, + deps: ["4@0"], + lamport: 5, + msg: null, + ops: [ + { + container: "cid:root-list:MovableList", + counter: 5, + content: { type: "move", from: 0, to: 1, elem_id: "L4@0" } as JsonOpContent, + }, + ], + }, + ], + }; + assertRejected(doc, json); + }); + + test("a move or set on a snapshot-imported document is checked", () => { + const snapshot = base().export({ mode: "snapshot" }); + for (const op of [ + { type: "move", from: 0, to: 1, elem_id: "L99@0" }, + { type: "set", elem_id: "L3@0", value: "z" }, + ]) { + assertRejected(LoroDoc.fromSnapshot(snapshot), forged(op)); + } + const doc = LoroDoc.fromSnapshot(snapshot); + doc.importJsonUpdates(forged({ type: "move", from: 1, to: 0, elem_id: "L2@0" })); + expect(doc.toJSON()).toEqual({ list: ["c", "a"] }); + }); + + test("a move of a snapshot element does not decode the snapshot's history", () => { + const source = base(); + const snapshot = decodeFastSnapshot(source.export({ mode: "snapshot" })); + // Decoding this snapshot's history fails, since its version vector claims + // one more op than the history holds. + const oplog = decodeSstable(snapshot.oplog).map((entry) => { + if (entry.key.length !== 2 || entry.key[0] !== 0x76 || entry.key[1] !== 0x76) { + return entry; + } + const version = decodePostcardVersionVector(entry.value).map((id) => ({ + ...id, + counter: id.counter + 1, + })); + return { ...entry, value: encodePostcardVersionVector(version) }; + }); + const doc = LoroDoc.fromSnapshot( + encodeFastSnapshot({ + ...snapshot, + oplog: encodeSstable(oplog, { compression: "none" }), + }), + ); + // Peer 2's edit, since the forged version already claims peer 1's next op. + const version = source.oplogVersion(); + source.setPeerId(2); + source.getMovableList("list").move(1, 0); + source.getMovableList("list").set(0, "C"); + source.commit(); + + doc.import(source.export({ mode: "update", from: version })); + expect(doc.toJSON()).toEqual({ list: ["C", "a"] }); + expect(() => doc.changeCount()).toThrow(/version does not match/u); + }); + + test("a move of an element outside the op's causal history is rejected", () => { + // Peer 2 inserts `y` concurrently with `base()`'s delete; the forged op by + // peer 1 depends only on `3@0` and so cannot have seen `y` (`L3@1`). + const doc = base(); + const beforeDelete = new LoroDoc(); + beforeDelete.setPeerId(2); + beforeDelete.import( + doc.export({ + mode: "updates-in-range", + spans: [{ id: { peer: "1", counter: 0 }, len: 3 }], + }), + ); + beforeDelete.getMovableList("list").push("y"); + beforeDelete.commit(); + doc.import(beforeDelete.export({ mode: "update" })); + expect(doc.toJSON()).toEqual({ list: ["a", "c", "y"] }); + + assertRejected(doc, forged({ type: "move", from: 0, to: 1, elem_id: "L3@1" })); + }); + + test("a move with an out-of-range index is rejected", () => { + const ops = [ + { type: "move", from: 9, to: 0, elem_id: "L0@0" }, + { type: "move", from: 0, to: 9, elem_id: "L0@0" }, + { type: "move", from: 5, to: 1, elem_id: "L1@0" }, + ]; + for (const op of ops) { + assertRejected(base(), forged(op)); + assertRejected(baseWithConcurrentEdit(), forged(op)); + } + }); + + test("a detached import of an unknown element is rejected", () => { + const doc = base(); + doc.detach(); + const before = snapshotOf(doc); + expect(() => + doc.importJsonUpdates(forged({ type: "move", from: 0, to: 1, elem_id: "L99@0" })), + ).toThrow(/causal history/u); + expect(snapshotOf(doc)).toEqual(before); + doc.attach(); + assertUsable(doc); + }); + + test("importBatch with an out-of-range move rolls back and stays attached", () => { + // Detached imports only touch history (bounds are checked when the document + // reattaches), which lets us export the invalid op as binary. + const carrier = base(); + carrier.detach(); + carrier.importJsonUpdates(forged({ type: "move", from: 9, to: 0, elem_id: "L0@0" })); + const forgedUpdate = carrier.export({ + mode: "updates-in-range", + spans: [{ id: { peer: "1", counter: 4 }, len: 1 }], + }); + + const doc = base(); + const valid = new LoroDoc(); + valid.setPeerId(5); + valid.import(doc.export({ mode: "update" })); + valid.getMovableList("list").push("v"); + valid.commit(); + const validUpdate = valid.export({ mode: "update", from: doc.oplogVersion() }); + + const before = snapshotOf(doc); + expect(() => doc.import(forgedUpdate)).toThrow(/out of range/u); + expect(snapshotOf(doc)).toEqual(before); + expect(() => doc.importBatch([validUpdate, forgedUpdate])).toThrow(/out of range/u); + expect(doc.isDetached()).toBe(false); + expect(snapshotOf(doc)).toEqual(before); + assertUsable(doc); + }); + + test("moves and sets of shallow-root elements still import", () => { + const doc = new LoroDoc(); + doc.setPeerId(1); + const list = doc.getMovableList("list"); + list.insert(0, "a"); + list.insert(1, "b"); + list.insert(2, "c"); + doc.commit(); + list.insert(3, "d"); + doc.commit(); + + const shallow = new LoroDoc(); + shallow.import( + doc.export({ mode: "shallow-snapshot", frontiers: doc.oplogFrontiers() }), + ); + const version = shallow.oplogVersion(); + + // Elements created before the shallow root are moved/set after it. + list.move(0, 3); + list.set(0, "B"); + doc.commit(); + shallow.import(doc.export({ mode: "update", from: version })); + expect(shallow.toJSON()).toEqual(doc.toJSON()); + + // Unknown elements are still rejected on a shallow document. + const json: JsonSchema = { + schema_version: 1, + start_version: {}, + peers: ["1"], + changes: [ + { + id: "6@0", + timestamp: 0, + deps: ["5@0"], + lamport: 6, + msg: null, + ops: [ + { + container: "cid:root-list:MovableList", + counter: 6, + content: { + type: "move", + from: 0, + to: 1, + elem_id: "L99@0", + } as JsonOpContent, + }, + ], + }, + ], + }; + assertRejected(shallow, json); + }); + + test("a move of an element deleted before the shallow root is rejected", () => { + // The shallow history only keeps elements alive at the root, and no valid op + // after the root can see `b`, which was deleted before it. + const source = base(); + const shallowSnapshot = source.export({ + mode: "shallow-snapshot", + frontiers: source.oplogFrontiers(), + }); + for (const op of [ + moveDeleted, + { type: "set", elem_id: "L1@0", value: "z" }, + { type: "move", from: 0, to: 1, elem_id: "L99@0" }, + ]) { + const doc = new LoroDoc(); + doc.import(shallowSnapshot); + assertRejected(doc, forged(op)); + } + + // `a` is alive at the root, so it can still be moved and set. + const doc = new LoroDoc(); + doc.import(shallowSnapshot); + doc.importJsonUpdates(forged({ type: "set", elem_id: "L0@0", value: "A" })); + expect(doc.toJSON()).toEqual({ list: ["A", "c"] }); + }); +});