From 7b0164e4e70c3e6fb441d43a7e9b60792a7f9869 Mon Sep 17 00:00:00 2001 From: Gert Pellin Date: Tue, 13 Oct 2026 04:26:28 +0200 Subject: [PATCH] ICC profiles written here have their tags 4-byte aligned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit moxcms's encode packs tagged elements back to back, so the built-in sRGB profile embedded in JPEG, TIFF and PSD exports was 634 bytes with `cprt` at offset 558, where ICC.1 §7.1.2 requires every tagged element on a 4-byte boundary (stricter readers reject the files). encode now lays the tag data out again with each element padded, updating the tag offsets, the size and the profile ID (zeroed, "not computed"); tags that share data keep sharing it. Reported to moxcms as awxkee/moxcms#198. Fixes #1584 --- crates/color/src/cms/icc.rs | 51 ++++++++++++++++++++++++++++++- crates/color/src/cms/tests_icc.rs | 23 ++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/crates/color/src/cms/icc.rs b/crates/color/src/cms/icc.rs index 94d316c8..df105751 100644 --- a/crates/color/src/cms/icc.rs +++ b/crates/color/src/cms/icc.rs @@ -249,7 +249,7 @@ pub(super) fn builtin_rgb_bytes(name: &str) -> Result, CmsError> { /// Encode `p` as an ICC file. The creation date is fixed so exports are reproducible. pub(super) fn encode(p: &ColorProfile) -> Result, CmsError> { - let mut bytes = p.encode().map_err(|e| CmsError::BadProfile(format!("{e:?}")))?; + let mut bytes = aligned(p.encode().map_err(|e| CmsError::BadProfile(format!("{e:?}")))?); // Header bytes 24..36: year, month, day, hours, minutes, seconds (u16 each). if let Some(date) = bytes.get_mut(24..36) { date.copy_from_slice(&[2026u16, 1, 1, 0, 0, 0].map(u16::to_be_bytes).concat()); @@ -257,6 +257,55 @@ pub(super) fn encode(p: &ColorProfile) -> Result, CmsError> { Ok(bytes) } +/// `icc` with every tagged element starting on a 4-byte boundary, padded with zeros, as ICC.1 +/// §7.1.2 requires (moxcms packs them, so a 62-byte `desc` left the next tag unaligned, #1584). +/// Tags that share data keep sharing it. A profile whose tag table doesn't read is returned as it is. +fn aligned(icc: Vec) -> Vec { + const TABLE: usize = 128; + let be = |b: &[u8], at: usize| b.get(at..at + 4).map(|v| u32::from_be_bytes([v[0], v[1], v[2], v[3]]) as usize); + let Some(n) = be(&icc, TABLE).filter(|n| *n <= 1000) else { return icc }; + let data_start = TABLE + 4 + 12 * n; + let mut entries = Vec::with_capacity(n); + for i in 0..n { + let e = TABLE + 4 + 12 * i; + let (Some(off), Some(size)) = (be(&icc, e + 4), be(&icc, e + 8)) else { return icc }; + if off < data_start || off.checked_add(size).is_none_or(|end| end > icc.len()) { + return icc; + } + entries.push((off, size)); + } + if entries.iter().all(|(off, _)| off.is_multiple_of(4)) && icc.len().is_multiple_of(4) { + return icc; + } + let mut out = icc.get(..data_start).map(<[u8]>::to_vec).unwrap_or_default(); + let mut moved: Vec<((usize, usize), usize)> = Vec::new(); + let mut order: Vec = (0..n).collect(); + order.sort_by_key(|&i| entries[i].0); + for i in order { + let (off, size) = entries[i]; + let at = match moved.iter().find(|(k, _)| *k == (off, size)) { + Some((_, at)) => *at, + None => { + out.resize(out.len().next_multiple_of(4), 0); + let at = out.len(); + out.extend_from_slice(&icc[off..off + size]); + moved.push(((off, size), at)); + at + } + }; + let e = TABLE + 4 + 12 * i + 4; + out[e..e + 4].copy_from_slice(&(at as u32).to_be_bytes()); + } + out.resize(out.len().next_multiple_of(4), 0); + let len = out.len() as u32; + out[0..4].copy_from_slice(&len.to_be_bytes()); + // The profile ID (an MD5 of the old bytes) no longer matches: zero is "not computed". + if let Some(id) = out.get_mut(84..100) { + id.fill(0); + } + out +} + fn text_tag(s: &str) -> Option { Some(ProfileText::Localizable(vec![LocalizableString::new("en".into(), "US".into(), s.into())])) } diff --git a/crates/color/src/cms/tests_icc.rs b/crates/color/src/cms/tests_icc.rs index 713b6539..2e5f099c 100644 --- a/crates/color/src/cms/tests_icc.rs +++ b/crates/color/src/cms/tests_icc.rs @@ -111,3 +111,26 @@ fn black_point_compensation_maps_the_darkest_ink_to_rgb_black() { let abs = |bpc| icc_cmyk(bpc).cmyk_to_rgb(rich, Intent::AbsoluteColorimetric); assert_eq!(abs(true), abs(false)); } + +/// #1584: the profiles written here (built-in RGB spaces) have every tag on a 4-byte boundary and +/// a size that is a multiple of 4 (ICC.1 §7.1.2), and still read back as the same space. +#[test] +fn written_profiles_align_their_tags() { + for name in [super::SRGB, super::DISPLAY_P3, super::WIDE_GAMUT_RGB, super::PROPHOTO_RGB] { + let icc = super::icc_bytes(name).unwrap(); + let be = |at: usize| u32::from_be_bytes([icc[at], icc[at + 1], icc[at + 2], icc[at + 3]]) as usize; + assert_eq!(be(0), icc.len(), "{name}: header size"); + assert_eq!(icc.len() % 4, 0, "{name}: {} bytes", icc.len()); + let n = be(128); + for i in 0..n { + let (off, size) = (be(132 + 12 * i + 4), be(132 + 12 * i + 8)); + assert_eq!(off % 4, 0, "{name}: tag {i} at {off}"); + assert!(off + size <= icc.len()); + } + let back = moxcms::ColorProfile::new_from_slice(&icc).unwrap(); + let orig = moxcms::ColorProfile::new_srgb(); + if name == super::SRGB { + assert_eq!((back.red_colorant, back.green_colorant, back.blue_colorant), (orig.red_colorant, orig.green_colorant, orig.blue_colorant)); + } + } +}