M2: a JP2 image in a colour space the decoder refuses is drawn by its component count (#3306) (#3363)

* M2: a JP2 image in a colour space the decoder refuses is drawn by its component count (#3306)

The veraPDF fixture's JPX header names CIEJab (enumerated colour space 19),
which hayro-jpeg2000 refuses, so the image failed and the page drew blank.
When the decoder refuses the image and its colr boxes name such a space, it is
decoded again from a copy naming greyscale, sRGB or CMYK by component count,
as MuPDF and Poppler draw it.

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

* M2: copy the JP2 stream only when a colour enum needs replacing (#3306)

Review follow-up on #3363: a stream the decoder refused for another reason was
copied for nothing.

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

* Renumber the JP2 colour-space patch (21): #3378 takes hayro-syntax (20)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
mtlcl410
2026-10-19 02:50:58 -04:00
committed by GitHub
co-authored by Claude Opus 5.5 Brandon Thomas
parent cbbeb61650
commit 43550c448d
3 changed files with 227 additions and 3 deletions
+54
View File
@@ -3665,6 +3665,60 @@ trailer << /Root 1 0 R >>
pdf
}
/// [`jpx_midpoint_pdf`]'s codestream (every sample 128) wrapped in a JP2 file whose header
/// has a colour specification naming enumerated colour space `enumerated`.
fn jp2_midpoint_pdf(components: u16, enumerated: u32) -> Vec<u8> {
let raw = jpx_midpoint_pdf(8, components);
let start = raw.windows(4).position(|w| w == [0xff, 0x4f, 0xff, 0x51]).unwrap();
let end = raw.windows(2).rposition(|w| w == [0xff, 0xd9]).unwrap() + 2;
let codestream = &raw[start..end];
let jp2_box = |kind: &[u8], body: &[u8]| [&((8 + body.len()) as u32).to_be_bytes()[..], kind, body].concat();
let mut ihdr = Vec::new();
ihdr.extend_from_slice(&1_u32.to_be_bytes()); // height
ihdr.extend_from_slice(&1_u32.to_be_bytes()); // width
ihdr.extend_from_slice(&components.to_be_bytes());
ihdr.extend_from_slice(&[7, 7, 0, 0]); // 8 bits, wavelet, colour known, no IPR
let colr = [&[1, 0, 0][..], &enumerated.to_be_bytes()].concat();
let jp2 = [
vec![0, 0, 0, 12, b'j', b'P', b' ', b' ', 0x0d, 0x0a, 0x87, 0x0a],
jp2_box(b"ftyp", b"jp2 \0\0\0\0jp2 "),
jp2_box(b"jp2h", &[jp2_box(b"ihdr", &ihdr), jp2_box(b"colr", &colr)].concat()),
jp2_box(b"jp2c", codestream),
]
.concat();
let content = "q 20 0 0 20 5 5 cm /Im1 Do Q";
let mut pdf = format!(
"%PDF-1.7\n\
1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n\
2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\
3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 40 40] /Contents 4 0 R /Resources << /XObject << /Im1 5 0 R >> >> >> endobj\n\
4 0 obj << /Length {} >> stream\n{content}\nendstream endobj\n\
5 0 obj << /Type /XObject /Subtype /Image /Width 1 /Height 1 /BitsPerComponent 8 /Filter /JPXDecode /Length {} >> stream\n",
content.len(),
jp2.len()
)
.into_bytes();
pdf.extend_from_slice(&jp2);
pdf.extend_from_slice(b"\nendstream endobj\ntrailer << /Root 1 0 R >>\n%%EOF");
pdf
}
/// #3306: a JP2 image whose colour specification names an enumerated colour space the
/// decoder refuses (CIEJab in a veraPDF fixture, or YPbPr, or bilevel for one component) is
/// drawn as if it had none, by its component count, as MuPDF and Poppler draw it: the
/// midpoint image shows mid-grey instead of nothing. A space the decoder knows (sRGB) is
/// drawn as before.
#[test]
fn a_jp2_image_in_a_refused_enumerated_colour_space_is_drawn() {
for (components, enumerated) in [(3, 19), (3, 22), (1, 15), (3, 16)] {
let mut r = PageRenderer::new(Arc::new(jp2_midpoint_pdf(components, enumerated)), RenderConfig::default());
let page = r.render(RenderRequest { page: 0, kind: RequestKind::Pixels, tile: None, scale: 1.0, tag: 0, ..Default::default() });
assert!(page.error.is_none(), "{enumerated}: {:?}", page.error);
let px = &page.rgba[((40 - 15) * 40 + 15) * 4..][..3];
assert!(px.iter().all(|&v| (120..=136).contains(&v)), "enum {enumerated}, {components} component(s): {px:?}");
}
}
#[test]
fn jpx_stream_bit_depth_boundaries_do_not_panic() {
use hayro::hayro_syntax::bit_reader::BitReader;
+1 -1
View File
File diff suppressed because one or more lines are too long
+172 -2
View File
@@ -26,7 +26,18 @@ pub(crate) fn decode(data: &[u8], params: &ImageDecodeParams) -> Option<FilterRe
target_resolution: params.target_dimension,
};
let image = hayro_jpeg2000::Image::new(data, &settings).ok()?;
// PdfCraft patch (21): a JP2 colour specification naming an enumerated colour space the
// decoder refuses (CIEJab, enum 19, in a veraPDF fixture; YCbCr, PhotoYCC, …) made the whole
// image fail, so the page drew blank (#3306). Then it is decoded as if it had no colour
// specification, as MuPDF and Poppler draw it: grey, RGB or CMYK by its component count.
let repaired;
let image = match hayro_jpeg2000::Image::new(data, &settings) {
Ok(image) => image,
Err(_) => {
repaired = without_refused_enumerated_colour(data)?;
hayro_jpeg2000::Image::new(&repaired, &settings).ok()?
}
};
let width = image.width();
let height = image.height();
@@ -87,6 +98,82 @@ pub(crate) fn decode(data: &[u8], params: &ImageDecodeParams) -> Option<FilterRe
})
}
/// The enumerated colour spaces (`EnumCS`, ISO 15444-1 I.5.3.3 and 15444-2 M.11.7.2) the decoder
/// handles: CMYK, CIELab, sRGB, greyscale, sYCC, e-sRGB and ROMM-RGB.
const DECODED_ENUMERATED: [u32; 7] = [12, 14, 16, 17, 18, 20, 21];
/// PdfCraft patch (21): a copy of JP2 file `data` whose colour specification boxes (`colr`, in
/// the `jp2h` header box) that name an enumerated colour space outside [`DECODED_ENUMERATED`]
/// name the space the decoder assumes without one instead: greyscale, sRGB or CMYK for 1, 3 or
/// 4 components (`ihdr`). `None` when there is nothing to change or the boxes can't be read.
pub(crate) fn without_refused_enumerated_colour(data: &[u8]) -> Option<Vec<u8>> {
let header = boxes(data).find(|&(kind, _, _)| kind == *b"jp2h")?;
let (_, start, end) = header;
let inner = data.get(start..end)?;
let (_, ih_start, _) = boxes(inner).find(|&(kind, _, _)| kind == *b"ihdr")?;
// ihdr: height (4), width (4), number of components (2).
let components = u16::from_be_bytes(inner.get(ih_start + 8..ih_start + 10)?.try_into().ok()?);
let replacement: u32 = match components {
1 => 17,
3 => 16,
4 => 12,
_ => return None,
};
// Where each refused enum is, found before anything is copied: a stream the decoder refused
// for another reason isn't copied for nothing.
let mut refused = Vec::new();
for (kind, body, body_end) in boxes(inner) {
if kind != *b"colr" {
continue;
}
// colr: METH (1), PREC (1), APPROX (1), then EnumCS (4) when METH is 1.
let colr = inner.get(body..body_end)?;
if colr.first() != Some(&1) {
continue;
}
let enumerated = u32::from_be_bytes(colr.get(3..7)?.try_into().ok()?);
if DECODED_ENUMERATED.contains(&enumerated) {
continue;
}
refused.push(start.checked_add(body)?.checked_add(3)?);
}
if refused.is_empty() {
return None;
}
let mut out = data.to_vec();
for at in refused {
out.get_mut(at..at.checked_add(4)?)?.copy_from_slice(&replacement.to_be_bytes());
}
Some(out)
}
/// The boxes in `data` (ISO 15444-1 I.4): each one's type and the start and end of its contents.
/// Stops at the first box whose length doesn't fit.
fn boxes(data: &[u8]) -> impl Iterator<Item = ([u8; 4], usize, usize)> + '_ {
let mut at = 0usize;
core::iter::from_fn(move || {
let length = u32::from_be_bytes(data.get(at..at.checked_add(4)?)?.try_into().ok()?);
let kind: [u8; 4] = data.get(at + 4..at.checked_add(8)?)?.try_into().ok()?;
let (header, size) = match length {
// The rest of the data.
0 => (8, data.len().checked_sub(at)?),
// An 8-byte length follows the type.
1 => {
let large = u64::from_be_bytes(data.get(at + 8..at.checked_add(16)?)?.try_into().ok()?);
(16, usize::try_from(large).ok()?)
}
n => (8, usize::try_from(n).ok()?),
};
let end = at.checked_add(size)?;
if size < header || end > data.len() {
return None;
}
let item = (kind, at + header, end);
at = end;
Some(item)
})
}
fn scale(
data: &[u8],
bit_per_component: u8,
@@ -134,7 +221,7 @@ fn scale(
#[cfg(test)]
mod tests {
use super::scale;
use super::{scale, without_refused_enumerated_colour};
use crate::bit_reader::{BitReader, bit_mask};
use alloc::vec::Vec;
@@ -188,4 +275,87 @@ mod tests {
}
}
}
/// A JP2 header (`jp2h`) holding `ihdr` for `components` and the given `colr` bodies, with
/// a codestream box after it.
fn jp2(components: u16, colrs: &[&[u8]]) -> Vec<u8> {
let jp2_box = |kind: &[u8], body: &[u8]| {
let mut b = ((8 + body.len()) as u32).to_be_bytes().to_vec();
b.extend_from_slice(kind);
b.extend_from_slice(body);
b
};
let mut ihdr = Vec::new();
ihdr.extend_from_slice(&[0, 0, 0, 1, 0, 0, 0, 1]);
ihdr.extend_from_slice(&components.to_be_bytes());
ihdr.extend_from_slice(&[7, 7, 0, 0]);
let mut header = jp2_box(b"ihdr", &ihdr);
for c in colrs {
header.extend(jp2_box(b"colr", c));
}
let mut out = jp2_box(b"ftyp", b"jp2 \0\0\0\0jp2 ");
out.extend(jp2_box(b"jp2h", &header));
out.extend(jp2_box(b"jp2c", &[0xff, 0x4f]));
out
}
fn enumerated(e: u32) -> Vec<u8> {
let mut c = alloc::vec![1, 0, 0];
c.extend_from_slice(&e.to_be_bytes());
c
}
/// The enumerated colour spaces found in the copy's `colr` boxes, in order.
fn enums_in(data: &[u8]) -> Vec<u32> {
data.windows(4)
.enumerate()
.filter(|(_, w)| *w == b"colr")
.filter(|(i, _)| data.get(i + 4) == Some(&1))
.map(|(i, _)| u32::from_be_bytes(data[i + 7..i + 11].try_into().unwrap()))
.collect()
}
#[test]
fn refused_enumerated_colour_spaces_become_the_component_counts_default() {
// CIEJab (19) and other refused spaces, by component count.
for (components, refused, expected) in [(3, 19, 16), (3, 22, 16), (1, 15, 17), (4, 13, 12), (3, 1, 16)] {
let data = jp2(components, &[&enumerated(refused)]);
let out = without_refused_enumerated_colour(&data).unwrap();
assert_eq!(enums_in(&out), [expected], "{refused} with {components} component(s)");
assert_eq!(out.len(), data.len(), "only the enum changes");
}
// Several colour specifications: each refused one, and only those.
let out = without_refused_enumerated_colour(&jp2(3, &[&enumerated(19), &enumerated(14), &enumerated(24)])).unwrap();
assert_eq!(enums_in(&out), [16, 14, 16]);
// An 8-byte box length (LBox 1) is read too.
let mut long = jp2(3, &[&enumerated(19)]);
let at = long.windows(4).position(|w| w == b"jp2h").unwrap() - 4;
let len = u32::from_be_bytes(long[at..at + 4].try_into().unwrap()) as u64 + 8;
long.splice(at..at + 4, [0, 0, 0, 1]);
long.splice(at + 8..at + 8, (len).to_be_bytes());
assert_eq!(enums_in(&without_refused_enumerated_colour(&long).unwrap()), [16]);
}
#[test]
fn what_the_decoder_handles_or_cant_be_read_is_left_alone() {
for known in [12, 14, 16, 17, 18, 20, 21] {
assert_eq!(without_refused_enumerated_colour(&jp2(3, &[&enumerated(known)])), None, "{known}");
}
// An ICC profile (METH 2), a component count with no default, no header at all.
// (Its bytes where an enum would be read say 19: only METH 1 is an enum.)
assert_eq!(without_refused_enumerated_colour(&jp2(3, &[&[2, 0, 0, 0, 0, 0, 19, 0, 0]])), None);
assert_eq!(without_refused_enumerated_colour(&jp2(2, &[&enumerated(19)])), None);
assert_eq!(without_refused_enumerated_colour(&[0xff, 0x4f, 0xff, 0x51]), None);
// Damaged lengths: none of these may panic, and none is changed.
let good = jp2(3, &[&enumerated(19)]);
for cut in 0..good.len() {
let _ = without_refused_enumerated_colour(&good[..cut]);
}
for bad in [0x0000_0004_u32, 0x7fff_ffff, 0xffff_ffff] {
let mut data = good.clone();
let at = data.windows(4).position(|w| w == b"colr").unwrap() - 4;
data[at..at + 4].copy_from_slice(&bad.to_be_bytes());
assert_eq!(without_refused_enumerated_colour(&data), None, "{bad:#x}");
}
}
}