fix: continue the plane depth count through model geometry

A construction plane built from model geometry resolves each of its sides
through the document again. That hop restarted the recursion depth at 0, so a
plane that is its own side never exceeded depth 1 and the 32-level cap never
fired: a ~600-byte design file recursed until the stack overflowed and the
process aborted, which no panic guard can intercept.

Carry the depth on the construction context so the geometry arms continue the
caller's count instead of restarting it.

Fixes #143

Co-Authored-By: Claude Opus 5 <[email protected]>
(cherry picked from commit 4bca1d49f27dfe9ef0140132746e1c7415daba8e)
This commit is contained in:
Matt committed 2026-10-11 08:22:07 -07:00
1 parent a094fa46b5
commit 0b5e15e2b9
5 files changed
+60 -5

No files matched your search

@@ -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<u8>)> {
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<String> = std::thread::scope(|scope| {
+6 -2
View File
@@ -105,6 +105,10 @@ pub(crate) struct Ctx<'a> {
pub doc: &'a Document,
pub vals: &'a BTreeMap<String, Value>,
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<String>) -> 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<Plane> {
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<Plane> {
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)),
}
}
+1 -1
View File
@@ -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)
}
}
}
+2 -2
View File
@@ -2905,7 +2905,7 @@ fn eval_feature(doc: &Document, vals: &BTreeMap<String, Value>, 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<String, Value>, 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(())
}
+33
View File
@@ -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);
}