M2: ungroup refuses unbalanced content, checks patterns for blending, reads each nested form once

Review follow-ups on #3067: unmatched EMC/BDC/BT/ET are refused; a shading
pattern's ExtGState and a tiling pattern's content count toward blending inside
a transparency group; nested forms with their own resources are cached and the
reads have a ceiling of 1024.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv2WwdgjmBz33GWkwgNn3Q

crates/edit/src/tests.rs

@@ -9043,6 +9043,141 @@ fn ungroup_looks_for_blend_modes_where_they_are_used() {
     assert!(!attempt(b"/Inner Do", Some((b"", "Inner")), true), "a nested form that draws itself");
 }
 
+/// Ungroup the form `Art` drawn by fixture page 1, with `content` and `resources`, as a
+/// transparency group or not; `build` adds what the resources refer to first.
+fn ungroup_art(content: &[u8], group: bool, build: impl FnOnce(&mut Document) -> Dict) -> Result<(), EditError> {
+    let mut doc = fixture();
+    let res = build(&mut doc);
+    let mut form = Dict::new();
+    form.set(b"Subtype".to_vec(), Object::name("Form"));
+    form.set(b"BBox".to_vec(), Object::Array([0, 0, 100, 50].map(Object::Int).to_vec()));
+    if group {
+        let mut g = Dict::new();
+        g.set(b"S".to_vec(), Object::name("Transparency"));
+        form.set(b"Group".to_vec(), Object::Dict(g));
+    }
+    form.set(b"Resources".to_vec(), Object::Dict(res));
+    let artwork = doc.add(Object::Stream(Stream::flate(form, content)));
+    doc.update_dict(pdfcraft_cos::ObjRef::new(6, 0), |d| {
+        let mut xo = Dict::new();
+        xo.set(b"Art".to_vec(), Object::Ref(artwork));
+        d.set(b"XObject".to_vec(), Object::Dict(xo));
+    })
+    .unwrap();
+    doc.set(pdfcraft_cos::ObjRef::new(7, 0), Object::Stream(Stream::flate(Dict::new(), b"q /Art Do Q")));
+    let mut doc = reopen(&doc);
+    images::change_image(&mut doc, 0, 0, &images::ImageChange::Ungroup).map(|_| ())
+}
+
+/// #3067 review: a form whose marked content or text objects don't balance within its own
+/// stream is refused: inlined, an extra EMC would close what the page opened, and one left open
+/// would take in the page's later content.
+#[test]
+fn ungroup_refuses_unbalanced_marked_content_and_text_objects() {
+    assert!(ungroup_art(b"/P BMC BT ET EMC /Q <</MCID 0>> BDC EMC", false, |_| Dict::new()).is_ok(), "balanced");
+    for content in [&b"EMC"[..], b"/P BMC", b"/P <</MCID 0>> BDC", b"BT", b"ET", b"BT BT ET ET", b"BT EMC ET"] {
+        let r = ungroup_art(content, false, |_| Dict::new());
+        assert!(
+            matches!(r, Err(EditError::Invalid(ref m)) if m.contains("marked content") || m.contains("text object")),
+            "{}: {r:?}",
+            String::from_utf8_lossy(content)
+        );
+    }
+}
+
+/// #3067 review: inside a transparency group, painting with a pattern blends when a shading
+/// pattern's own graphics state does, or a tiling pattern's content does.
+#[test]
+fn ungroup_looks_for_blend_modes_in_patterns() {
+    let attempt = |shading_bm: Option<&str>, tiling_content: Option<&[u8]>| {
+        ungroup_art(b"/Pattern cs /P1 scn 0 0 10 10 re f", true, |doc| {
+            let mut states = Dict::new();
+            for (name, bm) in [("Mul", "Multiply"), ("Norm", "Normal")] {
+                let mut gs = Dict::new();
+                gs.set(b"BM".to_vec(), Object::name(bm));
+                states.set(name.as_bytes().to_vec(), Object::Dict(gs));
+            }
+            let pattern = match (shading_bm, tiling_content) {
+                (Some(bm), _) => {
+                    let mut gs = Dict::new();
+                    gs.set(b"BM".to_vec(), Object::name(bm));
+                    let mut p = Dict::new();
+                    p.set(b"PatternType".to_vec(), Object::Int(2));
+                    p.set(b"ExtGState".to_vec(), Object::Dict(gs));
+                    doc.add(Object::Dict(p))
+                }
+                (None, content) => {
+                    let mut p = Dict::new();
+                    p.set(b"PatternType".to_vec(), Object::Int(1));
+                    p.set(b"BBox".to_vec(), Object::Array([0, 0, 10, 10].map(Object::Int).to_vec()));
+                    let mut res = Dict::new();
+                    res.set(b"ExtGState".to_vec(), Object::Dict(states.clone()));
+                    p.set(b"Resources".to_vec(), Object::Dict(res));
+                    doc.add(Object::Stream(Stream::flate(p, content.unwrap_or_default())))
+                }
+            };
+            let mut patterns = Dict::new();
+            patterns.set(b"P1".to_vec(), Object::Ref(pattern));
+            let mut res = Dict::new();
+            res.set(b"Pattern".to_vec(), Object::Dict(patterns));
+            res
+        })
+    };
+    assert!(attempt(Some("Normal"), None).is_ok(), "a shading pattern without blending");
+    assert!(attempt(Some("Multiply"), None).is_err(), "a shading pattern that multiplies");
+    assert!(attempt(None, Some(b"/Norm gs 0 0 1 1 re f")).is_ok(), "a tiling pattern without blending");
+    assert!(attempt(None, Some(b"/Mul gs 0 0 1 1 re f")).is_err(), "a tiling pattern that multiplies");
+}
+
+/// #3067 review: a form drawn many times inside a group is read once (its answer is kept), so
+/// shared forms nested to the limit can't multiply the work; and the reads have a ceiling, past
+/// which the group counts as blending.
+#[test]
+fn ungroup_reads_each_nested_form_once_and_within_a_ceiling() {
+    // Six levels, each drawing the one below ten times: 10^6 reads without the cache.
+    let shared = ungroup_art(&b"/F Do ".repeat(10), true, |doc| {
+        let mut below: Option<pdfcraft_cos::ObjRef> = None;
+        for _ in 0..6 {
+            let mut d = Dict::new();
+            d.set(b"Subtype".to_vec(), Object::name("Form"));
+            d.set(b"BBox".to_vec(), Object::Array([0, 0, 10, 10].map(Object::Int).to_vec()));
+            let mut res = Dict::new();
+            let content = match below {
+                Some(r) => {
+                    let mut xo = Dict::new();
+                    xo.set(b"F".to_vec(), Object::Ref(r));
+                    res.set(b"XObject".to_vec(), Object::Dict(xo));
+                    b"/F Do ".repeat(10)
+                }
+                None => b"0 0 1 1 re f".to_vec(),
+            };
+            d.set(b"Resources".to_vec(), Object::Dict(res));
+            below = Some(doc.add(Object::Stream(Stream::flate(d, &content))));
+        }
+        let mut xo = Dict::new();
+        xo.set(b"F".to_vec(), Object::Ref(below.unwrap()));
+        let mut res = Dict::new();
+        res.set(b"XObject".to_vec(), Object::Dict(xo));
+        res
+    });
+    assert!(shared.is_ok(), "{shared:?}");
+    // 2000 different forms: more reads than the ceiling allows.
+    let many = ungroup_art(&(0..2000).map(|i| format!("/F{i} Do ")).collect::<String>().into_bytes(), true, |doc| {
+        let mut xo = Dict::new();
+        for i in 0..2000 {
+            let mut d = Dict::new();
+            d.set(b"Subtype".to_vec(), Object::name("Form"));
+            d.set(b"BBox".to_vec(), Object::Array([0, 0, 10, 10].map(Object::Int).to_vec()));
+            d.set(b"Resources".to_vec(), Object::Dict(Dict::new()));
+            xo.set(format!("F{i}").into_bytes(), Object::Ref(doc.add(Object::Stream(Stream::flate(d, b"0 0 1 1 re f")))));
+        }
+        let mut res = Dict::new();
+        res.set(b"XObject".to_vec(), Object::Dict(xo));
+        res
+    });
+    assert!(matches!(many, Err(EditError::Invalid(ref m)) if m.contains("blend")), "{many:?}");
+}
+
 /// A transparency group that draws a nested form whose content can't be read is refused: what
 /// it blends can't be known, so it isn't guessed.
 #[test]

crates/edit/src/ungroup.rs

@@ -10,7 +10,10 @@
 //! compositing differs from painting its parts one by one: a knockout group, a group that uses
 //! a blend mode other than Normal inside, or a group drawn translucent, blended or under a soft
 //! mask. Those are refused rather than changed, as is tagged form content (its structure link
-//! would be lost). A form in a layer (`/OC`) keeps its membership as `/OC … BDC … EMC`.
+//! would be lost) and content whose marked content or text objects don't balance (inlined, they
+//! would change the scope of the page's own). A form in a layer (`/OC`) keeps its membership as `/OC … BDC … EMC`.
+
+use std::collections::HashMap;
 
 use pdfcraft_content::{Matrix, Op, parse};
 use pdfcraft_cos::{Dict, Document, Object};
@@ -78,8 +81,8 @@ pub(crate) fn ungroup_ops(doc: &Document, resources: &Dict, name: &[u8], translu
     let mut ops = parsed.ops;
     if group.is_some() {
         // The form itself is being read: drawing itself again counts as blending.
-        let mut seen: Vec<pdfcraft_cos::ObjRef> = form.as_ref().into_iter().collect();
-        if blends_inside(doc, &form_res, &ops, 0, &mut seen) {
+        let mut walk = BlendWalk { path: form.as_ref().into_iter().collect(), known: HashMap::new(), reads: 0 };
+        if blends_inside(doc, &form_res, &ops, 0, &mut walk) {
             return Err(refuse("it is a transparency group with blend modes inside"));
         }
     }
@@ -120,7 +123,20 @@ pub(crate) fn ungroup_ops(doc: &Document, resources: &Dict, name: &[u8], translu
         renames.iter().find(|(c, _)| *c == cat).and_then(|(_, m)| m.iter().find(|(from, _)| from == name)).map(|(_, to)| to.clone())
     };
     let mut depth = 0usize;
+    // Marked content and text objects must balance within the form's own stream: an extra EMC
+    // would close one the page opened, and one left open would take in the page's later content.
+    let mut marked = 0usize;
+    let mut in_text = false;
     for op in &mut ops {
+        match op.op.as_slice() {
+            b"BDC" | b"BMC" => marked = marked.saturating_add(1),
+            b"EMC" => marked = marked.checked_sub(1).ok_or_else(|| refuse("its content ends marked content it didn't begin"))?,
+            b"BT" if in_text => return Err(refuse("its content begins a text object inside another")),
+            b"BT" => in_text = true,
+            b"ET" if !in_text => return Err(refuse("its content ends a text object it didn't begin")),
+            b"ET" => in_text = false,
+            _ => {}
+        }
         let slot: Option<(&[u8], usize)> = match op.op.as_slice() {
             b"Tf" | b"Do" | b"gs" | b"sh" | b"cs" | b"CS" => {
                 let cat: &[u8] = match op.op.as_slice() {
@@ -162,6 +178,10 @@ pub(crate) fn ungroup_ops(doc: &Document, resources: &Dict, name: &[u8], translu
         }
     }
 
+    if marked > 0 || in_text {
+        return Err(refuse("its content leaves marked content or a text object open"));
+    }
+
     // A form in a layer (/OC) stays in it: its content is marked with the same membership.
     let layer = match s.dict.get(b"OC") {
         None => None,
@@ -210,41 +230,87 @@ pub(crate) fn ungroup_ops(doc: &Document, resources: &Dict, name: &[u8], translu
 /// How deep [`blends_inside`] follows nested forms; deeper ones count as blending.
 const MAX_NESTING: usize = 8;
 
+/// The most nested forms and tiling patterns [`blends_inside`] reads in all; past that the rest
+/// count as blending, so a file can't make the check run for ever.
+const MAX_NESTED_READS: usize = 1024;
+
+/// What [`blends_inside`] carries through its nested reads: the forms on the current path (a
+/// form drawing itself), the answer for each form with resources of its own (its answer doesn't
+/// depend on who draws it), and how many reads it has made.
+struct BlendWalk {
+    path: Vec<pdfcraft_cos::ObjRef>,
+    known: HashMap<pdfcraft_cos::ObjRef, bool>,
+    reads: usize,
+}
+
 /// Whether `ops` (drawn with `resources`) blend with something other than Normal: a `gs` they
-/// use, or one used by a form they draw, at any depth. A nested form that can't be read, is
-/// nested too deep or draws itself counts as blending, so the caller refuses rather than guesses.
-fn blends_inside(doc: &Document, resources: &Dict, ops: &[Op], depth: usize, seen: &mut Vec<pdfcraft_cos::ObjRef>) -> bool {
+/// use, or one used by a form or pattern they draw, at any depth. A shading pattern blends through
+/// its own `/ExtGState`, a tiling pattern through its content. A nested form or pattern that can't
+/// be read, is nested too deep or draws itself counts as blending, so the caller refuses rather
+/// than guesses.
+fn blends_inside(doc: &Document, resources: &Dict, ops: &[Op], depth: usize, walk: &mut BlendWalk) -> bool {
     let category = |cat: &[u8]| resources.get(cat).map(|x| doc.resolve(x)).and_then(|x| x.as_dict().cloned()).unwrap_or_default();
-    let (states, xobjects) = (category(b"ExtGState"), category(b"XObject"));
+    let (states, xobjects, patterns) = (category(b"ExtGState"), category(b"XObject"), category(b"Pattern"));
+    let state_blends = |g: Option<&Object>| g.map(|g| doc.resolve(g)).and_then(|g| g.as_dict().cloned()).is_some_and(|gs| blends(doc, &gs));
     for op in ops {
-        if op.is("gs")
-            && op.name(0).and_then(|n| states.get(n)).map(|g| doc.resolve(g)).and_then(|g| g.as_dict().cloned()).is_some_and(|gs| blends(doc, &gs))
-        {
+        if op.is("gs") && state_blends(op.name(0).and_then(|n| states.get(n))) {
             return true;
         }
-        if !op.is("Do") {
+        let o = if op.is("Do") {
+            op.name(0).and_then(|n| xobjects.get(n))
+        } else if op.is("scn") || op.is("SCN") {
+            op.operands.len().checked_sub(1).and_then(|i| op.name(i)).and_then(|n| patterns.get(n))
+        } else {
+            continue;
+        };
+        let Some(o) = o else { continue };
+        let resolved = doc.resolve(o);
+        let nested = match &*resolved {
+            Object::Stream(st) if op.is("Do") && st.dict.name(b"Subtype") == Some(b"Form") => st,
+            Object::Stream(st) if !op.is("Do") && st.dict.int(b"PatternType") == Some(1) => st,
+            // A shading pattern.
+            Object::Stream(st) if !op.is("Do") => {
+                if state_blends(st.dict.get(b"ExtGState")) {
+                    return true;
+                }
+                continue;
+            }
+            Object::Dict(d) if !op.is("Do") => {
+                if state_blends(d.get(b"ExtGState")) {
+                    return true;
+                }
+                continue;
+            }
+            _ => continue,
+        };
+        let r = o.as_ref();
+        if let Some(known) = r.and_then(|r| walk.known.get(&r)) {
+            if *known {
+                return true;
+            }
             continue;
         }
-        let Some(o) = op.name(0).and_then(|n| xobjects.get(n)) else { continue };
-        let Object::Stream(nested) = &*doc.resolve(o) else { continue };
-        if nested.dict.name(b"Subtype") != Some(b"Form") {
-            continue;
-        }
-        if depth >= MAX_NESTING || o.as_ref().is_some_and(|r| seen.contains(&r)) {
+        if depth >= MAX_NESTING || r.is_some_and(|r| walk.path.contains(&r)) || walk.reads >= MAX_NESTED_READS {
             return true;
         }
+        walk.reads += 1;
         let Ok(data) = nested.decoded_strict_within(MAX_FORM_BYTES) else { return true };
-        let own = nested.dict.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned()).unwrap_or_else(|| resources.clone());
-        if let Some(r) = o.as_ref() {
-            seen.push(r);
-        }
+        let own_resources = nested.dict.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned());
+        let has_own = own_resources.is_some();
+        let own = own_resources.unwrap_or_else(|| resources.clone());
         let inner = parse(&data);
         if inner.skipped > 0 {
             return true;
         }
-        let blends = blends_inside(doc, &own, &inner.ops, depth + 1, seen);
-        if let Some(r) = o.as_ref() {
-            seen.retain(|x| *x != r);
+        if let Some(r) = r {
+            walk.path.push(r);
+        }
+        let blends = blends_inside(doc, &own, &inner.ops, depth + 1, walk);
+        if let Some(r) = r {
+            walk.path.retain(|x| *x != r);
+            if has_own {
+                walk.known.insert(r, blends);
+            }
         }
         if blends {
             return true;