From 89a64d1ef5e39cfa1a05b45b2906eb178a9ec0cf Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 22 Oct 2026 16:04:24 +0100 Subject: [PATCH 01/13] =?UTF-8?q?#3692:=20sheet=20import=20=E2=80=94=20lin?= =?UTF-8?q?ear=20duplicate-header=20naming;=20ODS=20string=20attributes=20?= =?UTF-8?q?capped=20and=20over-budget=20rows=20refused=20before=20copying?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 40: field_names restarted every repeated header at "Name 2" and compared each candidate with every name so far (cubic). It now keeps the names in an ASCII-lowercased set and remembers each base name's next number; existing headers like "Name 2" stay collision-free and the output is unchanged. Finding 47: an ODS office:string-value bypassed MAX_CELL_CHARS and was copied for each repeated column before the text budget was checked. The attribute is now capped like text, a row stops being built (and is refused) once its text can't fit the remaining budget, and Grid::push measures what fits before cloning. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/formats/src/sheet.rs | 104 ++++++++++++++++++++++++++++++------ 1 file changed, 88 insertions(+), 16 deletions(-) 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()]]); + } } From 42c254c87a466f05147e1e54fcabf549d01a3a83 Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 22 Oct 2026 16:10:16 +0100 Subject: [PATCH 02/13] #3705: Para::trim_with drops the leading blank spans with one drain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 74: trimming removed each leading whitespace-only inline with Vec::remove(0), shifting the rest every time. A paragraph opening with many separately formatted line breaks (HTML

…, LaTeX \\) took quadratic time on open or paste. The removable prefix is now counted and drained once; the first remaining span is trimmed as before. 200,000 pairs ran past 60 s before, now milliseconds. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/formats/src/html.rs | 12 ++++++++++++ crates/formats/src/model.rs | 38 +++++++++++++++++++++++++++++-------- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/crates/formats/src/html.rs b/crates/formats/src/html.rs index fa6ec5799..06510d6b3 100644 --- a/crates/formats/src/html.rs +++ b/crates/formats/src/html.rs @@ -2424,4 +2424,16 @@ mod tests { assert!(html.contains("vertical-align:middle")); assert!(html.contains("vertical-align:bottom")); } + + #[test] + fn many_formatted_leading_breaks_trim_quickly() { + // #3705: a paragraph opening with alternating bold and italic `
` was trimmed one + // span at a time with `Vec::remove(0)`. + let html = format!("

{}x

", "

".repeat(100_000)); + let t = std::time::Instant::now(); + let f = parse(&html); + assert!(t.elapsed() < std::time::Duration::from_secs(2), "{:?}", t.elapsed()); + let texts: Vec = f.blocks.iter().filter_map(|b| if let FBlock::Para(p) = b { Some(p.text()) } else { None }).collect(); + assert_eq!(texts, ["x"]); + } } diff --git a/crates/formats/src/model.rs b/crates/formats/src/model.rs index ef050223e..46adc7fce 100644 --- a/crates/formats/src/model.rs +++ b/crates/formats/src/model.rs @@ -239,15 +239,13 @@ impl Para { /// Remove the characters in `start` from the start of the paragraph's text and those in /// `end` from its end. pub fn trim_with(&mut self, start: &[char], end: &[char]) { - while let Some(Inline::Text(t, _)) = self.inlines.first_mut() { - let n = t.trim_start_matches(start).len(); - let cut = t.len() - n; + // The leading spans that trim to nothing go in one `drain`: removing them one at a time + // shifted the rest each time, quadratic in a run of separately formatted line breaks. + let k = self.inlines.iter().take_while(|i| matches!(i, Inline::Text(t, _) if t.trim_start_matches(start).is_empty())).count(); + self.inlines.drain(..k); + if let Some(Inline::Text(t, _)) = self.inlines.first_mut() { + let cut = t.len() - t.trim_start_matches(start).len(); t.replace_range(..cut, ""); - if t.is_empty() { - self.inlines.remove(0); - } else { - break; - } } while let Some(Inline::Text(t, _)) = self.inlines.last_mut() { let n = t.trim_end_matches(end).len(); @@ -1558,4 +1556,28 @@ mod tests { assert_eq!(t.rows[1].len(), 2); assert!(t.rows[1][0].covered); } + + #[test] + fn trim_drops_a_long_run_of_blank_spans_in_linear_time() { + // Separately formatted line breaks don't merge; each used to be removed with + // `Vec::remove(0)`, shifting all the rest (quadratic). + let mut p = Para::default(); + let (b, i) = (Fmt { bold: true, ..Default::default() }, Fmt { italic: true, ..Default::default() }); + for _ in 0..200_000 { + p.push_text("\n", &b); + p.push_text(" ", &i); + } + p.push_text(" x ", &Fmt::default()); + let t = std::time::Instant::now(); + p.trim(); + assert!(t.elapsed() < std::time::Duration::from_secs(1), "{:?}", t.elapsed()); + assert_eq!(p.inlines, vec![Inline::Text("x".into(), Fmt::default())]); + // The first span left keeps its formatting and loses only its leading blanks. + let mut p = Para::default(); + p.push_text(" \t", &b); + p.push_text(" a ", &i); + p.push_text("b ", &b); + p.trim(); + assert_eq!(p.inlines, vec![Inline::Text("a ".into(), i), Inline::Text("b".into(), b)]); + } } From f6398e29a90fd725228e0494ea6827163bc1ee83 Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 22 Oct 2026 16:11:05 +0100 Subject: [PATCH 03/13] #3714: Markdown import tracks the text before each delimiter instead of searching back for it Finding 67: when emphasis was flattened, every delimiter run searched backwards for the nearest text node to borrow its formatting, reaching the start of the paragraph when the paragraph was all delimiters (or links): quadratic. The last text node's formatting is now carried forward through the loop, and nothing is looked up for a delimiter with no literal left. The result is the same. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/formats/src/markdown.rs | 43 +++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/crates/formats/src/markdown.rs b/crates/formats/src/markdown.rs index e483cc928..8274effe0 100644 --- a/crates/formats/src/markdown.rs +++ b/crates/formats/src/markdown.rs @@ -1202,18 +1202,16 @@ fn emit(mut nodes: Vec) -> Vec { } out.push(Inline::Text(t.to_string(), f)); }; - let base_of = |nodes: &[Node], k: usize| -> Fmt { - // Delimiter literals take the formatting (link, …) of neighbouring text. - for j in (0..k).rev() { - if let Some(Node::Text(_, f)) = nodes.get(j) { - return Fmt { bold: false, italic: false, strike: false, ..f.clone() }; - } - } - Fmt::default() - }; - for (k, n) in nodes.iter().enumerate() { + // Delimiter literals take the formatting (link, …) of the text before them: the last text + // node, tracked here rather than searched for backwards from every delimiter (quadratic on + // a paragraph of delimiters). + let mut last_text: Option<&Fmt> = None; + for n in &nodes { match n { - Node::Text(t, f) => push(&mut out, t, f, em, strong, strike), + Node::Text(t, f) => { + push(&mut out, t, f, em, strong, strike); + last_text = Some(f); + } Node::Inl(Inline::Text(t, f)) => push(&mut out, t, f, em, strong, strike), Node::Inl(other) => out.push(other.clone()), Node::Delim { ch, left, opens, closes, .. } => { @@ -1224,9 +1222,11 @@ fn emit(mut nodes: Vec) -> Vec { Emph::Strike => strike = strike.saturating_sub(1), } } - let lit: String = std::iter::repeat_n(*ch, *left).collect(); - let f = base_of(&nodes, k); - push(&mut out, &lit, &f, em, strong, strike); + if *left > 0 { + let lit: String = std::iter::repeat_n(*ch, *left).collect(); + let f = last_text.map_or_else(Fmt::default, |f| Fmt { bold: false, italic: false, strike: false, ..f.clone() }); + push(&mut out, &lit, &f, em, strong, strike); + } for e in opens { match e { Emph::Em => em += 1, @@ -2152,4 +2152,19 @@ mod tests { let texts: Vec = flow.blocks.iter().filter_map(|b| if let FBlock::Para(p) = b { Some(p.text()) } else { None }).collect(); assert_eq!(texts, ["Hello world!", "After comment."]); } + + #[test] + fn a_paragraph_of_delimiters_parses_in_linear_time() { + // #3714: each delimiter's literal looked back for the text before it, to the start of + // the paragraph when there was none (quadratic). + let src = "*_".repeat(200_000); + let t = std::time::Instant::now(); + let out = inl(&src); + assert!(t.elapsed() < std::time::Duration::from_secs(2), "{:?}", t.elapsed()); + let text: String = out.iter().filter_map(|i| if let Inline::Text(t, _) = i { Some(t.as_str()) } else { None }).collect(); + assert!(text.chars().all(|c| matches!(c, '*' | '_')), "only delimiters come out"); + // Literal delimiters still take the link of the text before them. + let s = inl("[a](http://x.org) [b*](http://y.org)"); + assert!(s.iter().any(|i| matches!(i, Inline::Text(t, f) if t == "b*" && f.link.as_deref() == Some("http://y.org"))), "{s:?}"); + } } From e9fe3d29b6056e53b15a92ef4040bb020c0395ab Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 22 Oct 2026 16:12:05 +0100 Subject: [PATCH 04/13] #3692: Markdown export escapes link destinations instead of deleting characters Finding 53: link_target removed `<`, `>` and newlines from a destination after is_safe_href had accepted it, so an accepted relative target such as `java` (or `/`, a backslash that a reader would treat as an escape, and `&` that starts an entity reference, so a CommonMark reader gets back exactly the checked target; control characters, which the check ignores, are still left out. The importer now ends a `<...>` destination at the first unescaped `>`, so these round-trip. Ordinary destinations are written as before. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/formats/src/markdown.rs | 75 +++++++++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 5 deletions(-) diff --git a/crates/formats/src/markdown.rs b/crates/formats/src/markdown.rs index 8274effe0..3617af530 100644 --- a/crates/formats/src/markdown.rs +++ b/crates/formats/src/markdown.rs @@ -799,10 +799,18 @@ fn link_dest(s: &str, at: usize) -> Option<(String, usize)> { i += ws; let r = s.get(i..)?; let (dest, used) = if let Some(inner) = r.strip_prefix('<') { - let end = inner.find(['>', '\n'])?; - if inner.get(end..)?.starts_with('\n') { - return None; - } + // The first `>` not escaped by a backslash ends it; a line break first means no link. + let b = inner.as_bytes(); + let mut k = 0; + let end = loop { + match b.get(k)? { + b'\\' => k += 1, + b'>' => break k, + b'\n' => return None, + _ => {} + } + k += 1; + }; (inner.get(..end)?.to_string(), end + 2) } else { let mut depth = 0i32; @@ -1436,8 +1444,35 @@ fn inlines_md_impl(inlines: &[Inline], in_table: bool) -> String { out } +/// `l` as a Markdown link destination that reads back as exactly `l`, the target +/// `is_safe_href` passed. What Markdown would decode (a backslash escape, an entity reference) +/// or end the destination on (`<`, `>`) is backslash-escaped, never deleted: deleting `<` or `>` +/// could turn an accepted relative target into `javascript:…` or `//host`. Control characters +/// can't be written in a destination and are left out, as the safety check ignores them. fn link_target(l: &str) -> String { - if l.contains([' ', '(', ')', '<', '>']) { format!("<{}>", l.replace(['<', '>', '\n'], "")) } else { l.to_string() } + let l: String = l.chars().filter(|c| !c.is_ascii_control()).collect(); + let mut out = String::with_capacity(l.len() + 2); + for (i, c) in l.char_indices() { + let rest = l.get(i + c.len_utf8()..).unwrap_or(""); + match c { + '<' | '>' => out.push('\\'), + // A backslash is literal unless punctuation (or the closing `)`/`>`) follows it. + '\\' if rest.chars().next().is_none_or(|n| n.is_ascii_punctuation()) => out.push('\\'), + '&' if entity_like(rest) => out.push('\\'), + _ => {} + } + out.push(c); + } + if l.contains([' ', '(', ')', '<', '>']) { format!("<{out}>") } else { out } +} + +/// Whether `rest` (the text after a `&`) starts like an entity reference (`amp;`, `#106;`, +/// `#x6A;`), which a Markdown reader would decode. +fn entity_like(rest: &str) -> bool { + let Some(end) = within(rest, 34).find(';') else { return false }; + let body = rest.get(..end).unwrap_or(""); + let body = body.strip_prefix('#').map_or(body, |n| n.strip_prefix(['x', 'X']).unwrap_or(n)); + !body.is_empty() && body.chars().all(|c| c.is_ascii_alphanumeric()) } fn marker(m: Mark) -> &'static str { @@ -2167,4 +2202,34 @@ mod tests { let s = inl("[a](http://x.org) [b*](http://y.org)"); assert!(s.iter().any(|i| matches!(i, Inline::Text(t, f) if t == "b*" && f.link.as_deref() == Some("http://y.org"))), "{s:?}"); } + + #[test] + fn exported_link_destinations_read_back_unchanged() { + // Removing `<` and `>` from a destination after the safety check could make a relative + // target a script or network-path one (`java)"); + for l in [ + "javab c", + "javascript:alert(1)", + "javascript:x", + "&", + "javascript\\:x", + "dir\\", + "dir\\(1)\\", + "q?a=1&b=2", + ] { + let out = md(l); + let back = inl(&out); + let got: Vec<_> = back.iter().filter_map(|i| if let Inline::Text(_, f) = i { f.link.clone() } else { None }).collect(); + assert_eq!(got, [l.to_string()], "{l:?} → {out:?}"); + } + // Ordinary destinations are written as before. + assert_eq!(md("https://example.com/a?b=1&c=2"), "[x](https://example.com/a?b=1&c=2)"); + assert_eq!(md("https://example.com/a b"), "[x]()"); + assert_eq!(md("https://e.com/x\ny"), "[x](https://e.com/xy)"); + } } From faa8221bca4b6e9b68b702ea9d6e3729a75d47d4 Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 22 Oct 2026 16:13:40 +0100 Subject: [PATCH 05/13] =?UTF-8?q?#3692:=20LaTeX=20=E2=80=94=20verbatim=20d?= =?UTF-8?q?elimiter=20check=20after=20control=20characters=20go;=20multico?= =?UTF-8?q?lumn/multirow=20unwrapping=20bounded?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 57: Writer::code replaced `\end{verbatim}` in a code line and then removed control characters, so `\end{verbatim}` became a real closing delimiter and the rest of the line was compiled as TeX. Control characters are now removed first and the replacement runs on the final text. Finding 52: the table reader unwrapped every nested \multicolumn / \multirow in a cell, rescanning the remaining braced content each time (quadratic; the nesting limit didn't apply). At most MAX_NEST wrappers are unwrapped now; real tables use one or two, which read as before. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/formats/src/latex.rs | 46 +++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/crates/formats/src/latex.rs b/crates/formats/src/latex.rs index 10de89212..663784624 100644 --- a/crates/formats/src/latex.rs +++ b/crates/formats/src/latex.rs @@ -243,8 +243,10 @@ impl Writer { } self.out.push_str("\\begin{verbatim}\n"); for l in lines { - // The environment ends at the first `\end{verbatim}`, so keep it from appearing. - let l: String = l.replace("\\end{verbatim}", "\\end {verbatim}").chars().filter(|c| *c == '\t' || !c.is_control()).collect(); + // The environment ends at the first `\end{verbatim}`, so keep it from appearing. The + // control characters go first: removing one afterwards could join a delimiter back up. + let l: String = l.chars().filter(|c| *c == '\t' || !c.is_control()).collect(); + let l = l.replace("\\end{verbatim}", "\\end {verbatim}"); self.out.push_str(&l); self.out.push('\n'); } @@ -2671,19 +2673,24 @@ fn table(toks: &[Tok], spec: &str, depth: usize) -> FTable { for cell_toks in r { let mut cell = Cell::default(); let mut content = trim_cell(cell_toks); - // `\multicolumn{n}{spec}{content}`, `\multirow{n}{width}{content}`. + // `\multicolumn{n}{spec}{content}`, `\multirow{n}{width}{content}`. Each wrapper + // rescans its content, so only MAX_NEST of them are unwrapped (a cell nests one or + // two); further ones stay in the content. let mut sub = Parser::new(content, depth + 1); + let mut wrappers = 0usize; loop { sub.skip_spaces(); match sub.t.get(sub.pos) { - Some(Tok::Cmd(c)) if c == "multicolumn" => { + Some(Tok::Cmd(c)) if c == "multicolumn" && wrappers < MAX_NEST => { + wrappers += 1; sub.pos += 1; cell.colspan = sub.arg_raw().trim().parse::().unwrap_or(1).clamp(1, 63); let _ = sub.arg_raw(); content = sub.arg_tokens(); sub = Parser::new(content, depth + 1); } - Some(Tok::Cmd(c)) if c == "multirow" || c == "multirow*" => { + Some(Tok::Cmd(c)) if (c == "multirow" || c == "multirow*") && wrappers < MAX_NEST => { + wrappers += 1; sub.pos += 1; let _ = sub.opt_arg(); cell.rowspan = sub.arg_raw().trim().parse::().map(|n| n.unsigned_abs().clamp(1, 1000) as u32).unwrap_or(1); @@ -3234,4 +3241,33 @@ Caf\'e na\"ive \c{c}a \v{s}\'{e} and {\bfseries grouped bold} after. let Some(Block::Table(bt_bordered)) = back_bordered.body.first().map(|b| &**b) else { panic!() }; assert!(bt_bordered.props.style.is_some(), "imported table should remain bordered"); } + + #[test] + fn verbatim_end_is_kept_out_after_control_characters_go() { + // A control character inside `\end{verbatim}` was removed after the check for it, so + // the code line closed the environment and the rest ran as TeX. + let mut d = Document::new(); + let line = "\\end{verb\u{E}atim}\\input{/etc/passwd}"; + d.body = vec![wordcraft_doc::para_block(wordcraft_doc::Paragraph::with_text(line, Default::default()).styled(model::CODE_STYLE))]; + let out = export(&d); + assert_eq!(out.matches("\\end{verbatim}").count(), 1, "{out}"); + assert!(out.contains("\\end {verbatim}\\input"), "{out}"); + } + + #[test] + fn nested_multicolumn_wrappers_are_bounded() { + // Each `\multicolumn` unwrapped rescanned the rest of the cell; nested ones were all + // unwrapped, quadratic in the nesting. + let n = 50_000; + let src = format!("\\begin{{tabular}}{{l}}{}x{}\\\\\\end{{tabular}}", "\\multicolumn{1}{c}{".repeat(n), "}".repeat(n)); + let t = std::time::Instant::now(); + let f = parse(&src); + assert!(t.elapsed() < std::time::Duration::from_secs(2), "{:?}", t.elapsed()); + assert!(f.blocks.iter().any(|b| matches!(b, FBlock::Table(_)))); + // One or two wrappers, as written in real tables, are still read. + let f = parse("\\begin{tabular}{ll}\\multicolumn{2}{c}{\\multirow{2}{*}{Both}}\\\\\\end{tabular}"); + let Some(FBlock::Table(t)) = f.blocks.first() else { panic!("{:?}", f.blocks) }; + assert_eq!((t.rows[0][0].colspan, t.rows[0][0].rowspan), (2, 2)); + assert_eq!(t.rows[0][0].text(), "Both"); + } } From 79e3610cbbe4dd1581f5a0e23a77747ccde4e20b Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 22 Oct 2026 16:24:47 +0100 Subject: [PATCH 06/13] #3713: importers drop overlong font names and links; spans share them; runs' copies budgeted Findings 58 and 65. Every span deep-copied its inherited link and font strings, so one long name or Markdown definition times many spans or references expanded a small file into gigabytes. Fmt now holds both as Arc (spans and Markdown references share one copy), names over 256 bytes and links over 8 KiB are dropped at the importers (RTF, HTML, Markdown, ODT, LaTeX), and the document builder stops giving runs link and font copies past 64 MiB. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/formats/src/html.rs | 6 +-- crates/formats/src/latex.rs | 15 ++++--- crates/formats/src/markdown.rs | 21 +++++---- crates/formats/src/model.rs | 56 +++++++++++++++++++----- crates/formats/src/odt.rs | 9 ++-- crates/formats/src/rtf.rs | 22 ++++++---- crates/formats/src/tests.rs | 78 ++++++++++++++++++++++++++++++++-- 7 files changed, 162 insertions(+), 45 deletions(-) diff --git a/crates/formats/src/html.rs b/crates/formats/src/html.rs index 06510d6b3..4970855ed 100644 --- a/crates/formats/src/html.rs +++ b/crates/formats/src/html.rs @@ -724,7 +724,7 @@ fn apply_style(css: &str, f: &mut Fmt, align: &mut Option, preserve: &mut if model::is_mono(&fam) || fam.eq_ignore_ascii_case("monospace") { f.code = true; } else if !fam.is_empty() && !matches!(fam.to_ascii_lowercase().as_str(), "serif" | "sans-serif" | "inherit" | "initial") { - f.font = Some(fam); + f.font = model::kept_font(&fam).or(f.font.take()); } } "vertical-align" => { @@ -1226,7 +1226,7 @@ impl<'r> Builder<'r> { if model::is_mono(&fam) { fmt.code = true; } else if !fam.is_empty() { - fmt.font = Some(fam); + fmt.font = model::kept_font(&fam).or(fmt.font.take()); } } if let Some(sz) = attr(attrs, "size") { @@ -1245,7 +1245,7 @@ impl<'r> Builder<'r> { } "a" => { if let Some(h) = attr(attrs, "href").filter(|h| wordcraft_doc::link::is_safe_href(h)) { - fmt.link = Some(h.trim().to_string()); + fmt.link = model::kept_link(h.trim()); } } _ => {} diff --git a/crates/formats/src/latex.rs b/crates/formats/src/latex.rs index 663784624..18d807fdd 100644 --- a/crates/formats/src/latex.rs +++ b/crates/formats/src/latex.rs @@ -2335,20 +2335,21 @@ impl<'a> Parser<'a> { } } "href" => { - let url = self.arg_raw(); - self.with_arg(move |f| f.link = Some(url)); + let url = model::kept_link(&self.arg_raw()); + self.with_arg(move |f| f.link = url); } "url" | "nolinkurl" => { let url = self.arg_raw(); let mut f = self.fmt(); if name == "url" { - f.link = Some(url.clone()); + f.link = model::kept_link(&url); } self.para.push_text(&url, &f); } "hyperlink" => { let target = self.arg_raw(); - self.with_arg(move |f| f.link = Some(format!("#{target}"))); + let link = model::kept_link(&format!("#{target}")); + self.with_arg(move |f| f.link = link); } "hypertarget" => { let target = self.arg_raw(); @@ -2362,7 +2363,7 @@ impl<'a> Parser<'a> { "ref" | "eqref" | "pageref" | "autoref" | "cref" | "Cref" | "nameref" => { let l = self.arg_raw(); let mut f = self.fmt(); - f.link = Some(format!("#{l}")); + f.link = model::kept_link(&format!("#{l}")); self.para.push_text(&l, &f); } "cite" | "citep" | "citet" | "parencite" | "textcite" | "autocite" => { @@ -3168,13 +3169,13 @@ Caf\'e na\"ive \c{c}a \v{s}\'{e} and {\bfseries grouped bold} after. let FBlock::Para(p) = &flow.blocks[0] else { panic!() }; assert_eq!( p.inlines[0], - Inline::Text("Homepage".to_string(), Fmt { link: Some("https://example.com/~user/page".to_string()), ..Default::default() }) + Inline::Text("Homepage".to_string(), Fmt { link: Some("https://example.com/~user/page".into()), ..Default::default() }) ); assert_eq!( p.inlines[2], Inline::Text( "https://example.com/~user/page".to_string(), - Fmt { link: Some("https://example.com/~user/page".to_string()), ..Default::default() } + Fmt { link: Some("https://example.com/~user/page".into()), ..Default::default() } ) ); } diff --git a/crates/formats/src/markdown.rs b/crates/formats/src/markdown.rs index 3617af530..e5346cca7 100644 --- a/crates/formats/src/markdown.rs +++ b/crates/formats/src/markdown.rs @@ -10,6 +10,7 @@ //! Export writes the same constructs; pictures become `![alt](media-name)` placeholders. use std::collections::HashMap; +use std::sync::Arc; use wordcraft_doc::{Align, Document}; @@ -33,7 +34,8 @@ pub fn import_on(text: &str, page: &wordcraft_doc::SectionProps) -> Document { } /// Link reference definitions: the normalised label to its destination. -pub type Refs = HashMap; +/// A destination is shared by every reference to it, not copied into each (#3713). +pub type Refs = HashMap>; /// A label as CommonMark compares them: case-folded, and any run of white space one space /// (§4.7, "matches"). `[FOO bar]` and `[foo BAR]` are the same label. @@ -141,7 +143,7 @@ fn take_definitions(lines: &mut Vec) -> Refs { if starts_block && let Some((label, dest)) = link_definition(line) { // The first definition of a label wins, as CommonMark says, and definitions may // follow one another. - refs.entry(label).or_insert(dest); + refs.entry(label).or_insert(dest.into()); return false; } starts_block = is_blank(line) || atx_heading(t).is_some() || thematic_break(t); @@ -991,7 +993,7 @@ fn tokenize(s: &str, base: &Fmt, depth: usize, refs: &Refs) -> Vec { let dest = link_dest(s, close + 1); scan_budget = scan_budget.saturating_sub(dest.as_ref().map_or(s.len(), |d| d.1).saturating_sub(close)); let (dest, end) = dest?; - return Some((text.to_string(), dest, end)); + return Some((text.to_string(), Arc::from(dest), end)); } // A reference link: `[text][label]`, `[text][]` and `[text]` all take // their destination from a definition collected before parsing (#3238). @@ -1028,7 +1030,8 @@ fn tokenize(s: &str, base: &Fmt, depth: usize, refs: &Refs) -> Vec { Some((inner, dest, end)) if base.link.is_none() => { flush!(); let mut f = base.clone(); - f.link = Some(dest); + // A destination too long to keep leaves its text unlinked. + f.link = (dest.len() <= model::MAX_LINK).then_some(dest); for x in parse_inlines(&inner, &f, depth + 1, refs) { nodes.push(Node::Inl(x)); } @@ -1055,7 +1058,7 @@ fn tokenize(s: &str, base: &Fmt, depth: usize, refs: &Refs) -> Vec { if is_auto { flush!(); let mut f = base.clone(); - f.link = Some(if inner.contains('@') && !inner.contains(':') { format!("mailto:{inner}") } else { inner.to_string() }); + f.link = Some(if inner.contains('@') && !inner.contains(':') { format!("mailto:{inner}").into() } else { inner.into() }); push_text(&mut nodes, inner, &f); i += inner.len() + 2; text_start = i; @@ -1386,7 +1389,7 @@ fn inlines_md_impl(inlines: &[Inline], in_table: bool) -> String { } let mut out = String::new(); let mut stack: Vec = Vec::new(); - let mut link: Option = None; + let mut link: Option> = None; let close_all = |out: &mut String, stack: &mut Vec| { while let Some(m) = stack.pop() { out.push_str(marker(m)); @@ -1675,7 +1678,7 @@ mod tests { let s = inl("see [the *site*](http://x.org \"T\") and `a*b*` \\*lit\\* & "); let link: Vec<_> = s .iter() - .filter_map(|i| if let Inline::Text(t, f) = i { f.link.as_ref().map(|l| (t.clone(), l.clone(), f.italic)) } else { None }) + .filter_map(|i| if let Inline::Text(t, f) = i { f.link.as_ref().map(|l| (t.clone(), l.to_string(), f.italic)) } else { None }) .collect(); assert_eq!(link[0], ("the ".into(), "http://x.org".into(), false)); assert_eq!(link[1], ("site".into(), "http://x.org".into(), true)); @@ -1696,7 +1699,7 @@ mod tests { .inlines .iter() .filter_map(|x| match x { - Inline::Text(t, f) => f.link.clone().map(|l| (t.clone(), l)), + Inline::Text(t, f) => f.link.as_deref().map(|l| (t.clone(), l.to_string())), _ => None, }) .collect(), @@ -2224,7 +2227,7 @@ mod tests { ] { let out = md(l); let back = inl(&out); - let got: Vec<_> = back.iter().filter_map(|i| if let Inline::Text(_, f) = i { f.link.clone() } else { None }).collect(); + let got: Vec<_> = back.iter().filter_map(|i| if let Inline::Text(_, f) = i { f.link.as_deref().map(str::to_string) } else { None }).collect(); assert_eq!(got, [l.to_string()], "{l:?} → {out:?}"); } // Ordinary destinations are written as before. diff --git a/crates/formats/src/model.rs b/crates/formats/src/model.rs index 46adc7fce..156269b1b 100644 --- a/crates/formats/src/model.rs +++ b/crates/formats/src/model.rs @@ -19,6 +19,26 @@ pub const MONO_FONT: &str = "Courier New"; pub const CODE_STYLE: &str = "Code"; /// Nesting limit for tables in tables (hostile input). pub const MAX_DEPTH: usize = 8; +/// Longest font name the importers keep, in bytes (as the .docx reader); a longer one (no real +/// font has one) is dropped (#3713). +pub const MAX_FONT_NAME: usize = 256; +/// Longest hyperlink target the importers keep, in bytes; a longer link is dropped and its text +/// kept (#3713). +pub const MAX_LINK: usize = 8 << 10; +/// Link and font bytes the document's runs may carry between them. A flow's spans share one +/// copy of a name ([`Fmt`]), but every run of a document owns its own: past this budget, runs +/// lose their link and font rather than multiply a small file into gigabytes (#3713). +const MAX_RUN_STRINGS: usize = 64 << 20; + +/// `name` as a span's font, unless it is longer than [`MAX_FONT_NAME`]. +pub fn kept_font(name: &str) -> Option> { + (name.len() <= MAX_FONT_NAME).then(|| name.into()) +} + +/// `target` as a span's link, unless it is longer than [`MAX_LINK`]. +pub fn kept_link(target: &str) -> Option> { + (target.len() <= MAX_LINK).then(|| target.into()) +} /// Character formatting the text formats can express. #[derive(Clone, Debug, Default, PartialEq)] @@ -30,11 +50,13 @@ pub struct Fmt { pub code: bool, pub sup: bool, pub sub: bool, - pub link: Option, + /// Shared: every span of a link holds the same target. + pub link: Option>, pub color: Option, /// Points. pub size: Option, - pub font: Option, + /// Shared, as `link`. + pub font: Option>, /// Background (highlight / shading). pub background: Option, /// Tracked insertion of this text (formats that keep revisions: RTF). @@ -429,10 +451,10 @@ pub fn char_props(f: &Fmt) -> CharProps { } else { None }, - link: f.link.clone().filter(|l| !l.is_empty()), + link: f.link.as_deref().filter(|l| !l.is_empty()).map(str::to_string), color: f.color.map(TextColor::Rgb), size: f.size.filter(|s| s.is_finite() && *s >= 1.0 && *s <= 1638.0), - font: if f.code { Some(MONO_FONT.to_string()) } else { f.font.clone().filter(|s| !s.is_empty()) }, + font: if f.code { Some(MONO_FONT.to_string()) } else { f.font.as_deref().filter(|s| !s.is_empty()).map(str::to_string) }, shading: f.background, ..Default::default() } @@ -516,6 +538,8 @@ struct Builder<'a> { doc: &'a mut Document, /// Revisions already in `doc` by (kind, author, date), so each change is added once. revs: std::collections::HashMap<(u8, RevMark), u32>, + /// Link and font bytes given to runs so far ([`MAX_RUN_STRINGS`]). + run_strings: usize, } fn rev_key(kind: RevisionKind) -> u8 { @@ -535,7 +559,7 @@ impl<'a> Builder<'a> { revs.entry((rev_key(r.kind), mark)).or_insert(i as u32); } } - Builder { doc, revs } + Builder { doc, revs, run_strings: 0 } } /// The document revision for a tracked change, added the first time it is seen. @@ -550,6 +574,17 @@ impl<'a> Builder<'a> { i } + /// `cp` as a run may keep it: without its link and font once the runs' copies of them are + /// over budget. + fn budget(&mut self, mut cp: CharProps) -> CharProps { + let n = cp.link.as_ref().map_or(0, String::len).saturating_add(cp.font.as_ref().map_or(0, String::len)); + match self.run_strings.checked_add(n).filter(|&t| t <= MAX_RUN_STRINGS) { + Some(t) => self.run_strings = t, + None => (cp.link, cp.font) = (None, None), + } + cp + } + fn para(&mut self, p: &Para, list_num: Option) -> Paragraph { let mut out = Paragraph::new(); out.props = para_props(p, list_num); @@ -575,11 +610,12 @@ impl<'a> Builder<'a> { match i { Inline::Text(t, f) => { let t = clean_text(t); - let mut cp = char_props(f); + let mut cp = self.budget(char_props(f)); cp.ins = f.ins.as_ref().map(|m| self.revision(RevisionKind::Insert, m)); cp.del = f.del.as_ref().map(|m| self.revision(RevisionKind::Delete, m)); if let Some(c) = &f.fmt_change { - cp.fmt_change = PropChange::boxed(self.revision(RevisionKind::Format, &c.mark), char_props(&c.old)); + let old = self.budget(char_props(&c.old)); + cp.fmt_change = PropChange::boxed(self.revision(RevisionKind::Format, &c.mark), old); } if p.kind == Kind::Code { // Code's font comes from its style, before a change as after it. @@ -614,7 +650,7 @@ impl<'a> Builder<'a> { } Inline::Ruby { base, ruby, fmt } => { let (base, ruby) = (clean_text(base), clean_text(ruby)); - let cp = char_props(fmt); + let cp = self.budget(char_props(fmt)); // A source without a base or without a reading (malformed markup) is just its // text: a guide that annotates nothing is no guide. if base.is_empty() || ruby.is_empty() { @@ -881,13 +917,13 @@ fn fmt_of(doc: &Document, cp: &CharProps, in_code: bool) -> Fmt { code, sup: e.vert_align == Some(VertAlign::Superscript), sub: e.vert_align == Some(VertAlign::Subscript), - link: e.link.clone(), + link: e.link.as_deref().map(Arc::from), color: match e.color { Some(TextColor::Rgb(c)) => Some(c), _ => None, }, size: e.size, - font: if code || in_code { None } else { e.shown_font().map(str::to_string) }, + font: if code || in_code { None } else { e.shown_font().map(Arc::from) }, background: e.highlight.and_then(|h| h.rgb()).or(e.shading), ins: None, del: None, diff --git a/crates/formats/src/odt.rs b/crates/formats/src/odt.rs index c73720819..6be960811 100644 --- a/crates/formats/src/odt.rs +++ b/crates/formats/src/odt.rs @@ -19,6 +19,7 @@ use std::collections::HashMap; use std::io::{Cursor, Read, Write}; +use std::sync::Arc; use quick_xml::XmlVersion; use quick_xml::events::{BytesStart, Event}; @@ -1034,7 +1035,7 @@ struct FmtDelta { color: Option, background: Option, size: Option, - font: Option, + font: Option>, code: Option, } @@ -1340,7 +1341,7 @@ impl Styles { if model::is_mono(&fam) || get(&a, "style:font-pitch").is_some_and(|p| p.trim().eq_ignore_ascii_case("fixed")) { d.code = Some(true); } else if !fam.is_empty() { - d.font = Some(fam); + d.font = model::kept_font(&fam); } } } @@ -1446,7 +1447,7 @@ impl Styles { /// The document's default font: the Standard paragraph style's, else the default style's. fn doc_font(&self) -> Option { - self.map.get("Standard").filter(|s| !s.auto).and_then(|s| s.fmt.font.clone()).or_else(|| self.default_font.clone()) + self.map.get("Standard").filter(|s| !s.auto).and_then(|s| s.fmt.font.as_deref().map(str::to_string)).or_else(|| self.default_font.clone()) } fn chain<'a>(&'a self, name: &'a str) -> Vec<(&'a str, &'a StyleInfo)> { @@ -2053,7 +2054,7 @@ impl Body<'_> { "a" => { let mut f = self.cur_fmt(); if let Some(h) = get(a, "xlink:href").filter(|h| !h.is_empty()) { - f.link = Some(h.to_string()); + f.link = model::kept_link(h); } if let Some(s) = get(a, "text:style-name") { f = self.styles.text_fmt(s, &f); diff --git a/crates/formats/src/rtf.rs b/crates/formats/src/rtf.rs index 280b2feae..7b06aa082 100644 --- a/crates/formats/src/rtf.rs +++ b/crates/formats/src/rtf.rs @@ -19,6 +19,7 @@ //! tables. Group nesting and picture sizes are capped. use std::collections::HashMap; +use std::sync::Arc; use wordcraft_doc::props::VAlign; use wordcraft_doc::{Align, Document, Rgb}; @@ -306,11 +307,13 @@ impl Writer { "\\nosupersub" }); } - let font = |f: &Fmt| if f.code { Some(model::MONO_FONT.to_string()) } else { f.font.clone() }; + fn font(f: &Fmt) -> Option<&str> { + if f.code { Some(model::MONO_FONT) } else { f.font.as_deref() } + } let old_font = font(old); if old_font != font(cur) { // No font is the document's default font, font 0 (Normal's). - let i = old_font.map_or(0, |n| self.t.font(&n)); + let i = old_font.map_or(0, |n| self.t.font(n)); ctl.push_str(&format!("\\f{i}")); } let half_points = |s: Option| s.map(|s| (s * 2.0).round().clamp(2.0, 3276.0) as i32); @@ -1106,7 +1109,7 @@ struct Reader { st: St, stack: Vec, overflow: usize, - fonts: HashMap, + fonts: HashMap>, font_cur: i32, font_name: String, /// Code pages of the fonts whose charset names one (`\fcharset204` → 1251). @@ -1398,7 +1401,9 @@ impl Reader { fn commit_font(&mut self) { let name = std::mem::take(&mut self.font_name).trim().trim_end_matches(';').trim().to_string(); - if !name.is_empty() && self.fonts.len() < 4096 { + if let Some(name) = model::kept_font(&name).filter(|n| !n.is_empty()) + && self.fonts.len() < 4096 + { self.fonts.insert(self.font_cur, name); if let Some(cp) = self.font_cp_cur.take() { self.font_cps.insert(self.font_cur, cp); @@ -1626,7 +1631,8 @@ impl Reader { for c in s.chars() { if c == ';' { self.commit_font(); - } else { + } else if self.font_name.len() <= model::MAX_FONT_NAME { + // One past the limit is enough for `commit_font` to drop the name. self.font_name.push(c); } } @@ -1646,7 +1652,7 @@ impl Reader { for c in s.chars() { if c == ';' { self.commit_style(); - } else { + } else if self.style_name.len() < model::MAX_FONT_NAME { self.style_name.push(c); } } @@ -2329,7 +2335,7 @@ impl Reader { "fldrslt" => { let link = self.st.field.and_then(|i| self.fields.get(i)).and_then(|s| hyperlink(s)); if link.is_some() { - self.st.fmt.link = link; + self.st.fmt.link = link.as_deref().and_then(model::kept_link); } } _ => {} @@ -2807,7 +2813,7 @@ fn parse_full(bytes: &[u8]) -> Result { if blocks.is_empty() { blocks.push(FBlock::Para(Para::default())); } - let default_font = r.fonts.get(&r.deff).filter(|n| !model::is_mono(n)).cloned(); + let default_font = r.fonts.get(&r.deff).filter(|n| !model::is_mono(n)).map(|n| n.to_string()); Ok(Parsed { flow: Flow { blocks, meta: r.meta }, page: r.page, last_section, default_font, headers_footers }) } diff --git a/crates/formats/src/tests.rs b/crates/formats/src/tests.rs index ca6b6f992..5ef26d5f1 100644 --- a/crates/formats/src/tests.rs +++ b/crates/formats/src/tests.rs @@ -749,7 +749,7 @@ fn code_spans_and_links_around_backticks_read_as_before() { let flow = crate::markdown::parse("\\``e` `` a ` b `` then ``` and `c` [x `]` y](http://a.org) [p ` q](http://b.org) ``f"); let FBlock::Para(p) = &flow.blocks[0] else { panic!("{:?}", flow.blocks) }; let spans: Vec<(String, bool, Option)> = - p.inlines.iter().filter_map(|i| if let model::Inline::Text(t, f) = i { Some((t.clone(), f.code, f.link.clone())) } else { None }).collect(); + p.inlines.iter().filter_map(|i| if let model::Inline::Text(t, f) = i { Some((t.clone(), f.code, f.link.as_deref().map(str::to_string))) } else { None }).collect(); let code: Vec<&str> = spans.iter().filter(|s| s.1).map(|s| s.0.as_str()).collect(); assert_eq!(code, ["e", "a ` b", "c", "]"], "{spans:?}"); let linked = |url: &str| spans.iter().filter(|s| s.2.as_deref() == Some(url)).map(|s| s.0.as_str()).collect::(); @@ -766,7 +766,7 @@ fn latex_href_keeps_ampersand() { .iter() .filter_map(|b| if let FBlock::Para(p) = b { Some(p) } else { None }) .flat_map(|p| p.inlines.iter()) - .filter_map(|i| if let model::Inline::Text(_, f) = i { Some(f.link.clone()) } else { None }) + .filter_map(|i| if let model::Inline::Text(_, f) = i { Some(f.link.as_deref().map(str::to_string)) } else { None }) .collect(); assert_eq!(links, [Some("https://e.com/a?b=1&c=2".to_string())]); @@ -775,7 +775,7 @@ fn latex_href_keeps_ampersand() { .iter() .filter_map(|b| if let FBlock::Para(p) = b { Some(p) } else { None }) .flat_map(|p| p.inlines.iter()) - .filter_map(|i| if let model::Inline::Text(t, f) = i { Some((t.to_string(), f.link.clone())) } else { None }) + .filter_map(|i| if let model::Inline::Text(t, f) = i { Some((t.to_string(), f.link.as_deref().map(str::to_string))) } else { None }) .collect(); let url = "https://e.com/a?b=1&c=2".to_string(); assert_eq!(texts, [(url.clone(), Some(url))]); @@ -789,7 +789,7 @@ fn brackets_do_not_turn_later_links_into_text() { let links = |md: &str| { let flow = crate::markdown::parse(md); let FBlock::Para(p) = &flow.blocks[0] else { panic!("{:?}", flow.blocks) }; - let mut urls: Vec = p.inlines.iter().filter_map(|i| if let model::Inline::Text(_, f) = i { f.link.clone() } else { None }).collect(); + let mut urls: Vec = p.inlines.iter().filter_map(|i| if let model::Inline::Text(_, f) = i { f.link.as_deref().map(str::to_string) } else { None }).collect(); urls.dedup(); urls }; @@ -875,3 +875,73 @@ fn hidden_drawings_are_left_out_of_exports() { // Two pictures, one hidden: one comes out. assert_eq!(crate::html::export(&d).matches(" Vec<(String, model::Fmt)> { + flow.blocks + .iter() + .filter_map(|b| if let FBlock::Para(p) = b { Some(p) } else { None }) + .flat_map(|p| p.inlines.iter()) + .filter_map(|i| if let model::Inline::Text(t, f) = i { Some((t.clone(), f.clone())) } else { None }) + .collect() +} + +/// A font name or link target too long for any real one is dropped, its text kept: every run +/// carried its own copy, so one long name times many runs was gigabytes (#3713). +#[test] +fn overlong_font_names_and_links_are_dropped() { + let long = "x".repeat(1 << 20); + let rtf = format!("{{\\rtf1{{\\fonttbl{{\\f0 Arial;}}{{\\f1 {long};}}}}\\f1 {}}}", "{\\b a}a".repeat(1000)); + let flow = crate::rtf::parse(rtf.as_bytes()).unwrap(); + let sp = spans(&flow); + assert!(sp.iter().map(|s| s.0.len()).sum::() >= 2000); + assert!(sp.iter().all(|(_, f)| f.font.is_none()), "an overlong RTF font name is no font"); + // A real one still is. + let flow = crate::rtf::parse(b"{\\rtf1{\\fonttbl{\\f0 Arial;}{\\f1 Georgia;}}\\f1 a}").unwrap(); + assert_eq!(spans(&flow)[0].1.font.as_deref(), Some("Georgia")); + + let html = format!("

kept too

"); + let sp = spans(&crate::html::parse(&html)); + assert!(sp.iter().any(|(t, _)| t.contains("kept")) && sp.iter().any(|(t, _)| t.contains("too"))); + assert!(sp.iter().all(|(_, f)| f.font.is_none() && f.link.is_none()), "{:?}", sp.iter().map(|s| &s.0).collect::>()); + + let sp = spans(&crate::markdown::parse(&format!("[kept](https://e.com/{long})"))); + assert_eq!(sp.len(), 1); + assert_eq!((sp[0].0.as_str(), sp[0].1.link.is_none()), ("kept", true)); +} + +/// Spans of one link share its target, and so do references to one definition: a long +/// definition and many references no longer copy the destination into each (#3713). +#[test] +fn spans_share_their_link_target() { + let dest = format!("https://e.com/{}", "x".repeat(4000)); + let md = format!("{}\n\n[r]: {dest}\n", "[a][r] **[b][r]** ".repeat(10_000)); + let sp = spans(&crate::markdown::parse(&md)); + let links: Vec<_> = sp.iter().filter_map(|(_, f)| f.link.as_ref()).collect(); + assert_eq!(links.len(), 20_000); + assert!(links.iter().all(|l| Arc::ptr_eq(l, links[0])), "one copy of the destination"); + + let html = format!("{}", "aa".repeat(1000)); + let sp = spans(&crate::html::parse(&html)); + assert!(sp.len() >= 2000); + let first = sp[0].1.link.clone().unwrap(); + assert!(sp.iter().all(|(_, f)| f.link.as_ref().is_some_and(|l| Arc::ptr_eq(l, &first)))); +} + +/// The document's runs own their link and font strings; past a budget further runs keep their +/// text but lose them, so a flow of shared names can't expand into gigabytes (#3713). +#[test] +fn run_link_and_font_copies_are_budgeted() { + let link: Arc = format!("https://e.com/{}", "x".repeat(model::MAX_LINK - 20)).into(); + let mut p = model::Para::default(); + for i in 0..20_000 { + let f = model::Fmt { bold: i % 2 == 0, link: Some(link.clone()), ..Default::default() }; + p.push_text("a", &f); + } + let doc = model::to_doc(&model::Flow { blocks: vec![FBlock::Para(p)], ..Default::default() }); + let paras: Vec<_> = doc.body.iter().filter_map(|b| if let Block::Para(p) = &**b { Some(p) } else { None }).collect(); + assert_eq!(paras.iter().map(|p| p.text.len()).sum::(), 20_000, "all text kept"); + let runs: Vec<_> = paras.iter().flat_map(|p| p.runs.iter()).collect(); + let kept: usize = runs.iter().filter_map(|r| r.props.link.as_ref()).map(String::len).sum(); + assert!(kept > 0 && kept <= 64 << 20, "{kept}"); +} From 9bee82220e89abdcf35e5631db28bd9cd73af53c Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 22 Oct 2026 16:24:47 +0100 Subject: [PATCH 07/13] Findings 40, 47, 52, 53, 57, 58, 65, 67, 74, 77 (#3692, #3705, #3713, Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/file-format-parity.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index c2b352424..2f58bcbe1 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -1,6 +1,6 @@ # File-format parity with Microsoft Word -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-25 · **Change:** trivial (the `.doc` reader logs what its copy budget leaves out, once a kind per document; previously trivial: repeated DOCX style IDs: save/reopen regression; previously trivial: Markdown code spans keep line-broken text literal, #3738; previously trivial: Arabic list labels: `arabicAlpha` and `arabicAbjad` draw the 28 Arabic letters (alphabetical and abjad order), doubling past the last letter; Hebrew, Hindi, Thai and Vietnamese still show decimal (`doc::section::tests::arabic_letters_count_in_both_orders`); previously trivial: localized TOC style ids read as the TOC styles, and a TOC in columns keeps its layout through Update Table, #3889; previously trivial: cu20we: outline caps; previously trivial: a relationship the markup names without the prefix is read as it wrote: an unqualified `id` on an element that carries a relationship names that relationship, and the writer keeps the attribute with the id the new package holds the part under; an ordinary `id` — a VML shape's, a drawing's — names no part; previously trivial: the reader takes the relationship ids the markup refers to from the relationship attributes of the main document it has already parsed, so a document with many relationships opens in the time its part takes, #3411; previously trivial: a table cell span from a `.json` file stays within 63 columns through Insert Column and Merge Cells, #3637; previously trivial: the file commands that change the document are checked on a protected document, the protection of a protected document needs an explicit stop, the session author can't change through `file.setAuthor` under protection (the CLI's `--author` uses the same protection check), and the post-check compares the settings and the metadata; previously trivial: a document with many content controls saves in the time the controls take, #13; previously trivial: `majorBidi`/`minorBidi` theme fonts follow the settings' theme font language, as measured in Word; previously trivial: Word signature lines are kept on save, still signable; previously trivial: Times New Roman keeps Liberation Serif under the roman-font rule; previously trivial: shape gradients and transparency, #1500; previously trivial: a folder that keeps no Unix modes, such as a NAS share mounted by GVFS, still takes a save, #3003; previously trivial: a text frame in the paragraph defaults frames nothing, #2360; previously trivial: table style band sizes written, a style without them unbanded as in Word, #3415; previously trivial: HTML export writes ruby (furigana, phonetic guides) and HTML import reads it back, #1681; previously trivial: RTF import decodes the East Asian multibyte code pages, #1653; previously trivial: PDF export: lam-alef drawn as two glyphs at one x extracts in reading order, #2310; previously trivial: native .json stores pictures and kept parts as base64, #586 follow-up; previously trivial: ODT headers and footers read from each master page, #863; previously trivial: RTF import compares heading and code formatting with the file's stylesheet; previously trivial: text typed into a content control takes the control's formatting; localized placeholder styles recognised; previously trivial: forms protection keeps its content controls: no inserting, Properties or Remove Content Control; previously trivial: Word XML and web pages saved as `.doc` open by what they hold, #2853; previously trivial: a VML shape in a header cell is always placed on the page, user-checked in Word for Mac, #1775; previously trivial: `w:sym` characters keep their symbol font when the text's font changes and are written back as `w:sym`, #2036; previously trivial: RTF export writes sections (gap 19); previously trivial: ODT export writes sections (gap 19); previously trivial: two lines in one and horizontal in vertical (`w:eastAsianLayout`) kept on save; previously trivial: PDF outline targets follow leading page breaks; previously trivial: positional tabs `w:ptab` read and kept on save, #2367; previously trivial: DOCX: mixed right-to-left runs split and marked `w:rtl` with their language, #1543; previously trivial: `w:doNotTrackMoves` round-trips and turns move tracking off; previously trivial: legacy outline, shadow, emboss and imprint text effects drawn, #2534; previously trivial: DOCX glossary documents (building blocks) read, kept and written; previously trivial: cached field results keep their visible run formatting; previously trivial: cached REF results retain hyperlink destinations; previously trivial: CLI conversions can explicitly refresh fields; previously trivial: a copied equation also goes in the clipboard's MathML formats, #1297; previously trivial: a pasted HTML `
` is an indented paragraph, not the Quote style, #1270); previously trivial: every table style part round-trips, #714; previously trivial: VML freeforms read and kept on save; previously trivial: multi-paragraph field instructions no longer add blank lines; previously trivial: `flowChartOr` draws its circle and cross spokes, as in Word; previously trivial: RTF: Mac OS Roman and DOS text, #1653; previously trivial: VML text boxes keep their internal margins, fit to their text and vertical alignment; hidden drawings not shown; previously trivial: PDF export draws horizontal and vertical lines inside groups, #1474; previously trivial: symbol-font text exported as Unicode, #776; previously trivial: text-box margins, Resize shape to fit text and freeforms in groups, #743; previously trivial: formatted paste from the system clipboard's HTML and RTF, #687; previously trivial: independent complex-script formatting inheritance and round-trip regression tests; previously trivial: import/export fixes for TXT, RTF, ODT, HTML and LaTeX, #433–#440; previously major: first version; every format Word reads or writes, measured from the readers and writers on origin/main) · **Target:** Microsoft Word (Microsoft 365) for Mac 16.113.4, plus Word for Windows' extra formats +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-25 · **Change:** trivial (the `.doc` reader logs what its copy budget leaves out, once a kind per document; importers bound hostile expansion: font names, links, ODS repeats, LaTeX wrappers, Markdown delimiters, #3692/#3713; previously trivial: repeated DOCX style IDs: save/reopen regression; previously trivial: Markdown code spans keep line-broken text literal, #3738; previously trivial: Arabic list labels: `arabicAlpha` and `arabicAbjad` draw the 28 Arabic letters (alphabetical and abjad order), doubling past the last letter; Hebrew, Hindi, Thai and Vietnamese still show decimal (`doc::section::tests::arabic_letters_count_in_both_orders`); previously trivial: localized TOC style ids read as the TOC styles, and a TOC in columns keeps its layout through Update Table, #3889; previously trivial: cu20we: outline caps; previously trivial: a relationship the markup names without the prefix is read as it wrote: an unqualified `id` on an element that carries a relationship names that relationship, and the writer keeps the attribute with the id the new package holds the part under; an ordinary `id` — a VML shape's, a drawing's — names no part; previously trivial: the reader takes the relationship ids the markup refers to from the relationship attributes of the main document it has already parsed, so a document with many relationships opens in the time its part takes, #3411; previously trivial: a table cell span from a `.json` file stays within 63 columns through Insert Column and Merge Cells, #3637; previously trivial: the file commands that change the document are checked on a protected document, the protection of a protected document needs an explicit stop, the session author can't change through `file.setAuthor` under protection (the CLI's `--author` uses the same protection check), and the post-check compares the settings and the metadata; previously trivial: a document with many content controls saves in the time the controls take, #13; previously trivial: `majorBidi`/`minorBidi` theme fonts follow the settings' theme font language, as measured in Word; previously trivial: Word signature lines are kept on save, still signable; previously trivial: Times New Roman keeps Liberation Serif under the roman-font rule; previously trivial: shape gradients and transparency, #1500; previously trivial: a folder that keeps no Unix modes, such as a NAS share mounted by GVFS, still takes a save, #3003; previously trivial: a text frame in the paragraph defaults frames nothing, #2360; previously trivial: table style band sizes written, a style without them unbanded as in Word, #3415; previously trivial: HTML export writes ruby (furigana, phonetic guides) and HTML import reads it back, #1681; previously trivial: RTF import decodes the East Asian multibyte code pages, #1653; previously trivial: PDF export: lam-alef drawn as two glyphs at one x extracts in reading order, #2310; previously trivial: native .json stores pictures and kept parts as base64, #586 follow-up; previously trivial: ODT headers and footers read from each master page, #863; previously trivial: RTF import compares heading and code formatting with the file's stylesheet; previously trivial: text typed into a content control takes the control's formatting; localized placeholder styles recognised; previously trivial: forms protection keeps its content controls: no inserting, Properties or Remove Content Control; previously trivial: Word XML and web pages saved as `.doc` open by what they hold, #2853; previously trivial: a VML shape in a header cell is always placed on the page, user-checked in Word for Mac, #1775; previously trivial: `w:sym` characters keep their symbol font when the text's font changes and are written back as `w:sym`, #2036; previously trivial: RTF export writes sections (gap 19); previously trivial: ODT export writes sections (gap 19); previously trivial: two lines in one and horizontal in vertical (`w:eastAsianLayout`) kept on save; previously trivial: PDF outline targets follow leading page breaks; previously trivial: positional tabs `w:ptab` read and kept on save, #2367; previously trivial: DOCX: mixed right-to-left runs split and marked `w:rtl` with their language, #1543; previously trivial: `w:doNotTrackMoves` round-trips and turns move tracking off; previously trivial: legacy outline, shadow, emboss and imprint text effects drawn, #2534; previously trivial: DOCX glossary documents (building blocks) read, kept and written; previously trivial: cached field results keep their visible run formatting; previously trivial: cached REF results retain hyperlink destinations; previously trivial: CLI conversions can explicitly refresh fields; previously trivial: a copied equation also goes in the clipboard's MathML formats, #1297; previously trivial: a pasted HTML `
` is an indented paragraph, not the Quote style, #1270); previously trivial: every table style part round-trips, #714; previously trivial: VML freeforms read and kept on save; previously trivial: multi-paragraph field instructions no longer add blank lines; previously trivial: `flowChartOr` draws its circle and cross spokes, as in Word; previously trivial: RTF: Mac OS Roman and DOS text, #1653; previously trivial: VML text boxes keep their internal margins, fit to their text and vertical alignment; hidden drawings not shown; previously trivial: PDF export draws horizontal and vertical lines inside groups, #1474; previously trivial: symbol-font text exported as Unicode, #776; previously trivial: text-box margins, Resize shape to fit text and freeforms in groups, #743; previously trivial: formatted paste from the system clipboard's HTML and RTF, #687; previously trivial: independent complex-script formatting inheritance and round-trip regression tests; previously trivial: import/export fixes for TXT, RTF, ODT, HTML and LaTeX, #433–#440; previously major: first version; every format Word reads or writes, measured from the readers and writers on origin/main) · **Target:** Microsoft Word (Microsoft 365) for Mac 16.113.4, plus Word for Windows' extra formats Every format Word opens or saves, what WordCraft does with it, and how it's tested. Fidelity is the share of a typical real-world file's content and formatting that survives (read: shows correctly; @@ -121,7 +121,7 @@ Measured by grepping `crates/docx/src/read` and `src/write` for the OOXML elemen | `w:fitText` | ❌ | ❌ | East Asian layout | | Glossary document (building blocks) | ✅ | ✅ | `w:docPart` entries (name, style, gallery, category, types, behaviours, description, guid, body) are read into `Document::glossary` and offered by Insert › Quick Parts and AutoText; `save` with `"in": "document"` stores one in the file. An untouched glossary is written back byte for byte with its own parts (styles, settings, fonts…); an edited one, or one whose parts weren't kept, is written from the model as `word/glossary1.xml` with copies of the document's styles and numbering. The glossary's own styles, settings and fonts, text boxes inside entries, and the notes and comments of a selection saved into the document don't survive an edit. Tests: `crates/docx/tests/it/glossary.rs` | | `w:altChunk`, sub-documents | ❌ | ❌ | | -| Hostile input | ✅ | — | `malformed.rs`, capped allocations, never-crash standard | +| Hostile input | ✅ | — | `malformed.rs`, capped allocations, never-crash standard. Text importers drop font names over 256 bytes and links over 8 KiB, share a link or font between spans, and cap the runs' copies of them at 64 MiB (#3713) | | Document paragraph defaults (`w:pPrDefault`) | ✅ | ✅ | Omission and explicit empty defaults remain distinct across repeated saves; nonempty defaults and later default-spacing edits are written | ## Saving many content controls @@ -268,6 +268,7 @@ Repeated DOCX style IDs use the last complete definition. The original regressio | 2026-10-25 | trivial | Repeated DOCX style IDs: repeated save/reopen regression for whole-definition replacement. | | 2026-10-25 | trivial | Arabic list labels: `arabicAlpha` and `arabicAbjad` draw the 28 Arabic letters (alphabetical and abjad order), doubling past the last letter; Hebrew, Hindi, Thai and Vietnamese still show decimal (`doc::section::tests::arabic_letters_count_in_both_orders`) | | 2026-10-23 | trivial | cu20we: preserve and render outline caps across DOCX/VML, screen and PDF; synthetic regressions; readiness estimates unchanged | +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-25 · **Change:** trivial (the `.doc` reader logs what its copy budget leaves out, once a kind per document; previously trivial: repeated DOCX style IDs: save/reopen regression; previously trivial: Markdown code spans keep line-broken text literal, #3738; previously trivial: Arabic list labels: `arabicAlpha` and `arabicAbjad` draw the 28 Arabic letters (alphabetical and abjad order), doubling past the last letter; Hebrew, Hindi, Thai and Vietnamese still show decimal (`doc::section::tests::arabic_letters_count_in_both_orders`); previously trivial: localized TOC style ids read as the TOC styles, and a TOC in columns keeps its layout through Update Table, #3889; previously trivial: cu20we: outline caps; previously trivial: a relationship the markup names without the prefix is read as it wrote: an unqualified `id` on an element that carries a relationship names that relationship, and the writer keeps the attribute with the id the new package holds the part under; an ordinary `id` — a VML shape's, a drawing's — names no part; previously trivial: the reader takes the relationship ids the markup refers to from the relationship attributes of the main document it has already parsed, so a document with many relationships opens in the time its part takes, #3411; previously trivial: a table cell span from a `.json` file stays within 63 columns through Insert Column and Merge Cells, #3637; previously trivial: the file commands that change the document are checked on a protected document, the protection of a protected document needs an explicit stop, the session author can't change through `file.setAuthor` under protection (the CLI's `--author` uses the same protection check), and the post-check compares the settings and the metadata; previously trivial: a document with many content controls saves in the time the controls take, #13; previously trivial: `majorBidi`/`minorBidi` theme fonts follow the settings' theme font language, as measured in Word; previously trivial: Word signature lines are kept on save, still signable; previously trivial: Times New Roman keeps Liberation Serif under the roman-font rule; previously trivial: shape gradients and transparency, #1500; previously trivial: a folder that keeps no Unix modes, such as a NAS share mounted by GVFS, still takes a save, #3003; previously trivial: a text frame in the paragraph defaults frames nothing, #2360; previously trivial: table style band sizes written, a style without them unbanded as in Word, #3415; previously trivial: HTML export writes ruby (furigana, phonetic guides) and HTML import reads it back, #1681; previously trivial: RTF import decodes the East Asian multibyte code pages, #1653; previously trivial: PDF export: lam-alef drawn as two glyphs at one x extracts in reading order, #2310; previously trivial: native .json stores pictures and kept parts as base64, #586 follow-up; previously trivial: ODT headers and footers read from each master page, #863; previously trivial: RTF import compares heading and code formatting with the file's stylesheet; previously trivial: text typed into a content control takes the control's formatting; localized placeholder styles recognised; previously trivial: forms protection keeps its content controls: no inserting, Properties or Remove Content Control; previously trivial: Word XML and web pages saved as `.doc` open by what they hold, #2853; previously trivial: a VML shape in a header cell is always placed on the page, user-checked in Word for Mac, #1775; previously trivial: `w:sym` characters keep their symbol font when the text's font changes and are written back as `w:sym`, #2036; previously trivial: RTF export writes sections (gap 19); previously trivial: ODT export writes sections (gap 19); previously trivial: two lines in one and horizontal in vertical (`w:eastAsianLayout`) kept on save; previously trivial: PDF outline targets follow leading page breaks; previously trivial: positional tabs `w:ptab` read and kept on save, #2367; previously trivial: DOCX: mixed right-to-left runs split and marked `w:rtl` with their language, #1543; previously trivial: `w:doNotTrackMoves` round-trips and turns move tracking off; previously trivial: legacy outline, shadow, emboss and imprint text effects drawn, #2534; previously trivial: DOCX glossary documents (building blocks) read, kept and written; previously trivial: cached field results keep their visible run formatting; previously trivial: cached REF results retain hyperlink destinations; previously trivial: CLI conversions can explicitly refresh fields; previously trivial: a copied equation also goes in the clipboard's MathML formats, #1297; previously trivial: a pasted HTML `
` is an indented paragraph, not the Quote style, #1270); previously trivial: every table style part round-trips, #714; previously trivial: VML freeforms read and kept on save; previously trivial: multi-paragraph field instructions no longer add blank lines; previously trivial: `flowChartOr` draws its circle and cross spokes, as in Word; previously trivial: RTF: Mac OS Roman and DOS text, #1653; previously trivial: VML text boxes keep their internal margins, fit to their text and vertical alignment; hidden drawings not shown; previously trivial: PDF export draws horizontal and vertical lines inside groups, #1474; previously trivial: symbol-font text exported as Unicode, #776; previously trivial: text-box margins, Resize shape to fit text and freeforms in groups, #743; previously trivial: formatted paste from the system clipboard's HTML and RTF, #687; previously trivial: independent complex-script formatting inheritance and round-trip regression tests; previously trivial: import/export fixes for TXT, RTF, ODT, HTML and LaTeX, #433–#440; previously major: first version; every format Word reads or writes, measured from the readers and writers on origin/main) · **Target:** Microsoft Word (Microsoft 365) for Mac 16.113.4, plus Word for Windows' extra formats | 2026-10-23 | trivial | `.doc` reader logs what its copy budget leaves out (floating objects, link targets, alt text, Prm formatting), once a kind per document (#3692) | | 2026-10-22 | trivial | `.doc` reader bounds hostile files: oversized sector tables refused, per-character re-reads removed, copy and decode budgets (#3642, #3692, #3708-#3712) | | 2026-10-23 | trivial | A relationship the markup names without the prefix is read as it wrote: an unqualified `id` on an element that carries a relationship names that relationship, and the writer keeps the attribute with the id the new package holds the part under; an ordinary `id` — a VML shape's, a drawing's — names no part (#3411 follow-up) | From d8501a5aeb58a85418e70bc9a8663945c2477241 Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 22 Oct 2026 16:47:09 +0100 Subject: [PATCH 08/13] #3713: rustfmt Finding 58 follow-up: formatting only. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/formats/src/markdown.rs | 3 ++- crates/formats/src/tests.rs | 13 +++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/crates/formats/src/markdown.rs b/crates/formats/src/markdown.rs index e5346cca7..7970729e3 100644 --- a/crates/formats/src/markdown.rs +++ b/crates/formats/src/markdown.rs @@ -2227,7 +2227,8 @@ mod tests { ] { let out = md(l); let back = inl(&out); - let got: Vec<_> = back.iter().filter_map(|i| if let Inline::Text(_, f) = i { f.link.as_deref().map(str::to_string) } else { None }).collect(); + let got: Vec<_> = + back.iter().filter_map(|i| if let Inline::Text(_, f) = i { f.link.as_deref().map(str::to_string) } else { None }).collect(); assert_eq!(got, [l.to_string()], "{l:?} → {out:?}"); } // Ordinary destinations are written as before. diff --git a/crates/formats/src/tests.rs b/crates/formats/src/tests.rs index 5ef26d5f1..16eb2afca 100644 --- a/crates/formats/src/tests.rs +++ b/crates/formats/src/tests.rs @@ -748,8 +748,11 @@ fn code_spans_and_links_around_backticks_read_as_before() { // inside a code span in link text, and link text with a backtick that nothing closes. let flow = crate::markdown::parse("\\``e` `` a ` b `` then ``` and `c` [x `]` y](http://a.org) [p ` q](http://b.org) ``f"); let FBlock::Para(p) = &flow.blocks[0] else { panic!("{:?}", flow.blocks) }; - let spans: Vec<(String, bool, Option)> = - p.inlines.iter().filter_map(|i| if let model::Inline::Text(t, f) = i { Some((t.clone(), f.code, f.link.as_deref().map(str::to_string))) } else { None }).collect(); + let spans: Vec<(String, bool, Option)> = p + .inlines + .iter() + .filter_map(|i| if let model::Inline::Text(t, f) = i { Some((t.clone(), f.code, f.link.as_deref().map(str::to_string))) } else { None }) + .collect(); let code: Vec<&str> = spans.iter().filter(|s| s.1).map(|s| s.0.as_str()).collect(); assert_eq!(code, ["e", "a ` b", "c", "]"], "{spans:?}"); let linked = |url: &str| spans.iter().filter(|s| s.2.as_deref() == Some(url)).map(|s| s.0.as_str()).collect::(); @@ -789,7 +792,8 @@ fn brackets_do_not_turn_later_links_into_text() { let links = |md: &str| { let flow = crate::markdown::parse(md); let FBlock::Para(p) = &flow.blocks[0] else { panic!("{:?}", flow.blocks) }; - let mut urls: Vec = p.inlines.iter().filter_map(|i| if let model::Inline::Text(_, f) = i { f.link.as_deref().map(str::to_string) } else { None }).collect(); + let mut urls: Vec = + p.inlines.iter().filter_map(|i| if let model::Inline::Text(_, f) = i { f.link.as_deref().map(str::to_string) } else { None }).collect(); urls.dedup(); urls }; @@ -900,7 +904,8 @@ fn overlong_font_names_and_links_are_dropped() { let flow = crate::rtf::parse(b"{\\rtf1{\\fonttbl{\\f0 Arial;}{\\f1 Georgia;}}\\f1 a}").unwrap(); assert_eq!(spans(&flow)[0].1.font.as_deref(), Some("Georgia")); - let html = format!("

kept too

"); + let html = + format!("

kept too

"); let sp = spans(&crate::html::parse(&html)); assert!(sp.iter().any(|(t, _)| t.contains("kept")) && sp.iter().any(|(t, _)| t.contains("too"))); assert!(sp.iter().all(|(_, f)| f.font.is_none() && f.link.is_none()), "{:?}", sp.iter().map(|s| &s.0).collect::>()); From 582f4118764eb3981c7208f8775e1b3b519570e9 Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 22 Oct 2026 19:56:02 +0100 Subject: [PATCH 09/13] =?UTF-8?q?#3692:=20Markdown=20export=20wraps=20link?= =?UTF-8?q?=20destinations=20with=20any=20Unicode=20whitespace=20in=20?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit review: only an ASCII space triggered the angle brackets, but the reader ends a bare destination at any whitespace (U+00A0, U+3000), so such a target read back as plain text. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/formats/src/markdown.rs | 2 +- crates/formats/src/tests.rs | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/crates/formats/src/markdown.rs b/crates/formats/src/markdown.rs index 7970729e3..cbf23326a 100644 --- a/crates/formats/src/markdown.rs +++ b/crates/formats/src/markdown.rs @@ -1466,7 +1466,7 @@ fn link_target(l: &str) -> String { } out.push(c); } - if l.contains([' ', '(', ')', '<', '>']) { format!("<{out}>") } else { out } + if l.contains(|c: char| c.is_whitespace() || matches!(c, '(' | ')' | '<' | '>')) { format!("<{out}>") } else { out } } /// Whether `rest` (the text after a `&`) starts like an entity reference (`amp;`, `#106;`, diff --git a/crates/formats/src/tests.rs b/crates/formats/src/tests.rs index 16eb2afca..dc2344534 100644 --- a/crates/formats/src/tests.rs +++ b/crates/formats/src/tests.rs @@ -867,6 +867,21 @@ fn exported_links_keep_only_safe_targets() { } } +/// A link target with any whitespace (a no-break or ideographic space too) is written in `<…>`, +/// so the reader, which ends a bare destination at the first whitespace, reads it back whole. +#[test] +fn markdown_link_targets_with_unicode_spaces_read_back() { + for l in ["https://e.com/a\u{a0}b", "https://e.com/a\u{3000}b", "https://e.com/a b"] { + let mut d = Document::new(); + let mut p = Paragraph::with_text("", CharProps::default()); + p.insert_text(0, "go", &CharProps { link: Some(l.to_string()), ..Default::default() }).unwrap(); + d.body = vec![para_block(p)]; + let md = String::from_utf8(export("md", &d).unwrap().unwrap()).unwrap(); + let sp = spans(&crate::markdown::parse(&md)); + assert_eq!(sp.iter().find(|(t, _)| t == "go").and_then(|(_, f)| f.link.as_deref()), Some(l), "md: {md}"); + } +} + #[test] fn hidden_drawings_are_left_out_of_exports() { let mut d = wordcraft_doc::Document::from_text("x"); From 41de47398a5d19da9d19d7e09e1f79619bf29b01 Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Fri, 23 Oct 2026 03:31:20 +0100 Subject: [PATCH 10/13] =?UTF-8?q?#3713:=20review=20=E2=80=94=20ODS=20repea?= =?UTF-8?q?ted=20cells=20are=20budgeted=20only=20for=20the=20copies=20the?= =?UTF-8?q?=20grid=20can=20still=20keep?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With one cell slot left, a long value repeated a thousand times was refused for all its copies' text although Grid::push keeps only the first, which fits. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01CX1hMKX28764Lrx5moxs1o --- crates/formats/src/sheet.rs | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/crates/formats/src/sheet.rs b/crates/formats/src/sheet.rs index 392b425c0..97cf1f868 100644 --- a/crates/formats/src/sheet.rs +++ b/crates/formats/src/sheet.rs @@ -979,9 +979,10 @@ fn ods_read(xml: &[u8], want: Option) -> Result<(Vec, Vec"#, + MAX_COLS - 1 + )); + let (_, rows) = read_rows(&book, Pick::Index(0)).unwrap(); + assert_eq!(rows.len(), full + 2); + assert_eq!(rows.last(), Some(&vec![wide])); + assert_eq!(rows.iter().map(Vec::len).sum::(), MAX_CELLS); + } } From 2c4f4669c605dd053342b2db8064b55239ae99fa Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Fri, 23 Oct 2026 11:08:38 +0100 Subject: [PATCH 11/13] =?UTF-8?q?#3713:=20review=20=E2=80=94=20importer=20?= =?UTF-8?q?limits=20that=20drop=20fonts,=20links,=20style=20names=20or=20s?= =?UTF-8?q?heet=20rows=20log=20a=20warning=20once=20per=20document?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every limit this branch added that leaves data out now says so: - font names over 256 bytes and links over 8 KiB (HTML, ODT, RTF, LaTeX, Markdown), and RTF style names cut to 256 bytes; - runs' links and fonts past the 64 MiB run-string budget; - spreadsheet rows refused for being over the text budget (ODS and Grid::push), once per sheet. model::dropped logs each kind once per imported document; each importer's entry point starts a new document with begin_import. Tests capture the warnings (crate::test_log) and check each is logged once, again for the next document, and not at all when nothing is dropped. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01CX1hMKX28764Lrx5moxs1o --- crates/formats/src/html.rs | 3 ++ crates/formats/src/latex.rs | 1 + crates/formats/src/lib.rs | 2 + crates/formats/src/markdown.rs | 5 +++ crates/formats/src/model.rs | 73 +++++++++++++++++++++++++++++++--- crates/formats/src/odt.rs | 1 + crates/formats/src/rtf.rs | 3 ++ crates/formats/src/sheet.rs | 32 ++++++++++++++- crates/formats/src/test_log.rs | 36 +++++++++++++++++ crates/formats/src/tests.rs | 39 +++++++++++++++++- 10 files changed, 188 insertions(+), 7 deletions(-) create mode 100644 crates/formats/src/test_log.rs diff --git a/crates/formats/src/html.rs b/crates/formats/src/html.rs index 4970855ed..f1eb1693c 100644 --- a/crates/formats/src/html.rs +++ b/crates/formats/src/html.rs @@ -1588,17 +1588,20 @@ fn collapse_ws(s: &str) -> String { /// Parse HTML into a document. pub fn import(s: &str) -> Document { + model::begin_import(); model::to_doc(&parse(s)) } /// [`import`], loading pictures that aren't `data:` URIs through `images`. pub fn import_with(s: &str, images: ImageLoader) -> Document { + model::begin_import(); model::to_doc(&parse_with(s, images)) } /// [`import_with`] onto `page` (its size and margins), which tables size to: HTML gives no page /// size, so the caller picks it (the region's paper, #1381). pub fn import_on(s: &str, images: ImageLoader, page: &wordcraft_doc::SectionProps) -> Document { + model::begin_import(); model::to_doc_in(&parse_with(s, images), page.clone()) } diff --git a/crates/formats/src/latex.rs b/crates/formats/src/latex.rs index 18d807fdd..a79f3a9f0 100644 --- a/crates/formats/src/latex.rs +++ b/crates/formats/src/latex.rs @@ -1057,6 +1057,7 @@ fn math_seq(t: &[MTok], pos: &mut usize, depth: usize, in_opt: bool) -> String { /// Import a LaTeX source file. pub fn import(bytes: &[u8]) -> Document { + model::begin_import(); let mut d = model::to_doc(&parse(&crate::txt::decode(bytes.get(..MAX_SOURCE).unwrap_or(bytes)))); d.ensure_nonempty(); d diff --git a/crates/formats/src/lib.rs b/crates/formats/src/lib.rs index 6678b5c95..8af0d2a50 100644 --- a/crates/formats/src/lib.rs +++ b/crates/formats/src/lib.rs @@ -91,5 +91,7 @@ pub fn export_with(ext: &str, doc: &Document, opts: &ExportOptions) -> Option Document { + model::begin_import(); model::to_doc(&parse(text)) } /// [`import`] onto `page`: Markdown gives no page size (#1381). pub fn import_on(text: &str, page: &wordcraft_doc::SectionProps) -> Document { + model::begin_import(); model::to_doc_in(&parse(text), page.clone()) } @@ -1032,6 +1034,9 @@ fn tokenize(s: &str, base: &Fmt, depth: usize, refs: &Refs) -> Vec { let mut f = base.clone(); // A destination too long to keep leaves its text unlinked. f.link = (dest.len() <= model::MAX_LINK).then_some(dest); + if f.link.is_none() { + model::dropped(model::Dropped::Link); + } for x in parse_inlines(&inner, &f, depth + 1, refs) { nodes.push(Node::Inl(x)); } diff --git a/crates/formats/src/model.rs b/crates/formats/src/model.rs index 156269b1b..61ae4aea5 100644 --- a/crates/formats/src/model.rs +++ b/crates/formats/src/model.rs @@ -30,14 +30,72 @@ pub const MAX_LINK: usize = 8 << 10; /// lose their link and font rather than multiply a small file into gigabytes (#3713). const MAX_RUN_STRINGS: usize = 64 << 20; -/// `name` as a span's font, unless it is longer than [`MAX_FONT_NAME`]. +/// `name` as a span's font, unless it is longer than [`MAX_FONT_NAME`] (then reported, see +/// [`dropped`]). pub fn kept_font(name: &str) -> Option> { - (name.len() <= MAX_FONT_NAME).then(|| name.into()) + let kept = (name.len() <= MAX_FONT_NAME).then(|| name.into()); + if kept.is_none() { + dropped(Dropped::Font); + } + kept } -/// `target` as a span's link, unless it is longer than [`MAX_LINK`]. +/// `target` as a span's link, unless it is longer than [`MAX_LINK`] (then reported, see +/// [`dropped`]). pub fn kept_link(target: &str) -> Option> { - (target.len() <= MAX_LINK).then(|| target.into()) + let kept = (target.len() <= MAX_LINK).then(|| target.into()); + if kept.is_none() { + dropped(Dropped::Link); + } + kept +} + +/// What an importer leaves out to stay within a limit (review of #3873: no limit drops data +/// without saying so). +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Dropped { + /// A font name over [`MAX_FONT_NAME`]. + Font, + /// A link target over [`MAX_LINK`]. + Link, + /// Runs' links and fonts past [`MAX_RUN_STRINGS`]. + RunStrings, + /// A style name cut to [`MAX_FONT_NAME`]. + StyleName, +} + +impl Dropped { + fn message(self) -> String { + match self { + Dropped::Font => format!("import: font names longer than {MAX_FONT_NAME} bytes are dropped; that text keeps the surrounding font"), + Dropped::Link => format!("import: links longer than {MAX_LINK} bytes are dropped; their text is kept"), + Dropped::RunStrings => { + format!("import: the document's runs carry more than {MAX_RUN_STRINGS} bytes of links and font names; later runs lose them") + } + Dropped::StyleName => format!("import: style names longer than {MAX_FONT_NAME} bytes are cut short"), + } + } +} + +thread_local! { + /// The kinds of [`Dropped`] already reported for the document being imported on this thread. + /// Per thread because a document is parsed and built on one, and the parsers drop things + /// from places that have no log of their own to hand. + static REPORTED: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Log that the importer left something of `kind` out: once per kind per imported document +/// ([`begin_import`]), not once per item. +pub(crate) fn dropped(kind: Dropped) { + let bit = 1u8 << (kind as u8); + if REPORTED.with(|r| r.replace(r.get() | bit)) & bit == 0 { + log::warn!("{}", kind.message()); + } +} + +/// Start a new document's reports: each importer's entry point calls this first. +pub(crate) fn begin_import() { + REPORTED.with(|r| r.set(0)); } /// Character formatting the text formats can express. @@ -580,7 +638,12 @@ impl<'a> Builder<'a> { let n = cp.link.as_ref().map_or(0, String::len).saturating_add(cp.font.as_ref().map_or(0, String::len)); match self.run_strings.checked_add(n).filter(|&t| t <= MAX_RUN_STRINGS) { Some(t) => self.run_strings = t, - None => (cp.link, cp.font) = (None, None), + None => { + if cp.link.is_some() || cp.font.is_some() { + dropped(Dropped::RunStrings); + } + (cp.link, cp.font) = (None, None); + } } cp } diff --git a/crates/formats/src/odt.rs b/crates/formats/src/odt.rs index 6be960811..472507f94 100644 --- a/crates/formats/src/odt.rs +++ b/crates/formats/src/odt.rs @@ -2627,6 +2627,7 @@ fn parse_tracked(bytes: &[u8]) -> Result<(Flow, SectionProps, Option, Opti /// Parse an `.odt` package into a document. pub fn import(bytes: &[u8]) -> Result { + model::begin_import(); let (flow, last, track, font, parts) = parse_tracked(bytes)?; let mut doc = model::to_doc(&flow); if font.is_some() { diff --git a/crates/formats/src/rtf.rs b/crates/formats/src/rtf.rs index 7b06aa082..2a9503fbd 100644 --- a/crates/formats/src/rtf.rs +++ b/crates/formats/src/rtf.rs @@ -1654,6 +1654,8 @@ impl Reader { self.commit_style(); } else if self.style_name.len() < model::MAX_FONT_NAME { self.style_name.push(c); + } else { + model::dropped(model::Dropped::StyleName); } } } @@ -2856,6 +2858,7 @@ fn column_grid(rows: &mut [Vec], rights: &[Vec]) -> Vec { /// Parse RTF bytes into a document. pub fn import(bytes: &[u8]) -> Result { + model::begin_import(); let Parsed { flow, last_section, default_font, headers_footers, .. } = parse_full(bytes)?; let mut doc = model::to_doc_in(&flow, last_section); // Each section's own headers and footers; those it leaves out stay linked to the previous. diff --git a/crates/formats/src/sheet.rs b/crates/formats/src/sheet.rs index 97cf1f868..e3ff85f29 100644 --- a/crates/formats/src/sheet.rs +++ b/crates/formats/src/sheet.rs @@ -343,6 +343,8 @@ struct Grid { rows: Vec>, cells: usize, text: TextBudget, + /// A row has been left out for its text (and logged, once per sheet). + refused: bool, } impl Grid { @@ -350,6 +352,15 @@ impl Grid { self.rows.len() >= MAX_ROWS || self.cells >= MAX_CELLS || self.text.0 >= MAX_TEXT_BYTES } + /// Note that a row is left out because its text is over what is left of [`MAX_TEXT_BYTES`]: + /// logged once per sheet, not once per row. + fn refuse(&mut self) { + if !self.refused { + self.refused = true; + log::warn!("sheet: rows with more text than the {MAX_TEXT_BYTES} byte budget has left are left out of the recipient list"); + } + } + /// Keep `row` (trailing blanks trimmed) `times` times, as far as the caps allow. fn push(&mut self, mut row: Vec, times: u64) { while row.last().is_some_and(String::is_empty) { @@ -364,6 +375,7 @@ impl Grid { 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()) { + self.refuse(); return; } self.cells = self.cells.saturating_add(r.len()); @@ -989,6 +1001,7 @@ fn ods_read(xml: &[u8], want: Option) -> Result<(Vec, Vec MAX_TEXT_BYTES.saturating_sub(grid.text.0) { cells.clear(); *budget = None; + grid.refuse(); return; } cells.resize(cells.len() + blanks, String::new()); @@ -1274,8 +1287,25 @@ mod tests { let book = ods(&format!( r#"next"# )); - let (_, rows) = read_rows(&book, Pick::Index(0)).unwrap(); + let ((_, rows), warned) = crate::test_log::warnings(|| read_rows(&book, Pick::Index(0)).unwrap()); assert_eq!(rows, vec![vec!["next".to_string()]]); + // The row left out is said to be (review of #3873). + assert_eq!(warned.iter().filter(|w| w.contains("are left out of the recipient list")).count(), 1, "{warned:?}"); + } + + #[test] + fn rows_over_the_text_budget_are_logged_once_per_sheet() { + let wide = "€".repeat(MAX_CELL_CHARS); + let over = format!( + r#""# + ); + let book = ods(&format!("{over}{over}{over}next")); + let ((_, rows), warned) = crate::test_log::warnings(|| read_rows(&book, Pick::Index(0)).unwrap()); + assert_eq!(rows, vec![vec!["next".to_string()]]); + assert_eq!(warned.iter().filter(|w| w.contains("recipient list")).count(), 1, "one warning, not one per row: {warned:?}"); + let book = ods("fine"); + let (_, warned) = crate::test_log::warnings(|| read_rows(&book, Pick::Index(0)).unwrap()); + assert!(warned.is_empty(), "{warned:?}"); } #[test] diff --git a/crates/formats/src/test_log.rs b/crates/formats/src/test_log.rs new file mode 100644 index 000000000..745ce21fe --- /dev/null +++ b/crates/formats/src/test_log.rs @@ -0,0 +1,36 @@ +//! Test helper: the warnings this crate logs, captured per test thread, so a test can check that +//! a limit which leaves something out says so. + +use std::cell::RefCell; + +struct Capture; + +thread_local! { + static LINES: RefCell> = const { RefCell::new(Vec::new()) }; +} + +impl log::Log for Capture { + fn enabled(&self, m: &log::Metadata) -> bool { + m.level() <= log::Level::Warn + } + + fn log(&self, r: &log::Record) { + if self.enabled(r.metadata()) { + LINES.with(|l| l.borrow_mut().push(r.args().to_string())); + } + } + + fn flush(&self) {} +} + +static CAPTURE: Capture = Capture; + +/// `f`'s result and the warnings logged on this thread while it ran. +pub(crate) fn warnings(f: impl FnOnce() -> T) -> (T, Vec) { + // Only this logger is ever installed in a test binary; a second call keeps it. + let _ = log::set_logger(&CAPTURE); + log::set_max_level(log::LevelFilter::Warn); + LINES.with(|l| l.borrow_mut().clear()); + let out = f(); + (out, LINES.with(|l| std::mem::take(&mut *l.borrow_mut()))) +} diff --git a/crates/formats/src/tests.rs b/crates/formats/src/tests.rs index dc2344534..bc2442d80 100644 --- a/crates/formats/src/tests.rs +++ b/crates/formats/src/tests.rs @@ -958,10 +958,47 @@ fn run_link_and_font_copies_are_budgeted() { let f = model::Fmt { bold: i % 2 == 0, link: Some(link.clone()), ..Default::default() }; p.push_text("a", &f); } - let doc = model::to_doc(&model::Flow { blocks: vec![FBlock::Para(p)], ..Default::default() }); + model::begin_import(); + let (doc, warned) = crate::test_log::warnings(|| model::to_doc(&model::Flow { blocks: vec![FBlock::Para(p)], ..Default::default() })); let paras: Vec<_> = doc.body.iter().filter_map(|b| if let Block::Para(p) = &**b { Some(p) } else { None }).collect(); assert_eq!(paras.iter().map(|p| p.text.len()).sum::(), 20_000, "all text kept"); let runs: Vec<_> = paras.iter().flat_map(|p| p.runs.iter()).collect(); let kept: usize = runs.iter().filter_map(|r| r.props.link.as_ref()).map(String::len).sum(); assert!(kept > 0 && kept <= 64 << 20, "{kept}"); + // The runs that lose theirs are said to, once for the document (review of #3873). + assert_eq!(warned.iter().filter(|w| w.contains("later runs lose them")).count(), 1, "{warned:?}"); +} + +/// Every limit that leaves a font name, link or style name out says so, once per kind per +/// document rather than once per item (review of #3873; the tracker complains of silent loss). +#[test] +fn dropped_names_and_links_are_logged_once_per_document() { + let long = "x".repeat(model::MAX_LINK + 1); + let html = format!( + "

a b c

" + ); + let count = |w: &[String], what: &str| w.iter().filter(|l| l.contains(what)).count(); + for _ in 0..2 { + // Each import is its own document: the second is reported too. + let (doc, warned) = crate::test_log::warnings(|| crate::html::import(&html)); + assert!(doc.body.len() == 1, "the text is kept"); + assert_eq!(count(&warned, "font names longer"), 1, "{warned:?}"); + assert_eq!(count(&warned, "links longer"), 1, "{warned:?}"); + } + let (_, warned) = crate::test_log::warnings(|| crate::html::import("

fine

")); + assert!(warned.is_empty(), "nothing left out, nothing said: {warned:?}"); + + let md = format!("[a](https://e.com/{long}) [b](https://e.com/{long})"); + let (_, warned) = crate::test_log::warnings(|| crate::markdown::import(&md)); + assert_eq!(count(&warned, "links longer"), 1, "{warned:?}"); + + let rtf = + format!("{{\\rtf1{{\\fonttbl{{\\f0 Arial;}}{{\\f1 {long};}}{{\\f2 {long};}}}}{{\\stylesheet{{\\s1 {long};}}{{\\s2 {long};}}}}\\f1 a\\f2 b}}"); + let (_, warned) = crate::test_log::warnings(|| crate::rtf::import(rtf.as_bytes()).unwrap()); + assert_eq!(count(&warned, "font names longer"), 1, "{warned:?}"); + assert_eq!(count(&warned, "style names longer"), 1, "{warned:?}"); + + let tex = format!("\\href{{https://e.com/{long}}}{{a}} \\url{{https://e.com/{long}}}"); + let (_, warned) = crate::test_log::warnings(|| crate::latex::import(tex.as_bytes())); + assert_eq!(count(&warned, "links longer"), 1, "{warned:?}"); } From 124243b1becc8056b23c8ddb210d7fa193420f31 Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Fri, 23 Oct 2026 13:20:09 +0100 Subject: [PATCH 12/13] Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01CX1hMKX28764Lrx5moxs1o --- docs/file-format-parity.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 2f58bcbe1..1ba787f06 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -271,6 +271,7 @@ Repeated DOCX style IDs use the last complete definition. The original regressio > **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-25 · **Change:** trivial (the `.doc` reader logs what its copy budget leaves out, once a kind per document; previously trivial: repeated DOCX style IDs: save/reopen regression; previously trivial: Markdown code spans keep line-broken text literal, #3738; previously trivial: Arabic list labels: `arabicAlpha` and `arabicAbjad` draw the 28 Arabic letters (alphabetical and abjad order), doubling past the last letter; Hebrew, Hindi, Thai and Vietnamese still show decimal (`doc::section::tests::arabic_letters_count_in_both_orders`); previously trivial: localized TOC style ids read as the TOC styles, and a TOC in columns keeps its layout through Update Table, #3889; previously trivial: cu20we: outline caps; previously trivial: a relationship the markup names without the prefix is read as it wrote: an unqualified `id` on an element that carries a relationship names that relationship, and the writer keeps the attribute with the id the new package holds the part under; an ordinary `id` — a VML shape's, a drawing's — names no part; previously trivial: the reader takes the relationship ids the markup refers to from the relationship attributes of the main document it has already parsed, so a document with many relationships opens in the time its part takes, #3411; previously trivial: a table cell span from a `.json` file stays within 63 columns through Insert Column and Merge Cells, #3637; previously trivial: the file commands that change the document are checked on a protected document, the protection of a protected document needs an explicit stop, the session author can't change through `file.setAuthor` under protection (the CLI's `--author` uses the same protection check), and the post-check compares the settings and the metadata; previously trivial: a document with many content controls saves in the time the controls take, #13; previously trivial: `majorBidi`/`minorBidi` theme fonts follow the settings' theme font language, as measured in Word; previously trivial: Word signature lines are kept on save, still signable; previously trivial: Times New Roman keeps Liberation Serif under the roman-font rule; previously trivial: shape gradients and transparency, #1500; previously trivial: a folder that keeps no Unix modes, such as a NAS share mounted by GVFS, still takes a save, #3003; previously trivial: a text frame in the paragraph defaults frames nothing, #2360; previously trivial: table style band sizes written, a style without them unbanded as in Word, #3415; previously trivial: HTML export writes ruby (furigana, phonetic guides) and HTML import reads it back, #1681; previously trivial: RTF import decodes the East Asian multibyte code pages, #1653; previously trivial: PDF export: lam-alef drawn as two glyphs at one x extracts in reading order, #2310; previously trivial: native .json stores pictures and kept parts as base64, #586 follow-up; previously trivial: ODT headers and footers read from each master page, #863; previously trivial: RTF import compares heading and code formatting with the file's stylesheet; previously trivial: text typed into a content control takes the control's formatting; localized placeholder styles recognised; previously trivial: forms protection keeps its content controls: no inserting, Properties or Remove Content Control; previously trivial: Word XML and web pages saved as `.doc` open by what they hold, #2853; previously trivial: a VML shape in a header cell is always placed on the page, user-checked in Word for Mac, #1775; previously trivial: `w:sym` characters keep their symbol font when the text's font changes and are written back as `w:sym`, #2036; previously trivial: RTF export writes sections (gap 19); previously trivial: ODT export writes sections (gap 19); previously trivial: two lines in one and horizontal in vertical (`w:eastAsianLayout`) kept on save; previously trivial: PDF outline targets follow leading page breaks; previously trivial: positional tabs `w:ptab` read and kept on save, #2367; previously trivial: DOCX: mixed right-to-left runs split and marked `w:rtl` with their language, #1543; previously trivial: `w:doNotTrackMoves` round-trips and turns move tracking off; previously trivial: legacy outline, shadow, emboss and imprint text effects drawn, #2534; previously trivial: DOCX glossary documents (building blocks) read, kept and written; previously trivial: cached field results keep their visible run formatting; previously trivial: cached REF results retain hyperlink destinations; previously trivial: CLI conversions can explicitly refresh fields; previously trivial: a copied equation also goes in the clipboard's MathML formats, #1297; previously trivial: a pasted HTML `
` is an indented paragraph, not the Quote style, #1270); previously trivial: every table style part round-trips, #714; previously trivial: VML freeforms read and kept on save; previously trivial: multi-paragraph field instructions no longer add blank lines; previously trivial: `flowChartOr` draws its circle and cross spokes, as in Word; previously trivial: RTF: Mac OS Roman and DOS text, #1653; previously trivial: VML text boxes keep their internal margins, fit to their text and vertical alignment; hidden drawings not shown; previously trivial: PDF export draws horizontal and vertical lines inside groups, #1474; previously trivial: symbol-font text exported as Unicode, #776; previously trivial: text-box margins, Resize shape to fit text and freeforms in groups, #743; previously trivial: formatted paste from the system clipboard's HTML and RTF, #687; previously trivial: independent complex-script formatting inheritance and round-trip regression tests; previously trivial: import/export fixes for TXT, RTF, ODT, HTML and LaTeX, #433–#440; previously major: first version; every format Word reads or writes, measured from the readers and writers on origin/main) · **Target:** Microsoft Word (Microsoft 365) for Mac 16.113.4, plus Word for Windows' extra formats | 2026-10-23 | trivial | `.doc` reader logs what its copy budget leaves out (floating objects, link targets, alt text, Prm formatting), once a kind per document (#3692) | | 2026-10-22 | trivial | `.doc` reader bounds hostile files: oversized sector tables refused, per-character re-reads removed, copy and decode budgets (#3642, #3692, #3708-#3712) | +| 2026-10-22 | trivial | Importer hardening (#3692, #3705, #3713, #3714): spreadsheet header numbering and ODS repeated cells, LaTeX wrapper nesting and verbatim ends, Markdown delimiter runs, blank-span trimming and link destinations, and font/link copies per run are bounded or escaped; normal documents unchanged | | 2026-10-23 | trivial | A relationship the markup names without the prefix is read as it wrote: an unqualified `id` on an element that carries a relationship names that relationship, and the writer keeps the attribute with the id the new package holds the part under; an ordinary `id` — a VML shape's, a drawing's — names no part (#3411 follow-up) | | 2026-10-21 | trivial | The reader takes the relationship ids the markup refers to from the relationship attributes of the main document it has already parsed, so a document with many relationships opens in the time its part takes, and an id that only appears as text is not a reference (#3411) | | 2026-10-23 | trivial | DOCX: TOC styles Word stores under ids in another language (`Sumrio1` named `toc 1`, `CabealhodoSumrio` named `TOC Heading`) read as `TOC1`…`TOC9` and `TOCHeading` unless the file uses those ids, so Update Table builds the entries in the document's own TOC styles (#3891); a TOC whose heading ends a section (entries in columns after a continuous break) is written and read as the engine's TOC, with the page-number tab at the column's edge (#3889) | From 22df7caf4deac7dfd4e43a81713b72de8898d18f Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Fri, 23 Oct 2026 14:50:14 +0100 Subject: [PATCH 13/13] =?UTF-8?q?#3692:=20review=20=E2=80=94=20formats=20t?= =?UTF-8?q?iming=20tests=20get=20a=20margin=20a=20busy=20CI=20machine=20ca?= =?UTF-8?q?n't=20break?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit many_formatted_leading_breaks_trim_quickly failed at its 2 s bound on a machine at load ~50 (it takes about a second there). Bounds now sit at least 10x over the fixed code's time on a loaded machine and far under the old code's on the same input (measured by running the pre-fix code, capped at 400 s): - HTML leading breaks: 2 s -> 20 s; ~1 s now, old trim ran past 6 minutes. - LaTeX nested multicolumn: 2 s -> 10 s; under 1 s now, old unwrapping took 213 s. - Markdown delimiters: 2 s -> 10 s; under 1 s now, old backwards search past 6 minutes. - Para::trim blank spans: 1 s -> 10 s; 0.16 s now, old Vec::remove(0) past 6 minutes. - repeated sheet headers: 2 s -> 10 s; 20 ms now, old numbering past 6 minutes. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01CX1hMKX28764Lrx5moxs1o --- crates/formats/src/html.rs | 3 ++- crates/formats/src/latex.rs | 3 ++- crates/formats/src/markdown.rs | 3 ++- crates/formats/src/model.rs | 3 ++- crates/formats/src/sheet.rs | 3 ++- 5 files changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/formats/src/html.rs b/crates/formats/src/html.rs index f1eb1693c..a3d76578c 100644 --- a/crates/formats/src/html.rs +++ b/crates/formats/src/html.rs @@ -2435,7 +2435,8 @@ mod tests { let html = format!("

{}x

", "

".repeat(100_000)); let t = std::time::Instant::now(); let f = parse(&html); - assert!(t.elapsed() < std::time::Duration::from_secs(2), "{:?}", t.elapsed()); + // About a second on a loaded machine; trimming one span at a time ran past 6 minutes. + assert!(t.elapsed() < std::time::Duration::from_secs(20), "{:?}", t.elapsed()); let texts: Vec = f.blocks.iter().filter_map(|b| if let FBlock::Para(p) = b { Some(p.text()) } else { None }).collect(); assert_eq!(texts, ["x"]); } diff --git a/crates/formats/src/latex.rs b/crates/formats/src/latex.rs index a79f3a9f0..d2c35f3cb 100644 --- a/crates/formats/src/latex.rs +++ b/crates/formats/src/latex.rs @@ -3264,7 +3264,8 @@ Caf\'e na\"ive \c{c}a \v{s}\'{e} and {\bfseries grouped bold} after. let src = format!("\\begin{{tabular}}{{l}}{}x{}\\\\\\end{{tabular}}", "\\multicolumn{1}{c}{".repeat(n), "}".repeat(n)); let t = std::time::Instant::now(); let f = parse(&src); - assert!(t.elapsed() < std::time::Duration::from_secs(2), "{:?}", t.elapsed()); + // Under a second on a loaded machine; unwrapping every wrapper took 213 s. + assert!(t.elapsed() < std::time::Duration::from_secs(10), "{:?}", t.elapsed()); assert!(f.blocks.iter().any(|b| matches!(b, FBlock::Table(_)))); // One or two wrappers, as written in real tables, are still read. let f = parse("\\begin{tabular}{ll}\\multicolumn{2}{c}{\\multirow{2}{*}{Both}}\\\\\\end{tabular}"); diff --git a/crates/formats/src/markdown.rs b/crates/formats/src/markdown.rs index 56d631e7a..9196c1968 100644 --- a/crates/formats/src/markdown.rs +++ b/crates/formats/src/markdown.rs @@ -2203,7 +2203,8 @@ mod tests { let src = "*_".repeat(200_000); let t = std::time::Instant::now(); let out = inl(&src); - assert!(t.elapsed() < std::time::Duration::from_secs(2), "{:?}", t.elapsed()); + // Under a second on a loaded machine; the backwards search ran past 6 minutes. + assert!(t.elapsed() < std::time::Duration::from_secs(10), "{:?}", t.elapsed()); let text: String = out.iter().filter_map(|i| if let Inline::Text(t, _) = i { Some(t.as_str()) } else { None }).collect(); assert!(text.chars().all(|c| matches!(c, '*' | '_')), "only delimiters come out"); // Literal delimiters still take the link of the text before them. diff --git a/crates/formats/src/model.rs b/crates/formats/src/model.rs index 61ae4aea5..ccc1f1c0c 100644 --- a/crates/formats/src/model.rs +++ b/crates/formats/src/model.rs @@ -1669,7 +1669,8 @@ mod tests { p.push_text(" x ", &Fmt::default()); let t = std::time::Instant::now(); p.trim(); - assert!(t.elapsed() < std::time::Duration::from_secs(1), "{:?}", t.elapsed()); + // A fraction of a second; `Vec::remove(0)` per span ran past 6 minutes. + assert!(t.elapsed() < std::time::Duration::from_secs(10), "{:?}", t.elapsed()); assert_eq!(p.inlines, vec![Inline::Text("x".into(), Fmt::default())]); // The first span left keeps its formatting and loses only its leading blanks. let mut p = Para::default(); diff --git a/crates/formats/src/sheet.rs b/crates/formats/src/sheet.rs index e3ff85f29..f7f300812 100644 --- a/crates/formats/src/sheet.rs +++ b/crates/formats/src/sheet.rs @@ -1252,7 +1252,8 @@ mod tests { 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()); + // Milliseconds; the old numbering ran past 6 minutes. + assert!(t.elapsed() < std::time::Duration::from_secs(10), "{:?}", t.elapsed()); assert_eq!(names.len(), 5_000); assert_eq!(names[0], "Name"); assert_eq!(names[4_999], "Name 5000");