Skip to content

Commit eb8ffc2

Browse files
rgbkrkrgbkrk
committed
fix(rust): reject dangling sequence update keys
1 parent 2869377 commit eb8ffc2

2 files changed

Lines changed: 184 additions & 34 deletions

File tree

‎rust/automerge/src/op_set2/change/batch.rs‎

Lines changed: 52 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -917,48 +917,54 @@ impl BatchApply {
917917
let mut edges_by_obj: SmallHashMap<ObjId, SmallHashMap<ElemId, Vec<ElemId>>> =
918918
HashMap::default();
919919
let mut inserts_by_obj: SmallHashMap<ObjId, Vec<(ElemId, ElemId)>> = HashMap::default();
920+
let mut references_by_obj: SmallHashMap<ObjId, Vec<ElemId>> = HashMap::default();
920921

921922
for ops in imported {
922923
for op in ops {
923-
if !op.insert() {
924-
continue;
925-
}
926924
if let KeyRef::Seq(key) = op.key() {
927925
let key = *key;
928-
let id = ElemId(op.id());
929-
edges_by_obj
930-
.entry(op.bld.obj)
931-
.or_default()
932-
.entry(key)
933-
.or_default()
934-
.push(id);
935-
inserts_by_obj
936-
.entry(op.bld.obj)
937-
.or_default()
938-
.push((id, key));
926+
if op.insert() {
927+
let id = ElemId(op.id());
928+
edges_by_obj
929+
.entry(op.bld.obj)
930+
.or_default()
931+
.entry(key)
932+
.or_default()
933+
.push(id);
934+
inserts_by_obj
935+
.entry(op.bld.obj)
936+
.or_default()
937+
.push((id, key));
938+
} else {
939+
references_by_obj.entry(op.bld.obj).or_default().push(key);
940+
}
939941
}
940942
}
941943
}
942944

943-
for (obj, inserts) in inserts_by_obj {
945+
let mut sequence_objs: HashSet<ObjId> = inserts_by_obj.keys().copied().collect();
946+
sequence_objs.extend(references_by_obj.keys().copied());
947+
948+
for obj in sequence_objs {
944949
let mut reachable = HashSet::new();
945950
let mut queue = VecDeque::from([ElemId::head()]);
946951

952+
let mut existing_candidates = HashSet::new();
947953
if let Some(edges) = edges_by_obj.get(&obj) {
948-
for key in edges.keys() {
949-
if key.is_head() {
950-
continue;
951-
}
952-
if let (Some(doc_key), Some(doc_obj)) = (
953-
Self::elem_id_in_current_doc(*key, actors, doc),
954-
Self::obj_id_in_current_doc(obj, actors, doc),
955-
) {
956-
if let Some((op, _)) =
957-
doc.ops().find_op_by_id_and_vis_slow(&doc_key.0, None)
958-
{
959-
if op.obj == doc_obj && op.insert {
960-
queue.push_back(*key);
961-
}
954+
existing_candidates.extend(edges.keys().copied().filter(|key| !key.is_head()));
955+
}
956+
if let Some(references) = references_by_obj.get(&obj) {
957+
existing_candidates.extend(references.iter().copied().filter(|key| !key.is_head()));
958+
}
959+
960+
for key in existing_candidates {
961+
if let (Some(doc_key), Some(doc_obj)) = (
962+
Self::elem_id_in_current_doc(key, actors, doc),
963+
Self::obj_id_in_current_doc(obj, actors, doc),
964+
) {
965+
if let Some((op, _)) = doc.ops().find_op_by_id_and_vis_slow(&doc_key.0, None) {
966+
if op.obj == doc_obj && op.insert {
967+
queue.push_back(key);
962968
}
963969
}
964970
}
@@ -974,11 +980,23 @@ impl BatchApply {
974980
}
975981
}
976982

977-
for (id, key) in inserts {
978-
if !reachable.contains(&id) {
979-
return Err(AutomergeError::InvalidSeqKey(Self::format_elem_id(
980-
key, actors,
981-
)));
983+
if let Some(inserts) = inserts_by_obj.remove(&obj) {
984+
for (id, key) in inserts {
985+
if !reachable.contains(&id) {
986+
return Err(AutomergeError::InvalidSeqKey(Self::format_elem_id(
987+
key, actors,
988+
)));
989+
}
990+
}
991+
}
992+
993+
if let Some(references) = references_by_obj.remove(&obj) {
994+
for key in references {
995+
if !reachable.contains(&key) || key.is_head() {
996+
return Err(AutomergeError::InvalidSeqKey(Self::format_elem_id(
997+
key, actors,
998+
)));
999+
}
9821000
}
9831001
}
9841002
}

‎rust/automerge/tests/test.rs‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1845,6 +1845,138 @@ fn apply_change_with_missing_sequence_insert_key_returns_error_without_panic() {
18451845
);
18461846
}
18471847

1848+
#[test]
1849+
fn apply_change_with_missing_sequence_update_key_returns_error() -> Result<(), AutomergeError> {
1850+
let mut doc = Automerge::new();
1851+
let actor = doc.get_actor().clone();
1852+
let mut tx = doc.transaction();
1853+
let list = tx.put_object(ROOT, "list", ObjType::List)?;
1854+
tx.insert(&list, 0, "seed")?;
1855+
let (heads, _) = tx.commit();
1856+
1857+
let change_actor = ActorId::from(b"change" as &[u8]);
1858+
let change = ExpandedChange {
1859+
operations: vec![automerge::legacy::Op {
1860+
action: automerge::legacy::OpType::Put("orphan".into()),
1861+
obj: automerge::legacy::ObjectId::Id(automerge::legacy::OpId(1, actor.clone())),
1862+
key: automerge::legacy::Key::Seq(automerge::legacy::ElementId::Id(
1863+
automerge::legacy::OpId(999, actor),
1864+
)),
1865+
pred: automerge::legacy::SortedVec::new(),
1866+
insert: false,
1867+
}],
1868+
actor_id: change_actor,
1869+
hash: None,
1870+
seq: 1,
1871+
start_op: std::num::NonZeroU64::MIN,
1872+
time: 0,
1873+
message: None,
1874+
deps: heads.into_iter().collect(),
1875+
extra_bytes: Vec::new(),
1876+
};
1877+
let change: Change = change.into();
1878+
let hash = change.hash();
1879+
1880+
let result = doc.apply_changes([change]);
1881+
1882+
assert!(
1883+
matches!(result, Err(AutomergeError::InvalidSeqKey(_))),
1884+
"expected InvalidSeqKey, got {result:?}"
1885+
);
1886+
assert!(!doc.get_heads().contains(&hash));
1887+
Ok(())
1888+
}
1889+
1890+
#[test]
1891+
fn apply_change_with_missing_sequence_delete_key_returns_error() -> Result<(), AutomergeError> {
1892+
let mut doc = Automerge::new();
1893+
let actor = doc.get_actor().clone();
1894+
let mut tx = doc.transaction();
1895+
let list = tx.put_object(ROOT, "list", ObjType::List)?;
1896+
tx.insert(&list, 0, "seed")?;
1897+
let (heads, _) = tx.commit();
1898+
1899+
let change_actor = ActorId::from(b"change" as &[u8]);
1900+
let change = ExpandedChange {
1901+
operations: vec![automerge::legacy::Op {
1902+
action: automerge::legacy::OpType::Delete,
1903+
obj: automerge::legacy::ObjectId::Id(automerge::legacy::OpId(1, actor.clone())),
1904+
key: automerge::legacy::Key::Seq(automerge::legacy::ElementId::Id(
1905+
automerge::legacy::OpId(999, actor),
1906+
)),
1907+
pred: automerge::legacy::SortedVec::new(),
1908+
insert: false,
1909+
}],
1910+
actor_id: change_actor,
1911+
hash: None,
1912+
seq: 1,
1913+
start_op: std::num::NonZeroU64::MIN,
1914+
time: 0,
1915+
message: None,
1916+
deps: heads.into_iter().collect(),
1917+
extra_bytes: Vec::new(),
1918+
};
1919+
let change: Change = change.into();
1920+
let hash = change.hash();
1921+
1922+
let result = doc.apply_changes([change]);
1923+
1924+
assert!(
1925+
matches!(result, Err(AutomergeError::InvalidSeqKey(_))),
1926+
"expected InvalidSeqKey, got {result:?}"
1927+
);
1928+
assert!(!doc.get_heads().contains(&hash));
1929+
Ok(())
1930+
}
1931+
1932+
#[test]
1933+
fn apply_change_can_reference_same_batch_sequence_insert() -> Result<(), AutomergeError> {
1934+
let mut doc = Automerge::new();
1935+
let actor = doc.get_actor().clone();
1936+
let mut tx = doc.transaction();
1937+
let list = tx.put_object(ROOT, "list", ObjType::List)?;
1938+
let (heads, _) = tx.commit();
1939+
1940+
let change_actor = ActorId::from(b"change" as &[u8]);
1941+
let inserted_id = automerge::legacy::OpId(1, change_actor.clone());
1942+
let change = ExpandedChange {
1943+
operations: vec![
1944+
automerge::legacy::Op {
1945+
action: automerge::legacy::OpType::Put("new".into()),
1946+
obj: automerge::legacy::ObjectId::Id(automerge::legacy::OpId(1, actor.clone())),
1947+
key: automerge::legacy::Key::Seq(automerge::legacy::ElementId::Head),
1948+
pred: automerge::legacy::SortedVec::new(),
1949+
insert: true,
1950+
},
1951+
automerge::legacy::Op {
1952+
action: automerge::legacy::OpType::Put("updated".into()),
1953+
obj: automerge::legacy::ObjectId::Id(automerge::legacy::OpId(1, actor)),
1954+
key: automerge::legacy::Key::Seq(automerge::legacy::ElementId::Id(
1955+
inserted_id.clone(),
1956+
)),
1957+
pred: automerge::legacy::SortedVec::from(vec![inserted_id]),
1958+
insert: false,
1959+
},
1960+
],
1961+
actor_id: change_actor,
1962+
hash: None,
1963+
seq: 1,
1964+
start_op: std::num::NonZeroU64::MIN,
1965+
time: 0,
1966+
message: None,
1967+
deps: heads.into_iter().collect(),
1968+
extra_bytes: Vec::new(),
1969+
};
1970+
1971+
doc.apply_changes([change.into()])?;
1972+
1973+
let Some((value, _)) = doc.get(&list, 0)? else {
1974+
panic!("expected list value");
1975+
};
1976+
assert_eq!(value.to_str(), Some("updated"));
1977+
Ok(())
1978+
}
1979+
18481980
#[test]
18491981
fn can_isolate() -> Result<(), AutomergeError> {
18501982
let mut doc1 = AutoCommit::new();

0 commit comments

Comments
 (0)