aleleba/designcraft
1
0
Fork 0
mirror of https://github.com/storytold/designcraft.git synced 2026-10-30 21:26:48 -06:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1868 from Hamza0101/fix/table-allocation-cap

Browse Source 
Tables: a shape a table cannot hold is clamped, not allocated (#1583)
This commit is contained in:
Brandon Thomas
2026-10-29 21:51:53 -04:00
committed by GitHub
parent 3e15ebc7b0 18f2dc2bde
commit cddf074cc4
5 changed files with 224 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/doc/src/table.rs
+68
View File
@@ -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 `<Table>` with 5,000 `<Row/>` and 5,000 `<Column/>` -- 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::<Cell>() > 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);
crates/doc/src/table/grid.rs
+9 -1
View File
@@ -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()));
crates/idml/src/import.rs
+11
View File
@@ -139,6 +139,14 @@ struct Importer<'r> {
/// Graphic `Self` → stand-in image for a linked file that can't be read.
previews: &'r HashMap<String, Arc<Vec<u8>>>,
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<Swatch>,
@@ -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,
crates/idml/src/import/story.rs
+5 -2
View File
@@ -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()));
crates/idml/src/import/table.rs
+131 -5
View File
@@ -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 `<Table>`
/// was unbounded in the one direction the budget was meant to close: `<Table></Table>` 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<designcraft_doc::CellStroke> {
@@ -21,12 +54,44 @@ pub(super) fn uniform_cell_stroke(edges: &[designcraft_doc::CellStrokeAttrs; 4])
impl<'r> Importer<'r> {
/// `<Table>`: 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<Table> {
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 `<Table>` 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 `<Table></Table>` 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}");
}
}
}