From 22b6aed8e5b75fa3170f3ac830aa04c5128abffc Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 21:50:12 +0000 Subject: [PATCH] address review: count enclosing groups against the nesting limit in pattern and mask builders --- crates/pdf/src/build.rs | 14 ++++++++++++-- crates/pdf/src/page.rs | 7 +++++-- crates/pdf/src/tests.rs | 19 +++++++++++++++++++ 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/crates/pdf/src/build.rs b/crates/pdf/src/build.rs index 540a8183a..cfa3a1fcd 100644 --- a/crates/pdf/src/build.rs +++ b/crates/pdf/src/build.rs @@ -23,6 +23,9 @@ pub(crate) struct Builder { pub top_layers: Vec, pub skipped: Vec, pub shapes: usize, + /// Groups the finished nodes will sit under once placed (an enclosing builder's open groups + /// and the pattern or mask groups around them), counted against [`MAX_DEPTH`]. + base: usize, } pub(crate) fn group(name: &str) -> Group { @@ -31,7 +34,14 @@ pub(crate) fn group(name: &str) -> Group { impl Builder { pub fn new() -> Builder { - Builder { stack: vec![(group("page"), Kind::Plain)], top_layers: vec![], skipped: vec![], shapes: 0 } + Builder { stack: vec![(group("page"), Kind::Plain)], top_layers: vec![], skipped: vec![], shapes: 0, base: 0 } + } + + /// A builder for content that will be placed inside this one's open groups and `wrap` more + /// groups (a pattern's `Pattern` and `Tile`, a soft mask's two), so its nesting limit counts + /// them. + pub fn nested(&self, wrap: usize) -> Builder { + Builder { base: self.base.saturating_add(self.stack.len()).saturating_add(wrap), ..Builder::new() } } pub fn depth(&self) -> usize { @@ -68,7 +78,7 @@ impl Builder { /// Open `g` as the group drawn into from now on; past [`MAX_DEPTH`] the group is dropped /// (what would have been drawn inside it goes to the open group) and noted as skipped. fn push_group(&mut self, g: Group, kind: Kind) { - if self.stack.len() >= MAX_DEPTH { + if self.base.saturating_add(self.stack.len()) >= MAX_DEPTH { self.skip("group nesting limit"); return; } diff --git a/crates/pdf/src/page.rs b/crates/pdf/src/page.rs index b7eb70fe0..511ea6e0c 100644 --- a/crates/pdf/src/page.rs +++ b/crates/pdf/src/page.rs @@ -636,7 +636,8 @@ impl<'a> Interp<'a> { let bc = f.get(&sm, "BC").map(|b| f.nums(b)).unwrap_or_default(); let gcs = f.get_dict(&gd, "Group").and_then(|gr| f.get(gr, "CS")).map(|c| color_space(f, c, Some(res))).unwrap_or(Cs::Rgb); let backdrop = if bc.is_empty() { [0.0; 3] } else { gcs.to_rgb(&bc) }; - let saved_b = std::mem::replace(&mut self.b, Builder::new()); + let inner = self.b.nested(2); + let saved_b = std::mem::replace(&mut self.b, inner); let ctm = self.gs.ctm; let saved_gs = std::mem::replace(&mut self.gs, GState { ctm, ..GState::default() }); let saved_stack = std::mem::take(&mut self.saved); @@ -752,7 +753,9 @@ impl<'a> Interp<'a> { return; } // The cell, drawn once in pattern space. - let saved_b = std::mem::replace(&mut self.b, Builder::new()); + // The cell ends up under the open groups and this pattern's `Pattern` and `Tile`. + let inner = self.b.nested(2); + let saved_b = std::mem::replace(&mut self.b, inner); let saved_gs = std::mem::take(&mut self.gs); let saved_stack = std::mem::take(&mut self.saved); let saved_path = std::mem::take(&mut self.path); diff --git a/crates/pdf/src/tests.rs b/crates/pdf/src/tests.rs index 2d09a7406..f0a0c5048 100644 --- a/crates/pdf/src/tests.rs +++ b/crates/pdf/src/tests.rs @@ -668,6 +668,25 @@ fn tiling_patterns_coloured_and_uncoloured() { assert_eq!(at(115, 15)[3], 0.0); } +/// A pattern cell nests under the page's open clipping groups and the pattern's own groups, and +/// the render tree stays within the group depth limit that counts all of them. +#[test] +fn a_pattern_cell_counts_the_groups_it_is_nested_in() { + fn depth(g: &effectcraft_svg::Group) -> usize { + 1 + g.children.iter().filter_map(|c| if let effectcraft_svg::Node::Group(g) = c { Some(depth(g)) } else { None }).max().unwrap_or(0) + } + let clips = "0 0 200 100 re W n ".repeat(250); + let cell = format!("{} 1 0 0 rg 0 0 10 10 re f", "0 0 20 20 re W n ".repeat(250)).into_bytes(); + let bytes = page_pdf( + &format!("{clips} /Pattern cs /P1 scn 0 0 100 100 re f"), + "/Pattern << /P1 10 0 R >>", + vec![(10, "<< /Type /Pattern /PatternType 1 /PaintType 1 /TilingType 1 /BBox [0 0 20 20] /XStep 20 /YStep 20 /Resources << >> >>".into(), Some(cell))], + ); + let doc = parse(&bytes).unwrap(); + assert!(depth(&doc.root) <= 260, "{}", depth(&doc.root)); + assert!(doc.skipped.iter().any(|s| s == "group nesting limit"), "{:?}", doc.skipped); +} + /// Build isolated interpreter objects without presenting a function or colour space as a /// document catalog. Complete-document parsing is covered by the page and reader fixtures. fn object_file(objs: Objs) -> object::File {