From 7cf4e6eca06340a82b6928108f7df8eebd9eec61 Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 20:52:48 +0000 Subject: [PATCH 1/7] JPEG: open 12-bit and arithmetic-coded files through craft-jpeg (#4934, #5003) Arithmetic-coded frames (SOF9-SOF11) and SOF0/SOF1 frames whose precision isn't 8 bits go to craft-jpeg; everything else stays on zune-jpeg. 12-bit files open as 16-bit images. PhotoCraft's own marker scan still supplies ICC, EXIF, XMP, resolution and Photoshop resources, and damaged or truncated entropy data adds the Truncated warning. A truncated scan too short for the frame it declares is refused, as on the zune-jpeg path. craft-jpeg is pinned to the aiongg fork until it lands on storytold. Co-authored-by: Michael Higgins --- Cargo.lock | 9 ++ Cargo.toml | 1 + crates/codecs/Cargo.toml | 2 + crates/codecs/src/codecs/jpeg.rs | 208 ++++++++++++++++++++++++++++++- 4 files changed, 219 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index b90e1f5cd5..3fdcda91b3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1311,6 +1311,14 @@ name = "craft-docpath" version = "0.1.0" source = "git+https://github.com/storytold/craft-libs?rev=82a24b006eefa036b43c09a67fb9a3fbd17ffe63#82a24b006eefa036b43c09a67fb9a3fbd17ffe63" +[[package]] +name = "craft-jpeg" +version = "0.1.0" +source = "git+https://github.com/aiongg/craft-libs?rev=059d1519a6c90ecb0064ffe39020311014ce760d#059d1519a6c90ecb0064ffe39020311014ce760d" +dependencies = [ + "thiserror 2.0.21", +] + [[package]] name = "craft-print" version = "0.1.0" @@ -4860,6 +4868,7 @@ dependencies = [ name = "photocraft-codecs" version = "0.15.1" dependencies = [ + "craft-jpeg", "exr", "flate2", "half", diff --git a/Cargo.toml b/Cargo.toml index 0787db33f8..dc8980032e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,6 +41,7 @@ photocraft-ipp = { path = "crates/ipp" } # external craft-print = { git = "https://github.com/storytold/craft-libs", rev = "3d37da389a3618ab9b76109da11416e9ca4ea719" } craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "82a24b006eefa036b43c09a67fb9a3fbd17ffe63" } +craft-jpeg = { git = "https://github.com/aiongg/craft-libs", rev = "059d1519a6c90ecb0064ffe39020311014ce760d" } thiserror = "2" serde = { version = "1", features = ["derive", "rc"] } serde_json = "1" diff --git a/crates/codecs/Cargo.toml b/crates/codecs/Cargo.toml index 25f3f09be0..062f7c4f98 100644 --- a/crates/codecs/Cargo.toml +++ b/crates/codecs/Cargo.toml @@ -31,6 +31,8 @@ image-webp = "0.2" zune-jpeg = "0.5" zune-core = "0.5" jpeg-encoder = "0.6" +# 12-bit and arithmetic-coded JPEG, which zune-jpeg does not decode. +craft-jpeg.workspace = true photocraft-heif = { path = "../heif", optional = true } # Orientation is applied a band of rows per task on native; wasm stays single-threaded. diff --git a/crates/codecs/src/codecs/jpeg.rs b/crates/codecs/src/codecs/jpeg.rs index 14d8f8ffec..8bb696638e 100644 --- a/crates/codecs/src/codecs/jpeg.rs +++ b/crates/codecs/src/codecs/jpeg.rs @@ -1,4 +1,5 @@ -//! JPEG: decode via `zune-jpeg` (gray, YCbCr, CMYK, YCCK), encode via +//! JPEG: decode via `zune-jpeg` (gray, YCbCr, CMYK, YCCK; arithmetic-coded and 12-bit files +//! via [`craft_jpeg`]), encode via //! `jpeg-encoder` (gray, RGB, CMYK). Metadata (APP0 JFIF density, APP1 //! EXIF/XMP, APP2 multi-segment ICC, APP13 Photoshop resources, APP14 Adobe) is //! parsed by our own marker scanner so behaviour does not depend on decoder internals. @@ -190,6 +191,43 @@ fn err(e: impl std::fmt::Display) -> CodecError { CodecError::malformed(F, e) } +/// Whether `craft-jpeg` (rather than `zune-jpeg`) decodes the file: arithmetic coding +/// (SOF9–SOF11) or a DCT frame whose precision isn't 8 bits (#4934, #5003). +fn uses_craft_jpeg(bytes: &[u8]) -> bool { + match craft_jpeg::frame(bytes) { + Some(f) if (0xC9..=0xCB).contains(&f.marker) => true, + Some(f) if matches!(f.marker, 0xC0 | 0xC1) => f.precision != 8, + _ => false, + } +} + +fn craft_jpeg_error(e: craft_jpeg::Error) -> CodecError { + match e { + craft_jpeg::Error::Malformed(m) => err(m), + craft_jpeg::Error::Unsupported(m) => CodecError::unsupported(F, m), + craft_jpeg::Error::LimitExceeded(m) => CodecError::LimitExceeded(m), + e => err(e), + } +} + +/// Decode a file [`uses_craft_jpeg`] routes away from `zune-jpeg`; `damaged` says entropy-coded +/// data was cut short or corrupt. +fn decode_craft_jpeg(bytes: &[u8], limits: &Limits, adobe_transform: Option) -> Result<(Image, bool), CodecError> { + let lim = craft_jpeg::Limits { max_pixels: limits.max_pixels, max_alloc: limits.max_alloc }; + let d = craft_jpeg::decode(bytes, &lim, adobe_transform).map_err(craft_jpeg_error)?; + let layout = match d.components { + 1 => ChannelLayout::Gray, + 3 => ChannelLayout::Rgb, + 4 => ChannelLayout::Cmyk, + n => return Err(err(format!("{n} components"))), + }; + let img = match d.samples { + craft_jpeg::Samples::U8(px) => Image::from_raw(d.width, d.height, layout, SampleType::U8, px)?, + craft_jpeg::Samples::U16(px) => Image::from_u16(d.width, d.height, layout, &px)?, + }; + Ok((img, d.damaged)) +} + /// Most pixels one byte of truncated scan data may account for. Even a flat image costs a few /// bits per 8×8 block, and truncated photographs are far above this. const MAX_TRUNCATED_PIXELS_PER_BYTE: u64 = 2048; @@ -203,6 +241,21 @@ pub(crate) fn decode(bytes: &[u8], limits: &Limits) -> Result if let Some((w, h, nc)) = meta.frame { limits.check_bytes(w, h, u64::from(nc.max(1)))?; } + if uses_craft_jpeg(bytes) { + if let Some((w, h, _)) = meta.frame + && end == DataEnd::Truncated + && u64::from(w) * u64::from(h) / MAX_TRUNCATED_PIXELS_PER_BYTE > scan_bytes + { + return Err(err("the truncated scan is too short for the frame it declares")); + } + let (mut img, damaged) = decode_craft_jpeg(bytes, limits, meta.adobe_transform)?; + img.icc = meta.icc; + img.meta = Metadata { exif: meta.exif, xmp: meta.xmp, dpi: meta.dpi, photoshop_resources: meta.photoshop, ..Default::default() }; + if end == DataEnd::Truncated || damaged { + img.warnings.push(DecodeWarning::Truncated { format: F }); + } + return Ok(img); + } let options = DecoderOptions::default().set_strict_mode(false).set_max_width(65535).set_max_height(65535); let mut dec = zune_jpeg::JpegDecoder::new_with_options(ZCursor::new(bytes), options); dec.decode_headers().map_err(err)?; @@ -553,4 +606,157 @@ mod tests { assert!(has_jfif_app0(&bytes)); assert_eq!(scan_metadata(&bytes).dpi, Some((96.0, 96.0))); } + + // --- Arithmetic-coded and 12-bit files, decoded by craft-jpeg --- + + fn segment(out: &mut Vec, marker: u8, body: &[u8]) { + out.extend_from_slice(&[0xFF, marker]); + out.extend_from_slice(&((body.len() + 2) as u16).to_be_bytes()); + out.extend_from_slice(body); + } + + /// One 8×8 gray block, quantizer 1, with entropy-coded scan data `data`. + fn gray_block(sof: u8, precision: u8, dht: Option<&[u8]>, data: &[u8]) -> Vec { + let mut s = vec![0xFF, 0xD8]; + let mut dqt = vec![0u8]; + dqt.extend([1u8; 64]); + segment(&mut s, 0xDB, &dqt); + segment(&mut s, sof, &[precision, 0, 8, 0, 8, 1, 1, 0x11, 0]); + if let Some(dht) = dht { + segment(&mut s, 0xC4, dht); + } + segment(&mut s, 0xDA, &[1, 1, 0x00, 0, 63, 0]); + s.extend_from_slice(data); + s.extend([0xFF, 0xD9]); + s + } + + /// SOF9 with only a DC coefficient (from a T.81 arithmetic encoder): 8-bit DC 200 → sample + /// 200 / 8 + 128 = 153; 12-bit DC 1000 → 1000 / 8 + 2048 = 2173. + fn arithmetic_block(precision: u8) -> Vec { + let data: &[u8] = if precision == 8 { &[210, 217, 68, 0] } else { &[210, 234, 134, 0] }; + gray_block(0xC9, precision, None, data) + } + + /// A 12-bit SOF1 stream: one DC coefficient. The DC table has one code (`0`, category 13), + /// the AC table one (`0`, end of block). DC = 7616 → sample 7616 / 8 + 2048 = 3000. + fn twelve_bit_huffman_block() -> Vec { + let mut dht = vec![0x00, 1]; + dht.extend([0u8; 15]); + dht.push(13); + dht.extend([0x10, 1]); + dht.extend([0u8; 15]); + dht.push(0x00); + // Bits: `0`, 7616 in 13 bits, `0`, one pad `1`. + let word: u16 = (7616 << 2) | 1; + gray_block(0xC1, 12, Some(&dht), &word.to_be_bytes()) + } + + fn twelve_bit(v: f32) -> u16 { + (v * 65535.0 / 4095.0).round() as u16 + } + + #[test] + fn routing() { + assert!(!uses_craft_jpeg(&[0xFF, 0xD8, 0xFF, 0xC0, 0, 11, 8, 0, 8, 0, 8, 1, 1, 0x11, 0])); + assert!(!uses_craft_jpeg(&[0xFF, 0xD8, 0xFF, 0xC1, 0, 11, 8, 0, 8, 0, 8, 1, 1, 0x11, 0])); + assert!(uses_craft_jpeg(&[0xFF, 0xD8, 0xFF, 0xC9, 0, 11, 8, 0, 8, 0, 8, 1, 1, 0x11, 0])); + assert!(uses_craft_jpeg(&[0xFF, 0xD8, 0xFF, 0xC1, 0, 11, 12, 0, 8, 0, 8, 1, 1, 0x11, 0])); + assert!(uses_craft_jpeg(&[0xFF, 0xD8, 0xFF, 0xC0, 0, 11, 12, 0, 8, 0, 8, 1, 1, 0x11, 0])); + assert!(!uses_craft_jpeg(&[0xFF, 0xD8])); + } + + #[test] + fn arithmetic_eight_bit_block_is_eight_bit() { + let img = crate::decode(&arithmetic_block(8)).unwrap(); + assert_eq!((img.dimensions(), img.sample_type(), img.layout()), ((8, 8), SampleType::U8, ChannelLayout::Gray)); + assert!(img.data().iter().all(|&v| v.abs_diff(153) <= 1), "{:?}", &img.data()[..4]); + assert!(img.warnings.is_empty()); + } + + #[test] + fn arithmetic_twelve_bit_block_is_sixteen_bit() { + let img = crate::decode(&arithmetic_block(12)).unwrap(); + assert_eq!((img.dimensions(), img.sample_type(), img.layout()), ((8, 8), SampleType::U16, ChannelLayout::Gray)); + let want = twelve_bit(2173.0); + assert!(img.to_u16_samples().unwrap().iter().all(|&v| v.abs_diff(want) <= 1)); + } + + #[test] + fn twelve_bit_huffman_block_decodes_to_sixteen_bit() { + let img = crate::decode(&twelve_bit_huffman_block()).unwrap(); + assert_eq!((img.dimensions(), img.sample_type()), ((8, 8), SampleType::U16)); + let want = twelve_bit(3000.0); + let px = img.to_u16_samples().unwrap(); + assert!(px.iter().all(|&v| v.abs_diff(want) <= 1), "{:?} vs {want}", &px[..4]); + assert!(img.warnings.is_empty()); + } + + #[test] + fn twelve_bit_jpeg_keeps_its_photoshop_resources() { + // An APP13 `8BIM` block (paths, Photoshop settings) right after SOI, as on the zune-jpeg path. + let resources = b"8BIM\x04\x04\0\0\0\0\0\0".to_vec(); + let mut app13 = PHOTOSHOP_HEADER.to_vec(); + app13.extend_from_slice(&resources); + let stream = twelve_bit_huffman_block(); + let mut with = stream[..2].to_vec(); + segment(&mut with, 0xED, &app13); + with.extend_from_slice(&stream[2..]); + assert!(uses_craft_jpeg(&with)); + let img = crate::decode(&with).unwrap(); + assert_eq!(img.meta.photoshop_resources.as_deref(), Some(resources.as_slice())); + } + + /// Where the entropy-coded data of the last scan starts. + fn last_scan_data(s: &[u8]) -> usize { + let sos = s.windows(2).rposition(|p| p == [0xFF, 0xDA]).unwrap(); + sos + 2 + usize::from(u16::from_be_bytes([s[sos + 2], s[sos + 3]])) + } + + /// A 12-bit scan whose entropy data runs out, EOI kept, is never a clean image. + #[test] + fn twelve_bit_scan_out_of_data_is_not_clean() { + let full = twelve_bit_huffman_block(); + let data = last_scan_data(&full); + for keep in 0..2 { + let mut s = full[..data + keep].to_vec(); + s.extend([0xFF, 0xD9]); + if let Ok(img) = crate::decode(&s) { + assert!(!img.warnings.is_empty(), "keep {keep}: clean decode of missing data"); + } + } + } + + /// A multi-MCU SOF1 12-bit scan cut in half, EOI kept, decodes with the Truncated warning; a + /// truncated scan too short for its frame is refused, as on the zune-jpeg path. + #[test] + fn damaged_or_truncated_twelve_bit_scan() { + let (w, h) = (64usize, 64usize); + let rgb: Vec = (0..w * h).flat_map(|i| [(i % w * 4) as u8, (i / w * 4) as u8, (i * 7) as u8]).collect(); + let mut out = Vec::new(); + jpeg_encoder::Encoder::new(&mut out, 90).encode(&rgb, w as u16, h as u16, jpeg_encoder::ColorType::Rgb).unwrap(); + // Relabel the baseline frame as 12-bit SOF1: the Huffman data still parses, as different values. + let sof = out.windows(2).position(|p| p == [0xFF, 0xC0]).unwrap(); + out[sof + 1] = 0xC1; + out[sof + 4] = 12; + let data = last_scan_data(&out); + let end = out.len() - 2; + let mut cut = out[..data + (end - data) / 2].to_vec(); + if cut.last() == Some(&0xFF) { + cut.pop(); + } + cut.extend([0xFF, 0xD9]); + let img = crate::decode(&out).unwrap(); + assert_eq!(img.sample_type(), SampleType::U16); + assert!(img.warnings.is_empty(), "{:?}", img.warnings); + let img = crate::decode(&cut).unwrap(); + assert_eq!(img.warnings, vec![DecodeWarning::Truncated { format: Format::Jpeg }]); + // Without EOI the same cut is a truncated file; under a 20000×20000 frame it is refused. + cut.truncate(cut.len() - 2); + assert_eq!(data_end(&cut).0, DataEnd::Truncated); + assert!(crate::decode(&cut).is_ok()); + cut[sof + 5..sof + 9].copy_from_slice(&[0x4E, 0x20, 0x4E, 0x20]); + let e = crate::decode(&cut).unwrap_err(); + assert!(e.to_string().contains("too short for the frame it declares"), "{e}"); + } } From 4198b9fa3bb8e7e1c16002352cf857e26017074b Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 22:06:38 +0000 Subject: [PATCH 2/7] JPEG: follow craft-jpeg's review fixes (non_exhaustive Limits) --- Cargo.lock | 2 +- Cargo.toml | 2 +- crates/codecs/src/codecs/jpeg.rs | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3fdcda91b3..9e7318eae6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1314,7 +1314,7 @@ source = "git+https://github.com/storytold/craft-libs?rev=82a24b006eefa036b43c09 [[package]] name = "craft-jpeg" version = "0.1.0" -source = "git+https://github.com/aiongg/craft-libs?rev=059d1519a6c90ecb0064ffe39020311014ce760d#059d1519a6c90ecb0064ffe39020311014ce760d" +source = "git+https://github.com/aiongg/craft-libs?rev=955831a3a260e398a8e060c4336fe1277d82a2fa#955831a3a260e398a8e060c4336fe1277d82a2fa" dependencies = [ "thiserror 2.0.21", ] diff --git a/Cargo.toml b/Cargo.toml index dc8980032e..2369d9ec1d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,7 +41,7 @@ photocraft-ipp = { path = "crates/ipp" } # external craft-print = { git = "https://github.com/storytold/craft-libs", rev = "3d37da389a3618ab9b76109da11416e9ca4ea719" } craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "82a24b006eefa036b43c09a67fb9a3fbd17ffe63" } -craft-jpeg = { git = "https://github.com/aiongg/craft-libs", rev = "059d1519a6c90ecb0064ffe39020311014ce760d" } +craft-jpeg = { git = "https://github.com/aiongg/craft-libs", rev = "955831a3a260e398a8e060c4336fe1277d82a2fa" } thiserror = "2" serde = { version = "1", features = ["derive", "rc"] } serde_json = "1" diff --git a/crates/codecs/src/codecs/jpeg.rs b/crates/codecs/src/codecs/jpeg.rs index 8bb696638e..323268ff39 100644 --- a/crates/codecs/src/codecs/jpeg.rs +++ b/crates/codecs/src/codecs/jpeg.rs @@ -213,7 +213,9 @@ fn craft_jpeg_error(e: craft_jpeg::Error) -> CodecError { /// Decode a file [`uses_craft_jpeg`] routes away from `zune-jpeg`; `damaged` says entropy-coded /// data was cut short or corrupt. fn decode_craft_jpeg(bytes: &[u8], limits: &Limits, adobe_transform: Option) -> Result<(Image, bool), CodecError> { - let lim = craft_jpeg::Limits { max_pixels: limits.max_pixels, max_alloc: limits.max_alloc }; + let mut lim = craft_jpeg::Limits::default(); + lim.max_pixels = limits.max_pixels; + lim.max_alloc = limits.max_alloc; let d = craft_jpeg::decode(bytes, &lim, adobe_transform).map_err(craft_jpeg_error)?; let layout = match d.components { 1 => ChannelLayout::Gray, From 48166f6ab37e7878f897546f82f64ab5585e817f Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 22:28:39 +0000 Subject: [PATCH 3/7] Pin craft-jpeg to storytold craft-libs 6e7f285 --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9e7318eae6..b27d26a78a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1314,7 +1314,7 @@ source = "git+https://github.com/storytold/craft-libs?rev=82a24b006eefa036b43c09 [[package]] name = "craft-jpeg" version = "0.1.0" -source = "git+https://github.com/aiongg/craft-libs?rev=955831a3a260e398a8e060c4336fe1277d82a2fa#955831a3a260e398a8e060c4336fe1277d82a2fa" +source = "git+https://github.com/storytold/craft-libs?rev=6e7f28518dd1cb57e1624bdc269c3c14316b13ae#6e7f28518dd1cb57e1624bdc269c3c14316b13ae" dependencies = [ "thiserror 2.0.21", ] diff --git a/Cargo.toml b/Cargo.toml index 2369d9ec1d..68d6ae90b1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,7 +41,7 @@ photocraft-ipp = { path = "crates/ipp" } # external craft-print = { git = "https://github.com/storytold/craft-libs", rev = "3d37da389a3618ab9b76109da11416e9ca4ea719" } craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "82a24b006eefa036b43c09a67fb9a3fbd17ffe63" } -craft-jpeg = { git = "https://github.com/aiongg/craft-libs", rev = "955831a3a260e398a8e060c4336fe1277d82a2fa" } +craft-jpeg = { git = "https://github.com/storytold/craft-libs", rev = "6e7f28518dd1cb57e1624bdc269c3c14316b13ae" } thiserror = "2" serde = { version = "1", features = ["derive", "rc"] } serde_json = "1" From ae368d5a615f161f0122780a51bc9148d55e1bbb Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 23:20:40 +0000 Subject: [PATCH 4/7] JPEG: lossless and 12-bit progressive files are unsupported, not damaged --- crates/codecs/src/codecs/jpeg.rs | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/crates/codecs/src/codecs/jpeg.rs b/crates/codecs/src/codecs/jpeg.rs index 323268ff39..f777022a23 100644 --- a/crates/codecs/src/codecs/jpeg.rs +++ b/crates/codecs/src/codecs/jpeg.rs @@ -192,11 +192,13 @@ fn err(e: impl std::fmt::Display) -> CodecError { } /// Whether `craft-jpeg` (rather than `zune-jpeg`) decodes the file: arithmetic coding -/// (SOF9–SOF11) or a DCT frame whose precision isn't 8 bits (#4934, #5003). +/// (SOF9–SOF11) or a DCT frame whose precision isn't 8 bits (#4934, #5003). Lossless (SOF3) and +/// 12-bit progressive Huffman (SOF2) go there too: craft-jpeg reports them unsupported, where +/// zune-jpeg's failure would read as a damaged file. fn uses_craft_jpeg(bytes: &[u8]) -> bool { match craft_jpeg::frame(bytes) { - Some(f) if (0xC9..=0xCB).contains(&f.marker) => true, - Some(f) if matches!(f.marker, 0xC0 | 0xC1) => f.precision != 8, + Some(f) if (0xC9..=0xCB).contains(&f.marker) || f.marker == 0xC3 => true, + Some(f) if matches!(f.marker, 0xC0..=0xC2) => f.precision != 8, _ => false, } } @@ -666,6 +668,19 @@ mod tests { assert!(uses_craft_jpeg(&[0xFF, 0xD8, 0xFF, 0xC1, 0, 11, 12, 0, 8, 0, 8, 1, 1, 0x11, 0])); assert!(uses_craft_jpeg(&[0xFF, 0xD8, 0xFF, 0xC0, 0, 11, 12, 0, 8, 0, 8, 1, 1, 0x11, 0])); assert!(!uses_craft_jpeg(&[0xFF, 0xD8])); + assert!(!uses_craft_jpeg(&[0xFF, 0xD8, 0xFF, 0xC2, 0, 11, 8, 0, 8, 0, 8, 1, 1, 0x11, 0])); + assert!(uses_craft_jpeg(&[0xFF, 0xD8, 0xFF, 0xC2, 0, 11, 12, 0, 8, 0, 8, 1, 1, 0x11, 0])); + assert!(uses_craft_jpeg(&[0xFF, 0xD8, 0xFF, 0xC3, 0, 11, 8, 0, 8, 0, 8, 1, 1, 0x11, 0])); + } + + #[test] + fn lossless_and_twelve_bit_progressive_are_unsupported_not_damaged() { + for marker in [0xC3, 0xC2] { + let mut bytes = vec![0xFF, 0xD8, 0xFF, marker, 0, 11, 12, 0, 8, 0, 8, 1, 1, 0x11, 0]; + bytes.extend_from_slice(&[0xFF, 0xD9]); + let e = crate::decode(&bytes).unwrap_err(); + assert!(matches!(e, CodecError::Unsupported { .. }), "{marker:#x}: {e:?}"); + } } #[test] From 703bc4c821f03c135147de10f50c4478f328dd92 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Sat, 24 Oct 2026 19:49:00 -0400 Subject: [PATCH 5/7] raster: a translated surface shares one rebuilt tile however many threads build it (#9230) Workers that missed the shared-tile memo at the same time each built and kept a copy, so the number of copies grew with the thread count and translated_shares_tiles_rebuilt_from_shared_sources failed on machines with 16 or more threads. Each worker now takes the tile stored first. --- crates/raster/src/lib.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/raster/src/lib.rs b/crates/raster/src/lib.rs index fb6a6fd388..f3778ccbe5 100644 --- a/crates/raster/src/lib.rs +++ b/crates/raster/src/lib.rs @@ -999,9 +999,12 @@ impl Surface { // A target that got only default pixels isn't stored, as `prune` would drop it. Checked // after the copy, a row at a time: a tile with content stops at its first such row. let finished = tile.data.chunks(default_row.len()).any(|row| row != default_row).then(|| Arc::new(tile)); - if let Some(k) = memo_key { - built.lock().unwrap_or_else(std::sync::PoisonError::into_inner).insert(k, finished.clone()); - } + // Workers that missed the memo at the same time each built a copy: all of them take + // the one stored first, so the targets share it however many threads raced. + let finished = match memo_key { + Some(k) => built.lock().unwrap_or_else(std::sync::PoisonError::into_inner).entry(k).or_insert(finished).clone(), + None => finished, + }; finished.map(|t| (dc, t)) }; #[cfg(not(target_arch = "wasm32"))] @@ -1869,8 +1872,9 @@ mod tests { let moved = s.translated(7, 5, s.content_bounds()); let distinct: std::collections::HashSet<_> = moved.tiles().map(|(_, t)| Arc::as_ptr(t)).collect(); // The 15 × 15 interior tiles read the same four source tiles at the same offset, so they are - // one tile; only the 64 border tiles (cut by the content's edge) are built one by one. - assert!(moved.tile_count() == 289 && distinct.len() <= 64 + 5, "{} tiles, {} distinct", moved.tile_count(), distinct.len()); + // one tile, however many threads build them; only the 64 border tiles (cut by the + // content's edge) are built one by one. + assert!(moved.tile_count() == 289 && distinct.len() == 64 + 1, "{} tiles, {} distinct", moved.tile_count(), distinct.len()); let all = Rect::new(-50, -50, 16 * TILE_SIZE + 50, 16 * TILE_SIZE + 50); assert_eq!(moved.read_region(all), s.read_region(all.translate(-7, -5))); } From 3d0c176e80eb1e8a03aedb391549cee59b21418b Mon Sep 17 00:00:00 2001 From: aiongg Date: Sun, 25 Oct 2026 00:00:01 +0000 Subject: [PATCH 6/7] JPEG: a craft-jpeg frame without scan data is refused before decoding --- crates/codecs/src/codecs/jpeg.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/codecs/src/codecs/jpeg.rs b/crates/codecs/src/codecs/jpeg.rs index f777022a23..3e533ccd08 100644 --- a/crates/codecs/src/codecs/jpeg.rs +++ b/crates/codecs/src/codecs/jpeg.rs @@ -246,6 +246,10 @@ pub(crate) fn decode(bytes: &[u8], limits: &Limits) -> Result limits.check_bytes(w, h, u64::from(nc.max(1)))?; } if uses_craft_jpeg(bytes) { + // craft-jpeg sizes its buffers from the frame header before it reads a scan. + if scan_bytes == 0 { + return Err(err("the file has no scan data")); + } if let Some((w, h, _)) = meta.frame && end == DataEnd::Truncated && u64::from(w) * u64::from(h) / MAX_TRUNCATED_PIXELS_PER_BYTE > scan_bytes @@ -677,12 +681,20 @@ mod tests { fn lossless_and_twelve_bit_progressive_are_unsupported_not_damaged() { for marker in [0xC3, 0xC2] { let mut bytes = vec![0xFF, 0xD8, 0xFF, marker, 0, 11, 12, 0, 8, 0, 8, 1, 1, 0x11, 0]; - bytes.extend_from_slice(&[0xFF, 0xD9]); + bytes.extend_from_slice(&[0xFF, 0xDA, 0, 8, 1, 1, 0, 0, 0x3F, 0, 0x12, 0x34, 0xFF, 0xD9]); let e = crate::decode(&bytes).unwrap_err(); assert!(matches!(e, CodecError::Unsupported { .. }), "{marker:#x}: {e:?}"); } } + #[test] + fn a_twelve_bit_frame_without_scan_data_is_refused_before_decoding() { + let mut bytes = vec![0xFF, 0xD8, 0xFF, 0xC1, 0, 11, 12, 0x40, 0, 0x40, 0, 1, 1, 0x11, 0]; + bytes.extend_from_slice(&[0xFF, 0xDA, 0, 8, 1, 1, 0, 0, 0x3F, 0, 0xFF, 0xD9]); + let e = crate::decode(&bytes).unwrap_err(); + assert!(e.to_string().contains("no scan data"), "{e}"); + } + #[test] fn arithmetic_eight_bit_block_is_eight_bit() { let img = crate::decode(&arithmetic_block(8)).unwrap(); From b278ff72459cbf9754863365edd822ba2248aa07 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 21:20:41 -0400 Subject: [PATCH 7/7] security(affinity): charge archive names, failed reads, corner arrays, attachments, pages and font names (#9257) * security(affinity): charge archive names, failed reads, corner arrays, attachments, pages and font names Bound the remaining per-occurrence and table-driven expansion work left after #8938: aggregate archive entry-name bytes (copied only for the winning name), extraction allowance charged before decoding so failed checksums are not free, CnrD corner arrays charged as curve records, AdCh and gradient arrays charged as slots (gradient positions copied only up to the stop limit), a document-wide page-rectangle cap, and per-field and aggregate limits on retained font names. Co-Authored-By: Claude Sonnet 5.5 * security(affinity): charge pixel state-array scans against the slot budget Co-Authored-By: Claude Sonnet 5.5 * fix(affinity): charge decoded name length and bound PagR rectangle collection --------- Co-authored-by: Claude Sonnet 5.5 --- crates/affinity/src/container.rs | 20 ++++++++-- crates/affinity/src/geometry.rs | 16 +++++++- crates/affinity/src/model.rs | 68 +++++++++++++++++++++++++------- crates/affinity/src/paint.rs | 8 +++- crates/affinity/src/raster.rs | 2 + crates/affinity/src/tests.rs | 14 +++++++ crates/affinity/src/text.rs | 25 ++++++++++-- 7 files changed, 131 insertions(+), 22 deletions(-) diff --git a/crates/affinity/src/container.rs b/crates/affinity/src/container.rs index 0250cc4f77..76ffb69f7d 100644 --- a/crates/affinity/src/container.rs +++ b/crates/affinity/src/container.rs @@ -17,6 +17,9 @@ const DOCUMENT_KIND: &[u8; 4] = b"nsrP"; const MAX_TABLES: usize = 4096; /// Distinct ids the allocation tables may name; each costs a map slot even when only deleted. const MAX_ENTRIES: usize = 1 << 22; +/// Bytes of entry names the allocation tables may name. A name is retained twice (in the entry +/// and as its map key), and tables can point many names at the same bytes. +const MAX_NAME_BYTES: usize = 32 << 20; /// zstd window ceiling: real documents use a few MiB; this stops a header from reserving gigabytes. const MAX_ZSTD_WINDOW: u64 = 64 << 20; @@ -168,6 +171,7 @@ impl<'a> Archive<'a> { // Newest record per id: (table date, table index, record). let mut latest: HashMap>)> = HashMap::new(); let mut names: HashMap = HashMap::new(); + let mut name_bytes = 0usize; let mut seen = HashSet::new(); let mut next = header.fat_offset; let mut tables = 0usize; @@ -231,8 +235,16 @@ impl<'a> Archive<'a> { } if flag == 0 { let len = usize::from(c.u16()?); - let name = String::from_utf8_lossy(c.take(len)?).into_owned(); - names.entry(id).or_insert(name); + let raw = c.take(len)?; + // Only the first name per id is kept: charge and copy just that one. + if let std::collections::hash_map::Entry::Vacant(slot) = names.entry(id) { + let name = String::from_utf8_lossy(raw).into_owned(); + name_bytes = name_bytes.saturating_add(name.len()); + if name_bytes > MAX_NAME_BYTES { + return Err(Error::Limit("archive entry names too large")); + } + slot.insert(name); + } } // Equal dates: the first table in the chain (the newest) wins, as in afread. let replace = latest.get(&id).is_none_or(|(d, _, _)| date > *d); @@ -284,6 +296,9 @@ impl<'a> Archive<'a> { if total > self.limits.max_total { return Err(Error::Limit("document too large")); } + // Charge before decoding: a corrupt entry that fails its checksum has already cost the + // decompression, and an importer that recovers must not get it again for free. + self.extracted = total; let mut c = Cursor::new(self.bytes, offset(e.offset)?); if c.take(4)? != b"#Fil" { return Err(Error::Malformed("archive entry has no data block")); @@ -303,7 +318,6 @@ impl<'a> Archive<'a> { if crc32fast::hash(&out) != e.crc { return Err(Error::Malformed("archive entry checksum mismatch")); } - self.extracted = total; Ok(out) } } diff --git a/crates/affinity/src/geometry.rs b/crates/affinity/src/geometry.rs index c62c206f58..fa45cf6213 100644 --- a/crates/affinity/src/geometry.rs +++ b/crates/affinity/src/geometry.rs @@ -148,7 +148,21 @@ pub(crate) fn curves(r: &mut Reader, crvs: ObjId) -> Option { Value::UInt(n) => usize::try_from(*n).ok()?, _ => return None, }; - let corners = s.obj(crvs, b"CnrD").and_then(|c| corner_radii(s.positional(c).collect())); + let corners = match s.obj(crvs, b"CnrD") { + Some(c) => { + let fields: Vec<&Value> = s.positional(c).collect(); + // The corner arrays are scanned for every occurrence of a shared curve, so charge + // them like records before they are walked. + if let Some(Value::Array(idx)) = fields.get(1) + && let Err(reason) = r.geometry_budget.records(idx.len()) + { + r.warn(reason); + return None; + } + corner_radii(fields) + } + None => None, + }; let mut path = Path::default(); let mut seen = 0usize; for index in 0..count { diff --git a/crates/affinity/src/model.rs b/crates/affinity/src/model.rs index 1d8e5918be..c59afa1cac 100644 --- a/crates/affinity/src/model.rs +++ b/crates/affinity/src/model.rs @@ -290,6 +290,10 @@ const MAX_NODES: usize = 500_000; const MAX_CHILD_SLOTS: usize = 8_000_000; /// Bytes of layer names the model may retain across all occurrences. const MAX_NAME_BYTES: usize = 64 << 20; +/// Page rectangles the model may retain across all spreads (real books have a few hundred). +const MAX_PAGES: usize = 1 << 18; +/// Bytes of font names the text runs may retain across the document. +const MAX_FONT_NAME_BYTES: usize = 16 << 20; /// Read a document: archive, `doc.dat` and the model. pub fn read(bytes: &[u8], limits: Limits) -> Result { @@ -307,6 +311,8 @@ pub(crate) struct Reader<'s, 'a, 'b> { nodes: usize, child_slots: usize, name_bytes: usize, + pages: usize, + font_name_bytes: usize, active: HashSet, pub(crate) geometry_budget: crate::geometry::Budget, /// Decoded pixels still available ([`Limits::max_pixel_bytes`] less what the layers read so @@ -324,6 +330,8 @@ impl<'s, 'a, 'b> Reader<'s, 'a, 'b> { nodes: 0, child_slots: 0, name_bytes: 0, + pages: 0, + font_name_bytes: 0, active: HashSet::new(), geometry_budget: crate::geometry::Budget::default(), pixels_left: limits.max_pixel_bytes, @@ -365,7 +373,7 @@ impl<'s, 'a, 'b> Reader<'s, 'a, 'b> { self.warn("an unknown kind of page"); continue; } - let pages = pages(s, spread); + let pages = self.pages(spread)?; let default_size = s.floats::<2>(doc, b"DfSz").filter(|[w, h]| *w > 0.0 && *h > 0.0).map(|[w, h]| Rect { x0: 0.0, y0: 0.0, x1: w, y1: h }); let bounds = s.floats::<4>(spread, b"SprB").map(rect).or_else(|| pages.iter().copied().reduce(union)).or(default_size).unwrap_or(Rect { x0: 0.0, @@ -421,7 +429,7 @@ impl<'s, 'a, 'b> Reader<'s, 'a, 'b> { } /// Counts raw array slots (null ones included) against [`MAX_CHILD_SLOTS`]. - fn charge_slots(&mut self, n: usize) -> Result<(), Error> { + pub(crate) fn charge_slots(&mut self, n: usize) -> Result<(), Error> { self.child_slots = self.child_slots.saturating_add(n); if self.child_slots > MAX_CHILD_SLOTS { return Err(Error::Limit("too many child slots in the layer tree")); @@ -553,6 +561,8 @@ impl<'s, 'a, 'b> Reader<'s, 'a, 'b> { /// an attached mask layer is a pixel mask. Adjustments attached the same way are reported. fn attached_masks(&mut self, id: ObjId, world: Affine) -> Result<(Option, Option), Error> { let s = self.s; + // Every occurrence of a shared layer scans its whole attachment array, null slots included. + self.charge_slots(s.array_len(id, b"AdCh").unwrap_or(0))?; let attached = s.objs(id, b"AdCh"); let mut mask: Option = None; let mut pixels: Option = None; @@ -601,21 +611,51 @@ impl<'s, 'a, 'b> Reader<'s, 'a, 'b> { } } -/// Page rectangles: `SpMd.PagR[].rctp` (Publisher 2, Affinity 3) or `SprB` split into `PagC` -/// equal pages (Publisher 1 facing spreads). -fn pages(s: &Stream, spread: ObjId) -> Vec { - if let Some(md) = s.obj(spread, b"SpMd") { - let v: Vec = s.objs(md, b"PagR").into_iter().filter_map(|p| s.floats::<4>(p, b"rctp").map(rect)).filter(|r| r.x1 > r.x0 && r.y1 > r.y0).collect(); - if !v.is_empty() { - return v; +impl Reader<'_, '_, '_> { + /// Page rectangles: `SpMd.PagR[].rctp` (Publisher 2, Affinity 3) or `SprB` split into `PagC` + /// equal pages (Publisher 1 facing spreads). A spread listed several times, or sharing one + /// large page array, is expanded into a new vector each time, so the slots scanned and the + /// rectangles retained are charged across the document. + fn pages(&mut self, spread: ObjId) -> Result, Error> { + let s = self.s; + if let Some(md) = s.obj(spread, b"SpMd") { + self.charge_slots(s.array_len(md, b"PagR").unwrap_or(0))?; + let remaining = MAX_PAGES.saturating_sub(self.pages); + let v: Vec = s + .objs(md, b"PagR") + .into_iter() + .filter_map(|p| s.floats::<4>(p, b"rctp").map(rect)) + .filter(|r| r.x1 > r.x0 && r.y1 > r.y0) + .take(remaining.saturating_add(1)) + .collect(); + if !v.is_empty() { + self.charge_pages(v.len())?; + return Ok(v); + } } + Ok(match (s.floats::<4>(spread, b"SprB").map(rect), s.int(spread, b"PagC")) { + (Some(b), Some(n @ 2..=16)) => { + self.charge_pages(n as usize)?; + let w = (b.x1 - b.x0) / n as f64; + (0..n).map(|i| Rect { x0: b.x0 + w * i as f64, x1: b.x0 + w * (i + 1) as f64, ..b }).collect() + } + _ => Vec::new(), + }) } - match (s.floats::<4>(spread, b"SprB").map(rect), s.int(spread, b"PagC")) { - (Some(b), Some(n @ 2..=16)) => { - let w = (b.x1 - b.x0) / n as f64; - (0..n).map(|i| Rect { x0: b.x0 + w * i as f64, x1: b.x0 + w * (i + 1) as f64, ..b }).collect() + + /// Counts retained font-name bytes against [`MAX_FONT_NAME_BYTES`]; false once it is spent. + pub(crate) fn charge_font_name(&mut self, n: usize) -> bool { + self.font_name_bytes = self.font_name_bytes.saturating_add(n); + self.font_name_bytes <= MAX_FONT_NAME_BYTES + } + + /// Counts retained page rectangles against [`MAX_PAGES`]. + fn charge_pages(&mut self, n: usize) -> Result<(), Error> { + self.pages = self.pages.saturating_add(n); + if self.pages > MAX_PAGES { + return Err(Error::Limit("too many pages")); } - _ => Vec::new(), + Ok(()) } } diff --git a/crates/affinity/src/paint.rs b/crates/affinity/src/paint.rs index 343c276b60..1847fad90f 100644 --- a/crates/affinity/src/paint.rs +++ b/crates/affinity/src/paint.rs @@ -232,9 +232,15 @@ pub(crate) fn descriptor(r: &mut Reader, id: ObjId, local: Affine) -> Option> = match s.field(grad, b"Posn") { - Some(Value::Array(a)) => a.iter().filter_map(floats).collect(), + Some(Value::Array(a)) => a.iter().filter_map(floats).take(MAX_STOPS).collect(), _ => Vec::new(), }; let mut stops = Vec::new(); diff --git a/crates/affinity/src/raster.rs b/crates/affinity/src/raster.rs index f2b99c195e..226c66a459 100644 --- a/crates/affinity/src/raster.rs +++ b/crates/affinity/src/raster.rs @@ -193,6 +193,8 @@ fn plane( } .iter() .filter_map(|v| if let Value::Obj(Some(o)) = v { Some(*o) } else { None }); + // The scan below runs for every occurrence of a shared bitmap, whatever it produces. + r.charge_slots(states.len().min(tw.saturating_mul(th)))?; let row = (w as usize) * bps; let mut out = vec![0u8; row * h as usize]; let (bx0, bx1) = (x0 as usize * bps, (x0 + w) as usize * bps); diff --git a/crates/affinity/src/tests.rs b/crates/affinity/src/tests.rs index c932aac399..ab59c440b4 100644 --- a/crates/affinity/src/tests.rs +++ b/crates/affinity/src/tests.rs @@ -258,6 +258,20 @@ mod archive { assert!(matches!(a.read("doc.dat"), Err(Error::Limit(_)))); } + #[test] + fn a_failed_read_still_spends_the_extraction_allowance() { + let d = doc(); + let file = synth::container(&[("doc.dat", &d, Method::Zstd)], None); + let mut broken = file.clone(); + broken[90] ^= 0x55; + // Room for exactly one read: the failed attempt must use it up, so a caller that + // recovers from the checksum error cannot make the same entry decode again. + let mut a = Archive::open(&broken, Limits { max_entry: usize::MAX, max_total: d.len(), ..Limits::default() }).unwrap(); + assert!(a.read("doc.dat").is_err()); + assert_eq!(a.extracted(), d.len()); + assert!(matches!(a.read("doc.dat"), Err(Error::Limit(_)))); + } + #[test] fn a_declared_size_smaller_than_the_data_is_rejected_not_truncated() { let big = vec![7u8; 1 << 20]; diff --git a/crates/affinity/src/text.rs b/crates/affinity/src/text.rs index 3b09c75124..bfad07217f 100644 --- a/crates/affinity/src/text.rs +++ b/crates/affinity/src/text.rs @@ -106,6 +106,24 @@ fn int(v: &Value) -> Option { } } +/// Longest font name kept; real PostScript and family names are far shorter. +const MAX_FONT_NAME: usize = 256; + +/// A font name for one run. Runs repeat the same shared string, so its copies are charged +/// against a document-wide byte budget; an overlong name or an exhausted budget leaves it empty. +fn font_name(r: &mut Reader, font: Option, tag: &[u8; 4]) -> String { + let name = font.and_then(|f| r.s.str(f, tag)).unwrap_or_default(); + if name.len() > MAX_FONT_NAME { + r.warn("font names longer than 256 bytes (left empty)"); + return String::new(); + } + if !r.charge_font_name(name.len()) { + r.warn("font names beyond the document metadata budget (left empty)"); + return String::new(); + } + name.to_string() +} + fn run_attrs(r: &mut Reader, attrs: ObjId, text: String, world: Affine) -> TextRun { let s = r.s; let doubles = s.field(attrs, b"Doub"); @@ -125,11 +143,12 @@ fn run_attrs(r: &mut Reader, attrs: ObjId, text: String, world: Affine) -> TextR r.warn("horizontally scaled text"); } let font = s.obj(attrs, b"DFnt"); - let get = |tag: &[u8; 4]| font.and_then(|f| s.str(f, tag)).unwrap_or_default().to_string(); + let postscript = font_name(r, font, b"Post"); + let family = font_name(r, font, b"Famy"); TextRun { text, - postscript: get(b"Post"), - family: get(b"Famy"), + postscript, + family, weight: font.and_then(|f| s.int(f, b"Wegt")).unwrap_or(400), italic: font.and_then(|f| s.bool(f, b"Ital")).unwrap_or(false), size,