diff --git a/crates/xlsx/src/tests/xls_tests.rs b/crates/xlsx/src/tests/xls_tests.rs index 762e512e0..d12cc7961 100644 --- a/crates/xlsx/src/tests/xls_tests.rs +++ b/crates/xlsx/src/tests/xls_tests.rs @@ -761,3 +761,72 @@ fn xls_short_window2_is_ignored() { assert_eq!(v(&wb, 0, "A1"), Value::Number(7.0)); } } + +/// Shared-formula followers each keep their own text, charged to the workbook's text allowance: +/// with no allowance left they keep their cached values, and the import still succeeds. +#[test] +fn shared_formula_text_counts_against_the_text_allowance() { + let file = compound("Workbook", &sample(false)); + // The smallest allowance that reads every string: nothing is left for expanded formulas. + let left = (0..1000).find(|n| crate::xls::read_xls_budgeted(&file, crate::xls::Budget::with_text_left(*n), Default::default()).is_ok()).unwrap(); + let (wb, report) = crate::xls::read_xls_budgeted(&file, crate::xls::Budget::with_text_left(left), Default::default()).unwrap(); + assert_eq!(ftext(&wb, "B6"), None); + assert_eq!(v(&wb, 0, "B6"), Value::Number(20.0)); + assert!(report.warnings.iter().any(|w| w.contains("shared-formula text")), "{:?}", report.warnings); + // an array anchor after the cut-off still gets its formula + assert!(ftext(&wb, "C5").is_some()); + // With room, the followers keep their formulas. + let (wb, _) = crate::xls::read_xls_budgeted(&file, crate::xls::Budget::default(), Default::default()).unwrap(); + assert!(ftext(&wb, "B6").is_some()); +} + +/// Shared-formula text is optional: an early sheet's expanded formulas must not use the allowance +/// a later sheet's inline strings need, or the whole import fails for text that fits. +#[test] +fn shared_formulas_do_not_starve_a_later_sheets_text() { + let shared = cat(&[&[0x4C], &u16s(&[0, 0xFFFF]), &ptg_int(10), &[0x05]]); + let boundsheet = |pos: u32, name: &str| rec(0x0085, &cat(&[&pos.to_le_bytes(), &[0, 0, name.len() as u8, 0], name.as_bytes()])); + let globals = |p: [u32; 2]| cat(&[&bof(0x0005), &boundsheet(p[0], "One"), &boundsheet(p[1], "Two"), &eof()]); + let one = cat(&[ + &bof(0x0010), + &formula(4, 1, num(10.0), &exp(4, 1)), + &rec(0x04BC, &cat(&[&u16s(&[4, 5]), &[1, 1, 0, 2], &(shared.len() as u16).to_le_bytes(), &shared])), + &formula(5, 1, num(20.0), &exp(4, 1)), + &eof(), + ]); + let two = cat(&[&bof(0x0010), &rec(0x0204, &cat(&[&cell(0, 0, 0), &xlstr("abcdefgh")])), &eof()]); + let p1 = globals([0; 2]).len() as u32; + let file = compound("Workbook", &cat(&[&globals([p1, p1 + one.len() as u32]), &one, &two])); + // The smallest allowance that reads every string leaves nothing for expanded formulas. + let left = (0..1000).find(|n| crate::xls::read_xls_budgeted(&file, crate::xls::Budget::with_text_left(*n), Default::default()).is_ok()).unwrap(); + let (wb, _) = crate::xls::read_xls_budgeted(&file, crate::xls::Budget::with_text_left(left), Default::default()).unwrap(); + assert_eq!(v(&wb, 1, "A1"), Value::text("abcdefgh")); + assert_eq!(ftext(&wb, "B6"), None); + assert_eq!(v(&wb, 0, "B6"), Value::Number(20.0)); + let (wb, _) = crate::xls::read_xls_budgeted(&file, crate::xls::Budget::default(), Default::default()).unwrap(); + assert!(ftext(&wb, "B6").is_some()); +} + +/// Two readable worksheets with the same name are refused at import, as they were when each sheet +/// was added to the workbook as it was read. +#[test] +fn xls_duplicate_sheet_names_are_refused() { + let boundsheet = |name: &str| rec(0x0085, &cat(&[&0u32.to_le_bytes(), &[0, 0, name.len() as u8, 0], name.as_bytes()])); + let sheet = cat(&[&bof(0x0010), &eof()]); + let globals = |a: &str, b: &str| cat(&[&bof(0x0005), &boundsheet(a), &boundsheet(b), &eof()]); + let file = |a: &str, b: &str| { + let p = globals(a, b).len() as u32; + let mut g = cat(&[ + &bof(0x0005), + &rec(0x0085, &cat(&[&p.to_le_bytes(), &[0, 0, a.len() as u8, 0], a.as_bytes()])), + &rec(0x0085, &cat(&[&(p + sheet.len() as u32).to_le_bytes(), &[0, 0, b.len() as u8, 0], b.as_bytes()])), + &eof(), + ]); + g.extend(&sheet); + g.extend(&sheet); + compound("Workbook", &g) + }; + let err = crate::xls::read_xls_budgeted(&file("One", "one"), crate::xls::Budget::default(), Default::default()).unwrap_err().to_string(); + assert!(err.contains("already taken"), "{err}"); + assert!(crate::xls::read_xls_budgeted(&file("One", "Two"), crate::xls::Budget::default(), Default::default()).is_ok()); +} diff --git a/crates/xlsx/src/xls.rs b/crates/xlsx/src/xls.rs index 1c9285bae..9049c6b12 100644 --- a/crates/xlsx/src/xls.rs +++ b/crates/xlsx/src/xls.rs @@ -78,11 +78,13 @@ pub(crate) struct Budget { text: usize, records: usize, max_records: usize, + /// A shared formula's text was refused: no later shared formula, in any sheet, is expanded. + shared_text_spent: bool, } impl Default for Budget { fn default() -> Self { - Budget { cells: 0, text: 0, records: 0, max_records: MAX_RECORDS } + Budget { cells: 0, text: 0, records: 0, max_records: MAX_RECORDS, shared_text_spent: false } } } @@ -92,6 +94,11 @@ impl Budget { pub(crate) fn with_max_records(max_records: usize) -> Self { Budget { max_records, ..Budget::default() } } + /// A budget with only `left` bytes of text allowance (tests). + #[cfg(test)] + pub(crate) fn with_text_left(left: usize) -> Self { + Budget { text: MAX_TEXT.saturating_sub(left), ..Budget::default() } + } fn record(&mut self) -> Result<(), IoError> { self.records = self.records.saturating_add(1); if self.records > self.max_records { @@ -106,6 +113,23 @@ impl Budget { } Ok(()) } + /// Charges `len` bytes of text if they fit the allowance, leaving it untouched otherwise: + /// for text the import can do without (expanded shared formulas keep their cached values). + fn try_text(&mut self, len: usize) -> bool { + let total = self.text.saturating_add(len); + let fits = total <= MAX_TEXT; + if fits { + self.text = total; + } + fits + } + /// [`try_text`](Self::try_text) for a shared formula's text. The first refusal ends shared + /// expansion for the whole workbook, so a shorter formula on a later sheet doesn't fit in the + /// room the refused one left, and refused formulas aren't decoded again on each sheet. + fn try_shared_text(&mut self, len: usize) -> bool { + self.shared_text_spent = self.shared_text_spent || !self.try_text(len); + !self.shared_text_spent + } fn text(&mut self, len: usize) -> Result<(), IoError> { self.text = self.text.saturating_add(len); if self.text > MAX_TEXT { @@ -182,6 +206,9 @@ pub(crate) fn read_xls_budgeted(bytes: &[u8], mut budget: Budget, functions: Fun // BoundSheet8 index → model sheet index. let mut model_index: Vec> = Vec::with_capacity(g.sheets.len()); + let mut parsed = Vec::new(); + // Names of the readable sheets so far: `wb.sheets` is only filled once every sheet is parsed. + let mut names: Vec<&str> = Vec::new(); for bs in &g.sheets { if bs.kind != 0 { report.warn(match bs.kind { @@ -193,17 +220,26 @@ pub(crate) fn read_xls_budgeted(bytes: &[u8], mut budget: Budget, functions: Fun continue; } wb.check_sheet_name(&bs.name, None).map_err(invalid)?; - let Some((mut sheet, calc)) = worksheet(&stream, bs, &g, &styles, &mut budget, &mut report)? else { + if names.iter().any(|n| n.eq_ignore_ascii_case(&bs.name)) { + return Err(invalid("That name is already taken. Try a different one.")); + } + let Some(st) = worksheet(&stream, bs, &g, &styles, &mut budget, &mut report)? else { report.warn("Dialog sheets and unreadable sheets were skipped."); model_index.push(None); continue; }; - sheet.visibility = bs.visibility; + names.push(&bs.name); + model_index.push(Some(parsed.len())); + parsed.push((bs.visibility, st)); + } + // Every sheet's mandatory text is charged by now; shared formulas take what is left. + for (visibility, st) in parsed { + let (mut sheet, calc) = finish_sheet(st, &mut budget, &mut report); + sheet.visibility = visibility; // Every worksheet repeats the workbook's calculation settings; Excel applies the first. if wb.sheets.is_empty() { wb.calc = calc; } - model_index.push(Some(wb.sheets.len())); wb.sheets.push(Arc::new(sheet)); } if wb.sheets.is_empty() { @@ -508,6 +544,7 @@ fn rk(v: u32) -> Value { /// keep their cached values). fn resolve_pending( st: &mut SheetState<'_>, + budget: &mut Budget, arrays: &HashMap, shared: &HashMap, pending: &[Pending], @@ -520,11 +557,27 @@ fn resolve_pending( st.attach(p.pos, f, Mode::Cell, Some(*range)); } } else if let Some(f) = shared.get(&p.anchor) { + if budget.shared_text_spent { + st.unsupported("more shared-formula text than the import allows"); + continue; + } if failed.contains(&p.anchor) || shared_attaches >= MAX_SHARED_ATTACHES { continue; } shared_attaches += 1; st.attach(p.pos, f, Mode::Shared, None); + let len = st.sheet.cell(p.pos).and_then(|c| c.formula.as_ref()).map(|f| f.text.len()); + // Every follower keeps its own text: charge it to the workbook's text allowance, and + // past it drop the formula again so the cell keeps its cached value. + if let Some(len) = len + && !budget.try_shared_text(len) + { + if let Some(cell) = st.sheet.cells.get_mut(p.pos) { + cell.formula = None; + } + st.unsupported("more shared-formula text than the import allows"); + continue; + } let imported = st.sheet.cell(p.pos).and_then(|c| c.formula.as_ref()).is_some_and(|f| f.text.chars().count() <= MAX_SHARED_TEMPLATE); if !imported { failed.insert(p.anchor); @@ -552,14 +605,14 @@ fn blank_array_followers(st: &mut SheetState<'_>, pending: &[Pending]) { } } -fn worksheet( +fn worksheet<'g>( stream: &[u8], bs: &BoundSheet, - g: &Globals, - styles: &[StyleId], + g: &'g Globals, + styles: &'g [StyleId], budget: &mut Budget, report: &mut ReadReport, -) -> Result, IoError> { +) -> Result>, IoError> { if bs.pos >= stream.len() { return Err(invalid(format!("sheet '{}' points outside the Workbook stream", bs.name))); } @@ -604,10 +657,16 @@ fn worksheet( } sheet_record(&rec, &mut st, budget, report)?; } - // Shared and array formulas: their definitions follow the first cell that uses them. + Ok(Some(st)) +} + +/// Attaches a sheet's shared and array formulas (their definitions follow the first cell that +/// uses them). Run for every sheet only after all sheets' mandatory text is charged, so the +/// optional shared-formula text can't use up the allowance a later sheet's strings need. +fn finish_sheet(mut st: SheetState<'_>, budget: &mut Budget, report: &mut ReadReport) -> (Sheet, CalcSettings) { let (arrays, shared) = (std::mem::take(&mut st.arrays), std::mem::take(&mut st.shared)); let pending = std::mem::take(&mut st.pending); - resolve_pending(&mut st, &arrays, &shared, &pending); + resolve_pending(&mut st, budget, &arrays, &shared, &pending); blank_array_followers(&mut st, &pending); if st.dropped > 0 { report.warn(format!("{} cells outside the grid were skipped.", st.dropped)); @@ -615,7 +674,7 @@ fn worksheet( for why in &st.unsupported { report.warn(format!("Some formulas use {why}; their saved results were kept as values.")); } - Ok(Some((st.sheet, st.calc))) + (st.sheet, st.calc) } /// The shortest WINDOW2 a file holds (the chart-sheet form); a shorter one is truncated. @@ -899,3 +958,18 @@ fn defined_names(g: &Globals, model_index: &[Option], wb: &mut Workbook, } } } + +#[cfg(test)] +mod budget_tests { + use super::*; + + #[test] + fn a_refused_shared_formula_ends_shared_expansion_for_later_sheets() { + let mut budget = Budget::with_text_left(10); + assert!(budget.try_shared_text(8)); + assert!(!budget.try_shared_text(8), "over the allowance"); + // a shorter formula would fit the 2 bytes left, but the cut-off holds + assert!(!budget.try_shared_text(1)); + assert!(budget.try_text(1), "text the import needs is still charged normally"); + } +}