Fix up #513: keep every referenced media; cap picture decoding

picture.compress decided which originals were still in use by scanning JSON
keys, which missed artistic-effect originals (and the embedded-font shape it
assumed no longer exists), so compressing a picture whose bytes were also
another picture's artistic original deleted media still in use. It now uses
the engine's typed media walker (media_refs::presentation_media).

Decoding is capped: the header's size is read first and pictures over 64 MP
are left alone, and the decoder runs with image::Limits (512 MiB allocation),
instead of decoding up to 400 MP before checking.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas
2026-10-14 15:39:15 -04:00
co-authored by Claude Opus 5.5
parent 70a9a5b1cd
commit 6050c04ecd
+104 -36
View File
@@ -5,7 +5,7 @@ use std::collections::{HashMap, HashSet};
use std::sync::Arc;
use deckcraft_model::style::PictureFill;
use deckcraft_model::{MediaId, Presentation, Shape, ShapeId, ShapeKind};
use deckcraft_model::{MediaId, Shape, ShapeId, ShapeKind};
use serde_json::{Value, json};
use super::*;
@@ -35,6 +35,28 @@ fn real_crop(c: &[f64; 4]) -> bool {
c.iter().all(|v| (0.0..1.0).contains(v)) && c.iter().any(|v| *v > 0.0) && c[0] + c[2] < 1.0 && c[1] + c[3] < 1.0
}
/// Largest picture decoded for compression, in pixels (a 61 MP camera frame fits); bigger ones
/// are left as they are.
const MAX_PIXELS: u64 = 64_000_000;
/// Most memory the decoder may allocate for one picture.
const MAX_DECODE_BYTES: u64 = 512 << 20;
/// Decode `data` within [`MAX_PIXELS`] and [`MAX_DECODE_BYTES`]: the size is read from the header
/// first, so a picture claiming to be huge is never decoded.
fn decode(data: &[u8], format: image::ImageFormat) -> Option<image::DynamicImage> {
let (w, h) = image::ImageReader::with_format(std::io::Cursor::new(data), format).into_dimensions().ok()?;
if w == 0 || h == 0 || u64::from(w) * u64::from(h) > MAX_PIXELS {
return None;
}
let mut reader = image::ImageReader::with_format(std::io::Cursor::new(data), format);
let mut limits = image::Limits::default();
limits.max_image_width = Some(w);
limits.max_image_height = Some(h);
limits.max_alloc = Some(MAX_DECODE_BYTES);
reader.limits(limits);
reader.decode().ok()
}
/// Compress `data` for a picture shown `w_pt` × `h_pt` with `crop`, at `ppi`. `None` when the
/// format isn't a raster we re-encode (SVG, EMF, GIF…) or nothing would get smaller.
fn pack(data: &[u8], crop: [f64; 4], w_pt: f64, h_pt: f64, ppi: f64, delete_cropped: bool) -> Option<Packed> {
@@ -43,9 +65,9 @@ fn pack(data: &[u8], crop: [f64; 4], w_pt: f64, h_pt: f64, ppi: f64, delete_crop
if !matches!(format, F::Png | F::Jpeg | F::Bmp | F::Tiff | F::WebP) {
return None;
}
let mut img = image::load_from_memory_with_format(data, format).ok()?;
let mut img = decode(data, format)?;
let (iw, ih) = (img.width() as f64, img.height() as f64);
if iw < 1.0 || ih < 1.0 || iw * ih > 400_000_000.0 {
if iw < 1.0 || ih < 1.0 {
return None;
}
let cut = delete_cropped && real_crop(&crop);
@@ -108,37 +130,6 @@ fn pictures_mut<'a>(shapes: &'a mut [Shape], want: &dyn Fn(ShapeId) -> bool, out
}
}
/// Media ids anything in the presentation still refers to (everything but the media list).
fn referenced_media(doc: &Presentation) -> HashSet<u32> {
fn scan(v: &Value, out: &mut HashSet<u32>, depth: usize) {
if depth > 256 {
return;
}
match v {
Value::Object(o) => {
for (k, x) in o {
match (k.as_str(), x.as_u64()) {
("media" | "poster" | "preview" | "sound", Some(n)) => {
out.insert(n as u32);
}
_ => scan(x, out, depth + 1),
}
}
}
Value::Array(a) => a.iter().for_each(|x| scan(x, out, depth + 1)),
_ => {}
}
}
let mut out = HashSet::new();
let mut shell = doc.clone();
shell.media.clear();
if let Ok(v) = serde_json::to_value(&shell) {
scan(&v, &mut out, 0);
}
out.extend(doc.embedded_fonts.iter().flat_map(|e| [e.regular, e.bold, e.italic, e.bold_italic]).flatten().map(|m| m.0));
out
}
fn compress(s: &mut Session, p: &Value) -> Result<Value> {
let ppi = f64_param(p, "ppi").unwrap_or(220.0);
if !ppi.is_finite() || !(36.0..=1200.0).contains(&ppi) {
@@ -204,8 +195,9 @@ fn compress(s: &mut Session, p: &Value) -> Result<Value> {
}
// Drop the originals nothing uses any more, so the saved file actually shrinks.
if !changed.is_empty() {
let used = referenced_media(doc);
doc.media.retain(|m| !changed.contains(&m.id) || used.contains(&m.id.0));
// Typed walk of every reference (artistic originals, fills, posters, sounds, fonts…).
let used: HashSet<MediaId> = crate::media_refs::presentation_media(doc).into_iter().collect();
doc.media.retain(|m| !changed.contains(&m.id) || used.contains(&m.id));
}
Ok(json!({"pictures": count, "before": before, "after": after, "ppi": ppi}))
})
@@ -249,4 +241,80 @@ mod tests {
let img = image::load_from_memory(&pk.bytes).unwrap();
assert_eq!(img.width(), 200);
}
/// A PNG whose header claims `w` × `h` (no pixel data): enough for the size check.
fn png_header(w: u32, h: u32) -> Vec<u8> {
fn crc(bytes: &[u8]) -> u32 {
let mut c = 0xFFFF_FFFFu32;
for b in bytes {
c ^= u32::from(*b);
for _ in 0..8 {
c = if c & 1 == 1 { 0xEDB8_8320 ^ (c >> 1) } else { c >> 1 };
}
}
!c
}
fn chunk(out: &mut Vec<u8>, kind: &[u8], data: &[u8]) {
let body: Vec<u8> = kind.iter().chain(data).copied().collect();
out.extend((data.len() as u32).to_be_bytes());
out.extend(&body);
out.extend(crc(&body).to_be_bytes());
}
let mut ihdr = w.to_be_bytes().to_vec();
ihdr.extend(h.to_be_bytes());
ihdr.extend([8, 2, 0, 0, 0]);
let mut out = b"\x89PNG\r\n\x1a\n".to_vec();
chunk(&mut out, b"IHDR", &ihdr);
chunk(&mut out, b"IDAT", &[]);
chunk(&mut out, b"IEND", &[]);
out
}
#[test]
fn huge_pictures_are_not_decoded() {
// 72 MP: over the cap, though within what the decoder would allocate by default.
let data = png_header(9_000, 8_000);
assert_eq!(
image::ImageReader::with_format(std::io::Cursor::new(&data), image::ImageFormat::Png).into_dimensions().ok(),
Some((9_000, 8_000))
);
assert!(decode(&data, image::ImageFormat::Png).is_none());
assert!(pack(&data, [0.0; 4], 72.0, 72.0, 96.0, true).is_none());
}
/// Compressing a picture whose media is also another picture's artistic-effect original must
/// keep that media: the effect needs it to be changed or removed.
#[test]
fn media_still_used_as_an_artistic_original_is_kept() {
use deckcraft_model::style::Artistic;
let mut s = Session::with_new();
let big = png(1200, 600);
let processed = png(1100, 600);
s.edit(|doc, _| {
let original = doc.add_media("original.png", "image/png", big.clone());
let shown = doc.add_media("processed.png", "image/png", processed.clone());
let slide = Arc::make_mut(doc.slides.first_mut().ok_or_else(|| bad("test", "no slide"))?);
let xfrm = Some(deckcraft_geom::Xfrm::new(0.0, 0.0, 72.0, 36.0));
let with_effect =
PictureFill { media: shown, artistic: Some(Artistic { effect: "artisticBlur".into(), original }), ..Default::default() };
slide.shapes.push(Shape { id: ShapeId(70_001), xfrm, kind: ShapeKind::Picture { fill: with_effect }, ..Default::default() });
slide.shapes.push(Shape {
id: ShapeId(70_002),
xfrm,
kind: ShapeKind::Picture { fill: PictureFill { media: original, ..Default::default() } },
..Default::default()
});
Ok(())
})
.unwrap();
let r = s.execute("picture.compress", &json!({"all": true, "ppi": 96})).unwrap();
assert_eq!(r["pictures"], 2, "{r}");
let doc = &s.doc().unwrap().doc;
// Every reference left in the deck resolves to media that is still there.
for id in crate::media_refs::presentation_media(doc) {
assert!(doc.media(id).is_some(), "{id:?} was deleted while still referenced");
}
assert!(doc.media.iter().any(|m| m.data.as_slice() == big.as_slice()), "the artistic original was deleted");
assert!(!doc.media.iter().any(|m| m.data.as_slice() == processed.as_slice()), "the replaced processed picture is dropped");
}
}