shifathosn
/
cadcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

JSON draw/annotation commands refuse invalid sizes and coincident points (fixes #343) (#344) 

CIRCLE/ARC no longer make a negative radius positive; POLYGON refuses a
non-integer sides count and a zero/negative radius or zero-length edge;
DONUT, RECTANG, PLINE, HATCH and TABLE refuse negative or zero sizes
instead of clamping them; LINE drops repeated points like the interactive
command; SPLINE needs two different fit points; DIMRADIUS/DIMDIAMETER/
DIMANGULAR refuse coincident points; DDPTYPE refuses an invalid PDMODE;
DIMSTYLE/DIMOVERRIDE list a non-positive DIMTXT/DIMTFAC or a negative
DIMASZ/DIMSCALE/DIMTSZ as ignored.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Valentin Klein 2026-10-10 23:32:01 +02:00 committed by GitHub
parent 71706f8e6a
commit 7bed82838d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 212 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
crates/doc/src/tables.rs
View File

@ -373,6 +373,17 @@ impl DimStyle {
}
_ => coerced,
};
// Sizes that can't be negative (text height and tolerance scale: not zero either).
let x = coerced.as_f64();
let out_of_range = match field {
"textHeight" | "tolScale" => x.is_some_and(|x| x <= 0.0),
"scale" | "arrowSize" | "tickSize" => x.is_some_and(|x| x < 0.0),
_ => false,
};
if out_of_range {
rejected.push(k.clone());
continue;
}
obj.insert(field.to_string(), coerced);
match serde_json::from_value::<DimStyle>(cur.clone()) {
Ok(ns) => *self = ns,

6
crates/engine/src/cmd/annotate.rs
View File

@ -240,6 +240,9 @@ fn radial(s: &mut Session, p: &Value, diameter: bool) -> Result<Value> {
let at = point_param(p, "at").unwrap_or(c + Vec2::from_angle(std::f64::consts::FRAC_PI_4));
(c, c + (at - c).normalized() * r)
};
if c.near(pt, 1e-12) {
return Err(bad(id, "`point` must differ from `center`"));
}
let k = if diameter {
dim(s, DimKind::Diameter, c + (c - pt), Vec2::ZERO, Vec2::ZERO, pt, Vec2::ZERO, "")
} else {
@ -281,6 +284,9 @@ fn run_angular(s: &mut Session, p: &Value) -> Result<Value> {
let a = point_req("dimangular", p, "p1")?;
let b = point_req("dimangular", p, "p2")?;
let at = point_req("dimangular", p, "at")?;
if v.near(a, 1e-12) || v.near(b, 1e-12) {
return Err(bad("dimangular", "`p1` and `p2` must differ from `vertex`"));
}
let k = dim(s, DimKind::Angular3P, at, a, b, v, Vec2::ZERO, "");
Ok(json!({ "handle": add_dim(s, k)?.hex() }))
}

60
crates/engine/src/cmd/draw.rs
View File

@ -99,6 +99,11 @@ fn run_line(s: &mut Session, p: &Value) -> Result<Value> {
if pts.len() < 2 {
return Err(bad("line", "need at least 2 points"));
}
// A repeated point would make a zero-length line.
let pts = distinct_points(&pts);
if pts.len() < 2 {
return Err(bad("line", "need at least 2 different points"));
}
let mut hs = Vec::new();
for w in pts.windows(2) {
if let [a, b] = w {
@ -108,6 +113,7 @@ fn run_line(s: &mut Session, p: &Value) -> Result<Value> {
if bool_or(p, "closed", false)
&& pts.len() > 2
&& let (Some(a), Some(b)) = (pts.last(), pts.first())
&& !a.near(*b, 1e-12)
{
hs.push(s.add_entity(line(*a, *b))?.hex());
}
@ -123,29 +129,30 @@ fn vertex_value(v: &Value) -> Option<PolyVertex> {
}
fn run_pline(s: &mut Session, p: &Value) -> Result<Value> {
let vs: Vec<PolyVertex> = p
.get("vertices")
.or_else(|| p.get("points"))
.and_then(Value::as_array)
.map(|a| a.iter().filter_map(vertex_value).collect())
.unwrap_or_default();
let raw = p.get("vertices").or_else(|| p.get("points")).and_then(Value::as_array);
for v in raw.into_iter().flatten() {
size_param("pline", v, "startWidth", true)?;
size_param("pline", v, "endWidth", true)?;
}
let vs: Vec<PolyVertex> = raw.map(|a| a.iter().filter_map(vertex_value).collect()).unwrap_or_default();
if vs.len() < 2 {
return Err(bad("pline", "`vertices` needs at least 2 points"));
}
let width = size_param("pline", p, "width", true)?.unwrap_or(0.0);
let mut k = lwpoly(vs, bool_or(p, "closed", false));
if let EntityKind::LwPolyline(pl) = &mut k {
pl.const_width = f64_or(p, "width", 0.0).max(0.0);
pl.const_width = width;
}
added(s.add_entity(k)?)
}
fn run_circle(s: &mut Session, p: &Value) -> Result<Value> {
let c = if let Some(center) = point_param(p, "center") {
let r = p
.get("radius")
.and_then(Value::as_f64)
.or_else(|| p.get("diameter").and_then(Value::as_f64).map(|d| d / 2.0))
.ok_or_else(|| bad("circle", "`radius` or `diameter` is required"))?;
// Checked before `Circle::new`, which would turn a negative radius positive.
let r = match size_param("circle", p, "radius", false)? {
Some(r) => r,
None => size_param("circle", p, "diameter", false)?.ok_or_else(|| bad("circle", "`radius` or `diameter` is required"))? / 2.0,
};
Circle::new(center, r)
} else if let (Some(a), Some(b)) = (point_param(p, "p1"), point_param(p, "p2")) {
match point_param(p, "p3") {
@ -155,7 +162,7 @@ fn run_circle(s: &mut Session, p: &Value) -> Result<Value> {
} else {
return Err(bad("circle", "give {center, radius} or {p1, p2[, p3]}"));
};
if c.radius <= 0.0 || !c.radius.is_finite() {
if !(c.radius > 1e-12 && c.radius.is_finite()) {
return Err(bad("circle", "radius must be positive"));
}
added(s.add_entity(circle(c.center, c.radius))?)
@ -168,6 +175,11 @@ fn run_arc(s: &mut Session, p: &Value) -> Result<Value> {
Arc::from_start_center_end(st, c, e)
} else if let Some(c) = point_param(p, "center") {
let r = f64_req("arc", p, "radius")?;
// Checked before `Arc::new`, which would turn a negative radius positive (zero is
// refused below).
if r < 0.0 {
return Err(bad("arc", "`radius` must be positive"));
}
Arc::new(c, r, f64_req("arc", p, "start")?.to_radians(), f64_req("arc", p, "end")?.to_radians())
} else {
return Err(bad("arc", "give {p1,p2,p3}, {start,center,end} or {center,radius,start,end}"));
@ -179,16 +191,23 @@ fn run_arc(s: &mut Session, p: &Value) -> Result<Value> {
}
fn run_polygon(s: &mut Session, p: &Value) -> Result<Value> {
let n = p.get("sides").and_then(Value::as_u64).unwrap_or(4) as usize;
// A `sides` that is not a whole number (e.g. -3) is refused, not replaced by the default 4.
let n = match p.get("sides") {
None | Some(Value::Null) => 4,
Some(v) => v.as_u64().and_then(|n| usize::try_from(n).ok()).unwrap_or(0),
};
if !(3..=1024).contains(&n) {
return Err(bad("polygon", "sides must be 3..1024"));
}
let vs = if let Some(e) = points_param(p, "edge") {
let (Some(a), Some(b)) = (e.first(), e.get(1)) else { return Err(bad("polygon", "edge needs 2 points")) };
if a.near(*b, 1e-12) {
return Err(bad("polygon", "edge endpoints must differ"));
}
polygon_from_edge(*a, *b, n)
} else {
let c = point_req("polygon", p, "center")?;
let r = f64_req("polygon", p, "radius")?;
let r = size_param("polygon", p, "radius", false)?.ok_or_else(|| bad("polygon", "`radius` (number) is required"))?;
let inscribed = bool_or(p, "inscribed", true);
// Without an angle, the bottom edge is horizontal, as with a typed radius.
let rp = match p.get("angle").and_then(Value::as_f64).filter(|a| a.is_finite()) {
@ -236,8 +255,8 @@ fn run_ray(s: &mut Session, p: &Value) -> Result<Value> {
fn run_spline(s: &mut Session, p: &Value) -> Result<Value> {
let sp = if let Some(f) = points_param(p, "fit") {
if f.len() < 2 {
return Err(bad("spline", "need 2+ fit points"));
if distinct_points(&f).len() < 2 {
return Err(bad("spline", "need 2+ different fit points"));
}
Spline::from_fit_points(&f)
} else if let Some(c) = points_param(p, "control") {
@ -265,7 +284,12 @@ fn donut(center: Vec2, inside: f64, outside: f64) -> EntityKind {
fn run_donut(s: &mut Session, p: &Value) -> Result<Value> {
let c = point_req("donut", p, "center")?;
added(s.add_entity(donut(c, f64_or(p, "inside", 0.5), f64_or(p, "outside", 1.0)))?)
let inside = size_param("donut", p, "inside", true)?.unwrap_or(0.5);
let outside = size_param("donut", p, "outside", false)?.unwrap_or(1.0);
if inside > outside {
return Err(bad("donut", "`inside` must not be larger than `outside`"));
}
added(s.add_entity(donut(c, inside, outside))?)
}
/// Justification code → (halign, valign).

8
crates/engine/src/cmd/draw2.rs
View File

@ -1560,7 +1560,11 @@ pub(crate) fn run_rectang2(s: &mut Session, p: &Value) -> Result<Value> {
.map(|c| (c.first().and_then(Value::as_f64).unwrap_or(0.0), c.get(1).and_then(Value::as_f64).unwrap_or(0.0)))
.map(|(x, y)| (if x.is_finite() { x } else { 0.0 }, if y.is_finite() { y } else { 0.0 }))
.unwrap_or((0.0, 0.0));
let fillet = f64_or(p, "fillet", 0.0);
if ch.0 < 0.0 || ch.1 < 0.0 {
return Err(bad("rectang", "`chamfer` distances must be zero or positive"));
}
let fillet = size_param("rectang", p, "fillet", true)?.unwrap_or(0.0);
let width = size_param("rectang", p, "width", true)?.unwrap_or(0.0);
let (w, h) = if let Some(d) = p.get("dimensions").and_then(Value::as_array) {
let l = d.first().and_then(Value::as_f64).filter(|v| v.is_finite()).ok_or_else(|| bad("rectang", "dimensions: [length, width]"))?;
let wd = d.get(1).and_then(Value::as_f64).filter(|v| v.is_finite()).ok_or_else(|| bad("rectang", "dimensions: [length, width]"))?;
@ -1589,7 +1593,7 @@ pub(crate) fn run_rectang2(s: &mut Session, p: &Value) -> Result<Value> {
if w.abs() < 1e-12 || h.abs() < 1e-12 || !w.is_finite() || !h.is_finite() {
return Err(bad("rectang", "rectangle has no area"));
}
let k = rect_kind(a, w, h, rot, fillet, ch, f64_or(p, "width", 0.0), f64_or(p, "elevation", 0.0));
let k = rect_kind(a, w, h, rot, fillet, ch, width, f64_or(p, "elevation", 0.0));
let hd = s.add_entity(k)?;
let th = f64_or(p, "thickness", 0.0);
if th != 0.0 {

2
crates/engine/src/cmd/hatch.rs
View File

@ -206,6 +206,8 @@ impl HatchSettings {
}
fn make(s: &mut Session, p: &Value, gradient: bool) -> Result<Value> {
// A zero or negative pattern scale would be clamped to a near-zero spacing.
size_param(if gradient { "gradient" } else { "hatch" }, p, "scale", false)?;
let st = HatchSettings::from(s, p, gradient);
let mut loops = Vec::new();
if let Some(pts) = points_param(p, "points") {

23
crates/engine/src/cmd/mod.rs
View File

@ -198,6 +198,29 @@ pub(crate) fn f64_or(p: &Value, key: &str, default: f64) -> f64 {
pub(crate) fn f64_req(cmd: &str, p: &Value, key: &str) -> Result<f64> {
p.get(key).and_then(Value::as_f64).filter(|v| v.is_finite()).ok_or_else(|| bad(cmd, format!("`{key}` (number) is required")))
}
/// An optional size: `None` when absent or null, else a finite number above zero (or zero too
/// with `zero_ok`). Anything else is refused, not clamped or made absolute, so a bad value never
/// makes a degenerate object.
pub(crate) fn size_param(cmd: &str, p: &Value, key: &str, zero_ok: bool) -> Result<Option<f64>> {
match p.get(key) {
None | Some(Value::Null) => Ok(None),
Some(v) => match v.as_f64().filter(|x| x.is_finite()) {
Some(x) if x > 0.0 || (zero_ok && x == 0.0) => Ok(Some(x)),
_ => Err(bad(cmd, format!("`{key}` must be {}", if zero_ok { "zero or positive" } else { "positive" }))),
},
}
}
/// `pts` without consecutive repeats (closer than `1e-12`), as the interactive LINE ignores a
/// point picked twice.
pub(crate) fn distinct_points(pts: &[Vec2]) -> Vec<Vec2> {
let mut out: Vec<Vec2> = Vec::with_capacity(pts.len());
for p in pts {
if !out.last().is_some_and(|l| l.near(*p, 1e-12)) {
out.push(*p);
}
}
out
}
pub(crate) fn bool_or(p: &Value, key: &str, default: bool) -> bool {
p.get(key).and_then(Value::as_bool).unwrap_or(default)
}

9
crates/engine/src/cmd/props.rs
View File

@ -854,8 +854,15 @@ fn run_mleaderstyle(s: &mut Session, p: &Value) -> Result<Value> {
}
fn run_ptype(s: &mut Session, p: &Value) -> Result<Value> {
// PDMODE is a shape 0..=4 plus 32 (circle) and/or 64 (square).
let mode = match p.get("pdmode") {
None | Some(Value::Null) => None,
Some(v) => Some(
v.as_i64().filter(|m| (0..=100).contains(m) && m & !96 <= 4).ok_or_else(|| bad("ddptype", "`pdmode` must be 0..4, plus 32 and/or 64"))?,
),
};
let d = s.doc_mut()?;
if let Some(m) = p.get("pdmode").and_then(Value::as_i64) {
if let Some(m) = mode {
d.header.set_i64("PDMODE", m);
}
if let Some(z) = p.get("pdsize").and_then(Value::as_f64) {

5
crates/engine/src/cmd/table.rs
View File

@ -118,11 +118,12 @@ fn run_table(s: &mut Session, p: &Value) -> Result<Value> {
if rows > MAX_ROWS || cols > MAX_COLS || cols == 0 {
return Err(bad("table", format!("rows must be ≤ {MAX_ROWS} and columns 1..={MAX_COLS}")));
}
let pos = |k: &str| p.get(k).and_then(Value::as_f64).filter(|v| v.is_finite() && *v > 0.0);
let row_height = size_param("table", p, "rowHeight", false)?;
let col_width = size_param("table", p, "colWidth", false)?;
let header: Option<Vec<String>> =
p.get("header").and_then(Value::as_array).map(|a| a.iter().map(|v| v.as_str().unwrap_or("").to_string()).collect());
let st = style_of(s);
let t = make_table(&st, at, rows, cols, pos("rowHeight"), pos("colWidth"), str_param(p, "title"), header, &cells);
let t = make_table(&st, at, rows, cols, row_height, col_width, str_param(p, "title"), header, &cells);
let h = s.add_entity(EntityKind::Table(t))?;
Ok(json!({ "handle": h.hex() }))
}

111
crates/engine/tests/json_param_validation.rs Normal file
View File

@ -0,0 +1,111 @@
//! The JSON forms of the draw and annotation commands refuse sizes and points that would make a
//! degenerate or invalid object (a negative radius made positive, a zero hatch scale, a
//! zero-length line …) instead of adding it (#343).
use cadcraft_engine::Session;
use cadcraft_engine::doc::EntityKind;
use serde_json::{Value, json};
fn count(s: &Session) -> usize {
s.doc().map(|d| d.model.iter().count()).unwrap_or(0)
}
/// A session with a 10×10 square (for HATCH's internal point) and a circle (for DIMRADIUS).
fn session() -> Session {
let mut s = Session::new();
s.execute("rectang", &json!({"p1": [0, 0], "p2": [10, 10]})).unwrap();
s
}
#[test]
fn invalid_sizes_and_points_are_refused() {
let cases: &[(&str, Value, &str)] = &[
("circle", json!({"center": [0, 0], "radius": -5}), "`radius` must be positive"),
("circle", json!({"center": [0, 0], "radius": 1e-300}), "radius must be positive"),
("circle", json!({"center": [0, 0], "diameter": -2}), "`diameter` must be positive"),
("arc", json!({"center": [0, 0], "radius": -5, "start": 0, "end": 90}), "`radius` must be positive"),
("polygon", json!({"sides": -3, "center": [0, 0], "radius": 5}), "sides must be 3..1024"),
("polygon", json!({"sides": 6, "center": [0, 0], "radius": -5}), "`radius` must be positive"),
("polygon", json!({"sides": 6, "center": [0, 0], "radius": 0}), "`radius` must be positive"),
("polygon", json!({"sides": 6, "edge": [[1, 1], [1, 1]]}), "edge endpoints must differ"),
("donut", json!({"center": [0, 0], "inside": -1, "outside": 2}), "`inside` must be zero or positive"),
("donut", json!({"center": [0, 0], "inside": 0, "outside": 0}), "`outside` must be positive"),
("donut", json!({"center": [0, 0], "inside": 5, "outside": 2}), "must not be larger"),
("rectang", json!({"p1": [0, 0], "p2": [10, 5], "fillet": -1}), "`fillet` must be zero or positive"),
("rectang", json!({"p1": [0, 0], "p2": [10, 5], "chamfer": [-1, 1]}), "`chamfer` distances"),
("rectang", json!({"p1": [0, 0], "p2": [10, 5], "width": -1}), "`width` must be zero or positive"),
("pline", json!({"vertices": [[0, 0], [1, 0]], "width": -1}), "`width` must be zero or positive"),
("pline", json!({"vertices": [{"p": [0, 0], "startWidth": -1}, [1, 0]]}), "`startWidth` must be zero or positive"),
("line", json!({"points": [[2, 2], [2, 2]]}), "2 different points"),
("spline", json!({"fit": [[0, 0], [0, 0], [0, 0]]}), "different fit points"),
("hatch", json!({"points": [[1, 1]], "scale": 0}), "`scale` must be positive"),
("hatch", json!({"points": [[1, 1]], "scale": -1}), "`scale` must be positive"),
("table", json!({"at": [0, 0], "rows": 2, "cols": 2, "rowHeight": -1}), "`rowHeight` must be positive"),
("table", json!({"at": [0, 0], "rows": 2, "cols": 2, "colWidth": 0}), "`colWidth` must be positive"),
("dimradius", json!({"center": [3, 3], "point": [3, 3]}), "must differ from `center`"),
("dimdiameter", json!({"center": [3, 3], "point": [3, 3]}), "must differ from `center`"),
("dimangular", json!({"vertex": [0, 0], "p1": [0, 0], "p2": [5, 0], "at": [1, 1]}), "must differ from `vertex`"),
("ddptype", json!({"pdmode": 999}), "`pdmode` must be"),
("ddptype", json!({"pdmode": 5}), "`pdmode` must be"),
];
for (cmd, p, want) in cases {
let mut s = session();
let before = count(&s);
let err = s.execute(cmd, p).expect_err(&format!("{cmd} {p} should be refused")).to_string();
assert!(err.contains(want), "{cmd} {p}: {err}");
assert_eq!(count(&s), before, "{cmd} {p} added an object");
}
}
#[test]
fn valid_values_still_work() {
let cases: &[(&str, Value)] = &[
("circle", json!({"center": [0, 0], "radius": 5})),
("circle", json!({"center": [0, 0], "diameter": 4})),
("arc", json!({"center": [0, 0], "radius": 5, "start": 0, "end": 90})),
("polygon", json!({"center": [0, 0], "radius": 5})),
("polygon", json!({"sides": 6, "edge": [[0, 0], [1, 0]]})),
("donut", json!({"center": [0, 0], "inside": 0, "outside": 2})),
("donut", json!({"center": [0, 0], "inside": 2, "outside": 2})),
("rectang", json!({"p1": [0, 0], "p2": [10, 5], "fillet": 0, "width": 0, "chamfer": [0, 0]})),
("pline", json!({"vertices": [{"p": [0, 0], "startWidth": 0, "endWidth": 1}, [1, 0]], "width": 0.5})),
("hatch", json!({"points": [[1, 1]], "scale": 2})),
("table", json!({"at": [20, 0], "rows": 2, "cols": 2, "rowHeight": 1, "colWidth": 3})),
("dimradius", json!({"center": [3, 3], "point": [5, 3]})),
];
for (cmd, p) in cases {
let mut s = session();
let before = count(&s);
s.execute(cmd, p).unwrap_or_else(|e| panic!("{cmd} {p}: {e}"));
assert_eq!(count(&s), before + 1, "{cmd} {p}");
}
let mut s = Session::new();
for m in [0, 3, 35, 66, 100] {
s.execute("ddptype", &json!({"pdmode": m, "pdsize": -5})).unwrap();
}
}
#[test]
fn repeated_line_points_make_no_zero_length_line() {
let mut s = Session::new();
let r = s.execute("line", &json!({"points": [[0, 0], [0, 0], [5, 0], [5, 0], [5, 5]], "closed": true})).unwrap();
assert_eq!(r["handles"].as_array().map(Vec::len), Some(3), "{r}");
let d = s.doc().unwrap();
assert!(d.model.iter().all(|e| matches!(&e.kind, EntityKind::Line(l) if l.a != l.b)));
}
#[test]
fn dimension_style_sizes_out_of_range_are_ignored() {
let mut s = Session::new();
let r = s.execute("dimstyle", &json!({"name": "D", "DIMTXT": -1, "DIMASZ": -1, "DIMSCALE": -2, "DIMGAP": 0.1})).unwrap();
let ignored: Vec<&str> = r["ignored"].as_array().map(|a| a.iter().filter_map(Value::as_str).collect()).unwrap_or_default();
for k in ["DIMTXT", "DIMASZ", "DIMSCALE"] {
assert!(ignored.contains(&k), "{r}");
}
let st = s.doc().unwrap().dim_style("D").unwrap();
assert!(st.text_height > 0.0 && st.arrow_size >= 0.0 && st.scale >= 0.0);
assert!((st.text_gap - 0.1).abs() < 1e-12);
// Zero arrows and DIMSCALE 0 stay valid.
let r = s.execute("dimstyle", &json!({"name": "D", "DIMASZ": 0, "DIMSCALE": 0})).unwrap();
assert!(r.get("ignored").is_none(), "{r}");
}