From 0b7deaa3d80438cf4c5bf56988498e6fd8ca3b2b Mon Sep 17 00:00:00 2001 From: RedBearAK <64876997+RedBearAK@users.noreply.github.com> Date: Sat, 24 Oct 2026 17:32:33 -0800 Subject: [PATCH] XLSX: the XML element cap counts only elements kept in the tree `parse_streaming` counts every element it attaches to a parent against MAX_XML_NODES, but the cells of a worksheet are attached to their `row` while the row is open, and the row is then handed to the callback and dropped. So the count grew with every cell of a sheet although the tree never held more than one row, and a workbook with a sheet of about one million ordinary cells (12,274 rows by 103 columns) was reported damaged after 6,310 rows since #2413. Shared strings, streamed by `si`, had the same exposure: a table of about a million plain strings, or fewer rich-text ones. The count now returns to its value at the start of a split element when that element is handed out, so a streamed part of any size passes while a part that keeps a million elements in its tree is still refused. Regression test: 5,001 streamed rows of 100 cells are read whole; the same part kept in a tree is refused. Co-Authored-By: Claude Fable 5.1 --- crates/xlsx/src/xml.rs | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/crates/xlsx/src/xml.rs b/crates/xlsx/src/xml.rs index e671d0adf..26178a570 100644 --- a/crates/xlsx/src/xml.rs +++ b/crates/xlsx/src/xml.rs @@ -230,8 +230,11 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res let mut root: Option = None; let mut buf = Vec::new(); // Elements kept in the tree so far, so a pathological part that passes the byte limits can - // still be refused before the tree itself outgrows memory. + // still be refused before the tree itself outgrows memory. Elements inside a `split` element + // are handed out with it and do not stay in the tree, so a streamed worksheet or shared-string + // table of any size passes: `kept_at_start` remembers the count when each open element began. let mut kept = 0usize; + let mut kept_at_start: Vec = Vec::new(); // Attach a finished element to its parent (or emit it / make it the root). fn finish( stack: &mut [El], @@ -240,11 +243,16 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res split: &[&str], cb: &mut dyn FnMut(El) -> Result<(), IoError>, kept: &mut usize, + kept_at_start: Option, ) -> Result<(), IoError> { match stack.last_mut() { Some(parent) => { if split.contains(&el.name.as_str()) { cb(el)?; + // The element and everything parsed inside it have left the tree. + if let Some(k) = kept_at_start { + *kept = k; + } } else { *kept = kept.saturating_add(1); if *kept > MAX_XML_NODES { @@ -269,14 +277,16 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res return Err(IoError::Xml("XML nested too deeply".into())); } stack.push(start_el(&e)); + kept_at_start.push(kept); } Event::Empty(e) => { let el = start_el(&e); - finish(&mut stack, &mut root, el, split, cb, &mut kept)?; + finish(&mut stack, &mut root, el, split, cb, &mut kept, None)?; } Event::End(_) => { if let Some(el) = stack.pop() { - finish(&mut stack, &mut root, el, split, cb, &mut kept)?; + let at_start = kept_at_start.pop(); + finish(&mut stack, &mut root, el, split, cb, &mut kept, at_start)?; } } Event::Text(t) => { @@ -315,7 +325,8 @@ pub fn parse_streaming(xml: &[u8], split: &[&str], cb: &mut dyn FnMut(El) -> Res } // Unclosed elements (truncated part): close them so we keep what we have. while let Some(el) = stack.pop() { - finish(&mut stack, &mut root, el, split, cb, &mut kept)?; + let at_start = kept_at_start.pop(); + finish(&mut stack, &mut root, el, split, cb, &mut kept, at_start)?; } root.ok_or_else(|| IoError::Xml("no root element".into())) } @@ -495,6 +506,26 @@ mod tests { } } + #[test] + fn a_streamed_part_may_hold_more_elements_than_the_cap() { + // A worksheet of ordinary size: 5001 rows of 100 cells is 1,000,200 `c` and `v` elements, + // more than the cap, but the rows are streamed out one by one and never pile up in the + // tree, so the part is read whole. Kept in a tree (no split), the same part is refused. + let row = format!("{}", "1".repeat(100)); + let xml = format!("{}", row.repeat(5001)); + let mut rows = 0usize; + let root = parse_streaming(xml.as_bytes(), &["row"], &mut |r| { + assert_eq!(r.children.len(), 100); + rows += 1; + Ok(()) + }) + .unwrap(); + assert_eq!(rows, 5001); + assert!(root.children.is_empty()); + let err = parse(xml.as_bytes()).unwrap_err(); + assert!(err.to_string().contains("elements"), "{err}"); + } + #[test] fn a_part_of_endless_empty_elements_is_refused() { assert!(parse(b"").is_ok());