#3713: review — importer limits that drop fonts, links, style names or sheet rows log a warning once per document

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CX1hMKX28764Lrx5moxs1o

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())
 }
 

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

crates/formats/src/lib.rs

@@ -91,5 +91,7 @@ pub fn export_with(ext: &str, doc: &Document, opts: &ExportOptions) -> Option<Re
     Some(out.map(|bytes| comments::append(&ext, doc, bytes)))
 }
 
+#[cfg(test)]
+mod test_log;
 #[cfg(test)]
 mod tests;

crates/formats/src/markdown.rs

@@ -25,11 +25,13 @@ const MAX_NEST: usize = 32;
 
 /// Parse Markdown text into a document.
 pub fn import(text: &str) -> 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<Node> {
                         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));
                         }

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<Arc<str>> {
-    (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<Arc<str>> {
-    (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<u8> = 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
     }

crates/formats/src/odt.rs

@@ -2627,6 +2627,7 @@ fn parse_tracked(bytes: &[u8]) -> Result<(Flow, SectionProps, Option<bool>, Opti
 
 /// Parse an `.odt` package into a document.
 pub fn import(bytes: &[u8]) -> Result<Document, String> {
+    model::begin_import();
     let (flow, last, track, font, parts) = parse_tracked(bytes)?;
     let mut doc = model::to_doc(&flow);
     if font.is_some() {

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<Cell>], rights: &[Vec<i32>]) -> Vec<f32> {
 
 /// Parse RTF bytes into a document.
 pub fn import(bytes: &[u8]) -> Result<Document, String> {
+    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.

crates/formats/src/sheet.rs

@@ -343,6 +343,8 @@ struct Grid {
     rows: Vec<Vec<String>>,
     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<String>, 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<usize>) -> Result<(Vec<String>, Vec<Vec<Str
                     if *bytes > 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#"<table:table-row><table:table-cell office:value-type="string" office:string-value="{wide}" table:number-columns-repeated="1000"/></table:table-row><table:table-row><table:table-cell><text:p>next</text:p></table:table-cell></table:table-row>"#
         ));
-        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#"<table:table-row><table:table-cell office:value-type="string" office:string-value="{wide}" table:number-columns-repeated="1000"/></table:table-row>"#
+        );
+        let book = ods(&format!("{over}{over}{over}<table:table-row><table:table-cell><text:p>next</text:p></table:table-cell></table:table-row>"));
+        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("<table:table-row><table:table-cell><text:p>fine</text:p></table:table-cell></table:table-row>");
+        let (_, warned) = crate::test_log::warnings(|| read_rows(&book, Pick::Index(0)).unwrap());
+        assert!(warned.is_empty(), "{warned:?}");
     }
 
     #[test]

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<Vec<String>> = 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<T>(f: impl FnOnce() -> T) -> (T, Vec<String>) {
+    // 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())))
+}

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::<usize>(), 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!(
+        "<p><span style=\"font-family:'{long}'\"><a href=\"https://e.com/{long}\">a</a></span> <font face=\"{long}\">b</font> <a href=\"https://e.com/{long}\">c</a></p>"
+    );
+    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("<p><a href=\"https://e.com\" style=\"font-family:Georgia\">fine</a></p>"));
+    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:?}");
 }