diff --git a/crates/doc/src/table.rs b/crates/doc/src/table.rs index d5342b5c9..937fefe1f 100644 --- a/crates/doc/src/table.rs +++ b/crates/doc/src/table.rs @@ -25,6 +25,32 @@ mod story; pub use cell::{AltFills, AltStrokes, Cell, CellStroke, Column, Row, RowHeightMode, RowKind, TableOptions}; pub use grid::CellRange; +/// Most rows a table holds, as InDesign allows. +pub const MAX_TABLE_ROWS: usize = 10_000; +/// Most columns a table holds, as InDesign allows. +pub const MAX_TABLE_COLS: usize = 1_000; +/// Most cells a table holds, whatever its shape. +/// +/// A [`Cell`] is about 1.4 KB (it carries a [`Story`]), so this is roughly 340 MB of cells at the +/// limit. The row and column limits alone are not enough: 10,000 × 1,000 would be 13.6 GB, and an +/// IDML story holding one `` with 5,000 `` and 5,000 `` -- about 1 KB +/// zipped -- asked for 25M cells, some 34 GB. That allocation does not fail politely: it aborts +/// the process through `handle_alloc_error`, which no `catch_unwind` can save, so every open +/// document's unsaved work goes with it (#1583). +pub const MAX_TABLE_CELLS: usize = 250_000; + +/// The rows and columns a table of `rows × cols` is actually given, inside the limits above. +/// +/// Columns are clamped first and rows take what is left of the cell budget, because a table is +/// far more often long than wide: truncating a 5,000-column table to 1,000 columns and then its +/// rows loses less of it than the other way round. Callers that read a file report the +/// difference; the counts are a document's claim, not something to trust. +pub fn clamped_size(rows: usize, cols: usize) -> (usize, usize) { + let cols = cols.clamp(1, MAX_TABLE_COLS); + let rows = rows.clamp(1, MAX_TABLE_ROWS.min(MAX_TABLE_CELLS / cols).max(1)); + (rows, cols) +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Table { @@ -48,6 +74,48 @@ mod tests { Story::with_text(StoryId(1), t, ParaFormat::default()) } + /// A table's shape comes out of IDML and DOCX files unchecked, and `Cell` is about 1.4 KB, so + /// a file claiming 5,000 x 5,000 asked for 25M cells -- some 34 GB. That allocation aborts the + /// process rather than failing, taking every open document's unsaved work with it (#1583). + #[test] + fn a_table_larger_than_the_limits_is_clamped_not_allocated() { + // The reported shape: 5,000 rows and 5,000 columns. + let t = Table::new(1, 5_000, 5_000, 0, 0, 500.0); + assert_eq!(t.columns.len(), MAX_TABLE_COLS, "columns are clamped to InDesign's limit"); + assert!(t.cells.len() <= MAX_TABLE_CELLS, "{} cells is within the budget", t.cells.len()); + assert_eq!(t.rows.len() * t.columns.len(), t.cells.len(), "the grid stays rectangular"); + // What it would have been: 25,000,000 cells at 1,360 bytes each. + assert!(25_000_000 * std::mem::size_of::() > 30_000_000_000, "the size this test is about"); + // Freed before the next large table, so the peak stays at one of them. + drop(t); + + // A long table keeps its rows while it fits the budget (the budget itself is checked by + // `clamped_size`, which allocates nothing). + let long = Table::new(2, 10_000, 5, 0, 0, 500.0); + assert_eq!((long.rows.len(), long.columns.len()), (10_000, 5), "a long thin table is untouched"); + assert_eq!(clamped_size(10_000, 25), (10_000, 25), "10,000 x 25 is inside the budget"); + drop(long); + + // Past the row limit the rows are clamped too. + let longer = Table::new(3, 50_000, 2, 0, 0, 100.0); + assert_eq!(longer.rows.len(), MAX_TABLE_ROWS, "rows are clamped to InDesign's limit"); + drop(longer); + + // Header, body and footer share the budget, and the table still has a body. + let with_parts = Table::new(4, 5_000, 5_000, 3, 2, 500.0); + assert!(with_parts.rows.len() <= MAX_TABLE_ROWS, "{} rows", with_parts.rows.len()); + assert!(with_parts.cells.len() <= MAX_TABLE_CELLS, "{} cells", with_parts.cells.len()); + assert!(with_parts.rows.iter().any(|r| r.kind == RowKind::Header), "the header survives"); + assert!(with_parts.rows.iter().any(|r| r.kind == RowKind::Footer), "the footer survives"); + assert!(with_parts.rows.iter().any(|r| r.kind == RowKind::Body), "and so does a body row"); + drop(with_parts); + + // A header and footer bigger than the whole budget still leave a body row. + let all_header = Table::new(6, 1, 1, MAX_TABLE_ROWS * 2, MAX_TABLE_ROWS * 2, 100.0); + assert!(all_header.rows.len() <= MAX_TABLE_ROWS, "{} rows", all_header.rows.len()); + assert!(all_header.rows.iter().any(|r| r.kind == RowKind::Body), "a body row survives"); + } + #[test] fn move_row_keeps_header_and_footer_rows_in_place() { let mut t = Table::new(1, 3, 2, 1, 1, 120.0); diff --git a/crates/doc/src/table/grid.rs b/crates/doc/src/table/grid.rs index a64f2cd99..7ae118744 100644 --- a/crates/doc/src/table/grid.rs +++ b/crates/doc/src/table/grid.rs @@ -25,8 +25,16 @@ impl CellRange { impl Table { /// A `rows × cols` table (`header` + body + `footer` rows) whose columns share `width`. + /// + /// The shape is clamped to [`clamped_size`](super::clamped_size): the counts reach this from + /// IDML and DOCX files, where they are whatever the file says (#1583). pub fn new(id: u64, body_rows: usize, cols: usize, header: usize, footer: usize, width: f64) -> Table { - let cols = cols.max(1); + // Header, body and footer rows share one budget: the total is clamped first, then the + // header and footer take what they can of it, and the body keeps at least one row. + let (total, cols) = super::clamped_size(body_rows.max(1).saturating_add(header).saturating_add(footer), cols); + let header = header.min(total.saturating_sub(1)); + let footer = footer.min(total.saturating_sub(header).saturating_sub(1)); + let body_rows = total.saturating_sub(header).saturating_sub(footer).max(1); let mut rows = Vec::new(); rows.extend((0..header).map(|_| Row { kind: RowKind::Header, ..Default::default() })); rows.extend((0..body_rows.max(1)).map(|_| Row::default())); diff --git a/crates/idml/src/import.rs b/crates/idml/src/import.rs index 161594e1e..2c6185c5c 100644 --- a/crates/idml/src/import.rs +++ b/crates/idml/src/import.rs @@ -139,6 +139,14 @@ struct Importer<'r> { /// Graphic `Self` → stand-in image for a linked file that can't be read. previews: &'r HashMap>>, next_id: u64, + /// Cells every table in this import has reserved. One table is capped, but a file chooses + /// how many tables it has, so the cap has to be document-wide as well (noted in review). + table_cells: usize, + /// Tables whose size the file claimed and did not get, warned about so far. A file chooses + /// how many tables it has, so a line each is its own flood (noted in review). + table_warnings: usize, + /// Whether the file has already been told its tables ran out of cells between them. + table_budget_spent: bool, settings: DocSettings, default_margins: Option<(Margins, Columns)>, swatches: Vec, @@ -225,6 +233,9 @@ impl<'r> Importer<'r> { read_link, previews, next_id: 0, + table_cells: 0, + table_warnings: 0, + table_budget_spent: false, settings: DocSettings::default(), default_margins: None, swatches: specials, diff --git a/crates/idml/src/import/story.rs b/crates/idml/src/import/story.rs index 3d87fc35a..5471f4f96 100644 --- a/crates/idml/src/import/story.rs +++ b/crates/idml/src/import/story.rs @@ -171,8 +171,11 @@ impl<'r> Importer<'r> { b.fresh = true; } "Table" => { - let t = self.table(c); - b.push_table(t, pf, cf); + // `None` once the file-wide cell budget is spent: the table is not read + // at all rather than kept as a 1 x 1 (noted in review). + if let Some(t) = self.table(c) { + b.push_table(t, pf, cf); + } } "HyperlinkTextDestination" | "ParagraphDestination" => { b.anchors.push((c.get("Self").unwrap_or("").to_string(), c.get("Name").unwrap_or("Anchor").to_string())); diff --git a/crates/idml/src/import/table.rs b/crates/idml/src/import/table.rs index 5b5ca744b..268f1af97 100644 --- a/crates/idml/src/import/table.rs +++ b/crates/idml/src/import/table.rs @@ -1,6 +1,39 @@ use super::values::{span, tint}; use super::{CellRange, El, Importer, RowHeightMode, StoryId, Table, names, swatch}; +/// Cells every table in one import may hold between them. A `Cell` carries a whole story, so +/// it is 1,360 bytes: 400,000 of them is about 520 MB, which is the 512 MiB the package reader +/// already allows one part of the file to decompress to, and the same count the spreadsheet +/// importer collects at most. +const MAX_IMPORT_TABLE_CELLS: usize = 400_000; + +/// Clamp warnings one import writes before it says the rest were the same. +const MAX_TABLE_WARNINGS: usize = 20; + +/// How large a table of `rows` x `cols` may be read when `reserved` cells have already been +/// taken, and whether the file-wide budget is what cut it down -- or `None` when the budget is +/// spent and the table is not read at all. +/// +/// The per-table cap leaves a file free to hold many capped tables, so each is also clamped +/// against what is left. While there is budget, one row and one column are always kept: the table +/// is then still in the document, still says where its text went, and a file pays one cell for a +/// table it really does contain. +/// +/// Past that there is nothing left to pay with. Keeping a 1 x 1 table for every further `
` +/// was unbounded in the one direction the budget was meant to close: `
` is fifteen +/// bytes, so the 512 MiB a part may decompress to holds some 36 million of them, and a `Cell` +/// each is tens of gigabytes with every table inside both caps (noted in review). +fn budgeted_size(rows: usize, cols: usize, reserved: usize) -> Option<(usize, usize, bool)> { + let (mut nr, mut nc) = designcraft_doc::table::clamped_size(rows, cols); + let left = MAX_IMPORT_TABLE_CELLS.checked_sub(reserved).filter(|left| *left > 0)?; + let over_budget = nr.saturating_mul(nc) > left; + if over_budget { + nc = nc.min(left); + nr = nr.min((left / nc).max(1)); + } + Some((nr, nc, over_budget)) +} + /// Preserve the legacy uniform field only when all four sides explicitly specify every /// property. Partial records must stay sparse so they can inherit from a parent style. pub(super) fn uniform_cell_stroke(edges: &[designcraft_doc::CellStrokeAttrs; 4]) -> Option { @@ -21,12 +54,44 @@ pub(super) fn uniform_cell_stroke(edges: &[designcraft_doc::CellStrokeAttrs; 4]) impl<'r> Importer<'r> { /// ``: rows, columns and cells (cell `Name` is `column:row`). - pub(super) fn table(&mut self, e: &El) -> Table { - let id = self.alloc(); + pub(super) fn table(&mut self, e: &El) -> Option
{ let rows: Vec<&El> = e.find_all("Row").collect(); let cols: Vec<&El> = e.find_all("Column").collect(); - let nr = rows.len().max(1); - let nc = cols.len().max(1); + // A file's row and column counts are its claim, and a table of them is allocated whole: a + // story holding one `
` with 5,000 of each asked for 25M cells, around 34 GB, which + // aborts the process instead of failing (#1583). What is read is clamped, and the rest of + // this function walks the clamped slices -- indexing `t.rows`/`t.columns` by the file's + // own counts would be out of bounds. + // + // Reserved before `Table::new`, so a cell's own story cannot spend the budget this table + // is counting on. + let Some((nr, nc, over_budget)) = budgeted_size(rows.len().max(1), cols.len().max(1), self.table_cells) else { + // Said once: the file that reaches this is the one with millions of tables in it. + if !self.table_budget_spent { + self.table_budget_spent = true; + self.warnings.push(format!( + "the tables in this file claim more than {MAX_IMPORT_TABLE_CELLS} cells between them: the rest of them were not read" + )); + } + return None; + }; + self.table_cells = self.table_cells.saturating_add(nr.saturating_mul(nc)); + if nr < rows.len() || nc < cols.len() { + let why = if over_budget { "more than the tables in one file can hold between them" } else { "more than a table can hold" }; + self.table_warnings += 1; + match self.table_warnings.cmp(&MAX_TABLE_WARNINGS) { + std::cmp::Ordering::Less => { + self.warnings.push(format!("a table of {} rows and {} columns was read as {nr} x {nc}: {why}", rows.len(), cols.len())) + } + std::cmp::Ordering::Equal => { + self.warnings.push(format!("more than {MAX_TABLE_WARNINGS} tables were read smaller than this file claims")) + } + std::cmp::Ordering::Greater => {} + } + } + let id = self.alloc(); + let rows = &rows[..nr.min(rows.len())]; + let cols = &cols[..nc.min(cols.len())]; let mut t = Table::new(id, nr, nc, 0, 0, 0.0); t.options.direction = if e.get("TableDirection") == Some("RightToLeftDirection") { designcraft_doc::TextDirection::RightToLeft @@ -113,7 +178,7 @@ impl<'r> Importer<'r> { for rg in regions { let _ = t.merge(rg); } - t + Some(t) } /// A cell's local fill, insets, alignment, rotation and edge strokes. @@ -157,3 +222,64 @@ impl<'r> Importer<'r> { } } } + +#[cfg(test)] +mod budget_tests { + use super::{MAX_IMPORT_TABLE_CELLS, budgeted_size}; + + /// One table is capped, but a file chooses how many tables it has: three tables each at the + /// per-table cap asked for 750,000 cells, about 1 GB of `Cell`, with every table inside its + /// own limit (noted in review of #1583). + #[test] + fn the_tables_in_one_file_are_capped_between_them() { + let mut reserved = 0; + let mut sizes = Vec::new(); + for _ in 0..4 { + let size = budgeted_size(500, 1000, reserved); + if let Some((nr, nc, _)) = size { + reserved += nr * nc; + } + sizes.push(size); + } + assert_eq!(sizes[0], Some((250, 1000, false)), "the first table is held by the per-table cap alone"); + assert_eq!(sizes[1], Some((150, 1000, true)), "the second gets what is left of the file's budget"); + assert_eq!(sizes[2], None, "and the third is not read: there is nothing left to pay for it"); + assert_eq!(sizes[3], None, "nor any after it"); + assert_eq!(reserved, MAX_IMPORT_TABLE_CELLS, "the budget is spent exactly, not overspent"); + } + + /// The budget is a hard limit, not a floor: keeping one cell for every further table was + /// unbounded in the one direction the budget exists to close, since a `
` is + /// fifteen bytes and a part may decompress to 512 MiB (noted in review). + #[test] + fn a_spent_budget_stops_reading_tables_altogether() { + let claims = [(0, 0), (1, 1), (7, 3), (usize::MAX, usize::MAX), (1, usize::MAX), (usize::MAX, 1), (10_000, 1_000), (2, 0)]; + let mut reserved = 0usize; + let mut read = 0usize; + let mut skipped = 0usize; + // Every claim, several times over, in an order that exhausts the budget part way through. + for _ in 0..4 { + for (rows, cols) in claims { + match budgeted_size(rows.max(1), cols.max(1), reserved) { + Some((nr, nc, _)) => { + assert!(nr >= 1 && nc >= 1, "{rows} x {cols} was read as {nr} x {nc}: a table has nowhere to put its text"); + reserved += nr * nc; + read += 1; + } + None => skipped += 1, + } + assert!(reserved <= MAX_IMPORT_TABLE_CELLS, "{read} tables reserved {reserved} cells, past the budget"); + } + } + assert_eq!(reserved, MAX_IMPORT_TABLE_CELLS, "this sequence does reach the budget"); + assert!(skipped > 0, "and does go on asking after it: {skipped} tables were not read"); + } + + /// Below the budget nothing is clamped by it, so an ordinary file is read exactly as before. + #[test] + fn an_ordinary_file_is_not_touched_by_the_budget() { + for (rows, cols) in [(1, 1), (2, 3), (50, 20), (250, 1000)] { + assert_eq!(budgeted_size(rows, cols, 0), Some((rows, cols, false)), "{rows} x {cols}"); + } + } +}