From 058815cd802a81877fab31fa396aa4622faebd64 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Thu, 29 Oct 2026 17:27:32 -0500 Subject: [PATCH 1/7] Honor embedded raster resolution when placing graphics (#1991) A 600px photo saved at 600dpi places 1in wide, as in InDesign, instead of one pixel per point. Reads JFIF/EXIF (JPEG), pHYs (PNG), EXIF (WebP), pixels-per-meter (BMP) and TIFF resolution tags; insane metadata falls back to 72ppi. --- crates/engine/src/cmd/file/place.rs | 40 +++++ crates/images/src/lib.rs | 15 +- crates/images/src/resolution.rs | 236 ++++++++++++++++++++++++++++ crates/images/src/tests.rs | 228 +++++++++++++++++++++++++++ 4 files changed, 516 insertions(+), 3 deletions(-) create mode 100644 crates/images/src/resolution.rs diff --git a/crates/engine/src/cmd/file/place.rs b/crates/engine/src/cmd/file/place.rs index a30cb2321..29358d834 100644 --- a/crates/engine/src/cmd/file/place.rs +++ b/crates/engine/src/cmd/file/place.rs @@ -589,6 +589,46 @@ pub(super) mod image_place_tests { } } + #[test] + fn placed_photo_honors_its_embedded_resolution() { + // #1991: a placed graphic's embedded density sets its size (600 dpi: 40 px place + // 4.8 pt wide), instead of a pixel always becoming a point. + fn crc32(data: &[u8]) -> u32 { + let mut crc = 0xFFFF_FFFFu32; + for &b in data { + crc ^= b as u32; + for _ in 0..8 { + crc = if crc & 1 == 1 { (crc >> 1) ^ 0xEDB8_8320 } else { crc >> 1 }; + } + } + !crc + } + let png = designcraft_render::Rendered { width: 40, height: 20, pixels: [30, 90, 180, 255].repeat(800) }.to_png(); + assert_eq!(&png[12..16], b"IHDR"); + let mut phys = Vec::new(); + phys.extend_from_slice(&23622u32.to_be_bytes()); + phys.extend_from_slice(&23622u32.to_be_bytes()); + phys.push(1); + let mut bytes = png[..33].to_vec(); + bytes.extend_from_slice(&(phys.len() as u32).to_be_bytes()); + bytes.extend_from_slice(b"pHYs"); + bytes.extend_from_slice(&phys); + let mut c = b"pHYs".to_vec(); + c.extend_from_slice(&phys); + bytes.extend_from_slice(&crc32(&c).to_be_bytes()); + bytes.extend_from_slice(&png[33..]); + let file = tempfile::Builder::new().suffix(".png").tempfile().unwrap(); + std::fs::write(file.path(), bytes).unwrap(); + let mut session = Session::new(); + session.execute("file.new", &json!({})).unwrap(); + let placed = session.execute("file.place", &json!({"path": file.path(), "x": 10.0, "y": 20.0})).unwrap(); + let document = &session.doc().unwrap().doc; + let item = document.item(ItemId(placed["id"].as_u64().unwrap())).unwrap(); + let b = item.bounds(); + assert!((b.x0 - 10.0).abs() < 1e-9 && (b.y0 - 20.0).abs() < 1e-9, "{b:?}"); + assert!((b.width() - 4.8).abs() < 0.01 && (b.height() - 2.4).abs() < 0.01, "{b:?}"); + } + #[test] fn explicit_frame_wins_over_placement_and_selection() { let image = image_file().unwrap(); diff --git a/crates/images/src/lib.rs b/crates/images/src/lib.rs index 83c540e6e..7540eee90 100644 --- a/crates/images/src/lib.rs +++ b/crates/images/src/lib.rs @@ -12,6 +12,7 @@ pub mod colour; mod eps; mod jpeg; mod psd; +mod resolution; mod tone; pub use eps::{bounding_box as eps_bounding_box, eps_pixel_size, eps_png, eps_proxy, eps_size, is_eps}; pub use tone::{Tone, tone}; @@ -358,8 +359,11 @@ pub fn svg_tree(bytes: &[u8]) -> Option { usvg::Tree::from_data(&xml, svg_options()).ok() } -/// Natural size of a placed graphic in points: rasters at 72 ppi (one pixel per point), PSDs -/// likewise, SVGs from their width/height (CSS pixels), PDFs are the caller's. +pub use resolution::raster_resolution; + +/// Natural size of a placed graphic in points: rasters at their embedded resolution when sane +/// (one pixel per point, 72 ppi, without usable metadata), PSDs likewise, SVGs from their +/// width/height (CSS pixels), PDFs are the caller's. pub fn natural_size(bytes: &[u8]) -> Option<(f64, f64)> { if is_svg(bytes) { let s = svg_tree(bytes)?.size(); @@ -370,7 +374,12 @@ pub fn natural_size(bytes: &[u8]) -> Option<(f64, f64)> { return Some((bb[2] - bb[0], bb[3] - bb[1])).filter(|(w, h)| w.is_finite() && h.is_finite() && *w > 0.0 && *h > 0.0); } let (w, h) = pixel_size(bytes)?; - Some((w as f64, h as f64)) + // Honor the file's density (#1991): a 600 px photo saved at 600 dpi places one inch wide, + // as in InDesign. Absurd metadata falls back to a pixel per point. + match raster_resolution(bytes) { + Some(dpi) if resolution::sane_resolution(dpi) => Some((w as f64 * 72.0 / dpi.0, h as f64 * 72.0 / dpi.1)), + _ => Some((w as f64, h as f64)), + } } /// Pixel dimensions of a raster. diff --git a/crates/images/src/resolution.rs b/crates/images/src/resolution.rs new file mode 100644 index 000000000..ab1dc119f --- /dev/null +++ b/crates/images/src/resolution.rs @@ -0,0 +1,236 @@ +//! Embedded raster resolution: the density a placed graphic was saved with, so placing honors +//! it instead of assuming 72 ppi (#1991: a 600 px photo saved at 600 dpi places one inch wide, +//! as in InDesign). Reads JFIF/EXIF from JPEG (and EXIF from WebP), pHYs from PNG, the resolution +//! tags from TIFF, and the header density from BMP. GIF, PSD and files without (usable) metadata +//! report nothing and keep the pixel-is-a-point size. + +/// Dots per inch in x and y, when the file carries usable resolution metadata. +pub fn raster_resolution(bytes: &[u8]) -> Option<(f64, f64)> { + if bytes.len() < 8 { + return None; + } + if bytes.starts_with(b"\xFF\xD8\xFF") { + return jpeg_resolution(bytes); + } + if bytes.starts_with(b"\x89PNG\r\n\x1a\n") { + return png_resolution(bytes); + } + if bytes.starts_with(b"RIFF") && bytes.get(8..12) == Some(b"WEBP".as_slice()) { + return webp_resolution(bytes); + } + if bytes.starts_with(b"BM") { + return bmp_resolution(bytes); + } + if is_classic_tiff(bytes) { + return tiff_resolution(bytes); + } + None +} + +/// A parsed density is usable when both axes are finite, positive and below the absurdity cap +/// (a corrupt or hostile value falls back to 72 ppi at the call site). +pub fn sane_resolution(dpi: (f64, f64)) -> bool { + let (dx, dy) = dpi; + dx.is_finite() && dy.is_finite() && dx > 0.0 && dy > 0.0 && dx <= 100_000.0 && dy <= 100_000.0 +} + +fn is_classic_tiff(bytes: &[u8]) -> bool { + bytes.starts_with(b"II*\x00") || bytes.starts_with(b"MM\x00*") +} + +// ---------- JPEG ---------- + +/// Scan JPEG markers for a JFIF density, preferring an EXIF resolution when both are present. +/// Stops at the start of scan (entropy data can contain marker-like bytes). +fn jpeg_resolution(bytes: &[u8]) -> Option<(f64, f64)> { + let mut pos = 2; + let mut jfif: Option<(f64, f64)> = None; + for _ in 0..512 { + // Markers are FF-prefixed; skip fill bytes. Running out of bytes ends the scan + // (a truncated file keeps a JFIF density found before the truncation). + let Some(mut m) = bytes.get(pos).copied() else { break }; + while m == 0xFF { + pos += 1; + let Some(b) = bytes.get(pos).copied() else { break }; + m = b; + if m != 0xFF { + break; + } + } + if m == 0xFF { + break; + } + pos += 1; + // Standalone markers carry no payload; SOS starts entropy-coded data. + if m == 0xD8 || m == 0xD9 || (0xD0..=0xD7).contains(&m) || m == 0x01 { + continue; + } + if m == 0xDA { + break; + } + let Some(&a) = bytes.get(pos) else { break }; + let Some(&b) = bytes.get(pos + 1) else { break }; + let len = u16::from_be_bytes([a, b]) as usize; + if len < 2 || pos + len > bytes.len() { + break; + } + let payload = &bytes[pos + 2..pos + len]; + if m == 0xE0 && jfif.is_none() { + jfif = jfif_density(payload); + } else if m == 0xE1 && let Some(dpi) = exif_resolution(payload) { + // A non-EXIF APP1 (XMP, ICC, ...) parses to nothing: keep scanning, since a + // later APP1 can still carry the EXIF resolution. + return Some(dpi); + } + pos += len; + } + jfif +} + +/// JFIF APP0 density: version, units (0 aspect, 1 dpi, 2 dots/cm), x/y density. +fn jfif_density(payload: &[u8]) -> Option<(f64, f64)> { + if !payload.starts_with(b"JFIF\x00") || payload.len() < 14 { + return None; + } + let units = payload[7]; + let (x, y) = (u16::from_be_bytes([payload[8], payload[9]]), u16::from_be_bytes([payload[10], payload[11]])); + match (units, x, y) { + (1, 1.., 1..) => Some((x as f64, y as f64)), + (2, 1.., 1..) => Some((x as f64 * 2.54, y as f64 * 2.54)), + _ => None, + } +} + +/// EXIF APP1 payload ("Exif\0\0" + a TIFF header): the resolution tags of IFD0. +fn exif_resolution(payload: &[u8]) -> Option<(f64, f64)> { + let tiff = payload.strip_prefix(b"Exif\x00\x00")?; + if !is_classic_tiff(tiff) { + return None; + } + tiff_resolution(tiff) +} + +// ---------- TIFF (also the body of EXIF) ---------- + +/// Classic TIFF resolution tags of the first IFD: XResolution/YResolution rationals scaled by +/// ResolutionUnit (2 = inch, 3 = cm; a missing unit defaults to inch per the spec). +pub fn tiff_resolution(tiff: &[u8]) -> Option<(f64, f64)> { + let le = match &tiff[..2] { + b"II" => true, + b"MM" => false, + _ => return None, + }; + let u16at = |i: usize| -> Option { + let b: [u8; 2] = tiff.get(i..i + 2)?.try_into().ok()?; + Some(if le { u16::from_le_bytes(b) } else { u16::from_be_bytes(b) }) + }; + let u32at = |i: usize| -> Option { + let b: [u8; 4] = tiff.get(i..i + 4)?.try_into().ok()?; + Some(if le { u32::from_le_bytes(b) } else { u32::from_be_bytes(b) }) + }; + let ifd = u32at(4)? as usize; + let count = u16at(ifd)? as usize; + if count > 512 { + return None; + } + let mut xres: Option<(u32, u32)> = None; + let mut yres: Option<(u32, u32)> = None; + let mut unit: Option = None; + for n in 0..count { + let e = ifd.checked_add(2)?.checked_add(n.checked_mul(12)?)?; + let (tag, typ, cnt) = (u16at(e)?, u16at(e + 2)?, u32at(e + 4)?); + // SHORT values of count 1 live inline in the value field (low bytes LE, high bytes BE). + let short_inline = || -> Option { + let raw = u32at(e + 8)?; + Some(if le { (raw & 0xFFFF) as u16 } else { (raw >> 16) as u16 }) + }; + // RATIONAL values of count 1 live at the offset (relative to the TIFF header start). + let rational_at = || -> Option<(u32, u32)> { + if cnt != 1 { + return None; + } + let off = u32at(e + 8)? as usize; + if off.checked_add(8)? > tiff.len() { + return None; + } + Some((u32at(off)?, u32at(off + 4)?)) + }; + match (tag, typ) { + (0x011A, 5) if xres.is_none() => xres = rational_at(), + (0x011B, 5) if yres.is_none() => yres = rational_at(), + (0x0128, 3) if unit.is_none() => unit = short_inline(), + _ => {} + } + } + let num = |(n, d): (u32, u32)| (d != 0).then(|| n as f64 / d as f64); + let (x, y) = (num(xres?)?, num(yres?)?); + match unit.unwrap_or(2) { + 2 => Some((x, y)), + 3 => Some((x * 2.54, y * 2.54)), + _ => None, + } +} + +// ---------- PNG ---------- + +/// The pHYs chunk: pixels-per-unit in x and y plus the unit (1 = meter → dpi). +fn png_resolution(bytes: &[u8]) -> Option<(f64, f64)> { + let mut pos = 8; + for _ in 0..64 { + let len = u32::from_be_bytes(bytes.get(pos..pos + 4)?.try_into().ok()?) as usize; + let kind = bytes.get(pos + 4..pos + 8)?; + if kind == b"pHYs" { + let data = bytes.get(pos + 8..pos + 8 + 9)?; + let (x, y) = (u32::from_be_bytes(data[0..4].try_into().ok()?), u32::from_be_bytes(data[4..8].try_into().ok()?)); + if data[8] == 1 && x > 0 && y > 0 { + return Some((x as f64 * 0.0254, y as f64 * 0.0254)); + } + return None; + } + if kind == b"IEND" { + return None; + } + pos = pos.checked_add(8)?.checked_add(len)?.checked_add(4)?; + if pos >= bytes.len() { + return None; + } + } + None +} + +// ---------- WebP ---------- + +/// A WebP EXIF chunk carries a TIFF header: reuse the TIFF resolution tags. +fn webp_resolution(bytes: &[u8]) -> Option<(f64, f64)> { + let riff = u32::from_le_bytes(bytes.get(4..8)?.try_into().ok()?) as usize; + let end = 12usize.checked_add(riff)?.min(bytes.len()); + let mut pos = 12; + for _ in 0..64 { + let kind = bytes.get(pos..pos + 4)?; + let size = u32::from_le_bytes(bytes.get(pos + 4..pos + 8)?.try_into().ok()?) as usize; + let data = pos.checked_add(8)?; + if data.checked_add(size)? > end { + return None; + } + if kind == b"EXIF" && is_classic_tiff(bytes.get(data..data + size.min(4)).unwrap_or_default()) { + return tiff_resolution(&bytes[data..(data + size).min(bytes.len())]); + } + pos = data.checked_add(size.checked_add(size & 1)?)?; + } + None +} + +// ---------- BMP ---------- + +/// BITMAPINFOHEADER pixels-per-meter (offset 38/42): meters → dpi. +fn bmp_resolution(bytes: &[u8]) -> Option<(f64, f64)> { + if bytes.len() < 46 || u32::from_le_bytes(bytes.get(14..18)?.try_into().ok()?) < 40 { + return None; + } + let ppm = |i: usize| -> Option { Some(i32::from_le_bytes(bytes.get(i..i + 4)?.try_into().ok()?)) }; + let (x, y) = (ppm(38)?, ppm(42)?); + if x > 0 && y > 0 { + return Some((x as f64 * 0.0254, y as f64 * 0.0254)); + } + None +} diff --git a/crates/images/src/tests.rs b/crates/images/src/tests.rs index 3e5daf8ab..917693f87 100644 --- a/crates/images/src/tests.rs +++ b/crates/images/src/tests.rs @@ -432,6 +432,234 @@ fn cmyk_tiff_with_alpha_reports_its_pixel_size() { assert_eq!(pixel_size(&b), Some((2, 1))); } +// ---------- embedded resolution (#1991) ---------- + +/// A real encoder-written 600x900 JPEG with an APPn segment inserted after SOI (decoders skip +/// APPn, so the file stays fully valid and dimension readers exercise the real path). +fn jpeg_with(app: &[u8]) -> Vec { + use image::codecs::jpeg::JpegEncoder; + let img = image::RgbaImage::from_pixel(600, 900, image::Rgba([200, 30, 30, 255])); + let mut b = Vec::new(); + JpegEncoder::new_with_quality(&mut b, 90).encode_image(&img).unwrap(); + assert_eq!(&b[..2], &[0xFF, 0xD8]); + let mut out = b[..2].to_vec(); + out.extend_from_slice(app); + out.extend_from_slice(&b[2..]); + out +} + +/// JFIF APP0 with the given units (0 aspect, 1 dpi, 2 per cm) and densities. +fn jfif_app(units: u8, x: u16, y: u16) -> Vec { + let mut a = vec![0xFF, 0xE0, 0x00, 0x10, b'J', b'F', b'I', b'F', 0x00, 0x01, 0x02, units]; + a.extend_from_slice(&x.to_be_bytes()); + a.extend_from_slice(&y.to_be_bytes()); + a.extend_from_slice(&[0x00, 0x00]); + a +} + +/// Minimal little-endian TIFF with X/YResolution rationals and an optional ResolutionUnit. +fn tiff_bytes(x: (u32, u32), y: (u32, u32), unit: Option) -> Vec { + let mut t = vec![b'I', b'I', 0x2A, 0x00, 0x08, 0x00, 0x00, 0x00]; + let n = if unit.is_some() { 3 } else { 2 }; + t.extend_from_slice(&(n as u16).to_le_bytes()); + let data = 8 + 2 + n * 12 + 4; + let mut entry = |tag: u16, typ: u16, val: u32| { + t.extend_from_slice(&tag.to_le_bytes()); + t.extend_from_slice(&typ.to_le_bytes()); + t.extend_from_slice(&1u32.to_le_bytes()); + t.extend_from_slice(&val.to_le_bytes()); + }; + entry(0x011A, 5, data as u32); + entry(0x011B, 5, (data + 8) as u32); + if let Some(u) = unit { + // SHORT count 1: the value sits inline in the low bytes. + t.extend_from_slice(&0x0128u16.to_le_bytes()); + t.extend_from_slice(&3u16.to_le_bytes()); + t.extend_from_slice(&1u32.to_le_bytes()); + t.extend_from_slice(&u.to_le_bytes()); + t.extend_from_slice(&[0, 0]); + } + t.extend_from_slice(&[0, 0, 0, 0]); + assert_eq!(t.len(), data); + for (num, den) in [x, y] { + t.extend_from_slice(&num.to_le_bytes()); + t.extend_from_slice(&den.to_le_bytes()); + } + t +} + +/// EXIF APP1 wrapping a TIFF header. +fn exif_app(tiff: &[u8]) -> Vec { + let mut a = vec![0xFF, 0xE1]; + a.extend_from_slice(&((tiff.len() + 8) as u16).to_be_bytes()); + a.extend_from_slice(b"Exif\x00\x00"); + a.extend_from_slice(tiff); + a +} + +/// #1991: a 600x900 px photo saved at 600 dpi places 1 x 1.5 in, not 600 x 900 pt. +#[test] +fn jpeg_jfif_density_sets_natural_size() { + let jpg = jpeg_with(&jfif_app(1, 600, 600)); + assert_eq!(pixel_size(&jpg), Some((600, 900))); + assert_eq!(raster_resolution(&jpg), Some((600.0, 600.0))); + assert_eq!(natural_size(&jpg), Some((72.0, 108.0))); + // Dots per cm convert to dpi; aspect-only JFIF (units 0) carries no density. + let cm = jpeg_with(&jfif_app(2, 100, 100)); + assert_eq!(raster_resolution(&cm), Some((254.0, 254.0))); + let aspect = jpeg_with(&jfif_app(0, 600, 600)); + assert_eq!(raster_resolution(&aspect), None); + assert_eq!(natural_size(&aspect), Some((600.0, 900.0))); +} + +#[test] +fn jpeg_exif_resolution_beats_jfif() { + // EXIF wins over a conflicting JFIF density. + let mut app = jfif_app(1, 72, 72); + app.extend_from_slice(&exif_app(&tiff_bytes((300, 1), (300, 1), Some(2)))); + let jpg = jpeg_with(&app); + assert_eq!(raster_resolution(&jpg), Some((300.0, 300.0))); + assert_eq!(natural_size(&jpg), Some((600.0 * 72.0 / 300.0, 900.0 * 72.0 / 300.0))); + // A missing unit defaults to inch; per-cm scales by 2.54. + let no_unit = jpeg_with(&exif_app(&tiff_bytes((300, 1), (300, 1), None))); + assert_eq!(raster_resolution(&no_unit), Some((300.0, 300.0))); + // A non-EXIF APP1 (XMP) before the EXIF one does not stop the scan. + let mut xmp = vec![0xFF, 0xE1]; + let xmp_payload = b"http://ns.adobe.com/xap/1.0/\x00"; + xmp.extend_from_slice(&((xmp_payload.len() + 2) as u16).to_be_bytes()); + xmp.extend_from_slice(xmp_payload); + xmp.extend_from_slice(&exif_app(&tiff_bytes((300, 1), (300, 1), Some(2)))); + assert_eq!(raster_resolution(&jpeg_with(&xmp)), Some((300.0, 300.0))); + let cm = jpeg_with(&exif_app(&tiff_bytes((100, 1), (100, 1), Some(3)))); + assert_eq!(raster_resolution(&cm), Some((254.0, 254.0))); + // A zero denominator or a missing axis is unusable: back to a pixel per point. + let zero = jpeg_with(&exif_app(&tiff_bytes((300, 0), (300, 1), Some(2)))); + assert_eq!(raster_resolution(&zero), None); +} + +/// Big-endian EXIF parses too. +#[test] +fn jpeg_exif_big_endian_parses() { + let mut t = vec![b'M', b'M', 0x00, 0x2A, 0x00, 0x00, 0x00, 0x08]; + t.extend_from_slice(&[0x00, 0x02]); + let mut entry = |tag: u16, val: u32| { + t.extend_from_slice(&tag.to_be_bytes()); + t.extend_from_slice(&5u16.to_be_bytes()); + t.extend_from_slice(&1u32.to_be_bytes()); + t.extend_from_slice(&val.to_be_bytes()); + }; + entry(0x011A, 38); + entry(0x011B, 46); + t.extend_from_slice(&[0, 0, 0, 0]); + assert_eq!(t.len(), 38); + for (num, den) in [(600u32, 2u32), (600u32, 2u32)] { + t.extend_from_slice(&num.to_be_bytes()); + t.extend_from_slice(&den.to_be_bytes()); + } + let jpg = jpeg_with(&exif_app(&t)); + assert_eq!(raster_resolution(&jpg), Some((300.0, 300.0))); + assert_eq!(natural_size(&jpg), Some((144.0, 216.0))); +} + +fn crc32(data: &[u8]) -> u32 { + let mut crc = 0xFFFF_FFFFu32; + for &b in data { + crc ^= b as u32; + for _ in 0..8 { + crc = if crc & 1 == 1 { (crc >> 1) ^ 0xEDB8_8320 } else { crc >> 1 }; + } + } + !crc +} + +/// A real encoder-written PNG with a pHYs chunk spliced in after IHDR (valid CRC, so the +/// file stays fully decodable and exercises the real dimension path). +fn png_with_phys(ppu: u32) -> Vec { + let img = image::RgbaImage::from_pixel(600, 900, image::Rgba([30, 200, 30, 255])); + let mut b = Vec::new(); + img.write_to(&mut std::io::Cursor::new(&mut b), image::ImageFormat::Png).unwrap(); + // Signature (8) + IHDR chunk (length 4 + type 4 + data 13 + CRC 4). + assert_eq!(&b[..8], b"\x89PNG\r\n\x1a\n"); + assert_eq!(&b[12..16], b"IHDR"); + let mut out = b[..33].to_vec(); + let mut phys = Vec::new(); + phys.extend_from_slice(&ppu.to_be_bytes()); + phys.extend_from_slice(&ppu.to_be_bytes()); + phys.push(1); + out.extend_from_slice(&(phys.len() as u32).to_be_bytes()); + out.extend_from_slice(b"pHYs"); + out.extend_from_slice(&phys); + let mut c = b"pHYs".to_vec(); + c.extend_from_slice(&phys); + out.extend_from_slice(&crc32(&c).to_be_bytes()); + out.extend_from_slice(&b[33..]); + out +} + +#[test] +fn png_phys_sets_natural_size() { + // 23622 px/m = 600 dpi: the photo places 1 x 1.5 in. + let png = png_with_phys(23622); + assert_eq!(pixel_size(&png), Some((600, 900))); + let (dx, _) = raster_resolution(&png).unwrap(); + assert!((dx - 600.0).abs() < 0.05, "{dx}"); + let (w, h) = natural_size(&png).unwrap(); + assert!((w - 72.0).abs() < 0.01 && (h - 108.0).abs() < 0.02, "{w} x {h}"); + // No pHYs: a pixel is a point. + let plain_img = image::RgbaImage::from_pixel(4, 4, image::Rgba([0, 0, 0, 255])); + let mut plain = Vec::new(); + plain_img.write_to(&mut std::io::Cursor::new(&mut plain), image::ImageFormat::Png).unwrap(); + assert_eq!(raster_resolution(&plain), None); + assert_eq!(natural_size(&plain), Some((4.0, 4.0))); +} + +/// A real encoder-written BMP with the header pixels-per-meter patched (compression and image +/// size stay zero, as the encoder writes them). +fn bmp_with_ppm(ppm: i32) -> Vec { + let img = image::RgbaImage::from_pixel(600, 900, image::Rgba([30, 30, 200, 255])); + let mut b = Vec::new(); + img.write_to(&mut std::io::Cursor::new(&mut b), image::ImageFormat::Bmp).unwrap(); + assert_eq!(&b[..2], b"BM"); + b[38..42].copy_from_slice(&ppm.to_le_bytes()); + b[42..46].copy_from_slice(&ppm.to_le_bytes()); + b +} + +#[test] +fn bmp_density_sets_natural_size() { + // 2835 px/m = 72 dpi: the bitmap keeps its pixel size in points. + let bmp = bmp_with_ppm(2835); + assert_eq!(pixel_size(&bmp), Some((600, 900))); + let (dx, _) = raster_resolution(&bmp).unwrap(); + assert!((dx - 72.0).abs() < 0.05, "{dx}"); + // 600 dpi (23622 px/m): the bitmap places 1 x 1.5 in. + let dense = bmp_with_ppm(23622); + let (w, h) = natural_size(&dense).unwrap(); + assert!((w - 72.0).abs() < 0.01 && (h - 108.0).abs() < 0.02, "{w} x {h}"); + // Zero density: no metadata, a pixel is a point. + let flat = bmp_with_ppm(0); + assert_eq!(raster_resolution(&flat), None); + assert_eq!(natural_size(&flat), Some((600.0, 900.0))); +} + +#[test] +fn tiff_resolution_tags_set_natural_size() { + use tiff::encoder::colortype::Gray8; + // A TIFF the encoder writes carries no density: pixels are points. + let mut plain = Vec::new(); + { + let mut enc = tiff::encoder::TiffEncoder::new(std::io::Cursor::new(&mut plain)).unwrap(); + enc.new_image::(6, 9).unwrap().write_data(&[0u8; 54]).unwrap(); + } + assert_eq!(pixel_size(&plain), Some((6, 9))); + assert_eq!(natural_size(&plain), Some((6.0, 9.0))); + // The hand-built tag body parses (300 dpi, inch default and explicit). + for unit in [None, Some(2)] { + let t = tiff_bytes((300, 1), (300, 1), unit); + assert_eq!(resolution::tiff_resolution(&t), Some((300.0, 300.0))); + } +} + #[test] fn bmp_decodes() { let mut bmp = Vec::new(); From 581af01d88dffdabb0b96393f08dbb61868d7f49 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Tue, 3 Nov 2026 18:42:32 +0000 Subject: [PATCH 2/7] Apply review fixes - crates/images/src/resolution.rs --- crates/images/src/resolution.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/images/src/resolution.rs b/crates/images/src/resolution.rs index ab1dc119f..3a04f5fef 100644 --- a/crates/images/src/resolution.rs +++ b/crates/images/src/resolution.rs @@ -31,7 +31,7 @@ pub fn raster_resolution(bytes: &[u8]) -> Option<(f64, f64)> { /// (a corrupt or hostile value falls back to 72 ppi at the call site). pub fn sane_resolution(dpi: (f64, f64)) -> bool { let (dx, dy) = dpi; - dx.is_finite() && dy.is_finite() && dx > 0.0 && dy > 0.0 && dx <= 100_000.0 && dy <= 100_000.0 + dx.is_finite() && dy.is_finite() && dx >= 1.0 && dy >= 1.0 && dx <= 100_000.0 && dy <= 100_000.0 } fn is_classic_tiff(bytes: &[u8]) -> bool { @@ -115,17 +115,17 @@ fn exif_resolution(payload: &[u8]) -> Option<(f64, f64)> { /// Classic TIFF resolution tags of the first IFD: XResolution/YResolution rationals scaled by /// ResolutionUnit (2 = inch, 3 = cm; a missing unit defaults to inch per the spec). pub fn tiff_resolution(tiff: &[u8]) -> Option<(f64, f64)> { - let le = match &tiff[..2] { + let le = match tiff.get(..2)? { b"II" => true, b"MM" => false, _ => return None, }; let u16at = |i: usize| -> Option { - let b: [u8; 2] = tiff.get(i..i + 2)?.try_into().ok()?; + let b: [u8; 2] = tiff.get(i..i.checked_add(2)?)?.try_into().ok()?; Some(if le { u16::from_le_bytes(b) } else { u16::from_be_bytes(b) }) }; let u32at = |i: usize| -> Option { - let b: [u8; 4] = tiff.get(i..i + 4)?.try_into().ok()?; + let b: [u8; 4] = tiff.get(i..i.checked_add(4)?)?.try_into().ok()?; Some(if le { u32::from_le_bytes(b) } else { u32::from_be_bytes(b) }) }; let ifd = u32at(4)? as usize; @@ -212,8 +212,12 @@ fn webp_resolution(bytes: &[u8]) -> Option<(f64, f64)> { if data.checked_add(size)? > end { return None; } - if kind == b"EXIF" && is_classic_tiff(bytes.get(data..data + size.min(4)).unwrap_or_default()) { - return tiff_resolution(&bytes[data..(data + size).min(bytes.len())]); + if kind == b"EXIF" { + let exif = bytes.get(data..data + size)?; + let tiff = exif.strip_prefix(b"Exif\x00\x00").unwrap_or(exif); + if is_classic_tiff(tiff) { + return tiff_resolution(tiff); + } } pos = data.checked_add(size.checked_add(size & 1)?)?; } From 0f1f51cbd43b561e7205744e1a42f318a45eefa3 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:17:26 +0000 Subject: [PATCH 3/7] Fix failing CI check - crates/images/src/resolution.rs --- crates/images/src/resolution.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/images/src/resolution.rs b/crates/images/src/resolution.rs index 3a04f5fef..c4a95e8e5 100644 --- a/crates/images/src/resolution.rs +++ b/crates/images/src/resolution.rs @@ -77,7 +77,9 @@ fn jpeg_resolution(bytes: &[u8]) -> Option<(f64, f64)> { let payload = &bytes[pos + 2..pos + len]; if m == 0xE0 && jfif.is_none() { jfif = jfif_density(payload); - } else if m == 0xE1 && let Some(dpi) = exif_resolution(payload) { + } else if m == 0xE1 + && let Some(dpi) = exif_resolution(payload) + { // A non-EXIF APP1 (XMP, ICC, ...) parses to nothing: keep scanning, since a // later APP1 can still carry the EXIF resolution. return Some(dpi); From c8136c538ba91a6b977bc83ee15de87fd2bd1623 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 18:20:51 -0800 Subject: [PATCH 4/7] Place: an unusable EXIF resolution falls back to the JFIF density A JPEG whose EXIF resolution is 0 ppi or outside 1 to 100,000 ppi now takes its size from its JFIF density instead of one point per pixel. Tests place an 11 x 17 in image (330 x 510 px at 30 ppi) with a click, check that it comes in at 792 x 1224 pt and keeps that size through a save and reopen, and that an image with no stored resolution comes in at 72 ppi. --- crates/engine/src/cmd/file/place.rs | 84 +++++++++++++++++++++++------ crates/images/src/lib.rs | 4 +- crates/images/src/resolution.rs | 10 ++-- crates/images/src/tests.rs | 13 +++++ crates/render/src/decode.rs | 2 +- 5 files changed, 88 insertions(+), 25 deletions(-) diff --git a/crates/engine/src/cmd/file/place.rs b/crates/engine/src/cmd/file/place.rs index 29358d834..e0d285702 100644 --- a/crates/engine/src/cmd/file/place.rs +++ b/crates/engine/src/cmd/file/place.rs @@ -589,10 +589,8 @@ pub(super) mod image_place_tests { } } - #[test] - fn placed_photo_honors_its_embedded_resolution() { - // #1991: a placed graphic's embedded density sets its size (600 dpi: 40 px place - // 4.8 pt wide), instead of a pixel always becoming a point. + /// A `width` x `height` PNG with a pHYs chunk of `ppm` pixels per meter on both axes, or none. + fn png_with_density(width: u32, height: u32, ppm: Option) -> std::io::Result { fn crc32(data: &[u8]) -> u32 { let mut crc = 0xFFFF_FFFFu32; for &b in data { @@ -603,22 +601,33 @@ pub(super) mod image_place_tests { } !crc } - let png = designcraft_render::Rendered { width: 40, height: 20, pixels: [30, 90, 180, 255].repeat(800) }.to_png(); + let pixels = [30, 90, 180, 255].repeat((width * height) as usize); + let png = designcraft_render::Rendered { width, height, pixels }.to_png(); assert_eq!(&png[12..16], b"IHDR"); - let mut phys = Vec::new(); - phys.extend_from_slice(&23622u32.to_be_bytes()); - phys.extend_from_slice(&23622u32.to_be_bytes()); - phys.push(1); let mut bytes = png[..33].to_vec(); - bytes.extend_from_slice(&(phys.len() as u32).to_be_bytes()); - bytes.extend_from_slice(b"pHYs"); - bytes.extend_from_slice(&phys); - let mut c = b"pHYs".to_vec(); - c.extend_from_slice(&phys); - bytes.extend_from_slice(&crc32(&c).to_be_bytes()); + if let Some(ppm) = ppm { + let mut phys = Vec::new(); + phys.extend_from_slice(&ppm.to_be_bytes()); + phys.extend_from_slice(&ppm.to_be_bytes()); + phys.push(1); + bytes.extend_from_slice(&(phys.len() as u32).to_be_bytes()); + bytes.extend_from_slice(b"pHYs"); + bytes.extend_from_slice(&phys); + let mut c = b"pHYs".to_vec(); + c.extend_from_slice(&phys); + bytes.extend_from_slice(&crc32(&c).to_be_bytes()); + } bytes.extend_from_slice(&png[33..]); - let file = tempfile::Builder::new().suffix(".png").tempfile().unwrap(); - std::fs::write(file.path(), bytes).unwrap(); + let file = tempfile::Builder::new().suffix(".png").tempfile()?; + std::fs::write(file.path(), bytes)?; + Ok(file) + } + + #[test] + fn placed_photo_honors_its_embedded_resolution() { + // #1991: a placed graphic's embedded density sets its size (600 dpi: 40 px place + // 4.8 pt wide), instead of a pixel always becoming a point. + let file = png_with_density(40, 20, Some(23622)).unwrap(); let mut session = Session::new(); session.execute("file.new", &json!({})).unwrap(); let placed = session.execute("file.place", &json!({"path": file.path(), "x": 10.0, "y": 20.0})).unwrap(); @@ -629,6 +638,47 @@ pub(super) mod image_place_tests { assert!((b.width() - 4.8).abs() < 0.01 && (b.height() - 2.4).abs() < 0.01, "{b:?}"); } + /// File › Place, then a click: an 11 x 17 in image (330 x 510 px at 30 ppi) comes in at + /// 792 x 1224 pt, keeps that size through a save and reopen, and an image with no stored + /// resolution comes in at 72 ppi (330 x 510 pt). + #[test] + fn a_clicked_image_keeps_its_print_size_through_save_and_reopen() { + // 1181 pixels per meter is 29.9974 ppi. + let (w, h) = (330.0 * 72.0 / (1181.0 * 0.0254), 510.0 * 72.0 / (1181.0 * 0.0254)); + for (ppm, (ew, eh)) in [(Some(1181), (w, h)), (None, (330.0, 510.0))] { + let image = png_with_density(330, 510, ppm).unwrap(); + let mut session = Session::new(); + session.execute("file.new", &json!({})).unwrap(); + session.execute("file.place", &json!({"path": image.path(), "loadCursor": true})).unwrap(); + session.execute("place.drop", &json!({"x": 0, "y": 0})).unwrap(); + // The bounds and natural size of the document's one placed graphic. + let shown = |session: &Session| { + let doc = &session.doc().unwrap().doc; + let graphics: Vec<_> = doc + .all_items() + .into_iter() + .filter_map(|id| doc.item(id)) + .filter_map(|item| match &item.content { + Content::Graphic(g) => Some((item.bounds(), g.size)), + _ => None, + }) + .collect(); + assert_eq!(graphics.len(), 1, "{graphics:?}"); + graphics[0] + }; + let (b, size) = shown(&session); + assert!((b.width() - ew).abs() < 1e-6 && (b.height() - eh).abs() < 1e-6, "{ppm:?}: {b:?}"); + assert!((size.0 - ew).abs() < 1e-6 && (size.1 - eh).abs() < 1e-6, "{ppm:?}: {size:?}"); + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("placed.designcraft"); + session.execute("file.save", &json!({"path": path.to_string_lossy()})).unwrap(); + let mut reopened = Session::new(); + reopened.execute("file.open", &json!({"path": path.to_string_lossy()})).unwrap(); + assert_eq!(shown(&reopened), (b, size), "{ppm:?}"); + } + } + #[test] fn explicit_frame_wins_over_placement_and_selection() { let image = image_file().unwrap(); diff --git a/crates/images/src/lib.rs b/crates/images/src/lib.rs index 7540eee90..b32be1063 100644 --- a/crates/images/src/lib.rs +++ b/crates/images/src/lib.rs @@ -374,8 +374,8 @@ pub fn natural_size(bytes: &[u8]) -> Option<(f64, f64)> { return Some((bb[2] - bb[0], bb[3] - bb[1])).filter(|(w, h)| w.is_finite() && h.is_finite() && *w > 0.0 && *h > 0.0); } let (w, h) = pixel_size(bytes)?; - // Honor the file's density (#1991): a 600 px photo saved at 600 dpi places one inch wide, - // as in InDesign. Absurd metadata falls back to a pixel per point. + // Honor the file's density (#1991): a 600 px photo saved at 600 dpi places one inch wide. + // Absurd metadata falls back to a pixel per point. match raster_resolution(bytes) { Some(dpi) if resolution::sane_resolution(dpi) => Some((w as f64 * 72.0 / dpi.0, h as f64 * 72.0 / dpi.1)), _ => Some((w as f64, h as f64)), diff --git a/crates/images/src/resolution.rs b/crates/images/src/resolution.rs index c4a95e8e5..dabaf1c61 100644 --- a/crates/images/src/resolution.rs +++ b/crates/images/src/resolution.rs @@ -1,6 +1,6 @@ //! Embedded raster resolution: the density a placed graphic was saved with, so placing honors -//! it instead of assuming 72 ppi (#1991: a 600 px photo saved at 600 dpi places one inch wide, -//! as in InDesign). Reads JFIF/EXIF from JPEG (and EXIF from WebP), pHYs from PNG, the resolution +//! it instead of assuming 72 ppi (#1991: a 600 px photo saved at 600 dpi places one inch wide). +//! Reads JFIF/EXIF from JPEG (and EXIF from WebP), pHYs from PNG, the resolution //! tags from TIFF, and the header density from BMP. GIF, PSD and files without (usable) metadata //! report nothing and keep the pixel-is-a-point size. @@ -78,10 +78,10 @@ fn jpeg_resolution(bytes: &[u8]) -> Option<(f64, f64)> { if m == 0xE0 && jfif.is_none() { jfif = jfif_density(payload); } else if m == 0xE1 - && let Some(dpi) = exif_resolution(payload) + && let Some(dpi) = exif_resolution(payload).filter(|dpi| sane_resolution(*dpi)) { - // A non-EXIF APP1 (XMP, ICC, ...) parses to nothing: keep scanning, since a - // later APP1 can still carry the EXIF resolution. + // A non-EXIF APP1 (XMP, ICC, ...) or an unusable EXIF density parses to nothing: + // keep scanning, so a later APP1 or the JFIF density still sets the size. return Some(dpi); } pos += len; diff --git a/crates/images/src/tests.rs b/crates/images/src/tests.rs index 917693f87..f2fcf2bf0 100644 --- a/crates/images/src/tests.rs +++ b/crates/images/src/tests.rs @@ -537,6 +537,19 @@ fn jpeg_exif_resolution_beats_jfif() { assert_eq!(raster_resolution(&zero), None); } +/// An EXIF density of 0 ppi (or any value outside the usable range) does not hide the JFIF +/// density: the 600x900 px photo at a JFIF 300 dpi still places 144 x 216 pt. +#[test] +fn unusable_exif_density_keeps_the_jfif_density() { + for exif in [(0, 1), (1, 4), (1_000_000, 1)] { + let mut app = jfif_app(1, 300, 300); + app.extend_from_slice(&exif_app(&tiff_bytes(exif, exif, Some(2)))); + let jpg = jpeg_with(&app); + assert_eq!(raster_resolution(&jpg), Some((300.0, 300.0)), "EXIF {exif:?}"); + assert_eq!(natural_size(&jpg), Some((144.0, 216.0)), "EXIF {exif:?}"); + } +} + /// Big-endian EXIF parses too. #[test] fn jpeg_exif_big_endian_parses() { diff --git a/crates/render/src/decode.rs b/crates/render/src/decode.rs index 135dbf108..df8e1f047 100644 --- a/crates/render/src/decode.rs +++ b/crates/render/src/decode.rs @@ -87,7 +87,7 @@ pub fn image_size(bytes: &[u8]) -> Option<(u32, u32)> { } /// Natural size of a placed graphic, unrounded: a PDF page (crop box) and an SVG in points, a raster -/// in pixels, an EPS its bounding box independently of preview pixels. Placed graphics are drawn at +/// at its stored resolution (72 ppi without one), an EPS its bounding box independently of preview pixels. Placed graphics are drawn at /// this size; `image_size` rounds it to whole pixels for display. pub fn natural_size(bytes: &[u8], page: usize) -> Option<(f64, f64)> { if is_pdf(bytes) { From 81f0d766cfb54df1a61b11d07e7494cf19133949 Mon Sep 17 00:00:00 2001 From: "autocraft-bot[bot]" <354215646+autocraft-bot[bot]@users.noreply.github.com> Date: Thu, 12 Nov 2026 02:47:57 +0000 Subject: [PATCH 5/7] Apply review fixes - crates/images/src/resolution.rs - crates/render/src/decode.rs --- crates/images/src/resolution.rs | 8 +++----- crates/render/src/decode.rs | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/images/src/resolution.rs b/crates/images/src/resolution.rs index dabaf1c61..ea0a55eaa 100644 --- a/crates/images/src/resolution.rs +++ b/crates/images/src/resolution.rs @@ -178,7 +178,7 @@ pub fn tiff_resolution(tiff: &[u8]) -> Option<(f64, f64)> { /// The pHYs chunk: pixels-per-unit in x and y plus the unit (1 = meter → dpi). fn png_resolution(bytes: &[u8]) -> Option<(f64, f64)> { let mut pos = 8; - for _ in 0..64 { + loop { let len = u32::from_be_bytes(bytes.get(pos..pos + 4)?.try_into().ok()?) as usize; let kind = bytes.get(pos + 4..pos + 8)?; if kind == b"pHYs" { @@ -189,7 +189,7 @@ fn png_resolution(bytes: &[u8]) -> Option<(f64, f64)> { } return None; } - if kind == b"IEND" { + if kind == b"IEND" || kind == b"IDAT" { return None; } pos = pos.checked_add(8)?.checked_add(len)?.checked_add(4)?; @@ -197,7 +197,6 @@ fn png_resolution(bytes: &[u8]) -> Option<(f64, f64)> { return None; } } - None } // ---------- WebP ---------- @@ -207,7 +206,7 @@ fn webp_resolution(bytes: &[u8]) -> Option<(f64, f64)> { let riff = u32::from_le_bytes(bytes.get(4..8)?.try_into().ok()?) as usize; let end = 12usize.checked_add(riff)?.min(bytes.len()); let mut pos = 12; - for _ in 0..64 { + loop { let kind = bytes.get(pos..pos + 4)?; let size = u32::from_le_bytes(bytes.get(pos + 4..pos + 8)?.try_into().ok()?) as usize; let data = pos.checked_add(8)?; @@ -223,7 +222,6 @@ fn webp_resolution(bytes: &[u8]) -> Option<(f64, f64)> { } pos = data.checked_add(size.checked_add(size & 1)?)?; } - None } // ---------- BMP ---------- diff --git a/crates/render/src/decode.rs b/crates/render/src/decode.rs index df8e1f047..5047ca169 100644 --- a/crates/render/src/decode.rs +++ b/crates/render/src/decode.rs @@ -88,7 +88,7 @@ pub fn image_size(bytes: &[u8]) -> Option<(u32, u32)> { /// Natural size of a placed graphic, unrounded: a PDF page (crop box) and an SVG in points, a raster /// at its stored resolution (72 ppi without one), an EPS its bounding box independently of preview pixels. Placed graphics are drawn at -/// this size; `image_size` rounds it to whole pixels for display. +/// this size; `image_size` gives a raster's pixel size and rounds a PDF page or SVG to whole points. pub fn natural_size(bytes: &[u8], page: usize) -> Option<(f64, f64)> { if is_pdf(bytes) { return pdf_page_size(bytes, page).map(|(w, h)| (w.max(1e-3), h.max(1e-3))); From 7ba1f8b96f6f84874045294dc7d023b4af48f27d Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 19:07:29 -0800 Subject: [PATCH 6/7] Place: test PNG and WebP resolution past 64 chunks Tests read a pHYs chunk behind 100 text chunks in a PNG, ignore a pHYs after the image data, and read an EXIF chunk stored after 100 frames of an animated WebP. --- crates/images/src/tests.rs | 50 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/crates/images/src/tests.rs b/crates/images/src/tests.rs index f2fcf2bf0..1803b310a 100644 --- a/crates/images/src/tests.rs +++ b/crates/images/src/tests.rs @@ -626,6 +626,56 @@ fn png_phys_sets_natural_size() { assert_eq!(natural_size(&plain), Some((4.0, 4.0))); } +/// A pHYs chunk behind 100 text chunks is still read, and the scan stops at the first IDAT. +#[test] +fn png_phys_after_many_chunks_is_read() { + fn chunk(out: &mut Vec, kind: &[u8; 4], data: &[u8]) { + out.extend_from_slice(&(data.len() as u32).to_be_bytes()); + out.extend_from_slice(kind); + out.extend_from_slice(data); + out.extend_from_slice(&[0, 0, 0, 0]); + } + let mut phys = 11811u32.to_be_bytes().repeat(2); + phys.push(1); + let mut png = b"\x89PNG\r\n\x1a\n".to_vec(); + chunk(&mut png, b"IHDR", &[0; 13]); + for _ in 0..100 { + chunk(&mut png, b"tEXt", b"Comment\x00x"); + } + let mut late = png.clone(); + chunk(&mut png, b"pHYs", &phys); + let (dx, dy) = raster_resolution(&png).unwrap(); + assert!((dx - 300.0).abs() < 0.05 && (dy - 300.0).abs() < 0.05, "{dx} x {dy}"); + // A pHYs after the image data is not read. + chunk(&mut late, b"IDAT", &[0; 4]); + chunk(&mut late, b"pHYs", &phys); + assert_eq!(raster_resolution(&late), None); +} + +/// An EXIF chunk stored after 100 animation frames of a WebP is still read. +#[test] +fn webp_exif_after_many_frames_is_read() { + fn chunk(out: &mut Vec, kind: &[u8; 4], data: &[u8]) { + out.extend_from_slice(kind); + out.extend_from_slice(&(data.len() as u32).to_le_bytes()); + out.extend_from_slice(data); + if data.len() % 2 == 1 { + out.push(0); + } + } + let mut body = b"WEBP".to_vec(); + chunk(&mut body, b"VP8X", &[0; 10]); + chunk(&mut body, b"ANIM", &[0; 6]); + for _ in 0..100 { + chunk(&mut body, b"ANMF", &[0; 17]); + } + chunk(&mut body, b"EXIF", &tiff_bytes((300, 1), (300, 1), Some(2))); + let mut webp = b"RIFF".to_vec(); + webp.extend_from_slice(&(body.len() as u32).to_le_bytes()); + webp.extend_from_slice(&body); + assert_eq!(raster_resolution(&webp), Some((300.0, 300.0))); +} + /// A real encoder-written BMP with the header pixels-per-meter patched (compression and image /// size stay zero, as the encoder writes them). fn bmp_with_ppm(ppm: i32) -> Vec { From 58cf0ba28afa3f6c8df55ed148bd6f5fee706d17 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 12 Nov 2026 09:08:51 -0800 Subject: [PATCH 7/7] Place: WebP EXIF stops at the declared RIFF end The RIFF size counts the bytes after its own field, so the container ends at offset 8 plus the size, not 12. An EXIF chunk that ran up to four bytes past the declared container was read when trailing bytes followed it. A new test covers an exact size with and without trailing bytes, a size declared four bytes short, and a file cut inside the EXIF chunk. --- crates/images/src/resolution.rs | 3 ++- crates/images/src/tests.rs | 34 +++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/crates/images/src/resolution.rs b/crates/images/src/resolution.rs index ea0a55eaa..3b04d8e73 100644 --- a/crates/images/src/resolution.rs +++ b/crates/images/src/resolution.rs @@ -203,8 +203,9 @@ fn png_resolution(bytes: &[u8]) -> Option<(f64, f64)> { /// A WebP EXIF chunk carries a TIFF header: reuse the TIFF resolution tags. fn webp_resolution(bytes: &[u8]) -> Option<(f64, f64)> { + // The RIFF size counts the bytes after the size field itself (offset 8). let riff = u32::from_le_bytes(bytes.get(4..8)?.try_into().ok()?) as usize; - let end = 12usize.checked_add(riff)?.min(bytes.len()); + let end = 8usize.checked_add(riff)?.min(bytes.len()); let mut pos = 12; loop { let kind = bytes.get(pos..pos + 4)?; diff --git a/crates/images/src/tests.rs b/crates/images/src/tests.rs index 1803b310a..d7d46e4a4 100644 --- a/crates/images/src/tests.rs +++ b/crates/images/src/tests.rs @@ -676,6 +676,40 @@ fn webp_exif_after_many_frames_is_read() { assert_eq!(raster_resolution(&webp), Some((300.0, 300.0))); } +/// The RIFF size counts the bytes after its own field (offset 8): an EXIF chunk that runs past +/// the declared container is not read, even when trailing bytes follow the container. +#[test] +fn webp_exif_past_the_declared_riff_end_is_not_read() { + let mut body = b"WEBP".to_vec(); + body.extend_from_slice(b"VP8X"); + body.extend_from_slice(&10u32.to_le_bytes()); + body.extend_from_slice(&[0; 10]); + let exif = tiff_bytes((300, 1), (300, 1), Some(2)); + body.extend_from_slice(b"EXIF"); + body.extend_from_slice(&(exif.len() as u32).to_le_bytes()); + body.extend_from_slice(&exif); + if exif.len() % 2 == 1 { + body.push(0); + } + let riff = |size: usize, trailing: usize| { + let mut webp = b"RIFF".to_vec(); + webp.extend_from_slice(&(size as u32).to_le_bytes()); + webp.extend_from_slice(&body); + webp.extend_from_slice(&vec![0xAA; trailing]); + webp + }; + // The exact size, with or without trailing data, reads the chunk. + assert_eq!(raster_resolution(&riff(body.len(), 0)), Some((300.0, 300.0))); + assert_eq!(raster_resolution(&riff(body.len(), 4)), Some((300.0, 300.0))); + // A size declared 4 bytes short ends the container inside the EXIF chunk. + assert_eq!(raster_resolution(&riff(body.len() - 4, 4)), None); + assert_eq!(raster_resolution(&riff(body.len() - 4, 0)), None); + // A file cut inside the EXIF chunk is not read either. + let mut cut = riff(body.len(), 0); + cut.truncate(cut.len() - 6); + assert_eq!(raster_resolution(&cut), None); +} + /// A real encoder-written BMP with the header pixels-per-meter patched (compression and image /// size stay zero, as the encoder writes them). fn bmp_with_ppm(ppm: i32) -> Vec {