fix(selection): reject malformed anchor references before selection changes

crates/engine/src/cmd/select.rs

@@ -301,10 +301,34 @@ pub(crate) fn parse_refs(v: Option<&Value>) -> Vec<(usize, usize)> {
         .unwrap_or_default()
 }
 
+/// Parse an explicitly supplied anchor list without dropping invalid entries.
+fn checked_refs(v: Option<&Value>, cmd: &str) -> Result<Vec<(usize, usize)>> {
+    let refs = v.and_then(Value::as_array).ok_or_else(|| bad(cmd, "`anchors` must be an array of [subpath, anchor] pairs"))?;
+    refs.iter()
+        .map(|v| {
+            let Some([subpath, anchor]) = v.as_array().map(Vec::as_slice) else {
+                return Err(bad(cmd, format!("invalid anchor reference {v}: expected [subpath, anchor]")));
+            };
+            let coordinate = |v: &Value| {
+                v.as_u64()
+                    .and_then(|n| usize::try_from(n).ok())
+                    .ok_or_else(|| bad(cmd, format!("invalid anchor index {v}: expected a non-negative integer")))
+            };
+            Ok((coordinate(subpath)?, coordinate(anchor)?))
+        })
+        .collect()
+}
+
 fn anchors(s: &mut Session, p: &Value) -> Result<Value> {
-    let id = id_param(p, "id").ok_or_else(|| bad("select.anchors", "missing id"))?;
-    let refs = parse_refs(p.get("anchors"));
-    let mode = str_param(p, "mode").unwrap_or("set").to_string();
+    const C: &str = "select.anchors";
+    let id = checked_id(s, p.get("id").ok_or_else(|| bad(C, "missing id"))?, C)?;
+    let refs = checked_refs(p.get("anchors"), C)?;
+    let mode = match str_param(p, "mode") {
+        None | Some("set") => "set",
+        Some("add") => "add",
+        Some("toggle") => "toggle",
+        Some(m) => return Err(bad(C, format!("unknown mode {m:?}"))),
+    };
     s.select(|_, sel| {
         if mode == "set" {
             sel.clear();
@@ -323,11 +347,16 @@ fn anchors(s: &mut Session, p: &Value) -> Result<Value> {
 }
 
 fn anchors_many(s: &mut Session, p: &Value) -> Result<Value> {
-    let items: Vec<(NodeId, BTreeSet<(usize, usize)>)> = p
-        .get("items")
-        .and_then(Value::as_array)
-        .map(|a| a.iter().filter_map(|it| Some((NodeId(it.get("id")?.as_u64()?), parse_refs(it.get("anchors")).into_iter().collect()))).collect())
-        .unwrap_or_default();
+    const C: &str = "select.anchorsMany";
+    let raw = p.get("items").and_then(Value::as_array).ok_or_else(|| bad(C, "`items` must be an array of {id, anchors}"))?;
+    let items: Vec<(NodeId, BTreeSet<(usize, usize)>)> = raw
+        .iter()
+        .map(|it| {
+            let id = checked_id(s, it.get("id").ok_or_else(|| bad(C, "each item needs an id"))?, C)?;
+            let refs = checked_refs(it.get("anchors"), C)?;
+            Ok((id, refs.into_iter().collect()))
+        })
+        .collect::<Result<_>>()?;
     // `add: true` is the older spelling of mode add.
     let mode = match str_param(p, "mode") {
         Some(m @ ("set" | "add" | "toggle" | "subtract")) => m,

crates/engine/src/tests.rs

@@ -960,3 +960,36 @@ fn given_targets_are_used_as_given_and_range_style_takes_the_selected_type() {
     assert!(s.execute("text.setRangeStyle", &json!({"start": 0, "end": 3, "size": 40})).is_err(), "two type objects: which one?");
     assert!(s.execute("text.setRangeStyle", &json!({"id": "two", "size": 40})).is_err());
 }
+
+#[test]
+fn malformed_anchor_selection_never_replaces_or_partially_changes_selection() {
+    let mut s = session();
+    let a = rect(&mut s, 10.0, 10.0, 30.0, 20.0);
+    let b = rect(&mut s, 50.0, 10.0, 30.0, 20.0);
+    let expected = s.doc().unwrap().selection.objects.clone();
+    let expected_anchors = s.doc().unwrap().selection.anchors.clone();
+    for p in [
+        json!({"id": a.0, "anchors": [[0, 0], ["bad", 2]]}),
+        json!({"id": a.0, "anchors": [[0, 0, 1]]}),
+        json!({"id": a.0, "anchors": "not an array"}),
+        json!({"id": u64::MAX, "anchors": []}),
+        json!({"id": a.0, "anchors": [], "mode": "missing"}),
+    ] {
+        assert!(s.execute("select.anchors", &p).is_err(), "{p}");
+        assert_eq!(s.doc().unwrap().selection.objects, expected);
+        assert_eq!(s.doc().unwrap().selection.anchors, expected_anchors);
+    }
+    for p in [
+        json!({"items": [{"id": a.0, "anchors": [[0, 0]]}, {"id": b.0, "anchors": [[0, "bad"]]}]}),
+        json!({"items": [{"id": a.0, "anchors": [[0, 0]]}, {"id": "bad", "anchors": [[0, 0]]}]}),
+        json!({"items": [{"id": u64::MAX, "anchors": []}]}),
+        json!({"items": [{"id": a.0}]}),
+    ] {
+        assert!(s.execute("select.anchorsMany", &p).is_err(), "{p}");
+        assert_eq!(s.doc().unwrap().selection.objects, expected);
+        assert_eq!(s.doc().unwrap().selection.anchors, expected_anchors);
+    }
+    s.execute("select.anchorsMany", &json!({"items": [{"id": a.0, "anchors": [[0, 0]]}]})).unwrap();
+    assert!(s.doc().unwrap().selection.contains(a));
+    assert!(!s.doc().unwrap().selection.contains(b));
+}