diff --git a/apps/solvecraft-cli/tests/design_corpus.rs b/apps/solvecraft-cli/tests/design_corpus.rs index 7cc2fb24..93ab92cd 100644 --- a/apps/solvecraft-cli/tests/design_corpus.rs +++ b/apps/solvecraft-cli/tests/design_corpus.rs @@ -150,6 +150,23 @@ fn try_bytes(bytes: &[u8]) { } } +/// Hostile shapes that no mutation of a saved file produces: plane references that close a +/// cycle, which resolve through the model and back into the document. +fn generated() -> Vec<(String, Vec)> { + let to = |name: &str| json!({"type": "plane", "plane": {"type": "construction", "name": name}}); + let plane = |id: u64, name: &str, side: Value| { + json!({"id": id, "name": name, "suppressed": false, "type": "construction_plane", + "plane": {"type": "midplane", "a": side, "b": {"type": "plane", "plane": {"type": "origin", "name": "XZ"}}}}) + }; + let design = |name: &str, features: Value| { + json!({"format": "solvecraft/11", "name": name, "units": "mm", "params": [], "next_id": 9, "features": features}).to_string().into_bytes() + }; + vec![ + ("construction plane is its own side".into(), design("selfplane", json!([plane(1, "P1", to("P1"))]))), + ("two construction planes refer to each other".into(), design("twoplanes", json!([plane(1, "P1", to("P2")), plane(2, "P2", to("P1"))]))), + ] +} + #[test] fn damaged_and_hostile_design_files_never_panic() { let mut rng = Rng(0x5eed_cafe_f00d_d00d); @@ -201,6 +218,7 @@ fn damaged_and_hostile_design_files_never_panic() { cases.push(("binary".into(), (0..4096u32).map(|i| (i.wrapping_mul(2_654_435_761) >> 24) as u8).collect())); all_cases.extend(cases.into_iter().map(|(what, b)| (name.clone(), what, b))); } + all_cases.extend(generated().into_iter().map(|(what, b)| ("generated".to_string(), what, b))); let next = std::sync::atomic::AtomicUsize::new(0); let threads = std::thread::available_parallelism().map_or(4, |n| n.get()); let failures: Vec = std::thread::scope(|scope| { diff --git a/crates/doc/src/construct.rs b/crates/doc/src/construct.rs index c666faec..d6a7ee8e 100644 --- a/crates/doc/src/construct.rs +++ b/crates/doc/src/construct.rs @@ -105,6 +105,10 @@ pub(crate) struct Ctx<'a> { pub doc: &'a Document, pub vals: &'a BTreeMap, pub st: &'a ModelState, + /// How deep the plane resolution that led here already is. A plane built from model + /// geometry resolves its sides through the document again, so the count has to continue + /// across that hop; restarting it lets a plane that refers to itself recurse forever. + pub depth: usize, } fn bad(m: impl Into) -> DocError { @@ -281,7 +285,7 @@ pub(crate) fn line(ctx: &Ctx, g: &GeoRef) -> Result<(Vec3, Vec3)> { /// A reference as a plane. pub(crate) fn plane(ctx: &Ctx, g: &GeoRef) -> Result { match g { - GeoRef::Plane { plane } => ctx.doc.resolve_plane_in(ctx.vals, plane, 0, Some(ctx.st)), + GeoRef::Plane { plane } => ctx.doc.resolve_plane_in(ctx.vals, plane, ctx.depth + 1, Some(ctx.st)), GeoRef::Face { at, name } => { let (b, fi, q) = face_of(ctx.st, *at, name)?; match kernel::face_surface(&b.body, fi) { @@ -527,7 +531,7 @@ pub(crate) fn resolve_geo_plane(ctx: &Ctx, p: &PlaneRef) -> Result { let (p, tan) = along(&path_points(ctx, path)?, t).ok_or_else(|| bad("the path has no length"))?; Plane::from_normal(p, tan).ok_or_else(degenerate) } - _ => ctx.doc.resolve_plane_in(ctx.vals, p, 0, Some(ctx.st)), + _ => ctx.doc.resolve_plane_in(ctx.vals, p, ctx.depth + 1, Some(ctx.st)), } } diff --git a/crates/doc/src/document.rs b/crates/doc/src/document.rs index fc9b9e29..41cff42a 100644 --- a/crates/doc/src/document.rs +++ b/crates/doc/src/document.rs @@ -2132,7 +2132,7 @@ impl Document { | PlaneRef::AlongPath { .. } | PlaneRef::Perpendicular { .. } => { let st = st.ok_or_else(|| DocError::Invalid("this plane is built from the model: evaluate first".into()))?; - crate::construct::resolve_geo_plane(&crate::construct::Ctx { doc: self, vals, st }, p) + crate::construct::resolve_geo_plane(&crate::construct::Ctx { doc: self, vals, st, depth }, p) } } } diff --git a/crates/doc/src/eval.rs b/crates/doc/src/eval.rs index a54e83a1..90aca9f0 100644 --- a/crates/doc/src/eval.rs +++ b/crates/doc/src/eval.rs @@ -2905,7 +2905,7 @@ fn eval_feature(doc: &Document, vals: &BTreeMap, f: &Feature, st: Ok(()) } FeatureKind::ConstructionAxis { def } => { - let (origin, dir) = crate::construct::resolve_axis(&crate::construct::Ctx { doc, vals, st }, def)?; + let (origin, dir) = crate::construct::resolve_axis(&crate::construct::Ctx { doc, vals, st, depth: 0 }, def)?; st.construct.push(crate::construct::Construct { feature: f.id, name: f.name.clone(), @@ -2914,7 +2914,7 @@ fn eval_feature(doc: &Document, vals: &BTreeMap, f: &Feature, st: Ok(()) } FeatureKind::ConstructionPoint { def } => { - let p = crate::construct::resolve_point(&crate::construct::Ctx { doc, vals, st }, def)?; + let p = crate::construct::resolve_point(&crate::construct::Ctx { doc, vals, st, depth: 0 }, def)?; st.construct.push(crate::construct::Construct { feature: f.id, name: f.name.clone(), geom: crate::construct::ConstructGeom::Point(p) }); Ok(()) } diff --git a/crates/doc/src/tests.rs b/crates/doc/src/tests.rs index 3ea65a68..c4bea724 100644 --- a/crates/doc/src/tests.rs +++ b/crates/doc/src/tests.rs @@ -367,3 +367,36 @@ fn timeline_clones_share_meshes_even_when_cloned_before_tessellation() { assert!(std::sync::Arc::ptr_eq(&a.mesh(), &b.mesh())); assert!(!a.mesh().triangles.is_empty()); } + +/// A construction plane that is its own side resolves through the model and back into the +/// document. The depth count has to survive that hop, or the recursion overflows the stack and +/// aborts the process, which no panic guard can intercept. +#[test] +fn self_referencing_construction_planes_are_errors() { + let selfref = |name: &str| Box::new(construct::GeoRef::Plane { plane: PlaneRef::Construction { name: name.into() } }); + let xz = || Box::new(construct::GeoRef::Plane { plane: PlaneRef::Origin { name: "XZ".into() } }); + + // Directly its own first side. + let mut doc = Document::new("selfplane"); + doc.add_feature(FeatureKind::ConstructionPlane { plane: PlaneRef::Midplane { a: selfref("P1"), b: xz() } }, Some("P1")).unwrap(); + let mut m = Model::new(); + m.evaluate(&doc); + assert!(m.results.iter().any(|r| r.error.is_some()), "a plane that is its own side must report an error: {:?}", m.results); + + // Two planes that are each other's side. + let mut doc = Document::new("twoplanes"); + doc.add_feature(FeatureKind::ConstructionPlane { plane: PlaneRef::Midplane { a: selfref("P2"), b: xz() } }, Some("P1")).unwrap(); + doc.add_feature(FeatureKind::ConstructionPlane { plane: PlaneRef::Midplane { a: selfref("P1"), b: xz() } }, Some("P2")).unwrap(); + let mut m = Model::new(); + m.evaluate(&doc); + assert!(m.results.iter().any(|r| r.error.is_some()), "a two-plane cycle must report an error: {:?}", m.results); + + // A perpendicular plane whose base is itself, and a sketch placed on it. + let mut doc = Document::new("perpplane"); + let line = Box::new(construct::GeoRef::World { axis: "X".into() }); + doc.add_feature(FeatureKind::ConstructionPlane { plane: PlaneRef::Perpendicular { plane: selfref("P1"), line } }, Some("P1")).unwrap(); + doc.add_feature(FeatureKind::Sketch { plane: PlaneRef::Construction { name: "P1".into() }, sketch: Sketch::new() }, Some("Sketch1")).unwrap(); + let mut m = Model::new(); + m.evaluate(&doc); + assert!(m.results.iter().any(|r| r.error.is_some()), "a cyclic perpendicular plane must report an error: {:?}", m.results); +}