M7: Ungroup reads a group's /S and /K, and nested types, through indirect objects (#2964, #3005)

Review follow-up on #3067: the transparency-group checks read /S and /K
raw, so an indirect /K true let a knockout group be ungrouped, and an
indirect /S /Transparency skipped the knockout, translucency and blend
checks. The /Subtype and /PatternType of nested forms, Type 3 fonts and
tiling patterns were read raw too, so blend modes inside such a form and
inherited resources were missed. All are now resolved first (any value
may be an indirect object, ISO 32000-2 7.3.10).

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

crates/edit/src/tests.rs

@@ -9112,6 +9112,112 @@ 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`; `build` adds the form's other
+/// entries (`/Group`, `/Resources`) and the objects they refer to.
+fn ungroup_built(content: &[u8], build: impl FnOnce(&mut Document, &mut Dict)) -> Result<(), EditError> {
+    let mut doc = fixture();
+    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()));
+    build(&mut doc, &mut form);
+    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: the entries a refusal depends on count when they are indirect objects too
+/// (ISO 32000-2 §7.3.10): a group's `/S` and `/K`, and the `/Subtype` or `/PatternType` of what
+/// the form draws.
+#[test]
+fn ungroup_reads_indirect_group_and_type_entries() {
+    let refused = |r: Result<(), EditError>, why: &str| match r {
+        Err(EditError::Invalid(m)) => assert!(m.contains(why), "{m}"),
+        other => panic!("expected a refusal mentioning {why:?}, got {other:?}"),
+    };
+    let fill = b"1 0 0 rg 0 0 10 10 re f";
+    let group = |doc: &mut Document, form: &mut Dict, s: bool, k: Option<bool>| {
+        let mut g = Dict::new();
+        let name = Object::name("Transparency");
+        g.set(b"S".to_vec(), if s { Object::Ref(doc.add(name)) } else { name });
+        if let Some(k) = k {
+            g.set(b"K".to_vec(), Object::Ref(doc.add(Object::Bool(k))));
+        }
+        form.set(b"Group".to_vec(), Object::Dict(g));
+    };
+    refused(ungroup_built(fill, |doc, form| group(doc, form, true, Some(true))), "knockout");
+    refused(
+        ungroup_built(fill, |doc, form| {
+            group(doc, form, false, None);
+            let k = Object::Ref(doc.add(Object::Bool(true)));
+            if let Some(Object::Dict(g)) = form.get_mut(b"Group") {
+                g.set(b"K".to_vec(), k);
+            }
+        }),
+        "knockout",
+    );
+    ungroup_built(fill, |doc, form| group(doc, form, true, Some(false))).unwrap();
+
+    // A nested form whose /Subtype is indirect is still looked into for blend modes.
+    refused(
+        ungroup_built(b"/Inner Do", |doc, form| {
+            group(doc, form, true, None);
+            let mut bm = Dict::new();
+            bm.set(b"BM".to_vec(), Object::name("Multiply"));
+            let mut states = Dict::new();
+            states.set(b"Mul".to_vec(), Object::Dict(bm));
+            let mut inner_res = Dict::new();
+            inner_res.set(b"ExtGState".to_vec(), Object::Dict(states));
+            let mut inner = Dict::new();
+            inner.set(b"Subtype".to_vec(), Object::Ref(doc.add(Object::name("Form"))));
+            inner.set(b"BBox".to_vec(), Object::Array([0, 0, 10, 10].map(Object::Int).to_vec()));
+            inner.set(b"Resources".to_vec(), Object::Dict(inner_res));
+            let inner = doc.add(Object::Stream(Stream::flate(inner, b"/Mul gs 0 0 1 1 re f")));
+            let mut xo = Dict::new();
+            xo.set(b"Inner".to_vec(), Object::Ref(inner));
+            let mut res = Dict::new();
+            res.set(b"XObject".to_vec(), Object::Dict(xo));
+            form.set(b"Resources".to_vec(), Object::Dict(res));
+        }),
+        "blend",
+    );
+
+    // A nested form or tiling pattern without resources, typed through an indirect object, still
+    // inherits the form's renamed ones.
+    for (cat, content) in [("XObject", &b"BT /F1 9 Tf (a) Tj ET /N Do"[..]), ("Pattern", b"BT /F1 9 Tf (a) Tj ET /Pattern cs /N scn 0 0 10 10 re f")]
+    {
+        let r = ungroup_built(content, |doc, form| {
+            let mut d = Dict::new();
+            if cat == "XObject" {
+                d.set(b"Subtype".to_vec(), Object::Ref(doc.add(Object::name("Form"))));
+                d.set(b"BBox".to_vec(), Object::Array([0, 0, 100, 50].map(Object::Int).to_vec()));
+            } else {
+                d.set(b"PatternType".to_vec(), Object::Ref(doc.add(Object::Int(1))));
+            }
+            let nested = doc.add(Object::Stream(Stream::flate(d, b"BT /F1 9 Tf (n) Tj ET")));
+            let mut times = Dict::new();
+            times.set(b"Type".to_vec(), Object::name("Font"));
+            times.set(b"Subtype".to_vec(), Object::name("Type1"));
+            times.set(b"BaseFont".to_vec(), Object::name("Times-Roman"));
+            let mut fonts = Dict::new();
+            fonts.set(b"F1".to_vec(), Object::Dict(times));
+            let mut named = Dict::new();
+            named.set(b"N".to_vec(), Object::Ref(nested));
+            let mut res = Dict::new();
+            res.set(b"Font".to_vec(), Object::Dict(fonts));
+            res.set(cat.as_bytes().to_vec(), Object::Dict(named));
+            form.set(b"Resources".to_vec(), Object::Dict(res));
+        });
+        refused(r, "uses its resources");
+    }
+}
+
 /// 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> {

crates/edit/src/ungroup.rs

@@ -52,7 +52,7 @@ pub(crate) fn ungroup_ops(doc: &Document, resources: &Dict, name: &[u8], translu
     let xobjects = resources.get(b"XObject").map(|x| doc.resolve(x)).and_then(|x| x.as_dict().cloned()).ok_or_else(|| refuse("it is missing"))?;
     let form = xobjects.get(name).ok_or_else(|| refuse("it is missing"))?;
     let Object::Stream(s) = &*doc.resolve(form) else { return Err(refuse("it is not a Form XObject")) };
-    if s.dict.name(b"Subtype") != Some(b"Form") {
+    if !is_name(doc, &s.dict, b"Subtype", b"Form") {
         return Err(refuse("it is not a Form XObject"));
     }
     let numbers = |key: &[u8], n: usize| -> Option<Vec<f64>> {
@@ -76,9 +76,9 @@ pub(crate) fn ungroup_ops(doc: &Document, resources: &Dict, name: &[u8], translu
     if s.dict.get(b"StructParent").is_some() || s.dict.get(b"StructParents").is_some() {
         return Err(refuse("it is tagged content (Tags panel); edit its text in a copy without tags"));
     }
-    let group = s.dict.get(b"Group").map(|g| doc.resolve(g)).and_then(|g| g.as_dict().cloned()).filter(|g| g.name(b"S") == Some(b"Transparency"));
+    let group = s.dict.get(b"Group").map(|g| doc.resolve(g)).and_then(|g| g.as_dict().cloned()).filter(|g| is_name(doc, g, b"S", b"Transparency"));
     if let Some(group) = &group {
-        if matches!(group.get(b"K"), Some(Object::Bool(true))) {
+        if group.get(b"K").is_some_and(|k| matches!(*doc.resolve(k), Object::Bool(true))) {
             return Err(refuse("it is a knockout transparency group"));
         }
         if translucent {
@@ -371,10 +371,10 @@ fn inherits_resources(doc: &Document, resources: &Dict, op: &Op) -> bool {
         _ => return false,
     };
     let drawn = match cat {
-        b"XObject" => dict.name(b"Subtype") == Some(b"Form"),
-        b"Font" => dict.name(b"Subtype") == Some(b"Type3"),
+        b"XObject" => is_name(doc, &dict, b"Subtype", b"Form"),
+        b"Font" => is_name(doc, &dict, b"Subtype", b"Type3"),
         // A shading pattern has no content of its own.
-        _ => dict.int(b"PatternType") == Some(1),
+        _ => int_of(doc, &dict, b"PatternType") == Some(1),
     };
     drawn && dict.get(b"Resources").is_none()
 }
@@ -397,8 +397,8 @@ fn blends_inside(doc: &Document, resources: &Dict, ops: &[Op], depth: usize, wal
         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,
+            Object::Stream(st) if op.is("Do") && is_name(doc, &st.dict, b"Subtype", b"Form") => st,
+            Object::Stream(st) if !op.is("Do") && int_of(doc, &st.dict, b"PatternType") == Some(1) => st,
             // A shading pattern.
             Object::Stream(st) if !op.is("Do") => {
                 if state_blends(st.dict.get(b"ExtGState")) {
@@ -450,6 +450,17 @@ fn blends_inside(doc: &Document, resources: &Dict, ops: &[Op], depth: usize, wal
     false
 }
 
+/// Whether `d`'s `key` is the name `want`, directly or through an indirect object (any value may
+/// be one, ISO 32000-2 §7.3.10).
+fn is_name(doc: &Document, d: &Dict, key: &[u8], want: &[u8]) -> bool {
+    d.get(key).is_some_and(|v| doc.resolve(v).as_name() == Some(want))
+}
+
+/// `d`'s integer `key`, directly or through an indirect object.
+fn int_of(doc: &Document, d: &Dict, key: &[u8]) -> Option<i64> {
+    d.get(key).and_then(|v| doc.resolve(v).as_int())
+}
+
 /// Whether an ExtGState blends with something other than Normal.
 fn blends(doc: &Document, gs: &Dict) -> bool {
     let Some(bm) = gs.get(b"BM").map(|b| doc.resolve(b)) else { return false };