Merge pull request #1951 from cloutier/fix/1910-slide-import-integrity

PPTX: report missing slide relationships and refuse over-limit lists

ROADMAP.md

@@ -2,7 +2,7 @@
 
 **Stage: alpha** · next: beta, ~20 points and ~120–210 h away
 
-> **Last reviewed:** 2026-11-04 · **Last updated:** 2026-11-11 · **Change:** trivial (the Opus decoder and the Ogg, MP4 and Matroska demuxers come from craft-libs; parity scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365)
+> **Last reviewed:** 2026-11-04 · **Last updated:** 2026-11-11 · **Change:** trivial (slide-list entries with missing relationships are reported as lost slides and open as repaired copies; the Opus decoder and the Ogg, MP4 and Matroska demuxers come from craft-libs; parity scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365)
 > **Last reviewed:** 2026-11-02 · **Last updated:** 2026-11-03 · **Change:** trivial (progress log: web `?host=parent` bridge opens and saves through a same-origin framing page; scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365)
 
 > **Last reviewed:** 2026-11-02 · **Last updated:** 2026-11-03 · **Change:** trivial (community PR wave 10: PPTX and native saves keep table cell centred anchoring, text overflow, compatible line spacing, normalised character height, Thai distributed alignment, RTL table direction, alternate run language and reflection transforms; imported effects placed by parent time-node delays; Reuse Slides fits the destination size and brings custom table styles; deleted slides leave custom shows; merged shapes drop their animations; Compress Pictures measures grouped pictures at displayed size; parity scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365)
@@ -155,6 +155,7 @@ app's JSON control channel (`deckcraft --control PORT`).
 | Date | What landed |
 |---|---|
 | 2026-11-11 | Media: the Opus decoder and the Ogg, MP4/MOV and Matroska demuxers are craft-audio and craft-media from craft-libs; DeckCraft's own copies and re-export crates are removed |
+| 2026-11-07 | PPTX slide-list entries with missing relationships are reported in lostSlides instead of silently omitted. The engine opens the partial deck as a repaired copy with no original save path. Synthetic 13-slide regression; does not establish the cause of #1910's field report. |
 | 2026-11-04 | Ogg Opus recovery keeps trailing packets without granules in the total duration, matching their recovered positions; valid end trimming stays covered |
 | 2026-11-03 | Web: the `?host=parent` bridge (as in GridCraft, WordCraft and PdfCraft) lets a same-origin page that frames DeckCraft send a presentation and store its saves; Save writes an opened .pptx back as .pptx under its own name, as on the desktop |
 | 2026-11-02 | Slide show on Linux and the BSDs: Presenter View stays off when winit lists exactly one display, as in PowerPoint; if it lists none, the setting is kept. On one display the presenter window covered the show, and the hidden window stalled every frame: slide changes took up to ~1.7 s under Hyprland (#1156) |

crates/pptx/src/read.rs

@@ -554,12 +554,15 @@ pub fn import(bytes: &[u8]) -> Result<Presentation, PptxError> {
     import_checked(bytes).map(|(p, _)| p)
 }
 
-/// [`import`], plus the parts of the slide list the file names but that couldn't be read (a
-/// missing or truncated slide): those slides are left out, so a caller can say so (#203).
+/// [`import`], plus the slides the file's slide list names but that couldn't be read (a missing
+/// or truncated slide part, or an entry whose relationship is missing): those slides are left
+/// out, so a caller can say so (#203).
 pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec<String>), PptxError> {
     import_with_chart_points(bytes, chart::MAX_PRESENTATION_CHART_POINTS)
 }
 
+const MAX_SLIDE_ENTRIES: usize = 10_000;
+
 /// [`import_checked`] with the presentation's chart point budget set (tests lower it).
 pub(crate) fn import_with_chart_points(bytes: &[u8], chart_points_cap: usize) -> Result<(Presentation, Vec<String>), PptxError> {
     let mut lost_slides: Vec<String> = vec![];
@@ -571,9 +574,15 @@ pub(crate) fn import_with_chart_points(bytes: &[u8], chart_points_cap: usize) ->
     if !pres_el.is("presentation") {
         return Err(PptxError::NotPptx(format!("{pres_path} is not a presentation part")));
     }
+    // Refuse an oversized list before reading slide parts: truncating it would look like a
+    // complete import and allow a later save to overwrite the omitted slides (#1910).
+    // Looking only for the first entry past the cap keeps this check bounded.
+    if pres_el.child("sldIdLst").is_some_and(|list| list.children_named("sldId").nth(MAX_SLIDE_ENTRIES).is_some()) {
+        return Err(PptxError::Unsupported(format!("PPTX slide list exceeds the {MAX_SLIDE_ENTRIES}-entry import limit")));
+    }
     let pres_part = Part::new(&pkg, &pres_path, &pres_doc);
 
-    let (master_paths, slide_entries) = master_and_slide_paths(pres_el, &pres_part);
+    let (master_paths, slide_entries) = master_and_slide_paths(pres_el, &pres_part, &mut lost_slides);
     let (docs, max_id) = load_part_docs(&pkg, &master_paths, &slide_entries);
 
     let mut imp = Imp {
@@ -630,7 +639,7 @@ pub(crate) fn import_with_chart_points(bytes: &[u8], chart_points_cap: usize) ->
 
 /// Masters, layouts and slides to read, in file order and without duplicates: (master paths,
 /// (file slide id, slide path)).
-fn master_and_slide_paths(pres_el: &El, pres_part: &Part) -> (Vec<String>, Vec<(u32, String)>) {
+fn master_and_slide_paths(pres_el: &El, pres_part: &Part, lost_slides: &mut Vec<String>) -> (Vec<String>, Vec<(u32, String)>) {
     let master_paths: Vec<String> = pres_el
         .child("sldMasterIdLst")
         .map(|l| l.children_named("sldMasterId").filter_map(|m| pres_part.rels.target(m.attr("r:id").unwrap_or(""))).map(String::from).collect())
@@ -641,8 +650,17 @@ fn master_and_slide_paths(pres_el: &El, pres_part: &Part) -> (Vec<String>, Vec<(
         .child("sldIdLst")
         .map(|l| {
             l.children_named("sldId")
-                .take(10_000)
-                .filter_map(|s| Some((s.u32("id").unwrap_or(0), pres_part.rels.target(s.attr("r:id").unwrap_or(""))?.to_string())))
+                .take(MAX_SLIDE_ENTRIES)
+                .filter_map(|s| {
+                    let rid = s.attr("r:id").unwrap_or("");
+                    let Some(path) = pres_part.rels.target(rid) else {
+                        // A missing relationship still represents a lost slide. Report it so
+                        // the engine opens a repaired copy instead of overwriting the original.
+                        lost_slides.push(format!("slide {} (missing relationship {rid})", s.u32("id").unwrap_or(0)));
+                        return None;
+                    };
+                    Some((s.u32("id").unwrap_or(0), path.to_string()))
+                })
                 .collect()
         })
         .unwrap_or_default();

crates/pptx/tests/it/malformed.rs

@@ -288,6 +288,94 @@ fn unreadable_slide_is_reported() {
     assert_eq!(deckcraft_pptx::import(&broken).expect("import").slides.len(), 1);
 }
 
+/// Missing slide relationships must not turn a damaged deck into a clean partial load (#1910).
+#[test]
+fn missing_slide_relationships_are_reported() {
+    let mut p = defaults::new_presentation(None);
+    let layout = p.slides[0].layout;
+    for _ in 1..13 {
+        let slide = defaults::new_slide(&mut p, layout).expect("slide");
+        p.slides.push(std::sync::Arc::new(slide));
+    }
+    let mut es = entries(&deckcraft_pptx::export(&p).expect("export"));
+    let pres = es.iter_mut().find(|(n, _)| n == "ppt/presentation.xml").expect("presentation");
+    let xml = String::from_utf8(pres.1.clone()).expect("utf8");
+    let doc = deckcraft_pptx::parse_xml(xml.as_bytes()).expect("xml");
+    let slides = doc.root.child("sldIdLst").expect("slide list");
+    let mut broken = xml.clone();
+    for slide in slides.children_named("sldId").skip(6) {
+        let rid = slide.attr("r:id").expect("relationship id");
+        broken = broken.replace(&format!("r:id=\"{rid}\""), &format!("r:id=\"missing-{rid}\""));
+    }
+    pres.1 = broken.into_bytes();
+    let damaged = rezip(&es);
+    let (q, lost) = deckcraft_pptx::import_checked(&damaged).expect("import");
+    assert_eq!(q.slides.len(), 6);
+    assert_eq!(lost.len(), 7, "every omitted slide must be reported: {lost:?}");
+    let loaded = deckcraft_engine::cmd::file::loaded_file("Original.pptx", &damaged).expect("load");
+    let mut session = deckcraft_engine::Session::new();
+    let token = session.stage_loaded(loaded);
+    let result = session.execute("file.adopt", &serde_json::json!({"path": "Original.pptx", "token": token})).expect("adopt");
+    assert_eq!(result["lostSlides"].as_array().expect("lost slides").len(), 7);
+    assert!(session.doc().expect("doc").path.is_none(), "Save must not overwrite the damaged source");
+    assert!(session.doc().expect("doc").title.ends_with("(Repaired)"));
+    let (q, lost) = deckcraft_pptx::import_checked(&deckcraft_pptx::export(&p).expect("export")).expect("import");
+    assert_eq!(q.slides.len(), 13);
+    assert!(lost.is_empty());
+}
+
+/// Every listed entry has its own slide part and relationship: the resource cap must not
+/// silently turn an otherwise readable large deck into a complete-looking partial load.
+#[test]
+fn over_limit_slide_lists_are_refused() {
+    fn deck(count: usize) -> Vec<u8> {
+        let mut es = entries(&valid());
+        let pres = es.iter_mut().find(|(n, _)| n == "ppt/presentation.xml").expect("presentation");
+        let mut xml = String::from_utf8(pres.1.clone()).expect("utf8");
+        let start = xml.find("<p:sldIdLst>").expect("slide list");
+        let end = xml.find("</p:sldIdLst>").expect("slide list end") + "</p:sldIdLst>".len();
+        let ids: String = (1..=count).map(|i| format!(r#"<p:sldId id="{}" r:id="capSlide{i}"/>"#, 255 + i)).collect();
+        xml.replace_range(start..end, &format!("<p:sldIdLst>{ids}</p:sldIdLst>"));
+        pres.1 = xml.into_bytes();
+        let rels = es.iter_mut().find(|(n, _)| n == "ppt/_rels/presentation.xml.rels").expect("relationships");
+        let mut xml = String::from_utf8(rels.1.clone()).expect("utf8");
+        let targets: String = (1..=count).map(|i| format!(r#"<Relationship Id="capSlide{i}" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/slide" Target="slides/slide{i}.xml"/>"#)).collect();
+        xml = xml.replace("</Relationships>", &format!("{targets}</Relationships>"));
+        rels.1 = xml.into_bytes();
+        let types = es.iter_mut().find(|(n, _)| n == "[Content_Types].xml").expect("content types");
+        let xml = String::from_utf8(types.1.clone()).expect("utf8");
+        let overrides: String = (2..=count).map(|i| format!(r#"<Override PartName="/ppt/slides/slide{i}.xml" ContentType="application/vnd.openxmlformats-officedocument.presentationml.slide+xml"/>"#)).collect();
+        types.1 = xml.replace("</Types>", &format!("{overrides}</Types>")).into_bytes();
+        let blank = br#"<p:sld xmlns:p="http://schemas.openxmlformats.org/presentationml/2006/main" xmlns:a="http://schemas.openxmlformats.org/drawingml/2006/main"><p:cSld><p:spTree><p:nvGrpSpPr><p:cNvPr id="1" name=""/><p:cNvGrpSpPr/><p:nvPr/></p:nvGrpSpPr><p:grpSpPr/></p:spTree></p:cSld></p:sld>"#.to_vec();
+        es.iter_mut().find(|(n, _)| n == "ppt/slides/slide1.xml").expect("slide").1 = blank.clone();
+        let slide_rels = es.iter().find(|(n, _)| n == "ppt/slides/_rels/slide1.xml.rels").expect("slide relationships").1.clone();
+        for i in 2..=count {
+            es.push((format!("ppt/slides/slide{i}.xml"), blank.clone()));
+            es.push((format!("ppt/slides/_rels/slide{i}.xml.rels"), slide_rels.clone()));
+        }
+        rezip(&es)
+    }
+    let at_limit = deck(10_000);
+    let (p, lost) = deckcraft_pptx::import_checked(&at_limit).expect("exactly at the limit");
+    assert_eq!(p.slides.len(), 10_000);
+    assert!(lost.is_empty());
+    drop(p);
+
+    let over_limit = deck(10_001);
+    for error in [
+        deckcraft_pptx::import_checked(&over_limit).expect_err("must refuse a partial load"),
+        deckcraft_pptx::import(&over_limit).expect_err("plain import must also refuse"),
+    ] {
+        assert!(matches!(error, deckcraft_pptx::PptxError::Unsupported(_)));
+        assert!(error.to_string().contains("10000"), "{error}");
+        assert!(error.to_string().contains("slide"), "{error}");
+    }
+    assert!(
+        deckcraft_engine::cmd::file::loaded_file("Original.pptx", &over_limit).is_err(),
+        "no partial file may be adopted with the original save path"
+    );
+}
+
 /// Many slides naming one notes part: it is read for the first slide only, not copied onto every
 /// slide that names it.
 #[test]

docs/file-format-parity.md

@@ -1,6 +1,6 @@
 # File-format parity with PowerPoint
 
-> **Last reviewed:** 2026-11-02 · **Last updated:** 2026-11-03 · **Change:** minor (cell anchorCtr, vertOverflow/horzOverflow, compatLnSpc, normalizeH, thaiDist, table rtl, run altLang and reflection transforms round-trip; Reuse Slides brings referenced custom table styles; previous: minor (native chart payloads stored in bounded ZIP members; previous: major, community PR wave 2026-10-14: seven owned PPTX corpus files, embedded fonts, EMF/WMF clip regions, threaded comments (#70), chart trendlines and workbooks (#335), SmartArt written as a diagram, tags and custom XML kept, picture export DPI; duplicated rows and headers folded; previous: trivial, lines in scaled groups, #233)) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365)
+> **Last reviewed:** 2026-11-02 · **Last updated:** 2026-11-07 · **Change:** trivial (a slide whose relationship is missing is reported in `lostSlides`; previous: minor (cell anchorCtr, vertOverflow/horzOverflow, compatLnSpc, normalizeH, thaiDist, table rtl, run altLang and reflection transforms round-trip; Reuse Slides brings referenced custom table styles; previous: minor (native chart payloads stored in bounded ZIP members; previous: major, community PR wave 2026-10-14: seven owned PPTX corpus files, embedded fonts, EMF/WMF clip regions, threaded comments (#70), chart trendlines and workbooks (#335), SmartArt written as a diagram, tags and custom XML kept, picture export DPI; duplicated rows and headers folded; previous: trivial, lines in scaled groups, #233))) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365)
 
 Every format PowerPoint reads or writes, DeckCraft's support, fidelity and how it is tested.
 Gaps are itemized in [gaps.md](gaps.md#file-formats). Dimension score: **~58% ready**
@@ -119,6 +119,7 @@ the round trip even though DeckCraft doesn't model it.
 
 | Date | Change | Summary |
 |---|---|---|
+| 2026-11-07 | trivial | A slide-list entry whose relationship is missing is reported in `lostSlides` like an unreadable slide part, so the deck opens as a repaired copy (#1910) |
 | 2026-11-02 | minor | Optional table RTL direction is read and written as `a:tblPr/@rtl`; native and two PPTX saves preserve true, false and absent values with distinguishable column text |
 | 2026-11-02 | minor | Text runs preserve alternate-language metadata in native and PPTX round trips |
 | 2026-11-03 | trivial | Reuse Slides transfers referenced custom table styles from native and PPTX sources; grouped tables, undo, native save/reopen and style-id conflicts are covered |