From 291b845ab01b4fd649dd56c2c8d7a71366077e5f Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Thu, 1 Oct 2026 04:31:21 +0800 Subject: [PATCH 1/3] fix: tolerate JSON loss in known history See context/import-peer-id-reuse.md for the JSON relay comparison rules. Co-Authored-By: GPT-6 --- .changeset/known-history-json-lossy.md | 5 + context/import-peer-id-reuse.md | 23 +- crates/loro-internal/src/arena.rs | 13 +- crates/loro-internal/src/loro.rs | 4 +- .../loro-internal/src/oplog/known_history.rs | 200 +++++++++++-- crates/loro/tests/import_json_relay.rs | 271 ++++++++++++++++++ 6 files changed, 490 insertions(+), 26 deletions(-) create mode 100644 .changeset/known-history-json-lossy.md create mode 100644 crates/loro/tests/import_json_relay.rs diff --git a/.changeset/known-history-json-lossy.md b/.changeset/known-history-json-lossy.md new file mode 100644 index 000000000..24744c222 --- /dev/null +++ b/.changeset/known-history-json-lossy.md @@ -0,0 +1,5 @@ +--- +"loro-crdt": patch +--- + +Fix a 1.16.4 regression where binary updates or snapshots containing history relayed through JSON were rejected with `UsedOpID`. Known-history checks now tolerate JSON's binary-to-byte-list and non-finite-number-to-null conversions, including nested values and rich-text styles, while still rejecting genuine conflicts. The receiver keeps its existing prefix values. The JSON format is unchanged: a Binary value still comes back from JSON as a list, including mergeable container markers; this fix does not restore values lost during JSON import. diff --git a/context/import-peer-id-reuse.md b/context/import-peer-id-reuse.md index 638f8f612..d48174cba 100644 --- a/context/import-peer-id-reuse.md +++ b/context/import-peer-id-reuse.md @@ -1,6 +1,6 @@ # Imports That Reuse Local Op Ids -Verified against code 2026-09-30. +Verified against code 2026-10-01. An import skips the part of each change the doc already has by version vector and applies the rest on top of the local history. When two clients shared a peer id @@ -51,9 +51,18 @@ is per atom range, not per change: (`unicode_start` is an arena offset), and a one-atom delete ignores the sign of `signed_len`, because slicing a reversed delete down to one atom yields `-1` where a forward one yields `1`. -- **JSON imports** use `ImportedValues::Lossy`: JSON text does not round-trip every - value (`NaN` becomes `null`, binary may come back as a list), so value payloads - are not compared there. Containers, positions, text, keys, ids and deps still are. +- **Value payloads** use `ImportedValues::Lossy` for both JSON and binary imports: + binary history may have been relayed through JSON before it was exported. + `ImportedValues::eq` accepts `Binary` versus a list of the same u8 numbers, and + non-finite doubles (`NaN`, positive or negative infinity) versus `Null`, in either + direction. It applies the same rules recursively inside lists and maps and to + rich-text style values. All other values are compared exactly; containers, + positions, text, keys, ids, style metadata and deps still must match. + The known prefix is then trimmed, so the receiver keeps its own values. + This fixes the 1.16.4 rejection of JSON-relayed history without changing the JSON + format: a JSON importer still gets a list instead of `Binary` (including mergeable + container markers), or `Null` instead of a non-finite double. It does not repair + that importer's lost values or make its state identical to the binary receiver's. - History below the shallow root, or any local change `get_change` cannot find, is not compared. The import then behaves as it did before this check. @@ -88,3 +97,9 @@ for both "all updates + 1 change" and "snapshot + 1 change". Probe: and re-imports that must still succeed (piecewise vs merged change stores, every op kind including `NaN` and one-atom reversed deletes, shallow docs). - `crates/loro-wasm/tests/import_reused_peer_id.test.ts`. +- `crates/loro/tests/import_json_relay.rs`: JSON-to-binary relays through full + updates, snapshots and batch import; binary values, mergeable text/counter, + non-finite doubles, nested/sequence/style values, both comparison directions, + later relay edits, and a genuine text conflict that must still be rejected. +- `known_history.rs` unit tests: the precise value equivalences and rejection + boundaries, including nested values and byte-list contents. diff --git a/crates/loro-internal/src/arena.rs b/crates/loro-internal/src/arena.rs index ab7588ede..541858195 100644 --- a/crates/loro-internal/src/arena.rs +++ b/crates/loro-internal/src/arena.rs @@ -660,11 +660,18 @@ impl SharedArena { (self.inner.values.lock()[range]).to_vec() } - /// Whether the values in two ranges are equal, without cloning them. - pub(crate) fn value_slices_eq(&self, a: Range, b: Range) -> bool { + /// Compare values in two ranges without cloning them or taking the lock twice. + pub(crate) fn value_slices_eq( + &self, + a: Range, + b: Range, + eq: impl Fn(&LoroValue, &LoroValue) -> bool, + ) -> bool { let values = self.inner.values.lock(); match (values.get(a), values.get(b)) { - (Some(a), Some(b)) => a == b, + (Some(a), Some(b)) => { + a.len() == b.len() && a.iter().zip(b.iter()).all(|(a, b)| eq(a, b)) + } _ => false, } } diff --git a/crates/loro-internal/src/loro.rs b/crates/loro-internal/src/loro.rs index 6d86ab69d..9dda46c9c 100644 --- a/crates/loro-internal/src/loro.rs +++ b/crates/loro-internal/src/loro.rs @@ -660,7 +660,7 @@ impl LoroDoc { } else { self.import_changes_and_apply_delta_to_state_if_needed( |oplog| encoding::decode_oplog_changes(oplog, parsed), - ImportedValues::Exact, + ImportedValues::Lossy, origin, ) @@ -672,7 +672,7 @@ impl LoroDoc { } EncodeMode::FastUpdates => self.import_changes_and_apply_delta_to_state_if_needed( |oplog| encoding::decode_oplog_changes(oplog, parsed), - ImportedValues::Exact, + ImportedValues::Lossy, origin, ), EncodeMode::Auto => { diff --git a/crates/loro-internal/src/oplog/known_history.rs b/crates/loro-internal/src/oplog/known_history.rs index a5ad2fd09..2ad2d26c5 100644 --- a/crates/loro-internal/src/oplog/known_history.rs +++ b/crates/loro-internal/src/oplog/known_history.rs @@ -7,7 +7,7 @@ //! the doc) or silently scrambled it (loro-dev/loro#1118). See //! `context/import-peer-id-reuse.md`. -use loro_common::{Counter, HasCounterSpan, LoroError, LoroResult, PeerID, ID}; +use loro_common::{Counter, HasCounterSpan, LoroError, LoroResult, LoroValue, PeerID, ID}; use rle::{HasLength, Sliceable}; use rustc_hash::FxHashMap; @@ -23,17 +23,50 @@ use crate::{ #[cfg(feature = "counter")] use crate::op::FutureInnerContent; -/// How faithfully the import format carries op values. +/// How to compare imported op values with known history. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum ImportedValues { - /// Binary encodings round-trip every value. + /// Compare every value exactly. Exact, - /// JSON updates may have gone through JSON text, which does not round-trip - /// every value (`NaN` becomes `null`, binary may come back as a list). Values - /// are then not compared; everything else still is. + /// Allow only JSON's lossy value representations, recursively. Binary imports + /// need this too: their history may have been relayed through JSON earlier. Lossy, } +impl ImportedValues { + fn eq(self, a: &LoroValue, b: &LoroValue) -> bool { + if self == Self::Exact { + return a == b; + } + + match (a, b) { + (LoroValue::Binary(bytes), LoroValue::List(list)) + | (LoroValue::List(list), LoroValue::Binary(bytes)) => { + bytes.len() == list.len() + && bytes + .iter() + .zip(list.iter()) + .all(|(&byte, value)| match value { + LoroValue::I64(number) => *number == i64::from(byte), + LoroValue::Double(number) => *number == f64::from(byte), + _ => false, + }) + } + (LoroValue::Double(number), LoroValue::Null) + | (LoroValue::Null, LoroValue::Double(number)) => !number.is_finite(), + (LoroValue::List(a), LoroValue::List(b)) => { + a.len() == b.len() && a.iter().zip(b.iter()).all(|(a, b)| self.eq(a, b)) + } + (LoroValue::Map(a), LoroValue::Map(b)) => { + a.len() == b.len() + && a.iter() + .all(|(key, a)| b.get(key).is_some_and(|b| self.eq(a, b))) + } + _ => Self::Exact.eq(a, b), + } + } +} + impl OpLog { /// Check the decoded `changes` against the history this doc already has for the /// same op ids, then drop the parts it already has. @@ -265,20 +298,21 @@ fn op_eq(arena: &SharedArena, values: ImportedValues, a: &Op, b: &Op) -> bool { return false; } - let exact = values == ImportedValues::Exact; - match (&a.content, &b.content) { - (InnerContent::List(a), InnerContent::List(b)) => list_op_eq(arena, exact, a, b), + (InnerContent::List(a), InnerContent::List(b)) => list_op_eq(arena, values, a, b), (InnerContent::Map(a), InnerContent::Map(b)) => { a.key == b.key - && a.value.is_some() == b.value.is_some() - && (!exact || a.value == b.value) + && match (&a.value, &b.value) { + (Some(a), Some(b)) => values.eq(a, b), + (None, None) => true, + _ => false, + } } (InnerContent::Tree(a), InnerContent::Tree(b)) => a == b, (InnerContent::Future(a), InnerContent::Future(b)) => match (a, b) { #[cfg(feature = "counter")] (FutureInnerContent::Counter(a), FutureInnerContent::Counter(b)) => { - !exact || a == b || (a.is_nan() && b.is_nan()) + a == b || (a.is_nan() && b.is_nan()) } ( crate::op::FutureInnerContent::Unknown { @@ -289,7 +323,7 @@ fn op_eq(arena: &SharedArena, values: ImportedValues, a: &Op, b: &Op) -> bool { prop: b_prop, value: b_value, }, - ) => a_prop == b_prop && (!exact || a_value == b_value), + ) => a_prop == b_prop && a_value == b_value, #[allow(unreachable_patterns)] _ => false, }, @@ -297,7 +331,12 @@ fn op_eq(arena: &SharedArena, values: ImportedValues, a: &Op, b: &Op) -> bool { } } -fn list_op_eq(arena: &SharedArena, exact: bool, a: &InnerListOp, b: &InnerListOp) -> bool { +fn list_op_eq( + arena: &SharedArena, + values: ImportedValues, + a: &InnerListOp, + b: &InnerListOp, +) -> bool { match (a, b) { ( InnerListOp::Insert { @@ -310,7 +349,9 @@ fn list_op_eq(arena: &SharedArena, exact: bool, a: &InnerListOp, b: &InnerListOp }, ) => { a_pos == b_pos - && (!exact || arena.value_slices_eq(a_slice.to_range(), b_slice.to_range())) + && arena.value_slices_eq(a_slice.to_range(), b_slice.to_range(), |a, b| { + values.eq(a, b) + }) } ( InnerListOp::InsertText { @@ -355,7 +396,7 @@ fn list_op_eq(arena: &SharedArena, exact: bool, a: &InnerListOp, b: &InnerListOp elem_id: b_elem, value: b_value, }, - ) => a_elem == b_elem && (!exact || a_value == b_value), + ) => a_elem == b_elem && values.eq(a_value, b_value), ( InnerListOp::StyleStart { start: a_start, @@ -375,10 +416,135 @@ fn list_op_eq(arena: &SharedArena, exact: bool, a: &InnerListOp, b: &InnerListOp a_start == b_start && a_end == b_end && a_key == b_key - && (!exact || a_value == b_value) + && values.eq(a_value, b_value) && a_info == b_info } (InnerListOp::StyleEnd, InnerListOp::StyleEnd) => true, _ => false, } } + +#[cfg(test)] +mod tests { + use super::ImportedValues; + use loro_common::LoroValue; + + fn assert_lossy_pair(a: &LoroValue, b: &LoroValue, equal: bool) { + assert_eq!(ImportedValues::Lossy.eq(a, b), equal, "{a:?} vs {b:?}"); + assert_eq!(ImportedValues::Lossy.eq(b, a), equal, "{b:?} vs {a:?}"); + } + + #[test] + fn byte_lists_must_match_every_byte() { + let binary = LoroValue::from(vec![0u8, 1, 255]); + for list in [ + LoroValue::from(vec![0, 1, 255]), + LoroValue::from(vec![0.0, 1.0, 255.0]), + ] { + assert_lossy_pair(&binary, &list, true); + assert!(!ImportedValues::Exact.eq(&binary, &list)); + } + assert_lossy_pair( + &LoroValue::from(Vec::::new()), + &LoroValue::from(Vec::::new()), + true, + ); + for list in [ + LoroValue::from(vec![0, 1]), + LoroValue::from(vec![0, 1, 255, 0]), + LoroValue::from(vec![0, 1, 254]), + LoroValue::from(vec![0, -1, 255]), + LoroValue::from(vec![0, 1, 256]), + LoroValue::from(vec![0.0, 1.5, 255.0]), + LoroValue::from(vec![0.0, f64::NAN, 255.0]), + LoroValue::from(vec![ + LoroValue::I64(0), + LoroValue::Bool(true), + LoroValue::I64(255), + ]), + ] { + assert_lossy_pair(&binary, &list, false); + } + assert_lossy_pair(&binary, &LoroValue::from(vec![0u8, 1, 254]), false); + } + + #[test] + fn only_non_finite_numbers_match_null() { + for number in [f64::NAN, f64::INFINITY, f64::NEG_INFINITY] { + let number = LoroValue::Double(number); + assert_lossy_pair(&number, &LoroValue::Null, true); + assert!(!ImportedValues::Exact.eq(&number, &LoroValue::Null)); + assert!(ImportedValues::Exact.eq(&number, &number)); + } + for number in [0.0, -0.0, 1.5, f64::MAX] { + assert_lossy_pair(&LoroValue::Double(number), &LoroValue::Null, false); + } + assert_lossy_pair(&LoroValue::I64(0), &LoroValue::Null, false); + assert_lossy_pair( + &LoroValue::Double(f64::INFINITY), + &LoroValue::Double(f64::NEG_INFINITY), + false, + ); + } + + #[test] + fn nested_values_allow_only_json_representations() { + let nested = + |value| LoroValue::Map(vec![("key".into(), LoroValue::from(vec![value]))].into()); + let original = nested(LoroValue::from(vec![ + LoroValue::from(vec![1u8, 2]), + LoroValue::Double(f64::NAN), + ])); + let json: LoroValue = + serde_json::from_str(&serde_json::to_string(&original).unwrap()).unwrap(); + assert_lossy_pair(&original, &json, true); + assert!(!ImportedValues::Exact.eq(&original, &json)); + assert_lossy_pair( + &original, + &nested(LoroValue::from(vec![ + LoroValue::from(vec![1, 3]), + LoroValue::Null, + ])), + false, + ); + assert_lossy_pair( + &original, + &LoroValue::Map(vec![("other".into(), original["key"].clone())].into()), + false, + ); + assert_lossy_pair(&original, &LoroValue::Map(Default::default()), false); + assert_lossy_pair( + &LoroValue::from(vec![1, 2]), + &LoroValue::from(vec![1, 2, 3]), + false, + ); + } + + #[test] + fn ordinary_values_still_require_exact_equality() { + let values = [ + LoroValue::Null, + LoroValue::Bool(false), + LoroValue::Bool(true), + LoroValue::I64(1), + LoroValue::I64(2), + LoroValue::Double(1.0), + LoroValue::Double(1.5), + LoroValue::from("one"), + LoroValue::from("two"), + LoroValue::from(loro_common::ContainerID::new_root( + "a", + loro_common::ContainerType::Map, + )), + LoroValue::from(loro_common::ContainerID::new_root( + "b", + loro_common::ContainerType::Map, + )), + ]; + for a in &values { + for b in &values { + assert_lossy_pair(a, b, a == b); + } + } + } +} diff --git a/crates/loro/tests/import_json_relay.rs b/crates/loro/tests/import_json_relay.rs new file mode 100644 index 000000000..c85625818 --- /dev/null +++ b/crates/loro/tests/import_json_relay.rs @@ -0,0 +1,271 @@ +//! History relayed through JSON keeps its lossy values even after binary export. +//! Known prefixes must be compared with that loss in mind, then trimmed without +//! replacing the receiver's original values. + +use loro::{ + ExpandType, ExportMode, LoroDoc, LoroError, LoroValue, StyleConfig, ValueOrContainer, ID, +}; + +fn source() -> LoroDoc { + let doc = LoroDoc::new(); + doc.set_peer_id(1).unwrap(); + doc.config_default_text_style(Some(StyleConfig { + expand: ExpandType::After, + })); + doc +} + +fn json_copy(doc: &LoroDoc) -> LoroDoc { + let json = doc.export_json_updates(&Default::default(), &doc.oplog_vv()); + // A typed JsonSchema does not lose values: exercise the actual JSON wire path. + let json = serde_json::to_string(&json).unwrap(); + let copy = LoroDoc::new(); + copy.set_peer_id(2).unwrap(); + copy.import_json_updates(json.as_str()).unwrap(); + copy +} + +fn map_value(doc: &LoroDoc, key: &str) -> LoroValue { + match doc.get_map("m").get(key).unwrap() { + ValueOrContainer::Value(value) => value, + other => panic!("expected a value, got {other:?}"), + } +} + +fn style_delta(doc: &LoroDoc) -> Option> { + doc.get_deep_value() + .as_map() + .unwrap() + .contains_key("text") + .then(|| doc.get_text("text").to_delta()) +} + +fn assert_relay_keeps_prefix(a: &LoroDoc, check_loss: impl Fn(&LoroDoc)) { + a.commit(); + let prefix = a.export(ExportMode::all_updates()).unwrap(); + let b = json_copy(a); + check_loss(&b); + + // Full exports must keep working after later relay edits too. + for tail in 1..=2 { + b.get_map("relay").insert("extra", tail).unwrap(); + b.commit(); + for mode in [ExportMode::all_updates(), ExportMode::Snapshot] { + let blob = b.export(mode).unwrap(); + for batch in [false, true] { + let c = LoroDoc::new(); + c.set_peer_id(3).unwrap(); + c.import(&prefix).unwrap(); + let expected = c.get_deep_value().into_map().unwrap(); + let expected_style = style_delta(&c); + if batch { + c.import_batch(&[blob.clone()]).unwrap(); + } else { + c.import(&blob).unwrap(); + } + + let actual = c.get_deep_value().into_map().unwrap(); + for (key, value) in expected.iter() { + assert_eq!(actual.get(key), Some(value), "prefix root {key}"); + } + assert_eq!(actual.len(), expected.len() + 1); + assert_eq!(actual["relay"]["extra"], LoroValue::from(tail)); + assert_eq!(style_delta(&c), expected_style); + assert_eq!(c.oplog_vv(), b.oplog_vv()); + } + } + } + + // Compare the same representations in the opposite direction: the receiver + // owns the JSON prefix and the binary sender owns the original values. + let c = json_copy(a); + let expected = c.get_deep_value().into_map().unwrap(); + let expected_style = style_delta(&c); + a.get_map("relay").insert("extra", 3).unwrap(); + a.commit(); + for mode in [ExportMode::all_updates(), ExportMode::Snapshot] { + let receiver = LoroDoc::new(); + receiver + .import(&c.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + receiver.import(&a.export(mode).unwrap()).unwrap(); + let actual = receiver.get_deep_value().into_map().unwrap(); + for (key, value) in expected.iter() { + assert_eq!(actual.get(key), Some(value), "JSON prefix root {key}"); + } + assert_eq!(style_delta(&receiver), expected_style); + assert_eq!(actual.len(), expected.len() + 1); + assert_eq!(actual["relay"]["extra"], LoroValue::I64(3)); + assert_eq!(receiver.oplog_vv(), a.oplog_vv()); + } +} + +#[test] +fn binary_map_value_survives_json_then_binary_relay() { + let a = source(); + a.get_map("m").insert("bin", vec![0u8, 1, 255]).unwrap(); + assert_relay_keeps_prefix(&a, |b| { + assert_eq!(map_value(b, "bin"), LoroValue::from(vec![0, 1, 255])); + }); +} + +#[test] +fn mergeable_text_survives_json_then_binary_relay() { + let a = source(); + a.get_map("m") + .ensure_mergeable_text("mt") + .unwrap() + .insert(0, "hi") + .unwrap(); + assert_relay_keeps_prefix(&a, |b| assert!(map_value(b, "mt").is_list())); +} + +#[test] +fn mergeable_counter_survives_json_then_binary_relay() { + let a = source(); + a.get_map("m") + .ensure_mergeable_counter("mc") + .unwrap() + .increment(2.0) + .unwrap(); + assert_relay_keeps_prefix(&a, |b| assert!(map_value(b, "mc").is_list())); +} + +#[test] +fn non_finite_map_values_survive_json_then_binary_relay() { + for value in [f64::NAN, f64::INFINITY, f64::NEG_INFINITY] { + let a = source(); + a.get_map("m").insert("number", value).unwrap(); + assert_relay_keeps_prefix(&a, |b| assert_eq!(map_value(b, "number"), LoroValue::Null)); + } +} + +#[test] +fn style_values_survive_json_then_binary_relay() { + for value in [LoroValue::from(vec![0u8, 255]), LoroValue::Double(f64::NAN)] { + let a = source(); + let text = a.get_text("text"); + text.insert(0, "styled").unwrap(); + text.mark(0..6, "custom", value).unwrap(); + let expected = text.to_delta(); + assert_relay_keeps_prefix(&a, |b| assert_ne!(b.get_text("text").to_delta(), expected)); + } +} + +#[test] +fn nested_and_sequence_values_survive_json_then_binary_relay() { + let a = source(); + let value = LoroValue::Map( + vec![( + "nested".into(), + LoroValue::List( + vec![ + LoroValue::from(vec![1u8, 2]), + LoroValue::Double(f64::INFINITY), + ] + .into(), + ), + )] + .into(), + ); + a.get_map("m").insert("nested", value.clone()).unwrap(); + a.get_list("list").push(value.clone()).unwrap(); + let movable = a.get_movable_list("movable"); + movable.push(value.clone()).unwrap(); + movable.push(0).unwrap(); + movable.set(1, value.clone()).unwrap(); + a.get_text("text").insert(0, "nested style").unwrap(); + a.get_text("text") + .mark(0..6, "custom", value.clone()) + .unwrap(); + assert_relay_keeps_prefix(&a, |b| assert_ne!(map_value(b, "nested"), value)); +} + +#[test] +fn real_text_conflict_is_rejected_after_json_then_binary_relay() { + let a = source(); + a.get_text("text").insert(0, "hello").unwrap(); + a.commit(); + let conflict = source(); + conflict.get_text("text").insert(0, "hellO").unwrap(); + conflict.commit(); + let b = json_copy(&conflict); + b.get_map("relay").insert("extra", 1).unwrap(); + b.commit(); + + for mode in [ExportMode::all_updates(), ExportMode::Snapshot] { + let c = LoroDoc::new(); + c.import(&a.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + let before = c.get_deep_value(); + let vv = c.oplog_vv(); + let err = c.import(&b.export(mode).unwrap()).unwrap_err(); + assert!(matches!(err, LoroError::UsedOpID { id } if id == ID::new(1, 4))); + assert_eq!(c.get_deep_value(), before); + assert_eq!(c.oplog_vv(), vv); + } +} + +#[test] +fn genuine_value_conflicts_are_rejected_on_json_and_binary_imports() { + let nested = |byte| LoroValue::Map(vec![("nested".into(), LoroValue::from(vec![byte]))].into()); + for (local, conflicting) in [ + (LoroValue::from(vec![1u8, 2]), LoroValue::from(vec![1, 3])), + (LoroValue::Double(f64::NAN), LoroValue::I64(0)), + (nested(1), nested(2)), + ] { + // Check every operation that carries a value, including arena-backed inserts. + for kind in ["map", "list", "movable-set", "style"] { + let write = |doc: &LoroDoc, value: LoroValue| match kind { + "map" => doc.get_map("m").insert("value", value).unwrap(), + "list" => doc.get_list("list").push(value).unwrap(), + "movable-set" => { + doc.get_movable_list("movable").push(0).unwrap(); + doc.get_movable_list("movable").set(0, value).unwrap(); + } + "style" => { + doc.get_text("text").insert(0, "style").unwrap(); + doc.get_text("text").mark(0..5, "custom", value).unwrap(); + } + _ => unreachable!(), + }; + let a = source(); + write(&a, local.clone()); + a.commit(); + let conflict = source(); + write(&conflict, conflicting.clone()); + conflict.commit(); + let b = json_copy(&conflict); + b.get_map("relay").insert("extra", 1).unwrap(); + b.commit(); + let json = + serde_json::to_string(&b.export_json_updates(&Default::default(), &b.oplog_vv())) + .unwrap(); + for import_kind in ["json", "updates", "snapshot"] { + let c = LoroDoc::new(); + c.import(&a.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + let before = c.get_deep_value(); + let before_style = style_delta(&c); + let vv = c.oplog_vv(); + let err = match import_kind { + "json" => c.import_json_updates(json.as_str()).unwrap_err(), + "updates" => c + .import(&b.export(ExportMode::all_updates()).unwrap()) + .unwrap_err(), + "snapshot" => c + .import(&b.export(ExportMode::Snapshot).unwrap()) + .unwrap_err(), + _ => unreachable!(), + }; + assert!( + matches!(err, LoroError::UsedOpID { .. }), + "{kind}/{import_kind}: {err:?}" + ); + assert_eq!(c.get_deep_value(), before); + assert_eq!(style_delta(&c), before_style); + assert_eq!(c.oplog_vv(), vv); + } + } + } +} From 37482ac6a0a8fa26818b449eb935219bc67c3e8b Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Thu, 1 Oct 2026 05:26:30 +0800 Subject: [PATCH 2/3] fix: cover remaining JSON value conversions See context/import-peer-id-reuse.md for comparison rules and ambiguous payload limits. Co-Authored-By: GPT-6 --- .changeset/known-history-json-lossy.md | 4 +- context/import-peer-id-reuse.md | 55 ++++- .../loro-internal/src/oplog/known_history.rs | 153 +++++++++++++- crates/loro/tests/import_json_relay.rs | 195 ++++++++++++++++-- 4 files changed, 376 insertions(+), 31 deletions(-) diff --git a/.changeset/known-history-json-lossy.md b/.changeset/known-history-json-lossy.md index 24744c222..e1192a52a 100644 --- a/.changeset/known-history-json-lossy.md +++ b/.changeset/known-history-json-lossy.md @@ -2,4 +2,6 @@ "loro-crdt": patch --- -Fix a 1.16.4 regression where binary updates or snapshots containing history relayed through JSON were rejected with `UsedOpID`. Known-history checks now tolerate JSON's binary-to-byte-list and non-finite-number-to-null conversions, including nested values and rich-text styles, while still rejecting genuine conflicts. The receiver keeps its existing prefix values. The JSON format is unchanged: a Binary value still comes back from JSON as a list, including mergeable container markers; this fix does not restore values lost during JSON import. +Fix a 1.16.4 regression where binary updates or snapshots containing history relayed through JSON were rejected with `UsedOpID`. Known-history checks tolerate binary-to-byte-list, non-finite-number-to-null and integral-Double-to-I64 conversions, including nested values and rich-text styles. Marker-looking strings that JSON turns into Containers and opaque unknown-op payloads retain a value-comparison bypass. The receiver keeps its existing prefix values. + +The JSON format is unchanged: a Binary value still comes back from JSON as a list, including mergeable container markers; this fix does not restore values lost during JSON import. JSON imports now compare other representable value payloads rather than skipping all value comparisons, so genuinely different values may be rejected where they were previously silently accepted. Real peer-id conflicts that look like the permitted conversions or use the bypassed payload kinds remain indistinguishable and can still pass. diff --git a/context/import-peer-id-reuse.md b/context/import-peer-id-reuse.md index d48174cba..7145d3ffe 100644 --- a/context/import-peer-id-reuse.md +++ b/context/import-peer-id-reuse.md @@ -53,21 +53,53 @@ is per atom range, not per change: where a forward one yields `1`. - **Value payloads** use `ImportedValues::Lossy` for both JSON and binary imports: binary history may have been relayed through JSON before it was exported. - `ImportedValues::eq` accepts `Binary` versus a list of the same u8 numbers, and - non-finite doubles (`NaN`, positive or negative infinity) versus `Null`, in either - direction. It applies the same rules recursively inside lists and maps and to - rich-text style values. All other values are compared exactly; containers, - positions, text, keys, ids, style metadata and deps still must match. + `ImportedValues::eq` accepts `Binary` versus a list of the same u8 numbers, + non-finite doubles (`NaN`, positive or negative infinity) versus `Null`, and + `Double(d)` versus `I64(i)` when `d == i as f64`, in either direction. These + rules apply recursively inside lists and maps and to rich-text style values. + JS numbers erase the distinction between an integral Double and I64; JSON text + written by JS also drops the `.0` (`crates/loro-wasm/src/convert.rs`, + `js_json_schema_to_loro_json_schema`; `LoroValueVisitor` in `loro-common/src/value.rs`). + The test-only `ImportedValues::Exact` provides the strict baseline for unit tests. +- **Ambiguous payload kinds.** A valid `🦜:cid:` string becomes a Container in + JSON (`LoroValueVisitor::visit_str`). JSON peer compression can then reinterpret + the string's peer as an index, so a marker-string versus Container pair is + accepted without comparing the encoded id. Two Containers or two strings still + must match. Unknown-op payloads (`FutureInnerContent::Unknown`) are also not + compared in Lossy mode: they may carry nested LoroValues or arena references + whose meaning this version cannot establish. The op's prop and container still + must match. This retains the prior JSON bypass for that opaque kind and permits + its later binary relay. +- **Counter ops** carry tagged `OwnedValue::F64`/`I64` values in JSON + (`encoding/json_schema.rs::decode_op`), both decoded to `Counter(f64)`. + Finite increments therefore remain comparable numerically, including integral + increments. NaN equals NaN for binary reimports. A non-finite counter increment + serialized as JSON text is already invalid at decode because its tagged f64 + becomes null; this is not a new known-history rejection. WASM's configured + `serde_wasm_bindgen` serializer rejects I64 values outside JS's safe integer + range instead of rounding them, so no I64-to-I64 rounding equivalence is added. +- **Other values** are compared exactly. Unlike main's blanket JSON value bypass, + JSON imports now reject genuinely different representable payloads. Text, + op container ids, positions, keys, element ids, style metadata and deps still + must match on every import path. The known prefix is then trimmed, so the receiver keeps its own values. This fixes the 1.16.4 rejection of JSON-relayed history without changing the JSON format: a JSON importer still gets a list instead of `Binary` (including mergeable - container markers), or `Null` instead of a non-finite double. It does not repair - that importer's lost values or make its state identical to the binary receiver's. + container markers), `Null` instead of a non-finite double, an I64 instead of an + integral Double, or a Container instead of a marker-looking string. It does not + repair that importer's lost values or make its state identical to the binary receiver's. - History below the shallow root, or any local change `get_change` cannot find, is not compared. The import then behaves as it did before this check. ## What it does not catch +- **Conflicts indistinguishable from JSON loss.** Two actual writers using the + same op id can store Binary versus a list of the same bytes, NaN/positive or + negative infinity versus null, or an integral Double versus the matching I64. + Such a conflict passes the overlap check even without a JSON relay. A valid + marker-string versus Container pair and differing unknown-op payloads also + pass under the bypasses above. The receiver keeps its own prefix; the conflicting + value (or its type) remains different, while unrelated matching values stay the same. - **Delta sync.** If the conflicting prefix is not in the import (the sender exported from the receiver's version vector), nothing can be compared: the receiver's vv already claims those ids. A tail position past the end of a @@ -99,7 +131,10 @@ for both "all updates + 1 change" and "snapshot + 1 change". Probe: - `crates/loro-wasm/tests/import_reused_peer_id.test.ts`. - `crates/loro/tests/import_json_relay.rs`: JSON-to-binary relays through full updates, snapshots and batch import; binary values, mergeable text/counter, - non-finite doubles, nested/sequence/style values, both comparison directions, - later relay edits, and a genuine text conflict that must still be rejected. + non-finite/integral doubles, marker strings with peer compression, tagged counter + values and unknown payloads, nested/sequence/style values, both comparison + directions and JSON re-imports, later relay edits, retained prefix history, and + genuine text/value conflicts that must still be rejected. - `known_history.rs` unit tests: the precise value equivalences and rejection - boundaries, including nested values and byte-list contents. + boundaries, including nested values, byte-list contents, numeric representations, + marker-string ambiguity and unknown-op prop/container checks. diff --git a/crates/loro-internal/src/oplog/known_history.rs b/crates/loro-internal/src/oplog/known_history.rs index 2ad2d26c5..b46397b9c 100644 --- a/crates/loro-internal/src/oplog/known_history.rs +++ b/crates/loro-internal/src/oplog/known_history.rs @@ -26,20 +26,33 @@ use crate::op::FutureInnerContent; /// How to compare imported op values with known history. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum ImportedValues { - /// Compare every value exactly. + /// Test-only baseline for checking which equivalences Lossy adds. Production + /// imports all use Lossy because binary history can also have a JSON ancestor. + #[cfg(test)] Exact, - /// Allow only JSON's lossy value representations, recursively. Binary imports - /// need this too: their history may have been relayed through JSON earlier. + /// Compare values modulo JSON conversions, recursively, and bypass ambiguous + /// payload kinds. Binary imports need this too: their history may have been + /// relayed through JSON earlier. Lossy, } impl ImportedValues { fn eq(self, a: &LoroValue, b: &LoroValue) -> bool { + #[cfg(test)] if self == Self::Exact { return a == b; } match (a, b) { + (LoroValue::Double(d), LoroValue::I64(i)) + | (LoroValue::I64(i), LoroValue::Double(d)) => *d == *i as f64, + (LoroValue::String(string), LoroValue::Container(_)) + | (LoroValue::Container(_), LoroValue::String(string)) => { + // A marker-looking string also becomes a container reference. + // JSON peer compression can reinterpret its peer as an index, + // so the decoded id cannot safely be compared with that string. + loro_common::ContainerID::try_from_loro_value_string(string).is_some() + } (LoroValue::Binary(bytes), LoroValue::List(list)) | (LoroValue::List(list), LoroValue::Binary(bytes)) => { bytes.len() == list.len() @@ -62,7 +75,7 @@ impl ImportedValues { && a.iter() .all(|(key, a)| b.get(key).is_some_and(|b| self.eq(a, b))) } - _ => Self::Exact.eq(a, b), + _ => a == b, } } } @@ -317,13 +330,23 @@ fn op_eq(arena: &SharedArena, values: ImportedValues, a: &Op, b: &Op) -> bool { ( crate::op::FutureInnerContent::Unknown { prop: a_prop, - value: a_value, + value: _a_value, }, crate::op::FutureInnerContent::Unknown { prop: b_prop, - value: b_value, + value: _b_value, }, - ) => a_prop == b_prop && a_value == b_value, + ) => { + // A newer container's payload may contain LoroValues or arena + // indices whose meaning this version cannot interpret. Keep the + // old JSON value bypass for this kind, including binary relays. + a_prop == b_prop + && match values { + ImportedValues::Lossy => true, + #[cfg(test)] + ImportedValues::Exact => _a_value == _b_value, + } + } #[allow(unreachable_patterns)] _ => false, }, @@ -528,7 +551,6 @@ mod tests { LoroValue::Bool(true), LoroValue::I64(1), LoroValue::I64(2), - LoroValue::Double(1.0), LoroValue::Double(1.5), LoroValue::from("one"), LoroValue::from("two"), @@ -547,4 +569,119 @@ mod tests { } } } + + #[test] + fn integral_doubles_match_only_the_same_integer() { + for integer in [0, 2, -2, 9_007_199_254_740_991, i64::MIN, i64::MAX] { + let double = LoroValue::Double(integer as f64); + let integer = LoroValue::I64(integer); + assert_lossy_pair(&double, &integer, true); + assert!(!ImportedValues::Exact.eq(&double, &integer)); + } + assert_lossy_pair(&LoroValue::Double(-0.0), &LoroValue::I64(0), true); + for number in [2.5, 3.0, f64::NAN, f64::INFINITY] { + assert_lossy_pair(&LoroValue::Double(number), &LoroValue::I64(2), false); + } + } + + #[test] + fn container_marker_strings_are_ambiguous_after_peer_compression() { + use loro_common::{ContainerID, ContainerType}; + let id = ContainerID::new_root("target", ContainerType::Text); + let marker = LoroValue::from(id.to_loro_value_string()); + let container = LoroValue::Container(id.clone()); + assert_lossy_pair(&marker, &container, true); + assert!(!ImportedValues::Exact.eq(&marker, &container)); + for string in ["plain text", "🦜:invalid"] { + assert_lossy_pair(&LoroValue::from(string), &container, false); + } + assert_lossy_pair( + &LoroValue::from("🦜:cid:0@0:Text"), + &LoroValue::Container(ContainerID::new_normal( + loro_common::ID::new(1, 0), + ContainerType::Text, + )), + true, + ); + assert_lossy_pair(&marker, &LoroValue::from("🦜:cid:root-other:Text"), false); + assert_lossy_pair( + &container, + &LoroValue::Container(ContainerID::new_root("other", ContainerType::Text)), + false, + ); + } + + #[test] + fn unknown_payload_bypass_still_compares_props_and_containers() { + use crate::{ + arena::SharedArena, + encoding::OwnedValue, + op::{FutureInnerContent, InnerContent, Op}, + }; + use loro_common::{ContainerID, ContainerType}; + let arena = SharedArena::new(); + let container = + arena.register_container(&ContainerID::new_root("future", ContainerType::Unknown(9))); + let make_op = |prop, value| Op { + counter: 0, + container, + content: InnerContent::Future(FutureInnerContent::Unknown { + prop, + value: Box::new(value), + }), + }; + let a = make_op(1, OwnedValue::LoroValue(LoroValue::Double(2.0))); + let b = make_op(1, OwnedValue::LoroValue(LoroValue::I64(2))); + assert!(super::op_eq(&arena, ImportedValues::Lossy, &a, &b)); + assert!(!super::op_eq(&arena, ImportedValues::Exact, &a, &b)); + let different_prop = make_op(2, OwnedValue::LoroValue(LoroValue::I64(2))); + assert!(!super::op_eq( + &arena, + ImportedValues::Lossy, + &a, + &different_prop + )); + let mut different_container = b; + different_container.container = + arena.register_container(&ContainerID::new_root("other", ContainerType::Unknown(9))); + assert!(!super::op_eq( + &arena, + ImportedValues::Lossy, + &a, + &different_container + )); + } + + #[cfg(feature = "counter")] + #[test] + fn counter_values_keep_numeric_equality_in_lossy_mode() { + use crate::{ + arena::SharedArena, + op::{FutureInnerContent, InnerContent, Op}, + }; + use loro_common::{ContainerID, ContainerType}; + let arena = SharedArena::new(); + let container = + arena.register_container(&ContainerID::new_root("counter", ContainerType::Counter)); + let make_op = |value| Op { + counter: 0, + container, + content: InnerContent::Future(FutureInnerContent::Counter(value)), + }; + for (a, b, equal) in [ + (2.0, 2.0, true), + (2.0, 3.0, false), + (-0.0, 0.0, true), + (f64::NAN, f64::NAN, true), + (f64::INFINITY, f64::INFINITY, true), + (f64::NAN, f64::INFINITY, false), + ] { + for values in [ImportedValues::Lossy, ImportedValues::Exact] { + assert_eq!( + super::op_eq(&arena, values, &make_op(a), &make_op(b)), + equal + ); + } + } + } } diff --git a/crates/loro/tests/import_json_relay.rs b/crates/loro/tests/import_json_relay.rs index c85625818..805f8905e 100644 --- a/crates/loro/tests/import_json_relay.rs +++ b/crates/loro/tests/import_json_relay.rs @@ -3,7 +3,8 @@ //! replacing the receiver's original values. use loro::{ - ExpandType, ExportMode, LoroDoc, LoroError, LoroValue, StyleConfig, ValueOrContainer, ID, + ContainerID, ContainerType, ExpandType, ExportMode, JsonFutureOp, JsonOpContent, LoroDoc, + LoroError, LoroValue, StyleConfig, ValueOrContainer, ID, }; fn source() -> LoroDoc { @@ -15,10 +16,30 @@ fn source() -> LoroDoc { doc } -fn json_copy(doc: &LoroDoc) -> LoroDoc { +fn js_json(doc: &LoroDoc) -> String { let json = doc.export_json_updates(&Default::default(), &doc.oplog_vv()); - // A typed JsonSchema does not lose values: exercise the actual JSON wire path. - let json = serde_json::to_string(&json).unwrap(); + let mut json = serde_json::to_value(&json).unwrap(); + // Model JS numbers / JSON.stringify: 2.0 is written as 2. A typed JsonSchema + // or a Rust-only string round trip would preserve the Double in this case. + fn canonicalize_numbers(value: &mut serde_json::Value) { + match value { + serde_json::Value::Number(number) => { + let n = number.as_f64().unwrap(); + if number.is_f64() && n.fract() == 0.0 && n.abs() <= 9_007_199_254_740_991.0 { + *value = serde_json::Value::from(n as i64); + } + } + serde_json::Value::Array(values) => values.iter_mut().for_each(canonicalize_numbers), + serde_json::Value::Object(values) => values.values_mut().for_each(canonicalize_numbers), + _ => {} + } + } + canonicalize_numbers(&mut json); + serde_json::to_string(&json).unwrap() +} + +fn json_copy(doc: &LoroDoc) -> LoroDoc { + let json = js_json(doc); let copy = LoroDoc::new(); copy.set_peer_id(2).unwrap(); copy.import_json_updates(json.as_str()).unwrap(); @@ -50,18 +71,38 @@ fn assert_relay_keeps_prefix(a: &LoroDoc, check_loss: impl Fn(&LoroDoc)) { for tail in 1..=2 { b.get_map("relay").insert("extra", tail).unwrap(); b.commit(); - for mode in [ExportMode::all_updates(), ExportMode::Snapshot] { - let blob = b.export(mode).unwrap(); + for mode in [ + Some(ExportMode::all_updates()), + Some(ExportMode::Snapshot), + None, + ] { + let blob = mode.map(|mode| b.export(mode).unwrap()); for batch in [false, true] { + if batch && blob.is_none() { + continue; + } let c = LoroDoc::new(); c.set_peer_id(3).unwrap(); c.import(&prefix).unwrap(); let expected = c.get_deep_value().into_map().unwrap(); let expected_style = style_delta(&c); - if batch { - c.import_batch(&[blob.clone()]).unwrap(); - } else { - c.import(&blob).unwrap(); + let prefix_vv = c.oplog_vv(); + let expected_history = + serde_json::to_string(&c.export_json_updates_without_peer_compression( + &Default::default(), + &prefix_vv, + )) + .unwrap(); + match &blob { + Some(blob) if batch => { + c.import_batch(&[blob.clone()]).unwrap(); + } + Some(blob) => { + c.import(blob).unwrap(); + } + None => { + c.import_json_updates(js_json(&b).as_str()).unwrap(); + } } let actual = c.get_deep_value().into_map().unwrap(); @@ -71,6 +112,14 @@ fn assert_relay_keeps_prefix(a: &LoroDoc, check_loss: impl Fn(&LoroDoc)) { assert_eq!(actual.len(), expected.len() + 1); assert_eq!(actual["relay"]["extra"], LoroValue::from(tail)); assert_eq!(style_delta(&c), expected_style); + assert_eq!( + serde_json::to_string(&c.export_json_updates_without_peer_compression( + &Default::default(), + &prefix_vv + )) + .unwrap(), + expected_history + ); assert_eq!(c.oplog_vv(), b.oplog_vv()); } } @@ -81,19 +130,40 @@ fn assert_relay_keeps_prefix(a: &LoroDoc, check_loss: impl Fn(&LoroDoc)) { let c = json_copy(a); let expected = c.get_deep_value().into_map().unwrap(); let expected_style = style_delta(&c); + let prefix_vv = c.oplog_vv(); + let expected_history = serde_json::to_string( + &c.export_json_updates_without_peer_compression(&Default::default(), &prefix_vv), + ) + .unwrap(); a.get_map("relay").insert("extra", 3).unwrap(); a.commit(); - for mode in [ExportMode::all_updates(), ExportMode::Snapshot] { + for mode in [ + Some(ExportMode::all_updates()), + Some(ExportMode::Snapshot), + None, + ] { let receiver = LoroDoc::new(); receiver .import(&c.export(ExportMode::all_updates()).unwrap()) .unwrap(); - receiver.import(&a.export(mode).unwrap()).unwrap(); + if let Some(mode) = mode { + receiver.import(&a.export(mode).unwrap()).unwrap(); + } else { + receiver.import_json_updates(js_json(a).as_str()).unwrap(); + } let actual = receiver.get_deep_value().into_map().unwrap(); for (key, value) in expected.iter() { assert_eq!(actual.get(key), Some(value), "JSON prefix root {key}"); } assert_eq!(style_delta(&receiver), expected_style); + assert_eq!( + serde_json::to_string( + &receiver + .export_json_updates_without_peer_compression(&Default::default(), &prefix_vv) + ) + .unwrap(), + expected_history + ); assert_eq!(actual.len(), expected.len() + 1); assert_eq!(actual["relay"]["extra"], LoroValue::I64(3)); assert_eq!(receiver.oplog_vv(), a.oplog_vv()); @@ -140,6 +210,105 @@ fn non_finite_map_values_survive_json_then_binary_relay() { } } +#[test] +fn integral_double_survives_json_and_binary_relays() { + for number in [2.0, -2.0, -0.0] { + let a = source(); + a.get_map("m") + .insert("number", LoroValue::Double(number)) + .unwrap(); + assert_relay_keeps_prefix(&a, |b| { + assert_eq!(map_value(b, "number"), LoroValue::I64(number as i64)); + }); + } +} + +#[test] +fn integral_double_json_reimport_into_original_writer_succeeds() { + let a = source(); + a.get_map("m") + .insert("number", LoroValue::Double(2.0)) + .unwrap(); + a.commit(); + let b = json_copy(&a); + assert_eq!(map_value(&b, "number"), LoroValue::I64(2)); + b.get_map("relay").insert("extra", 1).unwrap(); + b.commit(); + a.import_json_updates(js_json(&b).as_str()).unwrap(); + assert_eq!(map_value(&a, "number"), LoroValue::Double(2.0)); + assert_eq!(a.get_map("relay").get_value()["extra"], LoroValue::I64(1)); + assert_eq!(a.oplog_vv(), b.oplog_vv()); +} + +#[test] +fn container_marker_string_survives_json_and_binary_relays() { + let a = source(); + // The string uses compressed peer index 0, which the JSON importer maps to + // actual peer 1. Its counter matches the map op, so this is accepted JSON. + a.get_map("m").insert("cid", "🦜:cid:0@0:Text").unwrap(); + assert_relay_keeps_prefix(&a, |b| { + assert!(matches!( + b.get_map("m").get("cid"), + Some(ValueOrContainer::Container(_)) + )); + assert_eq!( + b.get_map("m").get_value()["cid"], + LoroValue::Container(ContainerID::new_normal(ID::new(1, 0), ContainerType::Text)) + ); + }); +} + +#[test] +fn tagged_counter_values_keep_float_semantics_through_json() { + let a = source(); + for increment in [2.0, -0.25, 0.5] { + a.get_counter("counter").increment(increment).unwrap(); + a.commit(); + } + assert_relay_keeps_prefix(&a, |b| { + assert_eq!(b.get_counter("counter").get_value(), 2.25) + }); +} + +#[test] +fn non_finite_counter_json_remains_invalid_before_history_comparison() { + for increment in [f64::NAN, f64::INFINITY, f64::NEG_INFINITY] { + let a = source(); + a.get_counter("counter").increment(increment).unwrap(); + a.commit(); + let b = source(); + let before = b.oplog_vv(); + assert!(matches!( + b.import_json_updates(js_json(&a).as_str()), + Err(LoroError::InvalidJsonSchema) + )); + assert_eq!(b.oplog_vv(), before); + } +} + +#[test] +fn unknown_op_payload_survives_json_and_binary_relays() { + let template = source(); + template.get_counter("unknown").increment(1.0).unwrap(); + template.commit(); + let mut json = template + .export_json_updates_without_peer_compression(&Default::default(), &template.oplog_vv()); + let op = &mut json.changes[0].ops[0]; + op.container = ContainerID::new_root("unknown", ContainerType::Unknown(9)); + let JsonOpContent::Future(future) = &mut op.content else { + unreachable!() + }; + // A newer container can carry arbitrary nested LoroValues in its opaque op. + // The typed schema preserves Double(2.0); a JS relay turns it into I64(2). + future.value = JsonFutureOp::Unknown( + serde_json::from_str(r#"{"value_type":"loro_value","value":{"nested":[2.0]}}"#).unwrap(), + ); + let a = source(); + a.import_json_updates(json).unwrap(); + let original = js_json(&a); + assert_relay_keeps_prefix(&a, |b| assert_eq!(js_json(b), original)); +} + #[test] fn style_values_survive_json_then_binary_relay() { for value in [LoroValue::from(vec![0u8, 255]), LoroValue::Double(f64::NAN)] { @@ -162,6 +331,7 @@ fn nested_and_sequence_values_survive_json_then_binary_relay() { vec![ LoroValue::from(vec![1u8, 2]), LoroValue::Double(f64::INFINITY), + LoroValue::Double(2.0), ] .into(), ), @@ -212,6 +382,7 @@ fn genuine_value_conflicts_are_rejected_on_json_and_binary_imports() { for (local, conflicting) in [ (LoroValue::from(vec![1u8, 2]), LoroValue::from(vec![1, 3])), (LoroValue::Double(f64::NAN), LoroValue::I64(0)), + (LoroValue::Double(2.0), LoroValue::I64(3)), (nested(1), nested(2)), ] { // Check every operation that carries a value, including arena-backed inserts. From 54da34ad0c4dfeab2c366205bbb2dcd6550441dd Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Thu, 1 Oct 2026 05:39:14 +0800 Subject: [PATCH 3/3] fix: tolerate large integer JSON rounding See context/import-peer-id-reuse.md for numeric equivalence and WASM conversion limits. Co-Authored-By: GPT-6 --- .changeset/known-history-json-lossy.md | 2 +- context/import-peer-id-reuse.md | 37 ++++- .../loro-internal/src/oplog/known_history.rs | 52 ++++++- crates/loro/tests/import_json_relay.rs | 140 ++++++++++++++++-- 4 files changed, 208 insertions(+), 23 deletions(-) diff --git a/.changeset/known-history-json-lossy.md b/.changeset/known-history-json-lossy.md index e1192a52a..c4d44e999 100644 --- a/.changeset/known-history-json-lossy.md +++ b/.changeset/known-history-json-lossy.md @@ -2,6 +2,6 @@ "loro-crdt": patch --- -Fix a 1.16.4 regression where binary updates or snapshots containing history relayed through JSON were rejected with `UsedOpID`. Known-history checks tolerate binary-to-byte-list, non-finite-number-to-null and integral-Double-to-I64 conversions, including nested values and rich-text styles. Marker-looking strings that JSON turns into Containers and opaque unknown-op payloads retain a value-comparison bypass. The receiver keeps its existing prefix values. +Fix a 1.16.4 regression where binary updates or snapshots containing history relayed through JSON were rejected with `UsedOpID`. Known-history checks tolerate binary-to-byte-list, non-finite-number-to-null and integral-Double-to-I64 conversions, including nested values and rich-text styles. They also tolerate large-integer rounding through JS numbers: I64/I64 or I64/Double pairs match if they convert to the same f64 and at least one value is outside `[-2^53, 2^53]`; integers inside that range are still compared exactly. Marker-looking strings that JSON turns into Containers and opaque unknown-op payloads retain a value-comparison bypass. The receiver keeps its existing prefix values. The JSON format is unchanged: a Binary value still comes back from JSON as a list, including mergeable container markers; this fix does not restore values lost during JSON import. JSON imports now compare other representable value payloads rather than skipping all value comparisons, so genuinely different values may be rejected where they were previously silently accepted. Real peer-id conflicts that look like the permitted conversions or use the bypassed payload kinds remain indistinguishable and can still pass. diff --git a/context/import-peer-id-reuse.md b/context/import-peer-id-reuse.md index 7145d3ffe..294567855 100644 --- a/context/import-peer-id-reuse.md +++ b/context/import-peer-id-reuse.md @@ -60,7 +60,23 @@ is per atom range, not per change: JS numbers erase the distinction between an integral Double and I64; JSON text written by JS also drops the `.0` (`crates/loro-wasm/src/convert.rs`, `js_json_schema_to_loro_json_schema`; `LoroValueVisitor` in `loro-common/src/value.rs`). + Two I64s also match when they convert to the same f64 and at least one has + `|i| > 2^53`: a JS-number relay can round `2^60 + 1` to `2^60`. The same f64 + comparison permits an I64 versus its rounded Double. Inside `[-2^53, 2^53]`, + distinct integers still differ and Double/I64 equality is exact numerically. The test-only `ImportedValues::Exact` provides the strict baseline for unit tests. +- **Large-integer JSON paths.** Native JSON text preserves an I64, but parsing it + into JS numbers can lose precision. The WASM object importer uses + `serde_wasm_bindgen::Deserializer::deserialize_any`, which reads numbers outside + JS's safe integer range (`|n| > 2^53 - 1`) as Double. The generic + `js_value_to_loro_value` also returns Double for `|n| > 2^53`, but returns I64 + at the boundary; a JS-written JSON string can decode a rounded I64 instead. + Both representations are tolerated, including JS's shortest decimal spelling + (for example `JSON.stringify(2 ** 60)` writes `1152921504606847000`, which converts + to the same f64). The direct WASM schema exporter currently uses + `serde_wasm_bindgen::Serializer::serialize_i64`, which rejects unsafe I64s + rather than rounding them. The tolerance covers history relayed through JS + parsing native JSON; it does not change that exporter's existing limitation. - **Ambiguous payload kinds.** A valid `🦜:cid:` string becomes a Container in JSON (`LoroValueVisitor::visit_str`). JSON peer compression can then reinterpret the string's peer as an index, so a marker-string versus Container pair is @@ -75,9 +91,7 @@ is per atom range, not per change: Finite increments therefore remain comparable numerically, including integral increments. NaN equals NaN for binary reimports. A non-finite counter increment serialized as JSON text is already invalid at decode because its tagged f64 - becomes null; this is not a new known-history rejection. WASM's configured - `serde_wasm_bindgen` serializer rejects I64 values outside JS's safe integer - range instead of rounding them, so no I64-to-I64 rounding equivalence is added. + becomes null; this is not a new known-history rejection. - **Other values** are compared exactly. Unlike main's blanket JSON value bypass, JSON imports now reject genuinely different representable payloads. Text, op container ids, positions, keys, element ids, style metadata and deps still @@ -86,8 +100,9 @@ is per atom range, not per change: This fixes the 1.16.4 rejection of JSON-relayed history without changing the JSON format: a JSON importer still gets a list instead of `Binary` (including mergeable container markers), `Null` instead of a non-finite double, an I64 instead of an - integral Double, or a Container instead of a marker-looking string. It does not - repair that importer's lost values or make its state identical to the binary receiver's. + integral Double, a rounded large integer, or a Container instead of a + marker-looking string. It does not repair that importer's lost values or make + its state identical to the binary receiver's. - History below the shallow root, or any local change `get_change` cannot find, is not compared. The import then behaves as it did before this check. @@ -96,6 +111,10 @@ is per atom range, not per change: - **Conflicts indistinguishable from JSON loss.** Two actual writers using the same op id can store Binary versus a list of the same bytes, NaN/positive or negative infinity versus null, or an integral Double versus the matching I64. + Different large integers (or a large integer versus a Double) also pass when + they convert to the same f64 and at least one is outside `[-2^53, 2^53]`, such + as `2^60 + 1` versus `2^60`; distinct integers inside that range remain checked + exactly. This ambiguity exists even when both writers stored I64s directly. Such a conflict passes the overlap check even without a JSON relay. A valid marker-string versus Container pair and differing unknown-op payloads also pass under the bypasses above. The receiver keeps its own prefix; the conflicting @@ -131,9 +150,11 @@ for both "all updates + 1 change" and "snapshot + 1 change". Probe: - `crates/loro-wasm/tests/import_reused_peer_id.test.ts`. - `crates/loro/tests/import_json_relay.rs`: JSON-to-binary relays through full updates, snapshots and batch import; binary values, mergeable text/counter, - non-finite/integral doubles, marker strings with peer compression, tagged counter - values and unknown payloads, nested/sequence/style values, both comparison - directions and JSON re-imports, later relay edits, retained prefix history, and + non-finite/integral doubles, rounded large integers as I64 and Double (including + the `2^53` boundary, both signs and i64 extrema), marker strings with peer + compression, tagged counter values and unknown payloads, nested/sequence/style + values, both comparison directions and JSON re-imports, later relay edits, + retained prefix history, and genuine text/value conflicts that must still be rejected. - `known_history.rs` unit tests: the precise value equivalences and rejection boundaries, including nested values, byte-list contents, numeric representations, diff --git a/crates/loro-internal/src/oplog/known_history.rs b/crates/loro-internal/src/oplog/known_history.rs index b46397b9c..26daecbae 100644 --- a/crates/loro-internal/src/oplog/known_history.rs +++ b/crates/loro-internal/src/oplog/known_history.rs @@ -44,8 +44,21 @@ impl ImportedValues { } match (a, b) { + (LoroValue::I64(a), LoroValue::I64(b)) => { + // JS numbers can round distinct large integers to the same f64. + // Keep exact comparison in the range where every integer fits. + const MAX_EXACT_INTEGER: i64 = 1 << 53; + a == b + || ((a.unsigned_abs() > MAX_EXACT_INTEGER as u64 + || b.unsigned_abs() > MAX_EXACT_INTEGER as u64) + && *a as f64 == *b as f64) + } (LoroValue::Double(d), LoroValue::I64(i)) - | (LoroValue::I64(i), LoroValue::Double(d)) => *d == *i as f64, + | (LoroValue::I64(i), LoroValue::Double(d)) => { + // Inside +/-2^53 this is exact numeric equality; outside it also + // accepts the rounded Double produced by a JS-number relay. + *d == *i as f64 + } (LoroValue::String(string), LoroValue::Container(_)) | (LoroValue::Container(_), LoroValue::String(string)) => { // A marker-looking string also becomes a container reference. @@ -571,7 +584,7 @@ mod tests { } #[test] - fn integral_doubles_match_only_the_same_integer() { + fn doubles_match_the_integer_as_f64() { for integer in [0, 2, -2, 9_007_199_254_740_991, i64::MIN, i64::MAX] { let double = LoroValue::Double(integer as f64); let integer = LoroValue::I64(integer); @@ -584,6 +597,41 @@ mod tests { } } + #[test] + fn large_integers_match_only_the_same_js_number() { + const BOUND: i64 = 1 << 53; + const LARGE: i64 = 1 << 60; + for (original, rounded) in [ + (BOUND + 1, BOUND), + (-BOUND - 1, -BOUND), + (LARGE + 1, LARGE), + (LARGE + 1, LARGE + 24), // JSON.stringify spells 2^60 as ...7000. + (-LARGE - 1, -LARGE), + (-LARGE - 1, -LARGE - 24), + (i64::MAX, i64::MAX - 1), + (i64::MIN + 1, i64::MIN), + ] { + let original = LoroValue::I64(original); + for rounded in [LoroValue::I64(rounded), LoroValue::Double(rounded as f64)] { + assert_lossy_pair(&original, &rounded, true); + assert!(!ImportedValues::Exact.eq(&original, &rounded)); + } + } + for (a, b) in [ + (BOUND - 1, BOUND), + (-BOUND + 1, -BOUND), + (BOUND, BOUND + 2), + (-BOUND, -BOUND - 2), + (LARGE, LARGE + 256), + (-LARGE, -LARGE - 256), + (i64::MAX, i64::MAX - 1024), + (i64::MIN, i64::MIN + 1024), + ] { + assert_lossy_pair(&LoroValue::I64(a), &LoroValue::I64(b), false); + assert_lossy_pair(&LoroValue::I64(a), &LoroValue::Double(b as f64), false); + } + } + #[test] fn container_marker_strings_are_ambiguous_after_peer_compression() { use loro_common::{ContainerID, ContainerType}; diff --git a/crates/loro/tests/import_json_relay.rs b/crates/loro/tests/import_json_relay.rs index 805f8905e..0c00c6699 100644 --- a/crates/loro/tests/import_json_relay.rs +++ b/crates/loro/tests/import_json_relay.rs @@ -17,29 +17,57 @@ fn source() -> LoroDoc { } fn js_json(doc: &LoroDoc) -> String { + js_json_numbers(doc, false) +} + +fn js_json_numbers(doc: &LoroDoc, large_as_double: bool) -> String { let json = doc.export_json_updates(&Default::default(), &doc.oplog_vv()); let mut json = serde_json::to_value(&json).unwrap(); - // Model JS numbers / JSON.stringify: 2.0 is written as 2. A typed JsonSchema - // or a Rust-only string round trip would preserve the Double in this case. - fn canonicalize_numbers(value: &mut serde_json::Value) { + // Model JS-number precision and integral-number normalization. Large I64s + // passed through JSON.parse lose bits. In loro-wasm/src/convert.rs, + // js_json_schema_to_loro_json_schema uses serde_wasm_bindgen: unsafe JS + // integers become Double. A JSON string can instead decode a rounded I64; + // use its exact integral representative here (JS's decimal spelling varies). + // The direct WASM schema exporter currently rejects unsafe I64s, so these + // tests model relays parsing native JSON, rather than that exporter. + fn canonicalize_numbers(value: &mut serde_json::Value, large_as_double: bool) { match value { serde_json::Value::Number(number) => { + const BOUND: i64 = 1 << 53; let n = number.as_f64().unwrap(); - if number.is_f64() && n.fract() == 0.0 && n.abs() <= 9_007_199_254_740_991.0 { + let large = number + .as_i64() + .is_some_and(|i| i.unsigned_abs() > BOUND as u64) + || (number.is_f64() && n.abs() >= BOUND as f64); + if large { + *value = if large_as_double || n >= i64::MAX as f64 || n < i64::MIN as f64 { + serde_json::Value::from(n) + } else { + serde_json::Value::from(n as i64) + }; + } else if number.is_f64() && n.fract() == 0.0 { *value = serde_json::Value::from(n as i64); } } - serde_json::Value::Array(values) => values.iter_mut().for_each(canonicalize_numbers), - serde_json::Value::Object(values) => values.values_mut().for_each(canonicalize_numbers), + serde_json::Value::Array(values) => values + .iter_mut() + .for_each(|value| canonicalize_numbers(value, large_as_double)), + serde_json::Value::Object(values) => values + .values_mut() + .for_each(|value| canonicalize_numbers(value, large_as_double)), _ => {} } } - canonicalize_numbers(&mut json); + canonicalize_numbers(&mut json, large_as_double); serde_json::to_string(&json).unwrap() } fn json_copy(doc: &LoroDoc) -> LoroDoc { - let json = js_json(doc); + json_copy_with(doc, js_json) +} + +fn json_copy_with(doc: &LoroDoc, to_json: impl Fn(&LoroDoc) -> String) -> LoroDoc { + let json = to_json(doc); let copy = LoroDoc::new(); copy.set_peer_id(2).unwrap(); copy.import_json_updates(json.as_str()).unwrap(); @@ -62,9 +90,17 @@ fn style_delta(doc: &LoroDoc) -> Option> { } fn assert_relay_keeps_prefix(a: &LoroDoc, check_loss: impl Fn(&LoroDoc)) { + assert_relay_keeps_prefix_with_json(a, js_json, check_loss) +} + +fn assert_relay_keeps_prefix_with_json( + a: &LoroDoc, + to_json: impl Fn(&LoroDoc) -> String, + check_loss: impl Fn(&LoroDoc), +) { a.commit(); let prefix = a.export(ExportMode::all_updates()).unwrap(); - let b = json_copy(a); + let b = json_copy_with(a, &to_json); check_loss(&b); // Full exports must keep working after later relay edits too. @@ -101,7 +137,7 @@ fn assert_relay_keeps_prefix(a: &LoroDoc, check_loss: impl Fn(&LoroDoc)) { c.import(blob).unwrap(); } None => { - c.import_json_updates(js_json(&b).as_str()).unwrap(); + c.import_json_updates(to_json(&b).as_str()).unwrap(); } } @@ -127,7 +163,7 @@ fn assert_relay_keeps_prefix(a: &LoroDoc, check_loss: impl Fn(&LoroDoc)) { // Compare the same representations in the opposite direction: the receiver // owns the JSON prefix and the binary sender owns the original values. - let c = json_copy(a); + let c = json_copy_with(a, &to_json); let expected = c.get_deep_value().into_map().unwrap(); let expected_style = style_delta(&c); let prefix_vv = c.oplog_vv(); @@ -149,7 +185,7 @@ fn assert_relay_keeps_prefix(a: &LoroDoc, check_loss: impl Fn(&LoroDoc)) { if let Some(mode) = mode { receiver.import(&a.export(mode).unwrap()).unwrap(); } else { - receiver.import_json_updates(js_json(a).as_str()).unwrap(); + receiver.import_json_updates(to_json(a).as_str()).unwrap(); } let actual = receiver.get_deep_value().into_map().unwrap(); for (key, value) in expected.iter() { @@ -240,6 +276,82 @@ fn integral_double_json_reimport_into_original_writer_succeeds() { assert_eq!(a.oplog_vv(), b.oplog_vv()); } +#[test] +fn large_integer_survives_json_and_binary_relays_as_i64() { + for number in [ + (1i64 << 60) + 1, + -(1i64 << 60) - 1, + (1i64 << 53) + 1, + -(1i64 << 53) - 1, + ] { + let a = source(); + a.get_map("m").insert("number", number).unwrap(); + assert_relay_keeps_prefix(&a, |b| { + assert_eq!(map_value(b, "number"), LoroValue::I64(number as f64 as i64)); + assert_ne!(map_value(b, "number"), LoroValue::I64(number)); + }); + } +} + +#[test] +fn large_integer_survives_json_and_binary_relays_as_double() { + for number in [ + (1i64 << 60) + 1, + -(1i64 << 60) - 1, + (1i64 << 53) + 1, + -(1i64 << 53) - 1, + i64::MAX, + i64::MIN + 1, + ] { + let a = source(); + let nested = LoroValue::Map( + vec![( + "nested".into(), + LoroValue::from(vec![LoroValue::I64(number)]), + )] + .into(), + ); + a.get_map("m").insert("number", number).unwrap(); + a.get_map("m").insert("nested", nested.clone()).unwrap(); + a.get_list("list").push(nested.clone()).unwrap(); + a.get_text("text").insert(0, "styled").unwrap(); + a.get_text("text").mark(0..6, "custom", nested).unwrap(); + let expected_style = a.get_text("text").to_delta(); + assert_relay_keeps_prefix_with_json( + &a, + |doc| js_json_numbers(doc, true), + |b| { + assert_eq!(map_value(b, "number"), LoroValue::Double(number as f64)); + assert_eq!( + map_value(b, "nested")["nested"][0], + LoroValue::Double(number as f64) + ); + assert_ne!(b.get_text("text").to_delta(), expected_style); + }, + ); + } +} + +#[test] +fn large_integer_json_reimport_into_original_writer_succeeds() { + for large_as_double in [false, true] { + for number in [(1i64 << 60) + 1, -(1i64 << 60) - 1] { + let a = source(); + a.get_map("m").insert("number", number).unwrap(); + a.commit(); + let b = json_copy_with(&a, |doc| js_json_numbers(doc, large_as_double)); + assert_ne!(map_value(&b, "number"), LoroValue::I64(number)); + b.get_map("relay").insert("extra", 1).unwrap(); + b.commit(); + a.import_json_updates(js_json_numbers(&b, large_as_double).as_str()) + .unwrap(); + assert_eq!(map_value(&a, "number"), LoroValue::I64(number)); + assert_eq!(a.get_map("relay").get_value()["extra"], LoroValue::I64(1)); + assert_eq!(a.oplog_vv(), b.oplog_vv()); + } + } +} + #[test] fn container_marker_string_survives_json_and_binary_relays() { let a = source(); @@ -383,6 +495,10 @@ fn genuine_value_conflicts_are_rejected_on_json_and_binary_imports() { (LoroValue::from(vec![1u8, 2]), LoroValue::from(vec![1, 3])), (LoroValue::Double(f64::NAN), LoroValue::I64(0)), (LoroValue::Double(2.0), LoroValue::I64(3)), + (LoroValue::I64((1 << 53) - 1), LoroValue::I64(1 << 53)), + (LoroValue::I64(1 << 53), LoroValue::I64((1 << 53) + 2)), + (LoroValue::I64(1 << 60), LoroValue::I64((1 << 60) + 256)), + (LoroValue::I64(-(1 << 60)), LoroValue::I64(-(1 << 60) - 256)), (nested(1), nested(2)), ] { // Check every operation that carries a value, including arena-backed inserts.