From 6bf2a42e22886556a92b0edefcfd2c507b2f1614 Mon Sep 17 00:00:00 2001 From: Nigel Ihza Firdausy <106657905+Ni-zav@users.noreply.github.com> Date: Sun, 11 Oct 2026 00:00:25 +0700 Subject: [PATCH] fix(path): reject malformed anchors before creating or replacing geometry --- crates/engine/src/cmd/create.rs | 39 +++++++++++++++++++++++++++++++-- crates/engine/src/cmd/path.rs | 21 +++++++++++++----- crates/engine/src/tests.rs | 27 +++++++++++++++++++++++ 3 files changed, 79 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/cmd/create.rs b/crates/engine/src/cmd/create.rs index dae94db3..62ea7778 100644 --- a/crates/engine/src/cmd/create.rs +++ b/crates/engine/src/cmd/create.rs @@ -396,6 +396,41 @@ pub(crate) fn anchor_from_json(v: &Value) -> Option { Some(a) } +/// Strictly parse command-provided anchors: dropping invalid entries would change geometry. +pub(crate) fn checked_anchor_from_json(v: &Value, cmd: &str) -> Result { + let coord = |key: &str| { + v.get(key) + .and_then(Value::as_f64) + .filter(|n| n.is_finite()) + .ok_or_else(|| bad(cmd, format!("anchor `{key}` must be a finite number"))) + }; + let p = Point::new(coord("x")?, coord("y")?); + let handle = |key: &str| -> Result { + match v.get(key) { + None | Some(Value::Null) => Ok(p), + Some(value) => { + let Some([x, y]) = value.as_array().map(Vec::as_slice) else { + return Err(bad(cmd, format!("anchor `{key}` must be [x, y]"))); + }; + let (Some(x), Some(y)) = (x.as_f64(), y.as_f64()) else { + return Err(bad(cmd, format!("anchor `{key}` must contain two numbers"))); + }; + if !x.is_finite() || !y.is_finite() { + return Err(bad(cmd, format!("anchor `{key}` must contain finite numbers"))); + } + Ok(Point::new(x, y)) + } + } + }; + let mut anchor = Anchor::with_handles(p, handle("in")?, handle("out")?); + match v.get("smooth") { + None | Some(Value::Null) | Some(Value::Bool(false)) => {} + Some(Value::Bool(true)) => anchor.kind = AnchorKind::Smooth, + Some(_) => return Err(bad(cmd, "anchor `smooth` must be a boolean")), + } + Ok(anchor) +} + fn path_create(s: &mut Session, p: &Value) -> Result { let path = if let Some(d) = str_param(p, "d") { let bp = vectorcraft_geom::BezPath::from_svg(d).map_err(|e| bad("path.create", format!("bad path data: {e}")))?; @@ -406,8 +441,8 @@ fn path_create(s: &mut Session, p: &Value) -> Result { .and_then(Value::as_array) .ok_or_else(|| bad("path.create", "missing anchors"))? .iter() - .filter_map(anchor_from_json) - .collect(); + .map(|a| checked_anchor_from_json(a, "path.create")) + .collect::>()?; if anchors.is_empty() { return Err(bad("path.create", "need at least one anchor")); } diff --git a/crates/engine/src/cmd/path.rs b/crates/engine/src/cmd/path.rs index a4f6feaf..76367789 100644 --- a/crates/engine/src/cmd/path.rs +++ b/crates/engine/src/cmd/path.rs @@ -6,7 +6,7 @@ use serde_json::{Value, json}; use vectorcraft_doc::{NodeId, NodeKind}; use vectorcraft_geom::{AnchorKind, PathData, Point, SubPath, Vec2}; -use super::create::anchor_from_json; +use super::create::{anchor_from_json, checked_anchor_from_json}; use super::*; use crate::EngineError; @@ -217,12 +217,21 @@ fn set_anchors(s: &mut Session, p: &Value) -> Result { let data = PathData::new( subs.iter() .map(|sp| { - SubPath::new( - sp.get("anchors").and_then(Value::as_array).map(|a| a.iter().filter_map(anchor_from_json).collect()).unwrap_or_default(), - bool_or(sp, "closed", false), - ) + let anchors = sp + .get("anchors") + .and_then(Value::as_array) + .ok_or_else(|| bad("path.setAnchors", "each subpath needs an anchors array"))? + .iter() + .map(|a| checked_anchor_from_json(a, "path.setAnchors")) + .collect::>>()?; + let closed = match sp.get("closed") { + None | Some(Value::Null) => false, + Some(Value::Bool(v)) => *v, + Some(_) => return Err(bad("path.setAnchors", "closed must be a boolean")), + }; + Ok(SubPath::new(anchors, closed)) }) - .collect(), + .collect::>>()?, ); s.edit("Reshape", |d, _| { *path_mut(d, id)? = data; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 60deb977..46cd71d1 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -993,3 +993,30 @@ fn malformed_anchor_selection_never_replaces_or_partially_changes_selection() { assert!(s.doc().unwrap().selection.contains(a)); assert!(!s.doc().unwrap().selection.contains(b)); } + +#[test] +fn path_commands_reject_bad_anchors_before_modifying_geometry() { + let mut s = session(); + let a = rect(&mut s, 10.0, 20.0, 50.0, 30.0); + let original_points: Vec<_> = s.doc().unwrap().doc.node(a).unwrap().path_data().unwrap().anchors().map(|(_, _, an)| an.p).collect(); + let history = s.doc().unwrap().history.undo.len(); + let valid = json!({"x": 10, "y": 20}); + for invalid in [ + json!({"x": "bad", "y": 40}), + json!({"x": 50}), + json!({"x": 50, "y": 40, "in": [0]}), + json!({"x": 50, "y": 40, "out": [1, "bad"]}), + json!({"x": 50, "y": 40, "smooth": "yes"}), + ] { + let anchors = json!([valid.clone(), invalid]); + assert!(s.execute("path.create", &json!({"anchors": anchors})).is_err(), "{anchors}"); + assert!(s.execute("path.setAnchors", &json!({"id": a.0, "subpaths": [{"anchors": anchors}]})).is_err(), "{anchors}"); + let points: Vec<_> = s.doc().unwrap().doc.node(a).unwrap().path_data().unwrap().anchors().map(|(_, _, an)| an.p).collect(); + assert_eq!(points, original_points); + assert_eq!(s.doc().unwrap().history.undo.len(), history); + } + assert!(s.execute("path.setAnchors", &json!({"id": a.0, "subpaths": [{"anchors": []}, {"closed": true}]})).is_err()); + assert_eq!(s.doc().unwrap().history.undo.len(), history); + s.execute("path.setAnchors", &json!({"id": a.0, "subpaths": [{"anchors": [{"x": 0, "y": 0}, {"x": 30, "y": 40}], "closed": false}]})).unwrap(); + assert_eq!(s.doc().unwrap().doc.node(a).unwrap().path_data().unwrap().anchor_count(), 2); +}