mirror of
https://github.com/storytold/gridcraft.git
synced 2026-10-28 04:36:51 -06:00
fix(xlsx): bound repeated column expansion (#1022)
This commit is contained in:
@@ -38,6 +38,7 @@ struct RowState {
|
||||
}
|
||||
|
||||
const MAX_HYPERLINK_CELLS: u64 = 10_000;
|
||||
const MAX_COLUMN_ASSIGNMENTS: u64 = 1_000_000;
|
||||
|
||||
pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result<Sheet, IoError> {
|
||||
let bytes = cx.pkg.read(part)?.ok_or_else(|| IoError::Format(format!("missing part {part}")))?;
|
||||
@@ -61,6 +62,7 @@ pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result<Sheet, IoE
|
||||
drop(bytes);
|
||||
|
||||
let mut fit_to_page = false;
|
||||
let mut column_assignments_left = MAX_COLUMN_ASSIGNMENTS;
|
||||
cx.cf_ext_ids.clear();
|
||||
for e in &root.children {
|
||||
match e.name.as_str() {
|
||||
@@ -88,7 +90,7 @@ pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result<Sheet, IoE
|
||||
sheet.default_col_width = px as f32;
|
||||
}
|
||||
}
|
||||
"cols" => read_cols(cx, e, &mut sheet),
|
||||
"cols" => read_cols(cx, e, &mut sheet, &mut column_assignments_left)?,
|
||||
"mergeCells" => {
|
||||
for m in e.kids("mergeCell") {
|
||||
if let Some(r) = m.attr("ref").and_then(RangeRef::parse)
|
||||
@@ -300,7 +302,7 @@ fn read_views(e: &El, sheet: &mut Sheet) {
|
||||
}
|
||||
}
|
||||
|
||||
fn read_cols(cx: &Ctx<'_>, e: &El, sheet: &mut Sheet) {
|
||||
fn read_cols(cx: &Ctx<'_>, e: &El, sheet: &mut Sheet, remaining: &mut u64) -> Result<(), IoError> {
|
||||
for c in e.kids("col") {
|
||||
let (Some(min), Some(max)) = (c.attr_u32("min"), c.attr_u32("max")) else { continue };
|
||||
if min == 0 || min > MAX_COLS || max < min {
|
||||
@@ -328,10 +330,16 @@ fn read_cols(cx: &Ctx<'_>, e: &El, sheet: &mut Sheet) {
|
||||
if info == LineInfo::default() || (info.size == Some(sheet.default_col_width) && LineInfo { size: None, ..info } == LineInfo::default()) {
|
||||
continue;
|
||||
}
|
||||
// Overlapping declarations repeat work even when the final map stays small.
|
||||
// Charge the clipped span before expansion, sharing the budget across <cols> blocks.
|
||||
*remaining = remaining
|
||||
.checked_sub(u64::from(max - min + 1))
|
||||
.ok_or_else(|| IoError::TooLarge(format!("sheet \"{}\": more than {MAX_COLUMN_ASSIGNMENTS} expanded column assignments", sheet.name)))?;
|
||||
for col in (min - 1)..max {
|
||||
sheet.cols.insert(col, info);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn read_row(cx: &mut Ctx<'_>, sheet: &mut Sheet, st: &mut RowState, row: &El) {
|
||||
|
||||
@@ -256,3 +256,68 @@ fn hostile_formula_text_does_not_overflow_the_stack() {
|
||||
assert_eq!(a_len, 1_000_001);
|
||||
assert_eq!(a_value, gridcraft_core::Value::Number(3.0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn repeated_column_ranges_have_one_worksheet_work_budget() {
|
||||
// 62 * 16,384 = 1,015,808 assignments, while the map holds at most 16,384 columns.
|
||||
// This stays small even on the unbounded reader, but exceeds the one-million work cap.
|
||||
let col = r#"<col min="1" max="16384" width="10" customWidth="1"/>"#;
|
||||
for (first, second) in [(62, 0), (31, 31)] {
|
||||
let body = format!("<cols>{}</cols><cols>{}</cols><sheetData/>", col.repeat(first), col.repeat(second));
|
||||
assert!(matches!(read_xlsx(&minimal(&body, &[], "", "")), Err(IoError::TooLarge(_))), "blocks: {first}, {second}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn column_work_budget_counts_only_clipped_nondefault_assignments() {
|
||||
let full = r#"<col min="1" max="16384" width="10" customWidth="1"/>"#;
|
||||
// Default-only records and invalid coordinates do not expand into column assignments.
|
||||
let noise = format!(
|
||||
"{}{}",
|
||||
r#"<col min="1" max="16384" hidden="0" style="0"/>"#.repeat(62),
|
||||
r#"<col min="0" max="16384" width="20"/>
|
||||
<col min="16385" max="4294967295" width="20"/>
|
||||
<col min="5" max="4" width="20"/>
|
||||
<col min="bad" max="16384" width="20"/>"#
|
||||
);
|
||||
// The last declaration clips to 576 columns: 61 * 16,384 + 576 = 1,000,000.
|
||||
let cols = format!("{noise}{}<col min=\"15809\" max=\"4294967295\" width=\"20\"/>", full.repeat(61));
|
||||
let body = format!("<cols>{cols}</cols><sheetData/>");
|
||||
let (wb, _) = read_xlsx(&minimal(&body, &[], "", "")).unwrap();
|
||||
let sheet = wb.sheet(0).unwrap();
|
||||
assert_eq!(sheet.cols.len(), 16_384);
|
||||
assert_eq!(sheet.cols.get(&15_807).unwrap().size, Some(crate::tables::col_width_to_px(10.0)));
|
||||
assert_eq!(sheet.cols.get(&15_808).unwrap().size, Some(crate::tables::col_width_to_px(20.0)));
|
||||
assert_eq!(sheet.cols.get(&16_383).unwrap().size, Some(crate::tables::col_width_to_px(20.0)));
|
||||
|
||||
let body = format!("<cols>{cols}<col min=\"1\" max=\"1\" hidden=\"1\"/></cols><sheetData/>");
|
||||
assert!(matches!(read_xlsx(&minimal(&body, &[], "", "")), Err(IoError::TooLarge(_))));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ordinary_overlapping_column_ranges_keep_last_declaration() {
|
||||
let styles = r#"<styleSheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main">
|
||||
<cellXfs count="2"><xf numFmtId="0"/><xf numFmtId="2"/></cellXfs></styleSheet>"#;
|
||||
let rels = r#"<Relationship Id="rId2" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/styles" Target="styles.xml"/>"#;
|
||||
let body = r#"<cols><col min="1" max="3" width="10" hidden="1" style="1"/></cols>
|
||||
<cols><col min="2" max="4" width="20" hidden="0" style="0"/>
|
||||
<col min="5" max="5" hidden="1" style="1"/>
|
||||
<col min="1" max="5"/></cols><sheetData/>"#;
|
||||
let (wb, _) = read_xlsx(&minimal(body, &[("xl/styles.xml", styles)], rels, "")).unwrap();
|
||||
let sheet = wb.sheet(0).unwrap();
|
||||
assert_eq!(sheet.cols.len(), 5);
|
||||
let first = sheet.cols.get(&0).unwrap();
|
||||
assert_eq!(first.size, Some(crate::tables::col_width_to_px(10.0)));
|
||||
assert!(first.hidden);
|
||||
assert_eq!(wb.styles.get(first.style.unwrap()).num_fmt.as_str(), "0.00");
|
||||
for col in 1..4 {
|
||||
let info = sheet.cols.get(&col).unwrap();
|
||||
assert_eq!(info.size, Some(crate::tables::col_width_to_px(20.0)));
|
||||
assert!(!info.hidden);
|
||||
assert_eq!(info.style, None);
|
||||
}
|
||||
let last = sheet.cols.get(&4).unwrap();
|
||||
assert_eq!(last.size, None);
|
||||
assert!(last.hidden);
|
||||
assert_eq!(wb.styles.get(last.style.unwrap()).num_fmt.as_str(), "0.00");
|
||||
}
|
||||
|
||||
@@ -1,5 +1,14 @@
|
||||
# Decisions
|
||||
|
||||
## Bound repeated XLSX column expansion (#1022)
|
||||
|
||||
Limit each worksheet to 1,000,000 nondefault column assignments across all `<cols>` blocks.
|
||||
This allows more than 60 full-grid passes while bounding repeated work on overlapping spans;
|
||||
counting only the final map size would not limit that work. Charge each clipped span before
|
||||
inserting it, after the existing default-record skip. Exceeding the limit returns the existing
|
||||
`IoError::TooLarge`, which aborts the import instead of silently dropping a partially read sheet.
|
||||
Keep the current last-wins semantics rather than introduce interval storage or normalization.
|
||||
|
||||
## Formula localization uses the existing interface preference
|
||||
|
||||
PR #98 builds on the persisted `UiState.language` introduced by #31 and extended by #104.
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# File-format parity
|
||||
|
||||
> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-14 · **Change:** trivial (formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; previously trivial: XLSX chart sheets are kept on save (#727)) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4
|
||||
> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-16 · **Change:** minor (XLSX column declarations share a per-sheet expansion-work limit, #1022) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4
|
||||
|
||||
Every format Excel opens or saves, with GridCraft's read and write support, fidelity and tests.
|
||||
Part of [target-app-parity.md](target-app-parity.md). **File formats: ~50% ready, 80–120 h**
|
||||
@@ -13,6 +13,12 @@ support comes from `crates/engine/src/io.rs` (`FileKind`), `crates/xlsx/src` and
|
||||
|
||||
## Workbook formats
|
||||
|
||||
XLSX column declarations are limited to 1,000,000 expanded column assignments per worksheet,
|
||||
including repeated overlaps across `<cols>` blocks. Spans are clipped to the grid and records
|
||||
that would be skipped as defaults are excluded before charging the limit. Excess work returns
|
||||
the existing file-too-large error; accepted overlaps retain their existing last-wins behavior
|
||||
(#1022).
|
||||
|
||||
Ink created with Draw is written as standard, unfilled DrawingML polylines, without visible point
|
||||
captions or private metadata. Supported numeric polylines reopen as editable ink, retaining points,
|
||||
colour, integer width and anchor. Curves, filled paths, transforms and unsupported line styles are
|
||||
@@ -85,6 +91,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here.
|
||||
|
||||
| Date | Change | Summary |
|
||||
|---|---|---|
|
||||
| 2026-10-16 | minor | XLSX column imports refuse more than 1,000,000 cumulative expanded assignments per sheet before expanding the next span; repeated overlaps cannot bypass the work limit (#1022) |
|
||||
| 2026-10-14 | trivial | Formula parser: text longer than Excel's 8,192 characters, and postfix chains (`1%%%…`, `f(1)(1)…`) past the 256-level nesting limit, are parse errors (the cell keeps its formula text and cached value) instead of trees deep enough to overflow the stack when a hostile file is opened, saved or dropped |
|
||||
| 2026-10-12 | trivial | XLSX chart sheets are kept on save (#727) |
|
||||
| 2026-10-12 | trivial | CSV and TSV export is capped at 512 MiB and refuses a larger used range with an error (#528) |
|
||||
|
||||
Reference in New Issue
Block a user