diff --git a/crates/formats/src/odt.rs b/crates/formats/src/odt.rs --- a/crates/formats/src/odt.rs +++ b/crates/formats/src/odt.rs @@ -1152,8 +1152,10 @@ enum Ev { Text(String), } -/// Stream XML events (empty elements become start + end). Malformed XML ends the stream. -fn events(xml: &[u8], mut f: impl FnMut(Ev)) { +/// Stream XML events (empty elements become start + end). `false` when the reader stopped on +/// malformed XML before the end of the stream: a truncated or corrupt file, whose partial +/// content the caller must not mistake for a complete read (#1342). +fn events(xml: &[u8], mut f: impl FnMut(Ev)) -> bool { let mut r = quick_xml::Reader::from_reader(xml); let cfg = r.config_mut(); cfg.trim_text(false); @@ -1184,7 +1186,8 @@ fn events(xml: &[u8], mut f: impl FnMut(Ev)) { }; f(Ev::Text(s)); } - Ok(Event::Eof) | Err(_) => break, + Ok(Event::Eof) => return true, + Err(_) => return false, Ok(_) => {} } } @@ -1555,6 +1558,8 @@ struct TableB { hcover: usize, /// Widths in points of the `table:table-column`s so far, 0 where the style gives none. widths: Vec, + /// The open row's `table:number-rows-repeated` (bounded). + row_repeat: usize, } impl TableB { @@ -1565,6 +1570,12 @@ impl TableB { } } +/// An ODF repeat count (`table:number-rows-repeated` / `table:number-columns-repeated`, ODF 1.3 +/// ยง19.678): 1 when absent or unreadable, never zero, at most `max`. +fn repeat_count(a: &Attrs, key: &str, max: usize) -> usize { + get(a, key).and_then(|v| v.parse::().ok()).unwrap_or(1).max(1).min(max as u64) as usize +} + /// A `text:changed-region` read from `text:tracked-changes`. #[derive(Default)] struct Region { @@ -1612,6 +1623,8 @@ struct Body<'a> { lists_saved: Vec<(Vec<(Option, bool)>, Vec)>, tables: Vec, cells: Vec, + /// The open cells' `table:number-columns-repeated`, in step with `cells`. + cell_repeat: Vec, in_text: bool, skip: usize, depth: usize, @@ -1663,6 +1676,7 @@ impl<'a> Body<'a> { item_used: Vec::new(), tables: Vec::new(), cells: Vec::new(), + cell_repeat: Vec::new(), in_text: false, skip: 0, depth: 0, @@ -2102,7 +2116,7 @@ impl Body<'_> { self.skip = 1; return; } - self.tables.push(TableB { rows: Vec::new(), row: Vec::new(), hcover: 0, widths: Vec::new() }); + self.tables.push(TableB { rows: Vec::new(), row: Vec::new(), hcover: 0, widths: Vec::new(), row_repeat: 1 }); } "table-column" => { let w = get(a, "table:style-name").and_then(|n| self.styles.column_width(n)).unwrap_or(0.0); @@ -2116,27 +2130,34 @@ impl Body<'_> { if let Some(t) = self.tables.last_mut() { t.row.clear(); t.hcover = 0; + t.row_repeat = repeat_count(a, "table:number-rows-repeated", wordcraft_doc::table::MAX_ROWS); } } "table-cell" => { self.end_para(); let span = get(a, "table:number-columns-spanned").and_then(|v| v.trim().parse::().ok()).unwrap_or(1).clamp(1, 63); let rows = get(a, "table:number-rows-spanned").and_then(|v| v.trim().parse::().ok()).unwrap_or(1).clamp(1, 1000); + let repeat = repeat_count(a, "table:number-columns-repeated", wordcraft_doc::table::MAX_COLS); let shading = get(a, "table:style-name").and_then(|s| self.styles.cell_background(s)); let valign = get(a, "table:style-name").and_then(|s| self.styles.cell_valign(s)); if let Some(t) = self.tables.last_mut() { - t.hcover = span as usize - 1; + // Every copy of a spanning cell is owed its own covered columns. + t.hcover = (span as usize - 1) * repeat; } self.cells.push(Cell { colspan: span, rowspan: rows, shading, valign, ..Default::default() }); + self.cell_repeat.push(repeat); self.containers.push(Vec::new()); self.lists_saved.push((std::mem::take(&mut self.lists), std::mem::take(&mut self.item_used))); } "covered-table-cell" => { if let Some(t) = self.tables.last_mut() { - if t.hcover > 0 { - t.hcover -= 1; - } else if t.row.len() < 63 { + let mut n = repeat_count(a, "table:number-columns-repeated", wordcraft_doc::table::MAX_COLS); + let owed = t.hcover.min(n); + t.hcover -= owed; + n -= owed; + while n > 0 && t.row.len() < wordcraft_doc::table::MAX_COLS { t.row.push(Cell { covered: true, ..Default::default() }); + n -= 1; } } } @@ -2241,6 +2262,7 @@ impl Body<'_> { } self.tables.truncate(d.tables); self.cells.truncate(d.cells); + self.cell_repeat.truncate(d.cells); self.lists_saved.truncate(d.lists_saved); self.fmt.truncate(d.fmt); self.lists = d.lists; @@ -2391,19 +2413,27 @@ impl Body<'_> { let blocks = if self.containers.len() > 1 { self.containers.pop().unwrap_or_default() } else { Vec::new() }; (self.lists, self.item_used) = self.lists_saved.pop().unwrap_or_default(); let mut cell = self.cells.pop().unwrap_or_default(); + let repeat = self.cell_repeat.pop().unwrap_or(1); cell.blocks = blocks; - if let Some(t) = self.tables.last_mut() - && t.row.len() < 63 - { - t.row.push(cell); + if let Some(t) = self.tables.last_mut() { + for _ in 0..repeat { + if t.row.len() >= wordcraft_doc::table::MAX_COLS { + break; + } + t.row.push(cell.clone()); + } } } "table-row" => { if let Some(t) = self.tables.last_mut() { let r = std::mem::take(&mut t.row); - if !r.is_empty() && t.rows.len() < wordcraft_doc::table::MAX_ROWS { - t.rows.push(r); + if !r.is_empty() { + let room = wordcraft_doc::table::MAX_ROWS.saturating_sub(t.rows.len()); + for _ in 0..t.row_repeat.min(room) { + t.rows.push(r.clone()); + } } + t.row_repeat = 1; } } "table-header-rows" => { @@ -2587,7 +2617,10 @@ fn parse_tracked(bytes: &[u8]) -> Result<(Flow, SectionProps, Option, Opti }; let parts = styles_xml.as_deref().map(|x| page_parts::read(x, &styles, &mut pages, &mut fetch)).unwrap_or_default(); let mut body = Body::new(&styles, &pages, &mut fetch); - events(&content, |ev| body.feed(ev)); + let complete = events(&content, |ev| body.feed(ev)); + if !complete { + return Err("ODT: content.xml is not well-formed (the document is truncated or corrupt)".into()); + } let mut blocks = body.take_blocks(); // An open section with nothing in it (after a column section at the end) isn't one: the // section before it ends the document. @@ -3100,6 +3133,62 @@ mod tests { assert_eq!(t2.rows[0][1].valign, Some(VAlign::Bottom)); } + #[test] + fn table_repeats_expand_rows_and_cells() { + // #1342: `table:number-rows-repeated` and `table:number-columns-repeated` are counted, so + // repeated populated rows and cells survive and repeated blanks keep later cells in place. + let content = format!( + r#" + + REnd + AB + S + "# + ); + let f = parse(&package(&content, None)).unwrap(); + let Some(FBlock::Table(t)) = f.blocks.iter().find(|b| matches!(b, FBlock::Table(_))) else { panic!("no table") }; + let texts = |r: &[model::Cell]| r.iter().map(Cell::text).collect::>(); + // The row repeated twice, its first cell twice. + assert_eq!(t.rows.len(), 4); + assert_eq!(texts(&t.rows[0]), ["R", "R", "End"]); + assert_eq!(texts(&t.rows[1]), ["R", "R", "End"]); + // A covered cell repeated twice keeps B in the fourth column. + let cells: Vec<(String, bool)> = t.rows[2].iter().map(|c| (c.text(), c.covered)).collect(); + assert_eq!(cells, [("A".to_string(), false), (String::new(), true), (String::new(), true), ("B".to_string(), false)]); + // A spanning cell repeated twice, its covered columns owed twice. + let cells: Vec<(String, u32)> = t.rows[3].iter().map(|c| (c.text(), c.colspan)).collect(); + assert_eq!(cells, [("S".to_string(), 2), ("S".to_string(), 2)]); + } + + #[test] + fn table_repeats_stop_at_the_caps() { + // Hostile repeat counts are clamped to the document table's own limits. + let content = format!( + r#" + x + y + "# + ); + let f = parse(&package(&content, None)).unwrap(); + let Some(FBlock::Table(t)) = f.blocks.iter().find(|b| matches!(b, FBlock::Table(_))) else { panic!("no table") }; + assert_eq!(t.rows.first().map(Vec::len), Some(wordcraft_doc::table::MAX_COLS)); + assert_eq!(t.rows.len(), wordcraft_doc::table::MAX_ROWS); + } + + #[test] + fn malformed_content_is_an_error_not_a_partial_document() { + // #1342: a valid prefix followed by corruption fails the import, so a partial read + // cannot be mistaken for the file's content and saved or merged. + let prefix = format!(r#"Good"#); + let corrupt = format!("{prefix}<"); + let doc = import(&package(&ok, None)).unwrap(); + assert_eq!(doc.plain_text(wordcraft_doc::StoryRef::Body), "Good"); + } + /// #1379: three master pages (A4 upright; A4 wide with page numbering restarting at 1; A4 /// upright again with numbering continuing) and a two-column `text:section` in between. fn sections_package() -> Vec { diff --git a/crates/formats/src/sheet.rs b/crates/formats/src/sheet.rs --- a/crates/formats/src/sheet.rs +++ b/crates/formats/src/sheet.rs @@ -239,14 +239,16 @@ impl<'a> Package<'a> { // XML /// A streamed XML event with namespace prefixes dropped; elements deeper than -/// [`MAX_XML_DEPTH`] (and their text) are skipped. +/// [`MAX_XML_DEPTH`] (and their text) are skipped. `false` when the reader stopped on malformed +/// XML before the end of the stream: a truncated or corrupt part, whose partial content the +/// caller must not mistake for a complete read (#1342). enum Ev<'e> { Start(&'e BytesStart<'e>), End(Vec), Text(String), } -fn walk(xml: &[u8], mut f: impl FnMut(Ev<'_>)) { +fn walk(xml: &[u8], mut f: impl FnMut(Ev<'_>)) -> bool { let mut r = quick_xml::Reader::from_reader(xml); let cfg = r.config_mut(); cfg.trim_text(false); @@ -288,7 +290,8 @@ fn walk(xml: &[u8], mut f: impl FnMut(Ev<'_>)) { }; f(Ev::Text(s)); } - Ok(Event::Eof) | Err(_) => break, + Ok(Event::Eof) => return true, + Err(_) => return false, Ok(_) => {} } } @@ -434,7 +437,7 @@ fn workbook(pkg: &mut Package<'_>) -> Result { let rels = pkg.read(&rels_of(&path)).map(|r| relationships(&r, dir_of(&path))).unwrap_or_default(); let mut listed: Vec<(String, String)> = Vec::new(); let mut date1904 = false; - walk(&xml, |ev| { + let complete = walk(&xml, |ev| { if let Ev::Start(e) = ev { match e.local_name().as_ref() { b"sheet" if listed.len() < MAX_SHEETS => listed.push((attr(e, b"name").unwrap_or_default(), attr(e, b"id").unwrap_or_default())), @@ -443,6 +446,9 @@ fn workbook(pkg: &mut Package<'_>) -> Result { } } }); + if !complete { + return Err("the workbook part's XML is not well-formed (the file is truncated or corrupt)".into()); + } let sheets = listed .into_iter() .filter_map(|(name, id)| { @@ -622,7 +628,7 @@ fn xlsx_rows(pkg: &mut Package<'_>, path: &str) -> Result>, Stri let (mut in_data, mut row_no, mut next_col) = (false, 0u64, 0usize); let mut cell: Option = None; let (mut in_v, mut in_is_t, mut phonetic) = (false, false, 0usize); - walk(&xml, |ev| match ev { + let complete = walk(&xml, |ev| match ev { Ev::Start(e) => match e.local_name().as_ref() { b"sheetData" => in_data = true, b"row" if in_data => { @@ -686,6 +692,9 @@ fn xlsx_rows(pkg: &mut Package<'_>, path: &str) -> Result>, Stri _ => {} }, }); + if !complete { + return Err("the sheet's XML is not well-formed (the file is truncated or corrupt)".into()); + } let mut grid = Grid::default(); for (_, r) in rows { grid.push(r, 1); @@ -856,7 +865,7 @@ fn ods_read(xml: &[u8], want: Option) -> Result<(Vec, Vec, u64, u64)> = None; // cells, pending blank cells, repeat let mut cell: Option = None; let (mut in_p, mut skip) = (0usize, 0usize); - walk(xml, |ev| match ev { + let complete = walk(xml, |ev| match ev { Ev::Start(e) => { let n = e.local_name(); let n = n.as_ref(); @@ -981,6 +990,9 @@ fn ods_read(xml: &[u8], want: Option) -> Result<(Vec, VecAdaAda