From b49535624c6b6b6a6ca87f35c207a9a2ddb66951 Mon Sep 17 00:00:00 2001 From: Hamza Date: Wed, 28 Oct 2026 16:26:13 +0500 Subject: [PATCH 1/3] Tables: a shape a table cannot hold is clamped, not allocated (#1583) `Table::new` allocates `rows * cols` cells up front, and the counts reach it from IDML and DOCX, where they are whatever the file claims. Measured here: `size_of::()` is 1,360 bytes, so the reported IDML case -- one `` with 5,000 `` and 5,000 ``, about 1 KB zipped -- asks 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 catch, so every open document's unsaved work goes with it. A table is clamped to InDesign's own limits, 10,000 rows and 1,000 columns, and to 250,000 cells whatever its shape -- the row and column limits alone would still allow 13.6 GB. Columns are clamped first and rows take what is left, because a table is far more often long than wide. The IDML importer walks the clamped slices and reports the difference. Indexing its rows and columns by the file's own counts would be out of bounds, which is how a 34 GB abort would otherwise have become a panic. Rebased onto the post-split main: the constants and `clamped_size` live in `doc`'s `table` module root, `Table::new` in `table/grid.rs`, and the IDML table reader in `import/table.rs`. --- crates/doc/src/table.rs | 68 +++++++++++++++++++++++++++++++++ crates/doc/src/table/grid.rs | 10 ++++- crates/idml/src/import/table.rs | 17 ++++++++- 3 files changed, 92 insertions(+), 3 deletions(-) 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 48647cffc..f93379231 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/table.rs b/crates/idml/src/import/table.rs index 5e5a8685d..9bf0f920f 100644 --- a/crates/idml/src/import/table.rs +++ b/crates/idml/src/import/table.rs @@ -25,8 +25,21 @@ impl<'r> Importer<'r> { let id = self.alloc(); 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. + let (nr, nc) = designcraft_doc::table::clamped_size(rows.len().max(1), cols.len().max(1)); + if nr < rows.len() || nc < cols.len() { + self.warnings.push(format!( + "a table of {} rows and {} columns was read as {nr} x {nc}: more than a table can hold", + rows.len(), + cols.len() + )); + } + 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 From 1995fb5feb1b46e76fe76d6cc77635c42963b092 Mon Sep 17 00:00:00 2001 From: Hamza Date: Wed, 28 Oct 2026 16:31:34 +0500 Subject: [PATCH 2/3] IDML import: the tables in one file are capped between them Review pointed out that capping each table leaves a file free to hold many capped tables: three at the per-table limit claim 750,000 cells, about 1 GB of Cell, with every table inside its own limit. The package reader's limits count input bytes, which say nothing about how many cells a row and column count claim. budgeted_size clamps each table against what the import has left, and the cells are reserved before Table::new so a cell's own story cannot spend the budget the table around it is counting on. The budget is 400,000 cells: a Cell carries a whole story and is 1,360 bytes, so that is about the 512 MiB one part of the file may already decompress to, and the same count the spreadsheet importer collects at most. A table is never read as nothing. One row and one column are kept, so the table stays in the document and still says where its text went, and a file pays one cell for a table it really does contain -- which its own size already bounds. The warning says which limit cut the table down. The arithmetic is tested rather than the allocation: 400,000 cells is 544 MB and three such tables are not a thing to ask CI to allocate. The sequence test runs 32 claimed sizes, including usize::MAX, and holds the invariant across all of them. --- crates/idml/src/import.rs | 4 ++ crates/idml/src/import/table.rs | 91 ++++++++++++++++++++++++++++++--- 2 files changed, 89 insertions(+), 6 deletions(-) diff --git a/crates/idml/src/import.rs b/crates/idml/src/import.rs index 73bed14d3..c8e0512b5 100644 --- a/crates/idml/src/import.rs +++ b/crates/idml/src/import.rs @@ -139,6 +139,9 @@ 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, settings: DocSettings, default_margins: Option<(Margins, Columns)>, swatches: Vec, @@ -225,6 +228,7 @@ impl<'r> Importer<'r> { read_link, previews, next_id: 0, + table_cells: 0, settings: DocSettings::default(), default_margins: None, swatches: specials, diff --git a/crates/idml/src/import/table.rs b/crates/idml/src/import/table.rs index 9bf0f920f..3b1051b66 100644 --- a/crates/idml/src/import/table.rs +++ b/crates/idml/src/import/table.rs @@ -1,6 +1,30 @@ use super::values::{span, tint}; use super::*; +/// 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; + +/// 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. +/// +/// The per-table cap leaves a file free to hold many capped tables, so each is also clamped +/// against what is left. 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 -- which its own size already bounds. +fn budgeted_size(rows: usize, cols: usize, reserved: usize) -> (usize, usize, bool) { + let (mut nr, mut nc) = designcraft_doc::table::clamped_size(rows, cols); + let left = MAX_IMPORT_TABLE_CELLS.saturating_sub(reserved); + let over_budget = nr.saturating_mul(nc) > left; + if over_budget { + nc = nc.min(left.max(1)); + nr = nr.min((left / nc).max(1)); + } + (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 { @@ -30,13 +54,14 @@ impl<'r> Importer<'r> { // 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. - let (nr, nc) = designcraft_doc::table::clamped_size(rows.len().max(1), cols.len().max(1)); + // + // Reserved before `Table::new`, so a cell's own story cannot spend the budget this table + // is counting on. + let (nr, nc, over_budget) = budgeted_size(rows.len().max(1), cols.len().max(1), self.table_cells); + self.table_cells = self.table_cells.saturating_add(nr.saturating_mul(nc)); if nr < rows.len() || nc < cols.len() { - self.warnings.push(format!( - "a table of {} rows and {} columns was read as {nr} x {nc}: more than a table can hold", - rows.len(), - 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.warnings.push(format!("a table of {} rows and {} columns was read as {nr} x {nc}: {why}", rows.len(), cols.len())); } let rows = &rows[..nr.min(rows.len())]; let cols = &cols[..nc.min(cols.len())]; @@ -170,3 +195,57 @@ 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..3 { + let (nr, nc, over) = budgeted_size(500, 1000, reserved); + reserved += nr * nc; + sizes.push((nr, nc, over)); + } + assert_eq!(sizes[0], (250, 1000, false), "the first table is held by the per-table cap alone"); + assert_eq!(sizes[1], (150, 1000, true), "the second gets what is left of the file's budget"); + assert_eq!(sizes[2], (1, 1, true), "and the third is kept, so its text still has somewhere to be"); + assert_eq!(reserved, MAX_IMPORT_TABLE_CELLS + 1, "400,000 cells and the one the last table keeps"); + } + + /// The budget holds for any sequence of claimed sizes, and a table is never read as nothing: + /// a 0 x n table has no cells to hold the text the file puts in it. + #[test] + fn a_table_is_never_read_as_empty_and_the_budget_always_holds() { + 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 tables = 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 { + let (nr, nc, _) = budgeted_size(rows.max(1), cols.max(1), reserved); + assert!(nr >= 1 && nc >= 1, "{rows} x {cols} was read as {nr} x {nc}"); + reserved += nr * nc; + tables += 1; + assert!( + reserved <= MAX_IMPORT_TABLE_CELLS + tables, + "{tables} tables reserved {reserved} cells, past the budget by more than the one cell each keeps" + ); + } + } + assert!(reserved >= MAX_IMPORT_TABLE_CELLS, "this sequence does reach the budget: {reserved}"); + } + + /// 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), (rows, cols, false), "{rows} x {cols}"); + } + } +} From 18f2dc2bdea4e8d9d028a4c267352240240d8a44 Mon Sep 17 00:00:00 2001 From: Hamza Date: Thu, 29 Oct 2026 16:43:49 +0500 Subject: [PATCH 3/3] A spent cell budget stops reading tables, rather than keeping one each Keeping a 1 x 1 table for every further `
` was unbounded in the one direction the budget exists to close. `
` is fifteen bytes, so the 512 MiB a part may decompress to holds some 36 million of them, and a `Cell` is 1,360 bytes: tens of gigabytes, with every table inside both caps. The test I wrote said as much without my noticing -- its invariant was `reserved <= MAX + tables`, and `tables` had no bound. `budgeted_size` now returns `None` once the budget is spent and the table is not read at all, so the cells one import reserves are at most the budget, exactly. The invariant is now `reserved <= MAX_IMPORT_TABLE_CELLS`, with no term that grows with the file. The warnings needed the same treatment, for the same reason: a file of 400,000 tables each claiming one row and two columns got a line about every one of them. At most 20 are named, then one line saying the rest were the same, and the file is told once that its tables ran out of cells between them. --- crates/idml/src/import.rs | 7 +++ crates/idml/src/import/story.rs | 7 ++- crates/idml/src/import/table.rs | 104 +++++++++++++++++++++----------- 3 files changed, 81 insertions(+), 37 deletions(-) diff --git a/crates/idml/src/import.rs b/crates/idml/src/import.rs index c8e0512b5..ae7364647 100644 --- a/crates/idml/src/import.rs +++ b/crates/idml/src/import.rs @@ -142,6 +142,11 @@ struct Importer<'r> { /// 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, @@ -229,6 +234,8 @@ impl<'r> Importer<'r> { 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 694a2dbbd..72cde49b1 100644 --- a/crates/idml/src/import/story.rs +++ b/crates/idml/src/import/story.rs @@ -168,8 +168,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 3b1051b66..91ac287f9 100644 --- a/crates/idml/src/import/table.rs +++ b/crates/idml/src/import/table.rs @@ -7,22 +7,31 @@ use super::*; /// 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. +/// 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. 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 -- which its own size already bounds. -fn budgeted_size(rows: usize, cols: usize, reserved: usize) -> (usize, usize, bool) { +/// 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.saturating_sub(reserved); + 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.max(1)); + nc = nc.min(left); nr = nr.min((left / nc).max(1)); } - (nr, nc, over_budget) + Some((nr, nc, over_budget)) } /// Preserve the legacy uniform field only when all four sides explicitly specify every @@ -45,8 +54,7 @@ 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(); // A file's row and column counts are its claim, and a table of them is allocated whole: a @@ -57,12 +65,31 @@ impl<'r> Importer<'r> { // // Reserved before `Table::new`, so a cell's own story cannot spend the budget this table // is counting on. - let (nr, nc, over_budget) = budgeted_size(rows.len().max(1), cols.len().max(1), self.table_cells); + 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.warnings.push(format!("a table of {} rows and {} columns was read as {nr} x {nc}: {why}", rows.len(), cols.len())); + 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); @@ -151,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. @@ -207,45 +234,52 @@ mod budget_tests { fn the_tables_in_one_file_are_capped_between_them() { let mut reserved = 0; let mut sizes = Vec::new(); - for _ in 0..3 { - let (nr, nc, over) = budgeted_size(500, 1000, reserved); - reserved += nr * nc; - sizes.push((nr, nc, over)); + 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], (250, 1000, false), "the first table is held by the per-table cap alone"); - assert_eq!(sizes[1], (150, 1000, true), "the second gets what is left of the file's budget"); - assert_eq!(sizes[2], (1, 1, true), "and the third is kept, so its text still has somewhere to be"); - assert_eq!(reserved, MAX_IMPORT_TABLE_CELLS + 1, "400,000 cells and the one the last table keeps"); + 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 holds for any sequence of claimed sizes, and a table is never read as nothing: - /// a 0 x n table has no cells to hold the text the file puts in it. + /// 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_table_is_never_read_as_empty_and_the_budget_always_holds() { + 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 tables = 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 { - let (nr, nc, _) = budgeted_size(rows.max(1), cols.max(1), reserved); - assert!(nr >= 1 && nc >= 1, "{rows} x {cols} was read as {nr} x {nc}"); - reserved += nr * nc; - tables += 1; - assert!( - reserved <= MAX_IMPORT_TABLE_CELLS + tables, - "{tables} tables reserved {reserved} cells, past the budget by more than the one cell each keeps" - ); + 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!(reserved >= MAX_IMPORT_TABLE_CELLS, "this sequence does reach the budget: {reserved}"); + 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), (rows, cols, false), "{rows} x {cols}"); + assert_eq!(budgeted_size(rows, cols, 0), Some((rows, cols, false)), "{rows} x {cols}"); } } }