M11: the optimizer reads numbers inside /DecodeParms given by reference

Document::with_direct_parms resolved an indirect /DecodeParms and its
array entries but not the numbers inside (/Predictor 9 0 R), which
Dict::int doesn't follow. A 1-bit image with such a predictor was
decoded without it, its rows shifted by their tag bytes, and Group 4 of
that stored in its place; resampling read images the same way. It now
keeps, of each parameters dictionary, the numbers and flags the decoders
read, resolved, and /JBIG2Globals as a reference. Only those small
values are copied (an entry that is no dictionary is null), so a file
naming one big object from every key and filter can't have it copied a
hundred times.

The change is #4384's, moved down so that every PR stacked on this one
reads such parameters.

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

crates/cos/src/document/objects.rs

@@ -160,9 +160,12 @@ impl Document {
 
     /// `s` with the `/DecodeParms` its filters use made direct, so [`Stream::decoded`] reads them:
     /// an indirect dictionary resolved, and of an array (itself perhaps indirect), the entries at
-    /// the filters' places (`/Crypt` counts, ISO 32000-2 §7.4.1) resolved, the rest dropped. `s`
-    /// itself when none of those is indirect, or when the chain is longer than any decoder takes
-    /// ([`pdfcraft_filters::MAX_CHAIN`]), so at most that many objects are copied.
+    /// the filters' places (`/Crypt` counts, ISO 32000-2 §7.4.1) resolved, the rest dropped. Of
+    /// each dictionary only the entries the decoders read are kept, numbers and flags given by
+    /// reference (`/Predictor 7 0 R`) resolved; `/JBIG2Globals` stays a reference. `s` itself
+    /// when none of those is indirect, or when the chain is longer than any decoder takes
+    /// ([`pdfcraft_filters::MAX_CHAIN`]). Only small values are copied, so a file can't have a
+    /// large object copied once for each key and filter.
     pub fn with_direct_parms<'s>(&self, s: &'s Stream) -> Cow<'s, Stream> {
         let Some(p) = s.dict.get(b"DecodeParms") else { return Cow::Borrowed(s) };
         let filters = match s.dict.get(b"Filter") {
@@ -173,24 +176,45 @@ impl Document {
         if filters > pdfcraft_filters::MAX_CHAIN {
             return Cow::Borrowed(s);
         }
-        let whole = matches!(p, Object::Ref(_));
-        let resolved = self.resolve(p);
-        let direct = match &*resolved {
-            Object::Array(a) => {
-                let used = a.get(..filters.min(a.len())).unwrap_or_default();
-                if !whole && !used.iter().any(|e| matches!(e, Object::Ref(_))) {
-                    return Cow::Borrowed(s);
+        let mut changed = matches!(p, Object::Ref(_));
+        // Its numbers and flags kept, resolved; anything else is no filter's parameters (null).
+        let one = |e: &Object, changed: &mut bool| -> Object {
+            *changed |= matches!(e, Object::Ref(_));
+            let Object::Dict(d) = &*self.held(e) else { return Object::Null };
+            let mut made = Dict::new();
+            for key in DECODE_PARMS_KEYS {
+                let Some(v) = d.get(key) else { continue };
+                *changed |= matches!(v, Object::Ref(_));
+                let v = self.held(v);
+                if matches!(*v, Object::Int(_) | Object::Real(_) | Object::Bool(_)) {
+                    made.set(key.to_vec(), (*v).clone());
                 }
-                Object::Array(used.iter().map(|e| (*self.resolve(e)).clone()).collect())
             }
-            other if whole => other.clone(),
-            _ => return Cow::Borrowed(s),
+            if let Some(g @ Object::Ref(_)) = d.get(b"JBIG2Globals") {
+                made.set(b"JBIG2Globals".to_vec(), g.clone());
+            }
+            Object::Dict(made)
         };
+        let made = match &*self.held(p) {
+            Object::Array(a) => Object::Array(a.get(..filters.min(a.len())).unwrap_or_default().iter().map(|e| one(e, &mut changed)).collect()),
+            _ => one(p, &mut changed),
+        };
+        if !changed {
+            return Cow::Borrowed(s);
+        }
         let mut s = s.clone();
-        s.dict.set(b"DecodeParms".to_vec(), direct);
+        s.dict.set(b"DecodeParms".to_vec(), made);
         Cow::Owned(s)
     }
 
+    /// `o`, or the object it refers to, without copying either.
+    fn held<'o>(&self, o: &'o Object) -> Held<'o> {
+        match o {
+            Object::Ref(_) => Held::Resolved(self.resolve(o)),
+            o => Held::Direct(o),
+        }
+    }
+
     /// Resolve and return a dictionary (a stream's dictionary counts).
     pub fn dict(&self, o: &Object) -> Option<Dict> {
         self.resolve(o).as_dict().cloned()
@@ -603,3 +627,40 @@ impl Document {
         matches!(self.try_get(r.num).as_deref(), Ok(Object::Dict(d)) if d.name(b"Type") == Some(b"Catalog") || d.contains(b"Pages"))
     }
 }
+
+/// The `/DecodeParms` entries the decoders read (ISO 32000-2 Tables 8, 11 and 13: the Flate and
+/// LZW predictors, CCITT, DCT's colour transform): numbers and flags a file may give by
+/// reference, which [`Dict::int`] doesn't follow.
+const DECODE_PARMS_KEYS: [&[u8]; 13] = [
+    b"Predictor",
+    b"Colors",
+    b"BitsPerComponent",
+    b"Columns",
+    b"EarlyChange",
+    b"K",
+    b"Rows",
+    b"EndOfLine",
+    b"EncodedByteAlign",
+    b"EndOfBlock",
+    b"BlackIs1",
+    b"DamagedRowsBeforeError",
+    b"ColorTransform",
+];
+
+/// An object read in place, or the object a reference names as the document holds it: either way
+/// nothing is copied ([`Document::held`]).
+enum Held<'o> {
+    Direct(&'o Object),
+    Resolved(Arc<Object>),
+}
+
+impl std::ops::Deref for Held<'_> {
+    type Target = Object;
+
+    fn deref(&self) -> &Object {
+        match self {
+            Held::Direct(o) => o,
+            Held::Resolved(o) => o,
+        }
+    }
+}

crates/cos/src/document/tests.rs

@@ -1,7 +1,7 @@
 use super::*;
 
-/// /DecodeParms given by reference, whole or as array entries, are read once made direct: here a
-/// Flate predictor, which the stream alone doesn't see. Only the entries the filters use are
+/// /DecodeParms given by reference, whole, as array entries or as the numbers inside, are read
+/// once made direct: here a Flate predictor, which the stream alone doesn't see. Only the entries the filters use are
 /// resolved, and nothing is for a chain longer than a decoder takes.
 #[test]
 fn decode_parms_given_by_reference_are_read_once_direct() {
@@ -25,6 +25,30 @@ fn decode_parms_given_by_reference_are_read_once_direct() {
         assert_ne!(s.decoded().unwrap(), rows, "{p:?} read as it is");
         assert_eq!(doc.with_direct_parms(&s).decoded().unwrap(), rows, "{p:?}");
     }
+    // A number inside given by reference, which `Dict::int` doesn't follow: in the dictionary
+    // itself, and in an array's entry.
+    let twelve = doc.add(Object::Int(12));
+    let mut inner = parms.clone();
+    inner.set(b"Predictor".to_vec(), Object::Ref(twelve));
+    for p in [Object::Dict(inner.clone()), Object::Array(vec![Object::Dict(inner)])] {
+        let s = stream(1, p.clone());
+        assert_ne!(s.decoded().unwrap(), rows, "{p:?} read as it is");
+        assert_eq!(doc.with_direct_parms(&s).decoded().unwrap(), rows, "{p:?}");
+    }
+    // Only numbers and flags are copied: a key naming a big array, or an entry that is no
+    // dictionary, is left out (a file could name one big object from every key and filter).
+    let big = doc.add(Object::Array(vec![Object::Int(1); 100_000]));
+    let mut hostile = Dict::new();
+    hostile.set(b"Predictor".to_vec(), Object::Ref(big));
+    hostile.set(b"Columns".to_vec(), Object::Ref(big));
+    hostile.set(b"Colors".to_vec(), Object::Ref(twelve));
+    let entry = doc.add(Object::Dict(hostile));
+    let two = stream(2, Object::Array(vec![Object::Ref(entry), Object::Ref(big)]));
+    let made = doc.with_direct_parms(&two);
+    let made = made.dict.get(b"DecodeParms").and_then(Object::as_array).unwrap();
+    let first = made[0].as_dict().unwrap();
+    assert_eq!((first.contains(b"Predictor"), first.contains(b"Columns"), first.int(b"Colors")), (false, false, Some(12)));
+    assert!(matches!(made[1], Object::Null), "{:?}", made[1]);
     let direct = stream(1, Object::Dict(parms));
     assert!(matches!(doc.with_direct_parms(&direct), Cow::Borrowed(_)), "nothing to resolve");
     assert_eq!(direct.decoded().unwrap(), rows);

crates/optimize/README.md

@@ -32,8 +32,8 @@ let report = optimize_with_progress(&mut doc, &settings, &mut |stage| keep_going
   its colour transform (an Adobe marker's, else `/ColorTransform`, ISO 32000-2 §7.4.8), so one
   stored untransformed keeps its colours; one whose marker and `/ColorTransform` disagree is left
   alone, since readers differ on which wins. Samples are read with `/DecodeParms` given by
-  reference too (`Document::with_direct_parms`), here and below: without them a predictor's tag
-  bytes would shift every row of an image written back.
+  reference too, the numbers inside included (`Document::with_direct_parms`), here and below:
+  without them a predictor's tag bytes would shift every row of an image written back.
 - **Monochrome images** (`Settings::mono`, CCITT Group 4 by default): a 1-bit image with one
   colour component (grey, indexed, a separation) or an image mask, stored with no filter or with
   general-purpose ones, is stored again as CCITT Group 4 (`pdfcraft_filters::encode_g4`) when

crates/optimize/tests/optimize.rs

@@ -413,13 +413,15 @@ fn black_and_white_images_become_ccitt_group_4() {
     assert!(render(&doc) == before, "the page looks the same");
 }
 
-/// An image whose /DecodeParms are a reference is read through them, as one with them direct: a
-/// predictor's tag bytes would otherwise shift every row, and the image would be stored again
-/// wrong. Here a 1-bit image with a PNG predictor (stored again as Group 4) and an eight-bit grey
-/// one with a PNG predictor at 600 ppi (resampled) come out byte for byte as with direct parameters.
+/// An image whose /DecodeParms are a reference, or hold numbers given by reference
+/// (`/Predictor 9 0 R`, which `Dict::int` doesn't follow), is read through them, as one with them
+/// direct: a predictor's tag bytes would otherwise shift every row, and the image would be stored
+/// again wrong. Here a 1-bit image with a PNG predictor (stored again as Group 4) and an eight-bit
+/// grey one with a PNG predictor at 600 ppi (resampled) come out byte for byte as with direct
+/// parameters.
 #[test]
 fn images_whose_decode_parms_are_references_come_out_as_with_them_direct() {
-    let build = |by_reference: bool| {
+    let build = |how: &str| {
         let mut doc = Document::new_empty();
         let (w, h) = (1200u32, 480u32);
         let grey = [("BitsPerComponent", 1.into()), ("ColorSpace", Object::name("DeviceGray"))];
@@ -440,13 +442,22 @@ fn images_whose_decode_parms_are_references_come_out_as_with_them_direct() {
         d.set(b"DecodeParms".to_vec(), Object::Dict(parms));
         let coded = pdfcraft_filters::encode(&pdfcraft_filters::Filter::Flate, &params, &shades).unwrap();
         let eight_bit = doc.add(Object::Stream(Stream::from_raw(d, coded)));
-        if by_reference {
-            for r in [one_bit, eight_bit] {
-                let mut s = stream(&doc, r);
-                let parms = s.dict.get(b"DecodeParms").cloned().unwrap();
-                s.dict.set(b"DecodeParms".to_vec(), Object::Ref(doc.add(parms)));
-                doc.set(r, Object::Stream(s));
-            }
+        for r in [one_bit, eight_bit] {
+            let mut s = stream(&doc, r);
+            let Some(Object::Dict(mut parms)) = s.dict.get(b"DecodeParms").cloned() else { panic!("{:?}", s.dict) };
+            let made = match how {
+                "whole" => Object::Ref(doc.add(Object::Dict(parms))),
+                "numbers" => {
+                    for k in [&b"Predictor"[..], b"Columns"] {
+                        let v = parms.get(k).cloned().unwrap();
+                        parms.set(k.to_vec(), Object::Ref(doc.add(v)));
+                    }
+                    Object::Dict(parms)
+                }
+                _ => continue,
+            };
+            s.dict.set(b"DecodeParms".to_vec(), made);
+            doc.set(r, Object::Stream(s));
         }
         // 300 × 120 pt is 288 ppi; 144 × 57.6 pt is 600 ppi.
         page(&mut doc, &[("A", one_bit), ("B", eight_bit)], "q 300 0 0 120 20 600 cm /A Do Q q 144 0 0 57.6 20 400 cm /B Do Q");
@@ -458,7 +469,9 @@ fn images_whose_decode_parms_are_references_come_out_as_with_them_direct() {
         assert_eq!(after[0].dict.name(b"Filter"), Some(&b"CCITTFaxDecode"[..]));
         after.map(|s| s.raw)
     };
-    assert_eq!(build(true), build(false));
+    let direct = build("direct");
+    assert_eq!(build("whole"), direct, "the dictionary by reference");
+    assert_eq!(build("numbers"), direct, "the numbers inside by reference");
 }
 
 #[test]