collective
Browse / github.com/storytold/designcraft / patch
Books: a PDF export is bounded, and an update says which chapters are outside (#1597)
Books: a PDF export is bounded, and an update says which chapters are outside (#1597) Audit **DC-16** (#1597), both of its actionable parts. ## The export was unbounded `book.exportPdf` never applied `MAX_BATCH_CHAPTERS` — which the staged updates (`paginate`, `syncStyles`) do — and it keeps **every chapter's whole PDF in memory** to merge them. A long book of heavy chapters had no limit at all. The cap now applies to export too, and what it holds is bounded at 512 MB: ``` the book's chapters are more than 512 MB of PDF: export them in parts ``` ## The writes: reported, not refused A book names its chapters by path, and an update writes numbering and styles into each of t
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- Hamza0101 (imported from pull request #1912, open)
- Base
50650f74d7328745cfcdd16098313a9fc31b23d5- Size
- +77 −2 in 1 file
- Review
- pending
- Shared
- 2026-10-28
- Id
8eb5b856ebb4bbe7d457c37b
How it went
No other operator has reported using it yet.
Diff
raw · crates/engine/src/cmd/book.rs
@@ -44,6 +44,26 @@ fn chapter(b: &Book, path: &str) -> Result<String> { .map_err(|why| EngineError::Other(format!("{path}: {why}"))) } +/// Chapters whose file is not under the book's own folder, by the path the book names.+///+/// A book names its chapters by path, and `book.paginate` and `book.syncStyles` write numbering+/// and styles into each of them. A book that arrived from somewhere else can therefore name+/// `~/Documents/x.designcraft` and have the user's own file rewritten by opening it and running+/// an update. Which files those are is reported, so the UI can ask before writing (#1597);+/// refusing them outright would break books whose chapters legitimately live in sibling folders,+/// which is how InDesign books are usually arranged.+#[cfg(not(target_arch = "wasm32"))]+fn outside_book_folder(b: &Book) -> Vec<String> {+ let Some(folder) = std::path::Path::new(&b.path).parent().map(std::path::Path::to_path_buf) else {+ return b.documents.clone();+ };+ b.documents+ .iter()+ .filter(|named| chapter(b, named).map(|resolved| !std::path::Path::new(&resolved).starts_with(&folder)).unwrap_or(true))+ .cloned()+ .collect()+}+ /// Every chapter's file ([`chapter`]), in order. fn chapters(b: &Book) -> Result<Vec<String>> { b.documents.iter().map(|path| chapter(b, path)).collect()@@ -174,7 +194,7 @@ fn paginate(s: &Session) -> Result<Value> { batch.stage(path, &d)?; } batch.commit()?;- Ok(json!({"pages": next - 1, "chapters": chapters}))+ Ok(json!({"pages": next - 1, "chapters": chapters, "outsideFolder": outside_book_folder(b)})) } #[cfg(not(target_arch = "wasm32"))]@@ -216,7 +236,7 @@ fn sync_styles(s: &Session) -> Result<Value> { n += 1; } batch.commit()?;- Ok(json!({"synced": n}))+ Ok(json!({"synced": n, "outsideFolder": outside_book_folder(b)})) } #[cfg(target_arch = "wasm32")]@@ -333,6 +353,15 @@ pub fn specs() -> Vec<CommandSpec> { }), cmd!(noundo "book.exportPdf", "Export Book to PDF…", [], None, "{path?} — every document in order as one PDF → {path, bytes, pages} (no path: {base64, …})", has_book, |s, p| { let b = s.book.clone().ok_or_else(no_book)?;+ // The same cap the staged updates use, which this path never applied, plus a budget+ // for what it holds: every chapter's whole PDF is kept in memory to be merged, so a+ // long book of heavy chapters was unbounded (#1597).+ #[cfg(not(target_arch = "wasm32"))]+ if b.documents.len() > MAX_BATCH_CHAPTERS {+ return Err(bad("book.exportPdf", format!("a book export supports at most {MAX_BATCH_CHAPTERS} chapters")));+ }+ const MAX_BOOK_PDF_BYTES: usize = 512 << 20;+ let mut held = 0usize; let mut parts = Vec::new(); for path in &chapters(&b)? { let mut d = load(path)?;@@ -342,6 +371,13 @@ pub fn specs() -> Vec<CommandSpec> { let r = designcraft_pdf::export_pdf_with_report(&super::interchange::with_pdf_boxes(&d), &cache, &designcraft_pdf::PdfOptions::default()).map_err(|e| EngineError::Other(e.to_string()))?; let n = designcraft_render::pdf_page_count(&r.bytes).unwrap_or(0); let sizes: Vec<(f32, f32)> = (0..n).filter_map(|i| designcraft_render::pdf_page_size(&r.bytes, i)).map(|(w, h)| (w as f32, h as f32)).collect();+ held = held.saturating_add(r.bytes.len());+ if held > MAX_BOOK_PDF_BYTES {+ return Err(bad(+ "book.exportPdf",+ format!("the book's chapters are more than {} MB of PDF: export them in parts", MAX_BOOK_PDF_BYTES / (1 << 20)),+ ));+ } parts.push((r.bytes, sizes)); } let pages: usize = parts.iter().map(|p| p.1.len()).sum();@@ -364,6 +400,45 @@ mod tests { use crate::Session; + /// A book names its chapters by path, and an update writes numbering and styles into each+ /// one, so a book from elsewhere can name a file of the user's own. Which chapters lie+ /// outside the book's folder is now reported, for the UI to ask about before writing (#1597).+ #[test]+ fn an_update_reports_the_chapters_outside_the_books_folder() {+ let root = std::env::temp_dir().join(format!("dc-book-outside-{}", std::process::id()));+ let (inside, elsewhere) = (root.join("book"), root.join("elsewhere"));+ std::fs::create_dir_all(&inside).unwrap();+ std::fs::create_dir_all(&elsewhere).unwrap();+ let mut s = Session::new();+ s.execute("book.new", &json!({"path": inside.join("Guide.dcbook").to_string_lossy()})).unwrap();+ for path in [inside.join("ch1.designcraft"), elsewhere.join("mine.designcraft")] {+ s.execute("file.new", &json!({})).unwrap();+ s.execute("file.saveAs", &json!({"path": path.to_string_lossy()})).unwrap();+ s.execute("book.add", &json!({"path": path.to_string_lossy()})).unwrap();+ s.execute("file.close", &json!({})).unwrap();+ }++ let r = s.execute("book.paginate", &json!({})).unwrap();+ let outside = r["outsideFolder"].as_array().expect("the report lists them").clone();+ assert_eq!(outside.len(), 1, "one chapter is outside the book's folder: {outside:?}");+ assert!(outside[0].as_str().unwrap_or_default().contains("mine.designcraft"), "{outside:?}");++ // Sync Styles reports the same thing, since it writes the same chapters.+ let r = s.execute("book.syncStyles", &json!({})).unwrap();+ assert_eq!(r["outsideFolder"].as_array().map(Vec::len), Some(1));++ // A book whose chapters are all beside it reports nothing.+ let mut s = Session::new();+ s.execute("book.new", &json!({"path": inside.join("Tidy.dcbook").to_string_lossy()})).unwrap();+ s.execute("file.new", &json!({})).unwrap();+ s.execute("file.saveAs", &json!({"path": inside.join("only.designcraft").to_string_lossy()})).unwrap();+ s.execute("book.add", &json!({"path": inside.join("only.designcraft").to_string_lossy()})).unwrap();+ s.execute("file.close", &json!({})).unwrap();+ let r = s.execute("book.paginate", &json!({})).unwrap();+ assert_eq!(r["outsideFolder"], json!([]), "nothing to ask about");+ let _ = std::fs::remove_dir_all(&root);+ }+ #[test] fn book_numbering_resolves_chapter_numbers_in_order() { let dir = std::env::temp_dir().join(format!("dc-book-chapters-{}", std::process::id()));