Merge pull request #997 from Ni-zav/fix/noissue-anchor-selection-input-s20261010T1654Z-8d74ec

fix(selection): reject malformed anchor references without changing selection

crates/engine/src/cmd/select.rs

@@ -294,17 +294,35 @@ fn key(s: &mut Session, p: &Value) -> Result<Value> {
     ok()
 }
 
-/// `[[subpath, anchor]…]` anchor references (malformed entries are left out).
-pub(crate) fn parse_refs(v: Option<&Value>) -> Vec<(usize, usize)> {
-    v.and_then(Value::as_array)
-        .map(|a| a.iter().filter_map(|x| Some((x.get(0)?.as_u64()? as usize, x.get(1)?.as_u64()? as usize))).collect())
-        .unwrap_or_default()
+/// `[[subpath, anchor]…]` anchor references of `cmd`: a malformed entry fails rather than being
+/// left out, which would act on other anchors than the ones given.
+pub(crate) 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 +341,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/cmd/typecmd.rs

@@ -628,7 +628,7 @@ fn area_options(s: &mut Session, p: &Value) -> Result<Value> {
 fn reshape_area(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "text.reshapeArea";
     let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
-    let mut refs = super::select::parse_refs(p.get("anchors"));
+    let mut refs = super::select::checked_refs(p.get("anchors"), C)?;
     refs.sort_unstable();
     refs.dedup();
     if refs.is_empty() {

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));
+}

crates/engine/src/tests_typearea.rs

@@ -236,6 +236,9 @@ fn reshape_area_takes_untrusted_params() {
         json!({"id": id.0, "dx": 1, "dy": 1}),
         json!({"id": id.0, "anchors": [], "dx": 1, "dy": 1}),
         json!({"id": id.0, "anchors": [["a", 1], [0], "x"], "dx": 1, "dy": 1}),
+        // A malformed reference among good ones fails rather than moving only the good ones.
+        json!({"id": id.0, "anchors": [[0, 2], [0, "3"]], "dx": 1, "dy": 1}),
+        json!({"id": id.0, "anchors": [[0, 2], [0, 3, 1]], "dx": 1, "dy": 1}),
         json!({"id": id.0, "anchors": [[0, 9]], "dx": 1, "dy": 1}),
         json!({"id": id.0, "anchors": [[0, 1], [7, 0]], "dx": 1, "dy": 1}),
         json!({"id": id.0, "anchors": [[0, 2]], "dy": 1}),