Clamp extracted Indexed image samples to the declared palette range (#1581)

* M10: clamp extracted Indexed image samples to hival

Validate Indexed hival as an integer in 0..=255 and clamp raw palette samples before lookup. Preserve existing Decode, mask, and short-palette behavior.

Refs #1201 (item 9 only).

AI-generated by Codex (GPT-6).

* ci: re-run (unrelated flaky test)

* test: make reduce-size no-op regression deterministic
This commit is contained in:
FredApps
2026-10-15 02:18:44 -04:00
committed by GitHub
parent 5105eb732c
commit 2fee7af241
3 changed files with 169 additions and 11 deletions
+10 -5
View File
@@ -179,8 +179,9 @@ fn image(doc: &Document, s: &Stream) -> Result<(&'static str, Vec<u8>), String>
Space::Gray => rgb.extend_from_slice(&[v(0); 3]),
Space::Rgb => rgb.extend_from_slice(&[v(0), v(1), v(2)]),
Space::Cmyk => rgb.extend_from_slice(&cmyk(v(0), v(1), v(2), v(3))),
Space::Indexed(base, table) => {
let i = px.first().copied().unwrap_or(0) as usize;
Space::Indexed(base, hival, table) => {
// Indexed samples outside the declared range use the nearest endpoint (ISO 32000-1, 8.6.6.3).
let i = px.first().copied().unwrap_or(0).min(*hival) as usize;
let k = base.components();
let e = table.get(i * k..i * k + k).unwrap_or(&[0, 0, 0, 0][..k]);
rgb.extend_from_slice(&match **base {
@@ -310,8 +311,8 @@ enum Space {
Gray,
Rgb,
Cmyk,
/// Base space and lookup table (one byte per base component).
Indexed(Box<Space>, Vec<u8>),
/// Base space, highest valid palette index, and lookup table.
Indexed(Box<Space>, u8, Vec<u8>),
}
impl Space {
@@ -352,12 +353,16 @@ fn space(doc: &Document, cs: Option<&Object>) -> Result<Space, String> {
if matches!(base, Space::Indexed(..)) {
return Err("nested indexed colour spaces aren't valid".into());
}
let hival = match a.get(2).map(|value| doc.resolve(value)).as_deref() {
Some(Object::Int(value)) if (0..=255).contains(value) => *value as u8,
_ => return Err("an indexed colour space with an invalid hival".into()),
};
let table = match a.get(3).map(|t| doc.resolve(t)).as_deref() {
Some(Object::String(s)) => s.bytes.clone(),
Some(Object::Stream(t)) => t.decoded().map_err(|e| e.to_string())?,
_ => return Err("an indexed colour space without a lookup table".into()),
};
Ok(Space::Indexed(Box::new(base), table))
Ok(Space::Indexed(Box::new(base), hival, table))
}
Some(other) => unsupported(other),
None => Err("a damaged colour space".into()),
+131
View File
@@ -744,6 +744,137 @@ fn damaged_masks_and_sizes_never_panic() {
}
}
#[test]
fn indexed_image_export_clamps_samples_to_hival() {
for palette in [vec![255, 0, 0, 0, 0, 255], vec![255, 0, 0, 0, 0, 255, 0, 255, 0, 255, 255, 255]] {
let mut doc = from_images(&[("flat.png".into(), png_bytes(false))]).unwrap();
let page = pdfcraft_model::pages(&doc)[0].clone();
let res = doc.resolve(page.dict.get(b"Resources").unwrap()).as_dict().cloned().unwrap();
let xo = doc.resolve(res.get(b"XObject").unwrap()).as_dict().cloned().unwrap();
let image = xo.iter().next().unwrap().1.as_ref().unwrap();
let mut dict = Dict::new();
dict.set(b"Type".to_vec(), Object::name("XObject"));
dict.set(b"Subtype".to_vec(), Object::name("Image"));
dict.set(b"Width".to_vec(), Object::Int(4));
dict.set(b"Height".to_vec(), Object::Int(1));
dict.set(b"BitsPerComponent".to_vec(), Object::Int(2));
dict.set(
b"ColorSpace".to_vec(),
Object::Array(vec![Object::name("Indexed"), Object::name("DeviceRGB"), Object::Int(1), Object::String(PdfString::literal(palette))]),
);
// Default Decode preserves sample values0,1,2,3. Values2 and3 clamp to hival1.
doc.set(image, Object::Stream(Stream::from_raw(dict, vec![0b0001_1011])));
let out = extract_images(&doc, &[0], 0);
assert!(out.skipped.is_empty(), "{:?}", out.skipped);
assert_eq!(out.images.len(), 1);
assert_eq!(exported_png_pixels(&out.images[0].data), vec![255, 0, 0, 0, 0, 255, 0, 0, 255, 0, 0, 255]);
}
}
fn exported_png_pixels(bytes: &[u8]) -> Vec<u8> {
let decoder = png::Decoder::new(std::io::Cursor::new(bytes));
let mut reader = decoder.read_info().unwrap();
let mut pixels = vec![0; reader.output_buffer_size().unwrap()];
let info = reader.next_frame(&mut pixels).unwrap();
pixels.truncate(info.buffer_size());
pixels
}
fn indexed_export_fixture(
hival: Object,
palette: Vec<u8>,
stream_table: bool,
indirect_hival: bool,
bpc: i64,
samples: Vec<u8>,
) -> (Document, pdfcraft_cos::ObjRef) {
let mut doc = from_images(&[("flat.png".into(), png_bytes(false))]).unwrap();
let page = pdfcraft_model::pages(&doc)[0].clone();
let res = doc.resolve(page.dict.get(b"Resources").unwrap()).as_dict().cloned().unwrap();
let xo = doc.resolve(res.get(b"XObject").unwrap()).as_dict().cloned().unwrap();
let image = xo.iter().next().unwrap().1.as_ref().unwrap();
let table = if stream_table {
Object::Ref(doc.add(Object::Stream(Stream::flate(Dict::new(), &palette))))
} else {
Object::String(PdfString::literal(palette))
};
let hival = if indirect_hival { Object::Ref(doc.add(hival)) } else { hival };
let mut dict = Dict::new();
dict.set(b"Type".to_vec(), Object::name("XObject"));
dict.set(b"Subtype".to_vec(), Object::name("Image"));
dict.set(b"Width".to_vec(), Object::Int(4));
dict.set(b"Height".to_vec(), Object::Int(1));
dict.set(b"BitsPerComponent".to_vec(), Object::Int(bpc));
dict.set(b"ColorSpace".to_vec(), Object::Array(vec![Object::name("Indexed"), Object::name("DeviceRGB"), hival, table]));
doc.set(image, Object::Stream(Stream::from_raw(dict, samples)));
(doc, image)
}
#[test]
fn indexed_hival_resolves_indirect_values_and_stream_palettes() {
for (bpc, samples) in [(2, vec![0x1b]), (4, vec![0x01, 0x23]), (8, vec![0, 1, 2, 255])] {
let (doc, image) = indexed_export_fixture(Object::Int(1), vec![255, 0, 0, 0, 0, 255], true, true, bpc, samples);
let (ext, bytes) = crate::extract::image_file(&doc, image).unwrap();
assert_eq!(ext, "png");
assert_eq!(exported_png_pixels(&bytes), vec![255, 0, 0, 0, 0, 255, 0, 0, 255, 0, 0, 255]);
}
}
#[test]
fn indexed_hival_boundary_values_keep_valid_palette_entries() {
let palette: Vec<u8> = (0..=255u8).flat_map(|i| [i, 255 - i, 17]).collect();
for high in [0, 255] {
let (doc, image) = indexed_export_fixture(Object::Int(high), palette.clone(), false, false, 8, vec![0, 1, 254, 255]);
let (_, bytes) = crate::extract::image_file(&doc, image).unwrap();
let expected: Vec<u8> = [0u8, 1, 254, 255]
.into_iter()
.flat_map(|i| {
let i = i.min(high as u8);
[i, 255 - i, 17]
})
.collect();
assert_eq!(exported_png_pixels(&bytes), expected);
}
}
#[test]
fn indexed_hival_rejects_invalid_values_without_panicking() {
for high in [Object::Int(-1), Object::Int(256), Object::Real(1.0), Object::Null, Object::name("bad")] {
let (doc, image) = indexed_export_fixture(high, vec![255, 0, 0], false, false, 8, vec![0; 4]);
assert!(crate::extract::image_file(&doc, image).unwrap_err().contains("invalid hival"));
let out = extract_images(&doc, &[0], 0);
assert!(out.images.is_empty());
assert_eq!(out.skipped.len(), 1);
assert!(out.skipped[0].2.contains("invalid hival"));
}
}
#[test]
fn indexed_hival_clamping_preserves_raw_colour_key_mask_samples() {
let (mut doc, image) = indexed_export_fixture(Object::Int(1), vec![255, 0, 0, 0, 0, 255], false, false, 2, vec![0x1b]);
let Object::Stream(mut stream) = (*doc.get(image)).clone() else { panic!("image stream") };
stream.dict.set(b"Mask".to_vec(), Object::Array(vec![Object::Int(2), Object::Int(2)]));
doc.set(image, Object::Stream(stream));
let (_, bytes) = crate::extract::image_file(&doc, image).unwrap();
assert_eq!(exported_png_pixels(&bytes), vec![255, 0, 0, 255, 0, 0, 255, 255, 0, 0, 255, 0, 0, 0, 255, 255]);
}
#[test]
fn indexed_hival_missing_element_returns_export_error() {
let (mut doc, image) = indexed_export_fixture(Object::Int(1), vec![255, 0, 0], false, false, 8, vec![0; 4]);
let Object::Stream(mut stream) = (*doc.get(image)).clone() else { panic!("image stream") };
stream.dict.set(b"ColorSpace".to_vec(), Object::Array(vec![Object::name("Indexed"), Object::name("DeviceRGB")]));
doc.set(image, Object::Stream(stream));
assert!(crate::extract::image_file(&doc, image).unwrap_err().contains("invalid hival"));
}
#[test]
fn indexed_hival_keeps_existing_short_palette_fallback() {
let (doc, image) = indexed_export_fixture(Object::Int(1), vec![255, 0, 0], false, false, 2, vec![0x1b]);
let (_, bytes) = crate::extract::image_file(&doc, image).unwrap();
assert_eq!(exported_png_pixels(&bytes), vec![255, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]);
}
/// An 8×8 baseline JPEG of one flat colour with four components, stored as `samples`, and an
/// Adobe APP14 segment with `transform` (0: CMYK, 2: YCCK).
fn flat_four_component_jpeg(samples: [u8; 4], transform: u8) -> Vec<u8> {
+28 -6
View File
@@ -69,12 +69,36 @@ fn opening_images_and_text_converts_them_to_new_pdfs() {
assert!(app.open_bytes("junk.png", None, b"\x89PNG\r\n\x1a\nnot really".to_vec()).is_err());
}
struct ReduceDir(std::path::PathBuf);
impl ReduceDir {
fn new() -> Self {
use std::sync::atomic::{AtomicU64, Ordering};
static NEXT: AtomicU64 = AtomicU64::new(0);
for _ in 0..128 {
let path = std::env::temp_dir().join(format!("pdfcraft-reduce-test-{}-{}", std::process::id(), NEXT.fetch_add(1, Ordering::Relaxed)));
match std::fs::create_dir(&path) {
Ok(()) => return Self(path),
Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => continue,
Err(error) => panic!("creating Reduce test directory: {error}"),
}
}
panic!("no unused Reduce test directory after 128 attempts");
}
}
impl Drop for ReduceDir {
fn drop(&mut self) {
let _ = std::fs::remove_dir_all(&self.0);
}
}
/// Open `name` and run Reduce File Size on it, saving to `out` without a dialog.
fn reduce(name: &'static str, bytes: Vec<u8>, out: &std::path::Path) -> egui_kittest::Harness<'static, PdfCraftApp> {
let out2 = out.to_path_buf();
let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(move |_cc| {
let mut app = PdfCraftApp::new();
app.set_option("language", "en").unwrap();
// Keep metadata identical: changing ModDate can make a compact PDF one byte smaller.
app.session = std::mem::take(&mut app.session).with_clock(|| 1_700_000_000);
app.open_bytes(name, None, bytes).unwrap();
app.save_override = Some(out2.to_string_lossy().into_owned());
app
@@ -105,9 +129,8 @@ fn reduce_file_size_writes_a_compact_copy() {
enc.set_depth(png::BitDepth::Eight);
enc.write_header().unwrap().write_image_data(&px).unwrap();
}
let dir = std::env::temp_dir().join(format!("pdfcraft-reduce-{}", std::process::id()));
std::fs::create_dir_all(&dir).unwrap();
let out = dir.join("reduced.pdf");
let dir = ReduceDir::new();
let out = dir.0.join("reduced.pdf");
let h = reduce("photo.png", photo, &out);
let bytes = std::fs::read(&out).unwrap();
assert!(bytes.starts_with(b"%PDF-"));
@@ -119,9 +142,8 @@ fn reduce_file_size_writes_a_compact_copy() {
#[test]
fn reduce_file_size_saves_nothing_when_the_file_is_already_small() {
use egui_kittest::kittest::Queryable;
let dir = std::env::temp_dir().join(format!("pdfcraft-reduce-small-{}", std::process::id()));
std::fs::create_dir_all(&dir).unwrap();
let out = dir.join("reduced.pdf");
let dir = ReduceDir::new();
let out = dir.0.join("reduced.pdf");
// A new text PDF is already compact: a copy would be no smaller (#490).
let h = reduce("notes.txt", "lorem ipsum ".repeat(500).into_bytes(), &out);
assert!(!out.exists(), "no copy was written");