Format: bound native load work for the manifest, failed media reads and layout checks

- presentation.json gets its own 256 MiB inflation limit instead of the 2 GiB
  per-entry allowance, and is parsed in place: only a file that needs its
  null-number repair is copied.
- A media member that fails decoding or its CRC is charged for the bytes it
  inflated, so later members cannot reuse the shared media budget.
- repair() and Presentation::validate() check slide layouts against a set
  built once instead of scanning every layout per slide.

crates/format/src/chart_parts_tests.rs

@@ -85,14 +85,14 @@ fn chart_parts_share_media_byte_and_item_budgets() {
     let mut p = fixture(vec![7; 600]);
     p.add_media("a.bin", "application/octet-stream", vec![8; 600]).unwrap();
     let bytes = save(&p).unwrap();
-    assert!(load_within(&bytes, Budget { total: 1200, items: 2 }).is_ok());
-    assert!(load_within(&bytes, Budget { total: 1199, items: 2 }).is_err(), "chart payloads share the cumulative media budget");
-    assert!(load_within(&bytes, Budget { total: 1200, items: 1 }).is_err(), "chart parts share the media item budget");
+    assert!(load_within(&bytes, Budget::media(1200, 2)).is_ok());
+    assert!(load_within(&bytes, Budget::media(1199, 2)).is_err(), "chart payloads share the cumulative media budget");
+    assert!(load_within(&bytes, Budget::media(1200, 1)).is_err(), "chart parts share the media item budget");
     let duplicated = rewrite_manifest(&bytes, |manifest| {
         let media = manifest["presentation"]["media"].as_array_mut().unwrap();
         media.push(media[0].clone());
     });
-    assert!(load_within(&duplicated, Budget { total: 1200, items: 2 }).is_err(), "duplicate media IDs still count as listed items");
+    assert!(load_within(&duplicated, Budget::media(1200, 2)).is_err(), "duplicate media IDs still count as listed items");
 }
 
 #[test]
@@ -104,9 +104,9 @@ fn legacy_inline_chart_bytes_are_budgeted_and_preserved() {
         part.remove("nativeEntry");
         part.insert("bytes".into(), json!(vec![9_u8; 600]));
     });
-    assert_eq!(load_within(&legacy, Budget { total: 1200, items: 2 }).unwrap().0, p);
-    assert!(load_within(&legacy, Budget { total: 1199, items: 2 }).is_err());
-    assert!(load_within(&legacy, Budget { total: 1200, items: 1 }).is_err());
+    assert_eq!(load_within(&legacy, Budget::media(1200, 2)).unwrap().0, p);
+    assert!(load_within(&legacy, Budget::media(1199, 2)).is_err());
+    assert!(load_within(&legacy, Budget::media(1200, 1)).is_err());
 }
 
 #[test]
@@ -207,7 +207,7 @@ fn corrupt_chart_members_stop_before_reusing_the_inflation_budget() {
     }
     // A failed CRC may already have inflated bytes. Continuing with the same remaining
     // budget would let each corrupt member consume that work budget again.
-    let result = load_within(&bytes, Budget { total: 10, items: 2 });
+    let result = load_within(&bytes, Budget::media(10, 2));
     assert!(result.is_err(), "two corrupt 10-byte members must not reuse a 10-byte budget");
     assert!(result.unwrap_err().to_string().contains("chart-parts/0"));
 }
@@ -228,7 +228,7 @@ fn duplicate_physical_zip_names_are_indexed_and_inflated_once() {
     assert_eq!(replaced, 2);
     let z = zip::ZipArchive::new(Cursor::new(bytes.as_slice())).unwrap();
     assert_eq!(z.file_names().filter(|n| n.starts_with("chart-parts/")).count(), 1);
-    let (q, damaged) = load_within(&bytes, Budget { total: 10, items: 2 }).unwrap();
+    let (q, damaged) = load_within(&bytes, Budget::media(10, 2)).unwrap();
     assert_eq!(damaged.len(), 1, "the second reference is missing, not inflated again");
     let ShapeKind::Group { children, .. } = &q.slides[0].shapes[0].kind else { panic!("group") };
     let ShapeKind::Chart(chart) = &children[0].kind else { panic!("chart") };

crates/format/src/lib.rs

@@ -29,12 +29,24 @@ const MAX_ENTRY: u64 = 2 << 30;
 const MAX_TOTAL_MEDIA: u64 = 4 << 30;
 /// Media items and chart sidecar parts one file may list.
 const MAX_MEDIA_ITEMS: usize = 100_000;
+/// Largest `presentation.json` we will inflate. The manifest holds no media bytes, so it is far
+/// smaller than a media entry may be; inflating and parsing one is what dominates memory.
+const MAX_MANIFEST: u64 = 256 << 20;
 
 /// What one load may inflate.
 #[derive(Clone, Copy)]
 struct Budget {
     total: u64,
     items: usize,
+    manifest: u64,
+}
+
+#[cfg(test)]
+impl Budget {
+    /// A media budget with the default manifest limit.
+    fn media(total: u64, items: usize) -> Self {
+        Budget { total, items, manifest: MAX_MANIFEST }
+    }
 }
 
 #[derive(Debug, thiserror::Error)]
@@ -107,6 +119,7 @@ pub fn sniff(bytes: &[u8]) -> bool {
     bytes.starts_with(b"PK") && [MIMETYPE, LEGACY_MIMETYPE].iter().any(|m| bytes.windows(m.len()).take(200).any(|w| w == m.as_bytes()))
 }
 
+#[cfg(test)]
 fn read_entry(z: &mut zip::ZipArchive<Cursor<&[u8]>>, name: &str) -> Result<Vec<u8>> {
     read_entry_max(z, name, MAX_ENTRY)
 }
@@ -114,13 +127,30 @@ fn read_entry(z: &mut zip::ZipArchive<Cursor<&[u8]>>, name: &str) -> Result<Vec<
 /// Read one member, refusing it before inflating if it declares more than `max` bytes, and
 /// stopping at `max` if it inflates further than it declared.
 fn read_entry_max(z: &mut zip::ZipArchive<Cursor<&[u8]>>, name: &str, max: u64) -> Result<Vec<u8>> {
+    read_entry_counted(z, name, max, &mut 0)
+}
+
+/// [`read_entry_max`] within `left` (and [`MAX_ENTRY`]), charging `left` for every byte inflated,
+/// including those inflated before a decode or CRC error, so a member that fails late still
+/// spends the shared budget it used.
+fn read_entry_charged(z: &mut zip::ZipArchive<Cursor<&[u8]>>, name: &str, left: &mut u64) -> Result<Vec<u8>> {
+    let mut spent = 0;
+    let out = read_entry_counted(z, name, (*left).min(MAX_ENTRY), &mut spent);
+    *left = left.saturating_sub(spent);
+    out
+}
+
+/// [`read_entry_max`], adding the bytes inflated to `spent` whether or not the read succeeds.
+fn read_entry_counted(z: &mut zip::ZipArchive<Cursor<&[u8]>>, name: &str, max: u64, spent: &mut u64) -> Result<Vec<u8>> {
     let f = z.by_name(name).map_err(|e| FormatError::Damaged(format!("{name}: {e}")))?;
     if f.size() > max {
         return Err(FormatError::Damaged(format!("{name} is too large")));
     }
     let mut out = Vec::with_capacity(f.size().min(64 << 20) as usize);
     let mut f = f.take(max.saturating_add(1));
-    f.read_to_end(&mut out).map_err(|e| FormatError::Damaged(format!("{name}: {e}")))?;
+    let read = f.read_to_end(&mut out);
+    *spent = spent.saturating_add(out.len() as u64);
+    read.map_err(|e| FormatError::Damaged(format!("{name}: {e}")))?;
     if out.len() as u64 > max {
         return Err(FormatError::Damaged(format!("{name} is too large")));
     }
@@ -137,12 +167,15 @@ pub fn load(bytes: &[u8]) -> Result<Presentation> {
 /// A chart member that is present but fails decoding, CRC or its entry limit refuses the file.
 /// Linked media saved without its bytes isn't damage.
 pub fn load_checked(bytes: &[u8]) -> Result<(Presentation, Vec<String>)> {
-    load_within(bytes, Budget { total: MAX_TOTAL_MEDIA, items: MAX_MEDIA_ITEMS })
+    load_within(bytes, Budget { total: MAX_TOTAL_MEDIA, items: MAX_MEDIA_ITEMS, manifest: MAX_MANIFEST })
 }
 
 fn load_within(bytes: &[u8], budget: Budget) -> Result<(Presentation, Vec<String>)> {
     let mut z = zip::ZipArchive::new(Cursor::new(bytes)).map_err(|e| FormatError::NotOurs(e.to_string()))?;
-    let json = read_entry(&mut z, "presentation.json").map_err(|_| FormatError::NotOurs("no presentation.json".into()))?;
+    if z.index_for_name("presentation.json").is_none() {
+        return Err(FormatError::NotOurs("no presentation.json".into()));
+    }
+    let json = read_entry_max(&mut z, "presentation.json", budget.manifest)?;
     let m = parse_manifest(&json)?;
     if m.format != "deckcraft" {
         return Err(FormatError::NotOurs(m.format));
@@ -164,11 +197,9 @@ fn load_within(bytes: &[u8], budget: Budget) -> Result<(Presentation, Vec<String
         if z.by_name(&name).is_ok_and(|f| f.size() > left) {
             return Err(FormatError::Damaged("embedded media is larger than DeckCraft can open".into()));
         }
-        match read_entry_max(&mut z, &name, left.min(MAX_ENTRY)) {
-            Ok(data) => {
-                left -= data.len() as u64;
-                item.data = Arc::new(data);
-            }
+        // A failed read is charged for what it inflated, so later members can't reuse that budget.
+        match read_entry_charged(&mut z, &name, &mut left) {
+            Ok(data) => item.data = Arc::new(data),
             Err(_) if linked_only => {}
             Err(e) => damaged.push(e.to_string()),
         }
@@ -188,9 +219,11 @@ pub fn repair(p: &mut Presentation) {
     }
     p.fix_next_id();
     let first_layout = p.masters.first().and_then(|m| m.layouts.first()).map(|l| l.id).unwrap_or_default();
+    // Indexed once: a per-slide scan of every layout is slides × layouts work.
+    let layouts: std::collections::HashSet<_> = p.masters.iter().flat_map(|m| m.layouts.iter().map(|l| l.id)).collect();
     let mut seen = std::collections::HashSet::new();
     for i in 0..p.slides.len() {
-        let needs_layout = p.slides.get(i).is_some_and(|s| p.layout(s.layout).is_none());
+        let needs_layout = p.slides.get(i).is_some_and(|s| !layouts.contains(&s.layout));
         let dup = p.slides.get(i).is_some_and(|s| !seen.insert(s.id));
         if needs_layout || dup {
             // An exhausted id space leaves a duplicate slide as it is: the engine refuses to
@@ -331,16 +364,20 @@ fn failure(src: &[u8]) -> std::result::Result<Manifest, (String, (usize, usize))
 /// nearest first, and a patch is kept only if the parse then gets further. (Files saved since
 /// [`finite::Finite`] never contain such `null`s; this reads older ones.)
 fn parse_manifest(json: &[u8]) -> Result<Manifest> {
-    let mut src = json.to_vec();
-    let mut last = match failure(&src) {
+    // Parse the inflated text as it is: only a file that fails on a `null` number is copied for
+    // patching (the copy is bounded by `MAX_MANIFEST`).
+    let mut last = match failure(json) {
         Ok(m) => return Ok(m),
         Err(f) => f,
     };
+    if !is_null_number(&last.0) {
+        return Err(FormatError::Damaged(last.0));
+    }
+    let mut src = json.to_vec();
     let mut budget = MAX_REPARSE_BYTES;
     'fixes: for _ in 0..MAX_NULL_FIXES {
         let (msg, pos) = &last;
-        let number = msg.starts_with("invalid type: null, expected f") || msg.starts_with("invalid type: null, expected a number");
-        if !number {
+        if !is_null_number(msg) {
             break;
         }
         let end = offset(&src, pos.0, pos.1).unwrap_or(src.len());
@@ -374,6 +411,11 @@ fn parse_manifest(json: &[u8]) -> Result<Manifest> {
     Err(FormatError::Damaged(last.0))
 }
 
+/// Is this parse error a `null` where a number belongs (a NaN or infinity serde_json wrote)?
+fn is_null_number(msg: &str) -> bool {
+    msg.starts_with("invalid type: null, expected f") || msg.starts_with("invalid type: null, expected a number")
+}
+
 /// Byte offset of a 1-based `line`:`column`.
 fn offset(src: &[u8], line: usize, column: usize) -> Option<usize> {
     let mut start = 0;

crates/format/src/tests.rs

@@ -93,9 +93,60 @@ fn media_budget_is_cumulative() {
     p.add_media("b.bin", "application/octet-stream", vec![2; 600]);
     let bytes = save(&p).unwrap();
     // Exactly at the limit opens; one byte under is refused, though each item fits.
-    assert!(load_within(&bytes, Budget { total: 1200, items: 2 }).is_ok());
-    assert!(load_within(&bytes, Budget { total: 1199, items: 2 }).is_err());
-    assert!(load_within(&bytes, Budget { total: 1200, items: 1 }).is_err());
+    assert!(load_within(&bytes, Budget::media(1200, 2)).is_ok());
+    assert!(load_within(&bytes, Budget::media(1199, 2)).is_err());
+    assert!(load_within(&bytes, Budget::media(1200, 1)).is_err());
+}
+
+#[test]
+fn corrupt_media_is_charged_for_what_it_inflated() {
+    let mut p = Presentation::default();
+    p.add_media("a.png", "image/png", b"PICTURE-AAAAAAAAAAAA".to_vec());
+    p.add_media("b.png", "image/png", b"PICTURE-BBBBBBBBBBBB".to_vec());
+    let mut bytes = save(&p).unwrap();
+    // Corrupt both stored members, so each fails its CRC only after inflating all its bytes.
+    for tag in [b"PICTURE-AAAA", b"PICTURE-BBBB"] {
+        let at = bytes.windows(tag.len()).position(|w| w == tag).unwrap();
+        bytes[at] ^= 0xff;
+    }
+    // Each member fits the budget on its own; the second must not reuse what the first spent.
+    let result = load_within(&bytes, Budget::media(20, 2));
+    assert!(result.is_err(), "two corrupt 20-byte members must not share a 20-byte budget");
+    // With room for both, the file opens and reports both.
+    let (_, damaged) = load_within(&bytes, Budget::media(40, 2)).unwrap();
+    assert_eq!(damaged.len(), 2, "{damaged:?}");
+}
+
+#[test]
+fn oversized_manifest_is_refused_before_inflating() {
+    let bytes = save(&Presentation::default()).unwrap();
+    let mut z = zip::ZipArchive::new(Cursor::new(bytes.as_slice())).unwrap();
+    let size = z.by_name("presentation.json").unwrap().size();
+    assert!(load_within(&bytes, Budget { total: 0, items: 0, manifest: size }).is_ok());
+    let err = load_within(&bytes, Budget { total: 0, items: 0, manifest: size - 1 }).unwrap_err();
+    assert!(err.to_string().contains("too large"), "{err}");
+}
+
+#[test]
+fn many_slides_on_missing_layouts_are_repaired() {
+    let mut p = Presentation::default();
+    let template = p.slides[0].clone();
+    let good = template.layout;
+    p.slides = (0..2000u32)
+        .map(|i| {
+            let mut s = (*template).clone();
+            s.id = deckcraft_model::SlideId(10_000 + i);
+            if i % 2 == 1 {
+                s.layout = deckcraft_model::LayoutId(900_000 + i);
+            }
+            Arc::new(s)
+        })
+        .collect();
+    p.next_id = 20_000;
+    repair(&mut p);
+    assert!(p.validate().is_empty(), "{:?}", p.validate().first());
+    assert!(p.slides.iter().all(|s| p.layout(s.layout).is_some()));
+    assert_eq!(p.slides[0].layout, good);
 }
 
 #[test]
@@ -124,7 +175,7 @@ fn repeated_media_ids_are_read_once() {
     }
     let bytes = out.finish().unwrap().into_inner();
     // 51 references to one 1000-byte member fit a 1000-byte budget: it's inflated once.
-    let q = load_within(&bytes, Budget { total: 1000, items: 51 }).unwrap().0;
+    let q = load_within(&bytes, Budget::media(1000, 51)).unwrap().0;
     assert_eq!(q.media.len(), 1);
     assert_eq!(q.media[0].data.len(), 1000);
     // And the deck saves again.

crates/model/src/lib.rs

@@ -1112,11 +1112,13 @@ impl Presentation {
     pub fn validate(&self) -> Vec<String> {
         let mut out = vec![];
         let mut seen = std::collections::HashSet::new();
+        // Indexed once: a per-slide scan of every layout is slides × layouts work.
+        let layouts: std::collections::HashSet<_> = self.masters.iter().flat_map(|m| m.layouts.iter().map(|l| l.id)).collect();
         for s in &self.slides {
             if !seen.insert(s.id) {
                 out.push(format!("duplicate slide id {}", s.id));
             }
-            if self.layout(s.layout).is_none() {
+            if !layouts.contains(&s.layout) {
                 out.push(format!("slide {} uses missing layout {}", s.id, s.layout));
             }
             let mut ids = std::collections::HashSet::new();