Compare commits

...
Author SHA1 Message Date
Dylan M. Taylor 1c4d723288 address review: keep the shared-formula text cut-off across worksheets 2026-11-03 23:20:26 +00:00
Dylan M. Taylor 690d1cea4d address review: keep resolving array formulas after the shared-formula text allowance is spent 2026-11-03 21:49:09 +00:00
Dylan M. Taylor 9334e74d06 xls: charge expanded shared-formula text to the import's text allowance
Shared-formula followers each kept their own printed formula text, but
none of it counted against the 256 MB text allowance, so compact PtgExp
records could expand into gigabytes of retained formulas. Each attach
now charges its text; past the allowance the formula is dropped and the
cell keeps its cached value.
2026-11-03 11:13:42 +00:00
2 changed files with 76 additions and 2 deletions
+18
View File
@@ -652,3 +652,21 @@ 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_with(&file, crate::xls::Budget::with_text_left(*n)).is_ok()).unwrap();
let (wb, report) = crate::xls::read_xls_with(&file, crate::xls::Budget::with_text_left(left)).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_with(&file, crate::xls::Budget::default()).unwrap();
assert!(ftext(&wb, "B6").is_some());
}
+58 -2
View File
@@ -75,11 +75,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 }
}
}
@@ -89,6 +91,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 {
@@ -103,6 +110,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 {
@@ -487,6 +511,7 @@ fn rk(v: u32) -> Value {
/// keep their cached values).
fn resolve_pending(
st: &mut SheetState<'_>,
budget: &mut Budget,
arrays: &HashMap<CellRef, (RangeRef, Parsed)>,
shared: &HashMap<CellRef, Parsed>,
pending: &[Pending],
@@ -499,11 +524,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);
@@ -586,7 +627,7 @@ fn worksheet(
// Shared and array formulas: their definitions follow the first cell that uses them.
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));
@@ -878,3 +919,18 @@ fn defined_names(g: &Globals, model_index: &[Option<usize>], 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");
}
}