fix(path): reject malformed anchors before creating or replacing geometry
This commit is contained in:
parent
93cadc8994
commit
6bf2a42e22
|
|
@ -396,6 +396,41 @@ pub(crate) fn anchor_from_json(v: &Value) -> Option<Anchor> {
|
|||
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<Anchor> {
|
||||
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<Point> {
|
||||
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<Value> {
|
||||
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<Value> {
|
|||
.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::<Result<_>>()?;
|
||||
if anchors.is_empty() {
|
||||
return Err(bad("path.create", "need at least one anchor"));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<Value> {
|
|||
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::<Result<Vec<_>>>()?;
|
||||
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::<Result<Vec<_>>>()?,
|
||||
);
|
||||
s.edit("Reshape", |d, _| {
|
||||
*path_mut(d, id)? = data;
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue