mirror of
https://github.com/storytold/gridcraft.git
synced 2026-10-24 02:36:48 -06:00
Merge PR #1648: fix(xlsx): bound repeated column expansion
Conflicts: - docs/file-format-parity.md: kept main's header and log rows, added this PR's change phrase and row once
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() {
|
||||
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-19 · **Change:** minor (pictures in XLSX headers and footers are kept on save; legacy XLSX `INDEX`/`MATCH` formulas stay scalar on save; XLSX text keeps carriage returns on save and reopen; .xls dialog sheets are skipped instead of read as worksheets; `.xls` import reads each sheet's display options from WINDOW2: right-to-left, gridlines, headings, zeros, formulas; XLSX text boxes and shapes keep their paragraphs' right-to-left direction on save; CSV/TSV/TXT can open with every field kept as text via `file.open {csvAsText}`; 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; XLSX PivotTables read and write `rowGrandTotals`/`colGrandTotals` the way Excel does, so a table saved with only one grand total opens with the right one; HTML export keeps right-to-left) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4
|
||||
> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-19 · **Change:** minor (XLSX column declarations share a per-sheet expansion-work limit; pictures in XLSX headers and footers are kept on save; legacy XLSX `INDEX`/`MATCH` formulas stay scalar on save; XLSX text keeps carriage returns on save and reopen; .xls dialog sheets are skipped instead of read as worksheets; `.xls` import reads each sheet's display options from WINDOW2: right-to-left, gridlines, headings, zeros, formulas; XLSX text boxes and shapes keep their paragraphs' right-to-left direction on save; CSV/TSV/TXT can open with every field kept as text via `file.open {csvAsText}`; 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; XLSX PivotTables read and write `rowGrandTotals`/`colGrandTotals` the way Excel does, so a table saved with only one grand total opens with the right one; HTML export keeps right-to-left) · **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**
|
||||
@@ -16,6 +16,12 @@ shared formulas are expanded during import (`array_export.rs`, #1505).
|
||||
|
||||
## 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
|
||||
@@ -95,6 +101,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here.
|
||||
| 2026-10-18 | trivial | XLSX PivotTables: `rowGrandTotals` is the Grand Total row and `colGrandTotals` the Grand Total column. Both were read and written the other way round, which a GridCraft round trip hid, so an Excel file with only one grand total opened showing the other |
|
||||
| 2026-10-19 | minor | HTML (Save as Web Page and clipboard) marks right-to-left sheets and right-to-left cell text with `dir` |
|
||||
| 2026-10-16 | trivial | XLSX: pictures in headers and footers (`&G`, the VML drawing `<legacyDrawingHF>` names and its images) are kept on save; they are still not shown (#1257) |
|
||||
| 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-15 | minor | XLSX text preserves CR and CRLF across save/reopen; shared strings use `_x000D_` and other XML text uses ` ` (#1488) |
|
||||
| 2026-10-15 | minor | XLSX save keeps legacy `INDEX`/`MATCH` formulas scalar instead of marking them as dynamic arrays (#1505) |
|
||||
|
||||
Reference in New Issue
Block a user