From a03e4b94132da79232641036f6e30708a635965e Mon Sep 17 00:00:00 2001 From: Matt Date: Sun, 11 Oct 2026 07:37:04 -0700 Subject: [PATCH] fix: budget the work 3MF and DXF import does, not only its output 3MF: flatten budgeted triangles but copied the whole vertex array for every placement, so an object with 100,000 vertices and one triangle placed by 500 build items reached 6.8 GB resident, and 10,000 items (still a ~200 KB file) asked for about 140 GB and aborted on the allocation. Copy only the vertices the triangles reach, keeping their relative order so an ordinary mesh is numbered as it was in the file. DXF: every bound in convert was on the geometry that comes out, and a block that emits none tripped none of them. Six self-inserts in one empty block fan out to 6^16 visits, and a 176-byte file never returned. Count entities visited and array cells placed against a work budget, skip the array loop for a block with no geometry, and bound the array size on its own. Fixes #144 Co-Authored-By: Claude Opus 5 (cherry picked from commit 3bf3b68a1340d9bdef8dcefd6c4fb5a7cae6eec4) --- crates/io/src/dxf.rs | 48 +++++++++++++++++++++---- crates/io/src/threemf.rs | 75 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 109 insertions(+), 14 deletions(-) diff --git a/crates/io/src/dxf.rs b/crates/io/src/dxf.rs index 2217220f..9c832bff 100644 --- a/crates/io/src/dxf.rs +++ b/crates/io/src/dxf.rs @@ -90,7 +90,7 @@ pub fn read_dxf(text: &str) -> Result> { } } let mut out = Vec::new(); - convert(&ents, k, &blocks, 0, &mut out)?; + convert(&ents, k, &blocks, 0, &mut out, &mut 0)?; Ok(out) } @@ -120,16 +120,22 @@ fn section(p: &[(i32, String)], start: usize) -> Result> { /// Deepest block nesting expanded. const MAX_BLOCK_DEPTH: usize = 16; +/// Entities visited and array cells placed while expanding blocks. Every bound below this one +/// is on the geometry that comes out, and a block that emits none trips none of them: N blocks +/// that each insert the same empty block N times fan out to N^MAX_BLOCK_DEPTH visits for no +/// output at all. This counts the work instead. +const MAX_WORK: usize = 4 * MAX_ENTITIES; /// Entities to geometry (millimetres), expanding block references. -fn convert(ents: &[Ent], k: f64, blocks: &HashMap)>, depth: usize, out: &mut Vec) -> Result<()> { +fn convert(ents: &[Ent], k: f64, blocks: &HashMap)>, depth: usize, out: &mut Vec, work: &mut usize) -> Result<()> { let pt = |x: f64, y: f64| Vec2::new(x * k, y * k); let mut i = 0; while i < ents.len() { let Some(e) = ents.get(i) else { break }; i += 1; - if out.len() > MAX_ENTITIES { - return Err(IoError::Invalid("too many entities".into())); + *work = work.saturating_add(1); + if out.len() > MAX_ENTITIES || *work > MAX_WORK { + return Err(IoError::Invalid("the drawing is too complex".into())); } // Paper space (layouts) is not the drawing. if e.get(67).is_some_and(|v| v != 0.0) { @@ -142,13 +148,18 @@ fn convert(ents: &[Ent], k: f64, blocks: &HashMap)>, dep continue; } let mut inner = Vec::new(); - convert(list, k, blocks, depth + 1, &mut inner)?; + convert(list, k, blocks, depth + 1, &mut inner, work)?; + if inner.is_empty() { + continue; + } let (sx, sy) = (e.get(41).unwrap_or(1.0), e.get(42).unwrap_or(1.0)); let rot = e.get(50).unwrap_or(0.0).to_radians(); let ins = pt(e.get(10).unwrap_or(0.0), e.get(20).unwrap_or(0.0)); let (cols, rows) = (e.get(70).unwrap_or(1.0).clamp(1.0, 10_000.0) as usize, e.get(71).unwrap_or(1.0).clamp(1.0, 10_000.0) as usize); - if cols.saturating_mul(rows).saturating_mul(inner.len()) > MAX_ENTITIES { - return Err(IoError::Invalid("too many entities".into())); + let cells = cols.saturating_mul(rows); + *work = work.saturating_add(cells); + if cells > MAX_ENTITIES || cells.saturating_mul(inner.len()) > MAX_ENTITIES || *work > MAX_WORK { + return Err(IoError::Invalid("the drawing is too complex".into())); } let (dc, dr) = (e.get(44).unwrap_or(0.0) * k, e.get(45).unwrap_or(0.0) * k); for r in 0..rows { @@ -492,4 +503,27 @@ mod tests { assert_eq!(read_dxf(selfref).unwrap().len(), 16); assert!(read_dxf("AutoCAD Binary DXF\r\n").is_err()); } + + /// Bounds on the geometry that comes out do not see a block that emits none: six + /// self-inserts fan out to 6^16 visits for no output. Both shapes must be refused quickly. + #[test] + fn empty_blocks_do_not_fan_out() { + let t = std::time::Instant::now(); + + let fanout = format!( + "0\nSECTION\n2\nBLOCKS\n0\nBLOCK\n2\nA\n{}0\nENDBLK\n0\nENDSEC\n0\nSECTION\n2\nENTITIES\n0\nINSERT\n2\nA\n0\nENDSEC\n0\nEOF\n", + "0\nINSERT\n2\nA\n".repeat(6) + ); + assert!(read_dxf(&fanout).is_err(), "a block of six self-inserts must be refused"); + + // An empty block placed as a 10,000 by 10,000 array, a thousand times over. + let array = format!( + "0\nSECTION\n2\nBLOCKS\n0\nBLOCK\n2\nE\n0\nENDBLK\n0\nENDSEC\n0\nSECTION\n2\nENTITIES\n{}0\nENDSEC\n0\nEOF\n", + "0\nINSERT\n2\nE\n70\n10000\n71\n10000\n".repeat(1000) + ); + let g = read_dxf(&array).unwrap_or_default(); + assert!(g.is_empty(), "an empty block has no geometry to place"); + + assert!(t.elapsed() < std::time::Duration::from_secs(5), "took {:?}", t.elapsed()); + } } diff --git a/crates/io/src/threemf.rs b/crates/io/src/threemf.rs index 1578a135..f87f8edd 100644 --- a/crates/io/src/threemf.rs +++ b/crates/io/src/threemf.rs @@ -528,20 +528,44 @@ fn flatten(m: &Model, id: u32, xf: &Xf, out: &mut MeshObject, stack: &mut Vec MAX_ELEMENTS { return bad("too many triangles after placing components"); } - let base = u32::try_from(out.positions.len()).map_err(|_| IoError::Invalid("3MF: too many vertices".into()))?; let n = o.positions.len(); - out.positions.extend(o.positions.iter().map(|p| apply(xf, *p) * m.unit)); + // One object can be placed by thousands of build items and components, and each placement + // needs its own transformed copy of the vertices. Copying only the vertices the triangles + // reach keeps that cost tied to the triangle budget above; copying the whole vertex array + // let an object with many vertices and one triangle multiply into tens of gigabytes. + let mut used: Vec = o.triangles.iter().flatten().copied().collect(); + used.sort_unstable(); + used.dedup(); + if used.last().is_some_and(|v| *v as usize >= n) { + return bad(format!("object {id}: triangle refers to a missing vertex")); + } + let base = u32::try_from(out.positions.len()).map_err(|_| IoError::Invalid("3MF: too many vertices".into()))?; + if base as usize + used.len() > u32::MAX as usize { + return bad("too many vertices"); + } + for v in &used { + let Some(p) = o.positions.get(*v as usize) else { + return bad(format!("object {id}: triangle refers to a missing vertex")); + }; + out.positions.push(apply(xf, *p) * m.unit); + } + // Vertices keep their relative order, so a mesh whose triangles use all of them is + // numbered exactly as it was in the file. + let at = |v: u32| used.binary_search(&v).ok().map(|i| base + i as u32); let flip = det(xf) < 0.0; for t in &o.triangles { - if t.iter().any(|v| *v as usize >= n) { - return bad(format!("object {id}: triangle refers to a missing vertex")); + let mut q = [0u32; 3]; + for (slot, v) in q.iter_mut().zip(t) { + let Some(i) = at(*v) else { + return bad(format!("object {id}: triangle refers to a missing vertex")); + }; + *slot = i; } - let t = t.map(|v| v + base); - out.triangles.push(if flip { [t[0], t[2], t[1]] } else { t }); + out.triangles.push(if flip { [q[0], q[2], q[1]] } else { q }); } if out.color.is_none() { out.color = o.material.and_then(|(g, i)| m.materials.get(&g).and_then(|g| g.get(i).copied().flatten())); @@ -734,4 +758,41 @@ mod tests { .is_err() ); } + + /// Every placement needs its own transformed copy of the vertices, so an object with many + /// vertices and one triangle used to cost its whole vertex array per build item: 100k + /// vertices times 10,000 items asked for about 140 GB and the process aborted on the + /// allocation, which no panic guard can intercept. + #[test] + fn many_placements_of_a_big_object_stay_bounded() { + let t = std::time::Instant::now(); + let verts: String = (0..100_000).map(|i| format!("", i % 100, i / 100)).collect(); + let model = format!( + "{}{verts}\ + {}", + r#""#, + "".repeat(10_000) + ); + let (objs, _) = read_3mf(&package(&model, None)).unwrap(); + assert_eq!(objs.len(), 10_000); + // Only the three vertices the one triangle reaches are copied per placement. + assert!(objs.iter().all(|o| o.positions.len() == 3 && o.triangles.len() == 1), "{:?}", objs.first().map(|o| o.positions.len())); + assert!(t.elapsed() < std::time::Duration::from_secs(30), "took {:?}", t.elapsed()); + } + + /// Triangles are budgeted across every placement, so fan-out cannot outrun the cap. + #[test] + fn placements_share_the_triangle_budget() { + let verts: String = (0..30_000).map(|i| format!("", i % 100)).collect(); + let tris: String = (0..10_000).map(|i| format!("", 3 * i, 3 * i + 1, 3 * i + 2)).collect(); + let model = format!( + "{}{verts}{tris}\ + {}", + r#""#, + "".repeat(4_000) + ); + let (objs, warnings) = read_3mf(&package(&model, None)).unwrap(); + assert!(objs.len() < 4_000, "the shared budget must stop the later items: {} built", objs.len()); + assert!(warnings.iter().any(|w| w.contains("too many triangles")), "{warnings:?}"); + } }