XLSX: warn when a pivot cache can't be read or is too large (#1039) (#2491)

read_cache turned every pivot cache error into absence with
optional_xml(part).ok().flatten(), so a cache over the part size limit
dropped its PivotTable with no warning, and a malformed or missing cache
only left a generic "skipped part" line. Each case now adds a warning that
the PivotTable was kept as plain cells, and the workbook keeps opening
(the oversized part is refused before it inflates).

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Harrison Booth
2026-11-14 16:56:12 +01:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 45ee4eceed
commit a8a51e3cf7
4 changed files with 64 additions and 1 deletions
+2
View File
@@ -180,6 +180,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in
| 2026-10-15 | File › Options › Formulas: Maximum Iterations and Maximum Change fields for iterative calculation, disabled while it is off, with Excel's limits (1 to 32767, 0 or more) |
| 2026-10-15 | Function help follows the interface language: argument hints, autocomplete, Insert Function (search and readable categories) and the Formulas-tab menus read per-language tables of descriptions and argument names; Spanish keeps its 10 functions in the new format |
| 2026-10-15 | Row auto-fit counts a cell's line breaks only when the cell wraps, as Excel does: a file whose unwrapped cells hold multi-line text no longer opens with tall rows (typing a break still turns wrap on and grows the row) |
| 2026-10-14 | XLSX import warns when a pivot cache is missing, malformed or too large, naming the cache and saying its PivotTable was kept as plain cells, instead of dropping the PivotTable silently (#1039) |
| 2026-10-14 | Set precision as displayed is applied: with the option on, formula results, spills and numbers typed or reformatted are stored as their number format shows them and formulas calculate from those values; `formulas.calculationOptions` gains `precisionAsDisplayed` (turning it on rounds existing constants; locked constants on a protected sheet keep their value; numbers written to a new sheet are rounded too) (#1343) |
| 2026-10-14 | Lookups (MATCH, XMATCH, VLOOKUP, HLOOKUP, XLOOKUP, LOOKUP) compare numbers exactly like Excel, so a computed key such as `=0.1+0.2` no longer finds 0.3; `=` and COUNTIF/SUMIF keep their 15-digit comparison |
| 2026-10-14 | Functions: EOMONTH, EDATE, WORKDAY, NETWORKDAYS, YEARFRAC, WEEKNUM, the bond and coupon functions, DISC, BIN2DEC, IM* and the other Analysis ToolPak functions give `#VALUE!` for a reference to more than one cell in a value argument, as Excel 365 does, instead of spilling; array constants and computed arrays still lift |
@@ -305,6 +306,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in
| 2026-10-16 | minor | Progress log: floating pictures' textures bounded and kept per workbook |
| 2026-10-16 | trivial | Progress log: web preferences kept between visits (#1562) |
| 2026-10-16 | minor | Progress log: interface fonts held once instead of twice |
| 2026-10-14 | trivial | Progress log: pivot cache import warnings (#1039) |
| 2026-10-14 | minor | Progress log: Set precision as displayed applied in calculation (#1343) |
| 2026-10-14 | minor | Progress log: `.xls` import reads cell formatting |
| 2026-10-14 | minor | Progress log: lookups compare numbers exactly |
+17 -1
View File
@@ -69,7 +69,23 @@ fn item_label_el(e: &El, sys: DateSystem) -> String {
}
fn read_cache(cx: &mut Ctx<'_>, part: &str, sheet_name: &str) -> Option<CacheIn> {
let x = cx.optional_xml(part).ok().flatten()?;
// A cache that can't be read loses its PivotTable: say so instead of dropping it silently.
// An oversized cache is refused before it inflates, so warning and opening the rest is safe.
let x = match cx.optional_xml(part) {
Ok(Some(x)) => x,
Ok(None) => {
cx.warn(format!("a PivotTable's cache {part} is missing or could not be read; kept as plain cells"));
return None;
}
Err(IoError::TooLarge(m)) => {
cx.warn(format!("a PivotTable's cache is too large to read ({m}); kept as plain cells"));
return None;
}
Err(e) => {
cx.warn(format!("a PivotTable's cache could not be read ({e}); kept as plain cells"));
return None;
}
};
if x.name != "pivotCacheDefinition" {
cx.warn(format!("{part} is not a pivot cache definition; its PivotTable was kept as plain cells"));
return None;
+44
View File
@@ -711,6 +711,50 @@ fn data_on_rows_with_one_value_field_reads_without_warning() {
assert!(back.sheet(0).unwrap().pivots[0].values_on_rows);
}
const SIMPLE_TABLE: &str = r#"<pivotTableDefinition name="Sales"><location ref="C1:D4"/><pivotFields><pivotField axis="axisRow"><items><item x="0"/><item t="default"/></items></pivotField><pivotField dataField="1"/></pivotFields><rowFields><field x="0"/></rowFields><dataFields><dataField name="Total" fld="1"/></dataFields></pivotTableDefinition>"#;
const CACHE_PART: &str = "xl/pivotCache/pivotCacheDefinition1.xml";
/// Rewrites the uncompressed size a zip's central directory declares for `name`.
fn declare_size(mut zip: Vec<u8>, name: &str, size: u32) -> Vec<u8> {
let at = (0..zip.len().saturating_sub(46))
.find(|&i| {
zip[i..i + 4] == [0x50, 0x4b, 0x01, 0x02] && {
let len = u16::from_le_bytes([zip[i + 28], zip[i + 29]]) as usize;
zip.get(i + 46..i + 46 + len) == Some(name.as_bytes())
}
})
.unwrap();
zip[at + 24..at + 28].copy_from_slice(&size.to_le_bytes());
zip
}
#[test]
fn unreadable_pivot_cache_warns() {
// Control: a readable cache gives a PivotTable and no cache warning.
let (wb, report) = read_xlsx(&with_pivot(SIMPLE_TABLE, Some(GOOD_CACHE))).unwrap();
assert_eq!(wb.sheet(0).unwrap().pivots.len(), 1, "{:?}", report.warnings);
assert!(!report.warnings.iter().any(|w| w.contains("PivotTable's cache")), "{:?}", report.warnings);
// Malformed and missing caches: the workbook opens and the lost PivotTable is reported.
let lost = format!("a PivotTable's cache {CACHE_PART} is missing or could not be read; kept as plain cells");
for cache in [Some("garbage <<<"), None] {
let (wb, report) = read_xlsx(&with_pivot(SIMPLE_TABLE, cache)).unwrap();
assert!(wb.sheet(0).unwrap().pivots.is_empty());
assert!(report.warnings.contains(&lost), "{cache:?}: {:?}", report.warnings);
}
}
#[test]
fn oversized_pivot_cache_warns() {
// The cache declares more than the per-part limit, so it is refused before it inflates.
let declared = u32::try_from(crate::package::MAX_PART + 1).unwrap();
let bytes = declare_size(with_pivot(SIMPLE_TABLE, Some(GOOD_CACHE)), CACHE_PART, declared);
let (wb, report) = read_xlsx(&bytes).unwrap();
assert_eq!(wb.sheet(0).unwrap().name, "Data");
assert!(wb.sheet(0).unwrap().pivots.is_empty());
let lost = format!("a PivotTable's cache is too large to read ({CACHE_PART} expands to {declared} bytes); kept as plain cells");
assert!(report.warnings.contains(&lost), "{:?}", report.warnings);
}
/// A one-sheet-of-data workbook with a pivot on "Report": `columns` are the source columns
/// (header, values), `pt` the pivot built on `Data!A1:<last>`.
fn small_pivot(columns: &[(&str, Vec<Value>)], pt: impl FnOnce(&mut PivotTable)) -> HashMap<String, String> {
+1
View File
@@ -209,6 +209,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here.
| 2026-10-15 | minor | XLSX: formulas and PivotTable sources quote sheet names like Excel (a leading `.`, R1C1-like and cell-like names), so `'.x'!A1` no longer saves as `.x!A1`, which Excel could not open (#1458) |
| 2026-10-15 | minor | XLSX text preserves CR and CRLF across save/reopen; shared strings use `_x000D_` and other XML text uses `&#13;` (#1488) |
| 2026-10-15 | minor | XLSX save keeps legacy `INDEX`/`MATCH` formulas scalar instead of marking them as dynamic arrays (#1505) |
| 2026-10-14 | trivial | XLSX import warns when a pivot cache is missing, malformed or too large instead of dropping its PivotTable silently (#1039) |
| 2026-10-14 | minor | `.xls` import reads cell formatting from the Font, XF, Palette and Style records: fonts, fills, borders, alignment, protection, the custom palette, the Normal font and user cell styles |
| 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-14 | minor | XLSX formulas read with dynamic-array metadata (`cm`) keep it on save, so Excel does not recalculate them with implicit intersection |