diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index c4b69e86d..2f839423d 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -643,14 +643,24 @@ impl Document { /// The page number (before formatting) of absolute page `abs`, per sections. A section without /// a start number continues from the previous section. + /// Saturates at `u32::MAX`, so start numbers loaded from a file can't overflow it. pub fn page_number(&self, abs: usize) -> u32 { - let Some(sec) = self.section_of(abs) else { return abs as u32 + 1 }; - let start = match sec.start_number { - Some(n) => n, - None if sec.start == 0 => 1, - None => self.page_number(sec.start - 1) + 1, - }; - start + (abs - sec.start) as u32 + // Walks back through continuing sections iteratively: one section per step, never recursing. + let (mut abs, mut offset) = (abs, 0u32); + loop { + let Some(sec) = self.section_of(abs) else { + return u32::try_from(abs).unwrap_or(u32::MAX).saturating_add(1).saturating_add(offset); + }; + let into = u32::try_from(abs - sec.start).unwrap_or(u32::MAX).saturating_add(offset); + match sec.start_number { + Some(n) => return n.saturating_add(into), + None if sec.start == 0 => return into.saturating_add(1), + None => { + offset = into.saturating_add(1); + abs = sec.start - 1; + } + } + } } /// The displayed page name ("1", "iv", "A-3"…) for an absolute page index, per sections. diff --git a/crates/doc/src/page.rs b/crates/doc/src/page.rs index f1eaa48ad..0a3cb4f68 100644 --- a/crates/doc/src/page.rs +++ b/crates/doc/src/page.rs @@ -328,6 +328,9 @@ impl Spread { } } +/// The highest start page number a document or section accepts. +pub const MAX_PAGE_NUMBER: u32 = 99_999; + /// A numbering section starting at a document page. #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] diff --git a/crates/doc/src/tests.rs b/crates/doc/src/tests.rs index 9ee28a139..5400e2f59 100644 --- a/crates/doc/src/tests.rs +++ b/crates/doc/src/tests.rs @@ -142,6 +142,18 @@ fn page_names_follow_sections() { assert_eq!(d.page_name(5), "4"); } +#[test] +fn hostile_section_start_numbers_saturate() { + let mut d = Document::new(&NewDocument { pages: 3, ..Default::default() }); + let first = d.sections.iter_mut().find(|s| s.start == 0).unwrap(); + first.start_number = Some(u32::MAX); + let continuing = Section { start: 2, start_number: None, ..first.clone() }; + d.sections.push(continuing); + assert_eq!(d.page_number(1), u32::MAX); + assert_eq!(d.page_number(2), u32::MAX); + assert_eq!(d.page_name(2), u32::MAX.to_string()); +} + #[test] fn document_serde_roundtrip() { let mut d = doc(); diff --git a/crates/engine/src/cmd/book.rs b/crates/engine/src/cmd/book.rs index 9631dac2b..60ace973b 100644 --- a/crates/engine/src/cmd/book.rs +++ b/crates/engine/src/cmd/book.rs @@ -139,12 +139,12 @@ pub fn specs() -> Vec { }), cmd!(query "book.list", "Book", [], None, "{} → {path, styleSource, documents: [{path, pages, firstPage}]}", has_book, |s, _| { let b = s.book.as_ref().ok_or_else(no_book)?; - let mut first = 1; + let mut first = 1usize; let mut docs = Vec::new(); for p in &b.documents { let pages = load(p).map(|d| d.page_count()).unwrap_or(0); docs.push(json!({"path": p, "pages": pages, "firstPage": first})); - first += pages; + first = first.saturating_add(pages); } Ok(json!({"path": b.path, "styleSource": b.style_source, "documents": docs})) }), @@ -154,10 +154,10 @@ pub fn specs() -> Vec { for p in &b.documents { let mut d = load(p)?; start_at(&mut d, next); - next += d.page_count() as u32; + next = next.saturating_add(u32::try_from(d.page_count()).unwrap_or(u32::MAX)); store(p, &d)?; } - Ok(json!({"pages": next - 1})) + Ok(json!({"pages": next.saturating_sub(1)})) }), cmd!(noundo "book.syncStyles", "Synchronize Book", [], None, "{} — paragraph and character styles and swatches of the style source go into every document (by name)", has_book, |s, _| { let b = s.book.clone().ok_or_else(no_book)?; diff --git a/crates/engine/src/cmd/layout.rs b/crates/engine/src/cmd/layout.rs index bd5320799..1bee570e4 100644 --- a/crates/engine/src/cmd/layout.rs +++ b/crates/engine/src/cmd/layout.rs @@ -6,6 +6,14 @@ use serde_json::{Value, json}; use super::{CommandSpec, bad, cmd, has_doc, ok, str_param}; use crate::Result; +/// A start page number parameter: a whole number in 1..=[`designcraft_doc::MAX_PAGE_NUMBER`]. +fn start_number(cmd: &str, key: &str, v: &Value) -> Result { + v.as_u64() + .and_then(|n| u32::try_from(n).ok()) + .filter(|n| (1..=designcraft_doc::MAX_PAGE_NUMBER).contains(n)) + .ok_or_else(|| bad(cmd, format!("`{key}` must be a whole number from 1 to {}", designcraft_doc::MAX_PAGE_NUMBER))) +} + /// Each page's margin box, by spread and page id. fn margin_boxes(d: &designcraft_doc::Document) -> Vec<(designcraft_doc::SpreadRef, designcraft_doc::PageId, designcraft_geom::Rect)> { d.spread_refs() @@ -269,7 +277,7 @@ pub fn specs() -> Vec { if let Some(i) = intent { d.settings.intent = i; } - if let Some(n) = p.get("startPage").and_then(Value::as_u64) { + if let Some(n) = p.get("startPage").map(|v| start_number("layout.documentSetup", "startPage", v)).transpose()? { if !d.sections.iter().any(|x| x.start == 0) { d.sections.insert( 0, @@ -284,7 +292,7 @@ pub fn specs() -> Vec { ); } if let Some(first) = d.sections.iter_mut().find(|x| x.start == 0) { - first.start_number = Some(n.max(1) as u32); + first.start_number = Some(n); } } // Number of Pages: add at the end (with the last page's parent) or remove from the end. @@ -410,7 +418,7 @@ pub fn specs() -> Vec { }); match p.get("startNumber") { Some(Value::Null) => sec.start_number = None, - Some(v) => sec.start_number = v.as_u64().map(|v| v.max(1) as u32), + Some(v) => sec.start_number = Some(start_number("layout.section", "startNumber", v)?), None => {} } if let Some(v) = p.get("style") { @@ -895,6 +903,20 @@ mod page_numbering_view_tests { assert_eq!(s.resolve_page("1"), Some(0)); assert_eq!(s.resolve_page("9"), None); } + + #[test] + fn section_start_numbers_are_bounded() { + let mut s = Session::new(); + s.execute("file.new", &json!({"pages": 2})).unwrap(); + for n in [json!(0), json!(100_000), json!(u32::MAX), json!(u64::from(u32::MAX) + 2), json!(-1), json!(2.5), json!("7")] { + let e = s.execute("layout.section", &json!({"page": 1, "startNumber": n})).unwrap_err().to_string(); + assert!(e.contains("startNumber"), "{n}: {e}"); + assert!(s.execute("layout.documentSetup", &json!({"startPage": n})).is_err(), "{n}"); + } + assert_eq!((0..2).map(|i| s.page_label(i)).collect::>(), ["1", "2"]); + let r = s.execute("layout.section", &json!({"page": 1, "startNumber": 99_999})).unwrap(); + assert_eq!(r["names"], json!(["99999", "100000"])); + } } #[cfg(test)]