From 0eebd533c58c357a1b5b09769b6890066b05d80e Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 25 Oct 2026 16:04:29 -0400 Subject: [PATCH] security(pathops): signed zeros, reported budget rejections, and a fill-only fallback Boxes that touch at -0.0/+0.0 now count as overlapping, so the pair cap cannot be skipped. try_boolean_n, try_unite_all and try_area report a refused input, and the Pathfinder shape-mode unite and the Blob Brush use them to show an error. When the planar map is over its budget, the fallback leaves out paths that fill nothing so a flood of cutting lines cannot take the filled shapes down with it. --- crates/engine/src/cmd/draw2.rs | 2 +- crates/engine/src/cmd/pathops.rs | 2 +- crates/pathops/src/boolean.rs | 37 +++++++++++++++++++++++++++++++- crates/pathops/src/budget.rs | 13 +++++++++++ crates/pathops/src/lib.rs | 4 +++- crates/pathops/src/planar.rs | 25 ++++++++++++++++++--- 6 files changed, 76 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/cmd/draw2.rs b/crates/engine/src/cmd/draw2.rs index ca5ef0909..5016e7405 100644 --- a/crates/engine/src/cmd/draw2.rs +++ b/crates/engine/src/cmd/draw2.rs @@ -1515,7 +1515,7 @@ fn blob(s: &mut Session, p: &Value) -> Result { } let mut inputs: Vec<(&PathData, FillRule)> = vec![(®ion, FillRule::NonZero)]; inputs.extend(merge_ids.iter().map(|(_, pd, r)| (pd, *r))); - let merged = po::unite_all(&inputs); + let merged = po::try_unite_all(&inputs).map_err(|e| EngineError::Other(e.to_string()))?; let Some(((keep, _, _), rest)) = merge_ids.split_last() else { return insert_look(s, "Blob Brush", path_kind(region), ap, None, keep_selected); }; diff --git a/crates/engine/src/cmd/pathops.rs b/crates/engine/src/cmd/pathops.rs index f898ff40e..dbe1c868e 100644 --- a/crates/engine/src/cmd/pathops.rs +++ b/crates/engine/src/cmd/pathops.rs @@ -452,7 +452,7 @@ fn run_pf(s: &mut Session, op: PathfinderOp) -> Result { let parts: Vec<(PathData, FillRule)> = lv.iter().filter_map(node_path).collect(); let refs: Vec<(&PathData, FillRule)> = parts.iter().map(|(p, r)| (p, *r)).collect(); let name = n.name.clone().or_else(|| lv.iter().find_map(|l| l.name.clone())); - push(po::unite_all(&refs), FillRule::NonZero, style_of(top), name); + push(po::try_unite_all(&refs).map_err(|e| EngineError::Other(e.to_string()))?, FillRule::NonZero, style_of(top), name); } else { for l in &lv { if let Some((p, r)) = node_path(l) { diff --git a/crates/pathops/src/boolean.rs b/crates/pathops/src/boolean.rs index f654ab35e..fb824813c 100644 --- a/crates/pathops/src/boolean.rs +++ b/crates/pathops/src/boolean.rs @@ -663,8 +663,17 @@ pub fn try_boolean(a: &PathData, a_rule: FillRule, b: &PathData, b_rule: FillRul Ok(all_contours_to_path(&c, &tidy)) } +/// [`boolean_n`] that reports a refused input ([`PathOpsError::TooComplex`], non-finite +/// coordinates) instead of returning an empty path. +pub fn try_boolean_n(paths: &[(&PathData, FillRule)], pred: impl Fn(&[bool]) -> bool) -> Result { + let bps: Vec<(BezPath, FillRule)> = paths.iter().map(|(p, r)| (fill_bezpath(p), *r)).collect(); + let arr = Arrangement::new(bps)?; + Ok(all_contours_to_path(&arr.contours(pred), &arr.tidy)) +} + /// N-ary boolean: the area where `pred(inside_flags)` holds, `inside_flags[i]` telling whether a -/// point is inside `paths[i]` under its fill rule. +/// point is inside `paths[i]` under its fill rule. A refused input gives an empty path; callers that +/// can report an error use [`try_boolean_n`]. pub fn boolean_n(paths: &[(&PathData, FillRule)], pred: impl Fn(&[bool]) -> bool) -> PathData { let bps: Vec<(BezPath, FillRule)> = paths.iter().map(|(p, r)| (fill_bezpath(p), *r)).collect(); match Arrangement::new(bps) { @@ -673,6 +682,26 @@ pub fn boolean_n(paths: &[(&PathData, FillRule)], pred: impl Fn(&[bool]) -> bool } } +/// [`unite_all`] that reports a refused input instead of leaving it out of the union or returning +/// an empty path. +pub fn try_unite_all(paths: &[(&PathData, FillRule)]) -> Result { + let mut all = BezPath::new(); + for (p, r) in paths { + let bp = fill_bezpath(p); + if bp.elements().is_empty() { + continue; + } + for ct in normalize_bez(&bp, *r)?.contours() { + all.extend(ct.path.iter()); + } + } + if all.elements().is_empty() { + return Ok(PathData::default()); + } + let tidy = Tidy::keeping(DEFAULT_PRECISION, paths.iter().map(|(p, _)| fill_bezpath(p)).collect::>().iter()); + Ok(all_contours_to_path(&normalize_bez(&all, FillRule::NonZero)?, &tidy)) +} + /// Union of many filled paths. Each input is first normalised on its own, then all are swept /// together with a single integer winding number, which keeps this fast for thousands of shapes. pub fn unite_all(paths: &[(&PathData, FillRule)]) -> PathData { @@ -697,6 +726,12 @@ pub fn unite_all(paths: &[(&PathData, FillRule)]) -> PathData { } } +/// [`area`] that reports a refused input instead of an area of zero. +pub fn try_area(path: &PathData, rule: FillRule) -> Result { + let c = normalize_bez(&fill_bezpath(path), rule)?; + Ok(c.contours().map(|ct| ct.path.area()).sum::().abs()) +} + /// Unsigned filled area of a path under a fill rule (normalises first, so it is exact for /// self-overlapping input). pub fn area(path: &PathData, rule: FillRule) -> f64 { diff --git a/crates/pathops/src/budget.rs b/crates/pathops/src/budget.rs index 8da5ec13a..ab7ead8a6 100644 --- a/crates/pathops/src/budget.rs +++ b/crates/pathops/src/budget.rs @@ -41,6 +41,10 @@ impl Counts { /// in two Fenwick trees over their y extents, so this costs O(n log n) however they lie, and stops /// as soon as the count passes `cap`. pub(crate) fn overlaps_within(boxes: &[Rect], cap: usize) -> bool { + // `f64::total_cmp` puts -0.0 before +0.0 although they are equal, which would hide an overlap + // between a box ending at -0.0 and one starting at +0.0; adding 0.0 maps -0.0 to +0.0. + let boxes: Vec = boxes.iter().map(|b| Rect::new(b.x0 + 0.0, b.y0 + 0.0, b.x1 + 0.0, b.y1 + 0.0)).collect(); + let boxes = boxes.as_slice(); let mut ys: Vec = boxes.iter().flat_map(|b| [b.y0, b.y1]).collect(); ys.sort_by(f64::total_cmp); ys.dedup_by(|a, b| a.total_cmp(b).is_eq()); @@ -98,4 +102,13 @@ mod tests { let apart: Vec = (0..10_000).map(|i| Rect::new(i as f64 * 3.0, 0.0, i as f64 * 3.0 + 1.0, 1.0)).collect(); assert!(overlaps_within(&apart, 0)); } + + /// Boxes that touch at a signed zero still overlap: -0.0 and +0.0 are the same position. + #[test] + fn touching_at_a_signed_zero_counts_as_an_overlap() { + let below = Rect::new(0.0, -1.0, 1.0, -0.0); + let above = Rect::new(0.0, 0.0, 1.0, 1.0); + assert!(!overlaps_within(&[below, above], 0)); + assert!(overlaps_within(&[below, above], 1)); + } } diff --git a/crates/pathops/src/lib.rs b/crates/pathops/src/lib.rs index 28061864e..74f5ac02d 100644 --- a/crates/pathops/src/lib.rs +++ b/crates/pathops/src/lib.rs @@ -15,7 +15,9 @@ mod pathfinder; mod planar; mod trap; -pub use boolean::{BoolOp, DEFAULT_PRECISION, area, boolean, boolean_n, normalize, try_boolean, try_normalize, unite_all}; +pub use boolean::{ + BoolOp, DEFAULT_PRECISION, area, boolean, boolean_n, normalize, try_area, try_boolean, try_boolean_n, try_normalize, try_unite_all, unite_all, +}; pub use edit::{ AverageAxis, SIMPLIFY_LEVEL, SimplifiedPart, SimplifyOptions, add_anchor_points, auto_simplify_level, average, join, remove_anchor, remove_redundant_points, simplify, simplify_level_tolerance, simplify_part, simplify_with, smooth, split_into_grid, diff --git a/crates/pathops/src/planar.rs b/crates/pathops/src/planar.rs index b53f10b69..9f375500b 100644 --- a/crates/pathops/src/planar.rs +++ b/crates/pathops/src/planar.rs @@ -799,7 +799,7 @@ pub fn shape_builder_with_gap(shapes: &[Shape], edges: bool, gap: f64) -> Builde }) .filter(|(faces, pieces)| !faces.is_empty() || !pieces.is_empty()); let Some((faces, pieces)) = planar else { - return BuilderArrangement { regions: crate::regions(shapes), ..Default::default() }; + return BuilderArrangement { regions: filled_regions(shapes), ..Default::default() }; }; let is_line = |i: u64| shapes.get(i as usize).is_some_and(|s| !s.path.subpaths.iter().any(|sp| sp.closed)); let (lines, outline): (Vec, Vec) = pieces.into_iter().partition(|e| is_line(e.key)); @@ -814,6 +814,25 @@ pub fn shape_builder_with_gap(shapes: &[Shape], edges: bool, gap: f64) -> Builde BuilderArrangement { regions, lines: if has_open { lines } else { Vec::new() }, edges: if edges { outline } else { Vec::new() } } } +/// The regions of the shapes' filled areas. A shape that fills nothing (open paths that enclose no +/// area, such as cutting lines) is left out, so a flood of them cannot push the arrangement past its +/// size budget and take the filled shapes down with it; the regions keep the original indices. +fn filled_regions(shapes: &[Shape]) -> Vec { + let fills = |s: &Shape| s.path.subpaths.iter().any(|sp| sp.closed || encloses_area(sp)); + let keep: Vec = (0..shapes.len()).filter(|&i| shapes.get(i).is_some_and(fills)).collect(); + if keep.len() == shapes.len() { + return crate::regions(shapes); + } + let kept: Vec = keep.iter().filter_map(|&i| shapes.get(i).cloned()).collect(); + crate::regions(&kept) + .into_iter() + .map(|mut r| { + r.sources = r.sources.iter().filter_map(|&s| keep.get(s).copied()).collect(); + r + }) + .collect() +} + /// Does `sp`, closed, enclose an area (rather than retrace itself)? The Shape Builder closes the /// open subpaths of filled paths that do. pub fn encloses_area(sp: &SubPath) -> bool { @@ -1907,9 +1926,9 @@ mod tests { fn shape_builder_closed_only_or_degenerate_is_the_filled_arrangement() { let s = [rect(0.0, 0.0, 100.0, 100.0, 0), rect(50.0, 0.0, 150.0, 100.0, 1)]; assert_eq!(shape_builder(&s, false), BuilderArrangement { regions: crate::regions(&s), ..Default::default() }); - // A broken line can't be swept: what the filled areas give (nothing here, without a panic). + // A broken line can't be swept and fills nothing: the filled areas still give their regions. let s = [rect(0.0, 0.0, 100.0, 100.0, 0), line((0.0, 0.0), (f64::NAN, 1.0), 1)]; - assert_eq!(shape_builder(&s, false), BuilderArrangement { regions: crate::regions(&s), ..Default::default() }); + assert_eq!(shape_builder(&s, false), BuilderArrangement { regions: crate::regions(&s[..1]), ..Default::default() }); // Zero-length and doubled-back lines don't break it either. let s = [ rect(0.0, 0.0, 100.0, 100.0, 0),