From a184db6f8f2d11bc20832c70ac438576d3d2dd1e Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Thu, 15 Oct 2026 19:18:11 -0600 Subject: [PATCH] fix(export): reject oversized page selections (#1089) Reject page selections that cannot fit the target index type before conversion, and return an error for the largest direct PPTX page index. Validated the combined main and PR tree with all seven cargo xtask ci stages (1,967 passing tests), four actual WASM export-page regressions, and successful FreeBSD, Windows ARM64, and packaging workflows. --- crates/engine/src/cmd/export.rs | 16 ++++-- crates/engine/tests/export_page_input.rs | 62 ++++++++++++++++++++++++ crates/pptx/src/lib.rs | 2 +- crates/pptx/src/tests.rs | 8 +++ 4 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 crates/engine/tests/export_page_input.rs diff --git a/crates/engine/src/cmd/export.rs b/crates/engine/src/cmd/export.rs index f10f3037d..bda757987 100644 --- a/crates/engine/src/cmd/export.rs +++ b/crates/engine/src/cmd/export.rs @@ -288,14 +288,22 @@ fn pages_param(id: &str, p: &Value, page_count: usize) -> Result { let mut v = Vec::new(); for x in a { - let n = x.as_u64().filter(|n| *n >= 1 && (*n as usize) <= page_count).ok_or_else(|| bad(id, format!("bad page {x}")))?; - v.push(n as usize - 1); + let n = x + .as_u64() + .and_then(|n| usize::try_from(n).ok()) + .filter(|n| *n >= 1 && *n <= page_count) + .ok_or_else(|| bad(id, format!("bad page {x}")))?; + v.push(n - 1); } Some(v) } Some(Value::Number(n)) => { - let n = n.as_u64().filter(|n| *n >= 1 && (*n as usize) <= page_count).ok_or_else(|| bad(id, format!("bad page {n}")))?; - Some(vec![n as usize - 1]) + let n = n + .as_u64() + .and_then(|n| usize::try_from(n).ok()) + .filter(|n| *n >= 1 && *n <= page_count) + .ok_or_else(|| bad(id, format!("bad page {n}")))?; + Some(vec![n - 1]) } Some(v) => return Err(bad(id, format!("bad `pages`: {v}"))), }) diff --git a/crates/engine/tests/export_page_input.rs b/crates/engine/tests/export_page_input.rs new file mode 100644 index 000000000..ebf0ac9ae --- /dev/null +++ b/crates/engine/tests/export_page_input.rs @@ -0,0 +1,62 @@ +//! Public export-command page input must be rejected before narrowing on 32-bit targets. +use designcraft_engine::{EngineError, Session}; +use serde_json::json; + +fn rejects(numbers: &[u64]) { + let mut session = Session::new(); + session.execute("file.new", &json!({"pages": 1})).unwrap(); + let before = serde_json::to_value(&session.doc().unwrap().doc).unwrap(); + for command in ["file.exportPdf", "file.exportPptx"] { + for &number in numbers { + for pages in [json!(number), json!([number])] { + #[cfg(target_arch = "wasm32")] + wasm_bindgen_test::console_log!("SAFETY_REJECTION_INPUT {command} {pages}"); + #[cfg(not(target_arch = "wasm32"))] + println!("SAFETY_REJECTION_INPUT {command} {pages}"); + match session.execute(command, &json!({"pages": pages})) { + Err(EngineError::BadParams { cmd, msg }) => { + assert_eq!(cmd, command); + assert_eq!(msg, format!("bad page {number}")); + } + Ok(_) => panic!("EXPECTED_BAD_PARAMS: {command} pages {pages} was accepted"), + Err(other) => panic!("EXPECTED_BAD_PARAMS: {command} pages {pages} returned {other:?}"), + } + assert_eq!(before, serde_json::to_value(&session.doc().unwrap().doc).unwrap()); + } + } + } +} + +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] +#[cfg_attr(not(target_arch = "wasm32"), test)] +fn zero_and_maximum_export_pages_are_bad_params() { + rejects(&[0, u64::MAX]); +} + +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] +#[cfg_attr(not(target_arch = "wasm32"), test)] +fn page_value_that_truncates_to_valid_page_is_bad_params() { + rejects(&[(1u64 << 32) + 1]); +} + +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] +#[cfg_attr(not(target_arch = "wasm32"), test)] +fn page_value_that_truncates_to_zero_is_bad_params() { + rejects(&[1u64 << 32]); +} + +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] +#[cfg_attr(not(target_arch = "wasm32"), test)] +fn page_one_exports_in_scalar_and_array_forms() { + let mut session = Session::new(); + session.execute("file.new", &json!({"pages": 1})).unwrap(); + let before = serde_json::to_value(&session.doc().unwrap().doc).unwrap(); + for (command, count) in [("file.exportPdf", "pages"), ("file.exportPptx", "slides")] { + for pages in [json!(1), json!([1])] { + let result = session.execute(command, &json!({"pages": pages})).unwrap(); + assert_eq!(result[count], 1); + assert!(!result["base64"].as_str().unwrap().is_empty()); + assert_eq!(before, serde_json::to_value(&session.doc().unwrap().doc).unwrap()); + } + } +} diff --git a/crates/pptx/src/lib.rs b/crates/pptx/src/lib.rs index c62bb707e..e05d05117 100644 --- a/crates/pptx/src/lib.rs +++ b/crates/pptx/src/lib.rs @@ -91,7 +91,7 @@ pub fn export_pptx(doc: &Document, cache: &Cache, opts: &PptxOptions) -> Result< return Err(PptxError::NoPages); } if let Some(bad) = pages.iter().find(|p| **p >= count) { - return Err(PptxError::BadPage(bad + 1)); + return Err(PptxError::BadPage(bad.saturating_add(1))); } let mut ex = slide::Exporter::new(doc, cache, opts); let mut slides = Vec::with_capacity(pages.len()); diff --git a/crates/pptx/src/tests.rs b/crates/pptx/src/tests.rs index 98e78384e..bb00fbd48 100644 --- a/crates/pptx/src/tests.rs +++ b/crates/pptx/src/tests.rs @@ -228,6 +228,14 @@ fn pictures_are_cropped_to_their_frames() { assert!(rels.contains("../media/image1.png")); } +#[test] +fn maximal_page_index_is_an_error() { + let d = Document::new(&NewDocument::default()); + let options = PptxOptions { pages: Some(vec![usize::MAX]), ..Default::default() }; + let result = export_pptx(&d, &Cache::new(), &options); + assert!(matches!(result, Err(PptxError::BadPage(usize::MAX)))); +} + #[test] fn bad_pages_and_hostile_geometry_are_errors_not_crashes() { let mut d = Document::new(&NewDocument::default());