diff --git a/crates/formats/src/sheet.rs b/crates/formats/src/sheet.rs index 520fca2f8..392b425c0 100644 --- a/crates/formats/src/sheet.rs +++ b/crates/formats/src/sheet.rs @@ -18,7 +18,7 @@ //! all), sheets, rows, columns, cells and string lengths are capped, repeat counts are clamped to //! what is left, and nothing recurses, so XML depth costs nothing. -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::io::{Cursor, Read}; use quick_xml::XmlVersion; @@ -138,18 +138,28 @@ pub fn read_rows(bytes: &[u8], pick: Pick<'_>) -> Result<(Vec, Vec Vec { let mut out: Vec = Vec::with_capacity(row.len()); + // The names given so far, ASCII-lowercased, and per base name the next number to try: a set + // lookup per candidate, and each base resumes where it stopped, instead of rescanning every + // name from `2` for every repeat (cubic on a row of equal headers). + let mut taken: HashSet = HashSet::with_capacity(row.len()); + let mut next: HashMap = HashMap::new(); for (i, h) in row.iter().enumerate() { let base = match h.trim() { "" => format!("Column {}", i + 1), t => t.to_string(), }; - let taken = |n: &str, out: &[String]| out.iter().any(|o| o.eq_ignore_ascii_case(n)); let mut name = base.clone(); - let mut k = 2usize; - while taken(&name, &out) { - name = format!("{base} {k}"); - k = k.saturating_add(1); + if taken.contains(&name.to_ascii_lowercase()) { + let k = next.entry(base.to_ascii_lowercase()).or_insert(2); + loop { + name = format!("{base} {k}"); + *k = k.saturating_add(1); + if !taken.contains(&name.to_ascii_lowercase()) { + break; + } + } } + taken.insert(name.to_ascii_lowercase()); out.push(name); } out @@ -350,14 +360,14 @@ impl Grid { } let mut left = times; while left > 0 && !self.full() { - let room = MAX_CELLS.saturating_sub(self.cells); - let mut r = row.clone(); - r.truncate(room); + // What fits is measured on `row` itself; only a copy that will be kept is made. + let keep = row.len().min(MAX_CELLS.saturating_sub(self.cells)); + let r = row.get(..keep).unwrap_or_default(); if !self.text.take(r.iter().map(String::len).sum()) { return; } self.cells = self.cells.saturating_add(r.len()); - self.rows.push(r); + self.rows.push(r.to_vec()); left -= 1; } } @@ -854,7 +864,9 @@ fn ods_read(xml: &[u8], want: Option) -> Result<(Vec, Vec, u64, u64)> = None; // cells, pending blank cells, repeat + // Cells, pending blank cells, repeat, and the cells' text bytes (`None` once the row is over + // the text budget and so refused). + let mut row: Option<(Vec, u64, u64, Option)> = None; let mut cell: Option = None; let (mut in_p, mut skip) = (0usize, 0usize); walk(xml, |ev| match ev { @@ -875,7 +887,7 @@ fn ods_read(xml: &[u8], want: Option) -> Result<(Vec, Vec { let repeat = attr(e, b"number-rows-repeated").and_then(|v| v.trim().parse().ok()).unwrap_or(1u64).max(1); - row = Some((Vec::new(), 0, repeat)); + row = Some((Vec::new(), 0, repeat, Some(0))); } b"table-cell" | b"covered-table-cell" if row.is_some() => { let repeat = attr(e, b"number-columns-repeated").and_then(|v| v.trim().parse().ok()).unwrap_or(1u64).max(1); @@ -959,7 +971,8 @@ fn ods_read(xml: &[u8], want: Option) -> Result<(Vec, Vec) -> Result<(Vec, Vec MAX_TEXT_BYTES.saturating_sub(grid.text.0) { + cells.clear(); + *budget = None; + return; + } cells.resize(cells.len() + blanks, String::new()); *pending = 0; - let times = c.repeat.min(MAX_COLS.saturating_sub(cells.len()) as u64) as usize; cells.extend(std::iter::repeat_n(text, times)); } b"table-row" => { - if let Some((cells, _, repeat)) = row.take() { + if let Some((cells, _, repeat, Some(_))) = row.take() { grid.push(cells, repeat); } } @@ -997,7 +1018,12 @@ fn ods_value(c: &OCell) -> String { ("time", Some(v)) => iso_time(v).unwrap_or_else(|| c.text.clone()), ("boolean", Some(v)) => if v.eq_ignore_ascii_case("true") || v == "1" { "TRUE" } else { "FALSE" }.into(), ("percentage" | "currency", Some(v)) if c.text.trim().is_empty() => v.parse::().map(number).unwrap_or_else(|_| c.text.clone()), - ("string", Some(v)) if c.text.trim().is_empty() => v.to_string(), + ("string", Some(v)) if c.text.trim().is_empty() => { + // Attribute text obeys the same per-cell cap as `` text. + let mut s = String::new(); + push_capped(&mut s, v); + s + } _ => c.text.clone(), } } @@ -1204,4 +1230,50 @@ mod tests { assert_eq!(rows.len(), 1); assert_eq!(rows[0], vec!["TRUE", "World", "Name"]); } + + #[test] + fn repeated_headers_are_numbered_in_linear_time() { + // Every repeat used to restart at `Name 2` and compare each candidate with every name + // so far: cubic in the number of equal headers. + let row = vec!["Name".to_string(); 5_000]; + let t = std::time::Instant::now(); + let names = field_names(&row); + assert!(t.elapsed() < std::time::Duration::from_secs(2), "{:?}", t.elapsed()); + assert_eq!(names.len(), 5_000); + assert_eq!(names[0], "Name"); + assert_eq!(names[4_999], "Name 5000"); + // Names a header already has stay collision-free, ignoring case. + let row: Vec = ["Name", "Name 2", "Name", "name", "", "Column 5", "NAME 2"].iter().map(|s| s.to_string()).collect(); + assert_eq!(field_names(&row), ["Name", "Name 2", "Name 3", "name 4", "Column 5", "Column 5 2", "NAME 2 2"]); + } + + fn ods(rows: &str) -> Vec { + let content = format!( + r#"{rows}"# + ); + zip(&[("mimetype", "application/vnd.oasis.opendocument.spreadsheet"), ("content.xml", &content)]) + } + + #[test] + fn ods_string_attributes_are_capped_before_repeating() { + // An `office:string-value` is a cell's text: it obeys MAX_CELL_CHARS like `` + // does, so repeating it across the columns can't build a huge row first. + let long = "v".repeat(MAX_CELL_CHARS * 4); + let book = ods(&format!( + r#""# + )); + let (_, rows) = read_rows(&book, Pick::Index(0)).unwrap(); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].len(), 3); + assert!(rows[0].iter().all(|c| c.chars().count() == MAX_CELL_CHARS)); + + // A row whose repeated text alone is over the text budget is dropped as it is built + // (it was built in full, then cloned, then refused); the rows after it still count. + let wide = "€".repeat(MAX_CELL_CHARS); + let book = ods(&format!( + r#"next"# + )); + let (_, rows) = read_rows(&book, Pick::Index(0)).unwrap(); + assert_eq!(rows, vec![vec!["next".to_string()]]); + } }