mirror of
https://github.com/storytold/designcraft.git
synced 2026-10-25 18:56:50 -06:00
Layout: section start numbers are bounded and page numbering can't overflow
layout.section refuses a startNumber outside 1..=99 999, and so does layout.documentSetup for startPage (both go through one start_number check against designcraft_doc::MAX_PAGE_NUMBER). Document::page_number walks continuing sections in a loop with saturating arithmetic, so start numbers loaded from a file can't overflow it, and book pagination saturates its running page count.
This commit is contained in:
+17
-7
@@ -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.
|
||||
|
||||
@@ -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")]
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -139,12 +139,12 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
}),
|
||||
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<CommandSpec> {
|
||||
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)?;
|
||||
|
||||
@@ -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<u32> {
|
||||
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<CommandSpec> {
|
||||
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<CommandSpec> {
|
||||
);
|
||||
}
|
||||
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<CommandSpec> {
|
||||
});
|
||||
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::<Vec<_>>(), ["1", "2"]);
|
||||
let r = s.execute("layout.section", &json!({"page": 1, "startNumber": 99_999})).unwrap();
|
||||
assert_eq!(r["names"], json!(["99999", "100000"]));
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
Reference in New Issue
Block a user