From 0a5f8bce2d22b32dbb57cab17cbd9fd53c9063d1 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Fri, 23 Oct 2026 21:57:34 +0200 Subject: [PATCH] M11: black-and-white images: only DeviceGray counts as a one-component name; refuse a malformed mono A bare /Indexed, /I or /CalGray is no colour space on its own, so it can only name a resource, which might be RGB: such an image is now left alone instead of being read as one bit per pixel. doc_optimize refuses a mono that isn't an object or a compression that isn't a known name, instead of reading it as the default (from CodeRabbit's review). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01BfMGHQNgdq5yf5tKZHGR3z --- crates/automation/src/documents.rs | 16 +++++++++----- crates/automation/tests/it/automation.rs | 9 ++++---- crates/optimize/src/images.rs | 6 +++-- crates/optimize/tests/optimize.rs | 28 +++++++++++++++++------- 4 files changed, 39 insertions(+), 20 deletions(-) diff --git a/crates/automation/src/documents.rs b/crates/automation/src/documents.rs index 1b26dec7f1..6e2c0a83f5 100644 --- a/crates/automation/src/documents.rs +++ b/crates/automation/src/documents.rs @@ -317,12 +317,16 @@ impl Automation { }; settings.color = images("color", settings.color)?; settings.gray = images("gray", settings.gray)?; - if let Some(c) = a.get("mono").and_then(|m| m.get("compression")).and_then(Value::as_str) { - settings.mono = match c { - "ccitt_g4" | "ccitt" | "g4" => MonoCompression::CcittG4, - "retain" => MonoCompression::Retain, - other => return Err(ToolError::InvalidArgs(format!("unknown mono compression {other:?} (ccitt_g4, retain)"))), - }; + if let Some(mono) = a.get("mono") { + let unknown = |c: &Value| ToolError::InvalidArgs(format!("unknown mono compression {c} (ccitt_g4, retain)")); + let mono = mono.as_object().ok_or_else(|| ToolError::InvalidArgs("mono must be an object: { compression: ccitt_g4|retain }".into()))?; + if let Some(c) = mono.get("compression") { + settings.mono = match c.as_str() { + Some("ccitt_g4" | "ccitt" | "g4") => MonoCompression::CcittG4, + Some("retain") => MonoCompression::Retain, + _ => return Err(unknown(c)), + }; + } } for (key, flag) in [ ("merge_fragments", &mut settings.merge_fragments), diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index aa4b50d087..8392b6722f 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -6455,10 +6455,11 @@ fn doc_optimize_stores_black_and_white_images_as_ccitt_group_4() { }) .collect(); assert_eq!(filters, [b"CCITTFaxDecode".to_vec()]); - assert!(matches!( - a.call("doc_optimize", &json!({ "doc": doc, "path": "x.pdf", "mono": { "compression": "jbig2" } })), - Err(ToolError::InvalidArgs(_)) - )); + // An unknown compression, one that isn't a name, or a mono that isn't an object is refused, + // not read as the default. + for mono in [json!({ "compression": "jbig2" }), json!({ "compression": false }), json!("retain")] { + assert!(matches!(a.call("doc_optimize", &json!({ "doc": doc, "path": "x.pdf", "mono": mono })), Err(ToolError::InvalidArgs(_))), "{mono}"); + } } /// A one-page PDF whose page picture is drawn as `rows` grey images one pixel row high, 600 px diff --git a/crates/optimize/src/images.rs b/crates/optimize/src/images.rs index 0a157b8c33..ad76003074 100644 --- a/crates/optimize/src/images.rs +++ b/crates/optimize/src/images.rs @@ -533,11 +533,13 @@ fn process(doc: &Document, s: &Stream, ppi: f64, settings: &ImageSettings, n: us } /// Whether colour space `cs` has one colour component, so a 1-bit image in it has one bit per -/// pixel: grey, a separation, an indexed space or a DeviceN of one colourant. +/// pixel: grey, a separation, an indexed space or a DeviceN of one colourant. Of names, only +/// DeviceGray counts: another name is no colour space on its own (`/Indexed` and `/CalGray` need +/// their parameters), so it can only name a resource, which might be RGB. fn one_component(doc: &Document, cs: &Object) -> bool { let cs = doc.resolve(cs); match &*cs { - Object::Name(n) => matches!(n.as_slice(), b"DeviceGray" | b"G" | b"CalGray" | b"Indexed" | b"I"), + Object::Name(n) => matches!(n.as_slice(), b"DeviceGray" | b"G"), Object::Array(a) => match a.first().and_then(Object::as_name) { Some(b"CalGray" | b"Separation" | b"Indexed" | b"I") => true, Some(b"ICCBased") => a.get(1).is_some_and(|icc| matches!(&*doc.resolve(icc), Object::Stream(s) if s.dict.int(b"N") == Some(1))), diff --git a/crates/optimize/tests/optimize.rs b/crates/optimize/tests/optimize.rs index 8c0afe89e7..db32ea7ad2 100644 --- a/crates/optimize/tests/optimize.rs +++ b/crates/optimize/tests/optimize.rs @@ -329,9 +329,9 @@ fn render(doc: &Document) -> Vec { /// Black-and-white images become CCITT Group 4, losslessly: the page looks the same to the pixel. /// Grey, an image mask (with a decode array that inverts it), an indexed image of two colours and -/// one stored with a PNG predictor qualify; noise (bigger as G4), a 1-bit RGB image, one wider -/// than Group 4 encodes and a tiny one (bigger as stored) are left alone, as is everything with -/// `mono: Retain`. +/// one stored with a PNG predictor qualify; noise (bigger as G4), a 1-bit RGB image, one whose +/// colour space is a resource name, one wider than Group 4 encodes and a tiny one (bigger as +/// stored) are left alone, as is everything with `mono: Retain`. #[test] fn black_and_white_images_become_ccitt_group_4() { let mut doc = Document::new_empty(); @@ -366,20 +366,32 @@ fn black_and_white_images_become_ccitt_group_4() { None, &[("BitsPerComponent", 1.into()), ("ColorSpace", Object::name("DeviceGray"))], ); + // A bare /Indexed is no colour space on its own: it names the page's resource, here RGB, so + // these samples are three to a pixel. + let named = + mono_image(&mut doc, 400, 160, &bilevel(1200, 160, false), None, &[("BitsPerComponent", 1.into()), ("ColorSpace", Object::name("Indexed"))]); // 64 × 8 white: its Group 4 data is smaller than its Flate data, but not by the longer // dictionary Group 4 needs. let tiny = mono_image(&mut doc, 64, 8, &[0xFF; 64], None, &[("BitsPerComponent", 1.into()), ("ColorSpace", Object::name("DeviceGray"))]); let tiny_g4 = pdfcraft_filters::encode_g4(&[0xFF; 64], 64, 8).unwrap(); assert!(tiny_g4.len() < stream(&doc, tiny).raw.len(), "{} against {}", tiny_g4.len(), stream(&doc, tiny).raw.len()); - page( + let p = page( &mut doc, - &[("G", gray), ("M", mask), ("I", indexed), ("P", predicted), ("N", noise), ("R", rgb), ("W", wide), ("T", tiny)], + &[("G", gray), ("M", mask), ("I", indexed), ("P", predicted), ("N", noise), ("R", rgb), ("W", wide), ("T", tiny), ("X", named)], "q 300 0 0 120 20 660 cm /G Do Q q 0 0.4 0.8 rg 300 0 0 120 320 660 cm /M Do Q q 300 0 0 120 20 520 cm /I Do Q \ - q 300 0 0 120 320 520 cm /P Do Q q 200 0 0 100 20 400 cm /N Do Q q 200 0 0 80 240 400 cm /R Do Q q 560 0 0 4 20 380 cm /W Do Q q 64 0 0 8 460 400 cm /T Do Q", + q 300 0 0 120 320 520 cm /P Do Q q 200 0 0 100 20 400 cm /N Do Q q 200 0 0 80 240 400 cm /R Do Q q 560 0 0 4 20 380 cm /W Do Q q 64 0 0 8 460 400 cm /T Do Q q 200 0 0 80 240 300 cm /X Do Q", ); + doc.update_dict(p, |d| { + if let Some(Object::Dict(res)) = d.get_mut(b"Resources") { + let mut cs = Dict::new(); + cs.set(b"Indexed".to_vec(), Object::name("DeviceRGB")); + res.set(b"ColorSpace".to_vec(), Object::Dict(cs)); + } + }) + .unwrap(); let before = render(&doc); let raw = |doc: &Document, r: ObjRef| stream(doc, r).raw; - let untouched = [noise, rgb, wide, tiny].map(|r| raw(&doc, r)); + let untouched = [noise, rgb, wide, tiny, named].map(|r| raw(&doc, r)); let mut kept = doc.clone(); let r = optimize(&mut kept, &Settings { mono: MonoCompression::Retain, ..Settings::default() }).unwrap(); @@ -397,7 +409,7 @@ fn black_and_white_images_become_ccitt_group_4() { } assert_eq!(stream(&doc, mask).dict.get(b"Decode"), Some(&decode), "the decode array stays"); assert_eq!(stream(&doc, indexed).dict.get(b"ColorSpace"), Some(&indexed_cs), "the colour space stays"); - assert_eq!([noise, rgb, wide, tiny].map(|r| raw(&doc, r)), untouched); + assert_eq!([noise, rgb, wide, tiny, named].map(|r| raw(&doc, r)), untouched); assert!(render(&doc) == before, "the page looks the same"); }