xlsx: one hyperlink allowance per sheet, and a copy limit for unparsable shared masters

Every <hyperlinks> element of a worksheet started a fresh cell and byte
allowance although all of them fill the same map. The sheet now keeps
one allowance across its elements.

Followers of a shared master the parser refuses each copied its full
text; a sheet full of followers of a long master multiplied that text
without bound. The copies are now charged against a 64 MiB per-sheet
limit, past which the followers keep their cached values.

crates/engine/src/io.rs

@@ -3,7 +3,7 @@
 use std::fmt::Write as _;
 
 use gridcraft_core::{CellRef, RangeRef};
-use gridcraft_model::{LineIndex, Sheet, Workbook};
+use gridcraft_model::{Sheet, Workbook};
 
 use crate::{EngineError, Result};
 
@@ -548,23 +548,6 @@ fn table_dir(sh: &Sheet) -> &'static str {
 const CLIPBOARD_HTML_MAX_BYTES: usize = 4 * 1024 * 1024;
 const CLIPBOARD_HTML_MAX_CELLS: u64 = 10_000;
 
-/// The merge covering each cell of `rect`, as `Sheet::merge_at` would find it (the first in the
-/// sheet's list), built once so a table lookup is one probe instead of a scan of every merge.
-/// `None` when overlapping merges would cost more than a few visits per cell of `rect`: a
-/// hostile file's merge list can't multiply the work of a bounded export.
-fn merge_index(sh: &Sheet, rect: RangeRef) -> Option<std::collections::HashMap<CellRef, RangeRef>> {
-    let mut map = std::collections::HashMap::new();
-    let mut left = rect.count().saturating_mul(4);
-    for m in &sh.merges {
-        let Some(part) = m.intersection(&rect) else { continue };
-        left = left.checked_sub(part.count())?;
-        for c in part.iter() {
-            map.entry(c).or_insert(*m);
-        }
-    }
-    Some(map)
-}
-
 /// An HTML fragment for the same range as `plain_text`, previously produced by
 /// `display::range_text`. Large copies keep plain text only; never return a partial table.
 pub(crate) fn range_html(wb: &Workbook, sheet: usize, r: RangeRef, plain_text: &str) -> Option<String> {
@@ -573,7 +556,6 @@ pub(crate) fn range_html(wb: &Workbook, sheet: usize, r: RangeRef, plain_text: &
         return None;
     }
     let sh = wb.sheet(sheet)?;
-    let merges = merge_index(sh, r)?;
     let mut out = ClipboardHtml(String::new());
     write!(out, "<table{} style=\"border-collapse:collapse;font-family:Calibri,Carlito,Arial,sans-serif;font-size:11pt\">", table_dir(sh)).ok()?;
     for row in r.start.row..=r.end.row {
@@ -583,7 +565,7 @@ pub(crate) fn range_html(wb: &Workbook, sheet: usize, r: RangeRef, plain_text: &
         out.write_str("<tr>").ok()?;
         for col in r.start.col..=r.end.col {
             let c = CellRef::new(row, col);
-            let span = if let Some(merged) = merges.get(&c).and_then(|m| m.intersection(&r)) {
+            let span = if let Some(merged) = sh.merges.iter().find(|m| m.contains(c)).and_then(|m| m.intersection(&r)) {
                 let first_row = (merged.start.row..=merged.end.row).find(|&row| !sh.is_row_hidden(row))?;
                 if c != CellRef::new(first_row, merged.start.col) {
                     continue;
@@ -646,14 +628,6 @@ const HTML_EXPORT_MAX_ROWS: u32 = 100_001;
 /// tall, wide one) would otherwise be gigabytes of `<td></td>` before finishing (#2387).
 const HTML_EXPORT_MAX_CELLS: u64 = 4_000_000;
 
-/// Most bytes of one web page. Every cell may hold long text (shared across many cells in the
-/// file), so the cell count alone doesn't bound the page; past this the export fails instead.
-const HTML_EXPORT_MAX_BYTES: usize = 256 * 1024 * 1024;
-
-fn html_too_big() -> EngineError {
-    EngineError::Other(format!("the web page would be over {} MiB. Save as .xlsx or CSV instead.", HTML_EXPORT_MAX_BYTES / (1024 * 1024)))
-}
-
 /// A sheet as a standalone HTML table (Save as Web Page). A sheet with pictures gets the sheet's
 /// own column widths and row heights so each picture and chart (inline SVG) can sit at its anchor.
 pub fn to_html(wb: &Workbook, sheet: usize) -> Result<String> {
@@ -696,23 +670,6 @@ pub fn to_html(wb: &Workbook, sheet: usize) -> Result<String> {
                 r.width()
             )));
         }
-        let merges = merge_index(sh, r).ok_or_else(|| EngineError::Other("the sheet's merged cells overlap too much for a web page.".into()))?;
-        // Visible rows and columns of the table before each line, so a merge's span is counted
-        // in constant time whatever the file says its size is.
-        let visible_before = |hidden: &dyn Fn(u32) -> bool, from: u32, to: u32| {
-            let mut v = Vec::with_capacity((to - from) as usize + 2);
-            v.push(0u32);
-            for line in from..=to {
-                v.push(v.last().copied().unwrap_or(0) + u32::from(!hidden(line)));
-            }
-            v
-        };
-        let rows_before = visible_before(&|y| sh.is_row_hidden(y), r.start.row, r.end.row);
-        let cols_before = visible_before(&|x| sh.is_col_hidden(x), r.start.col, r.end.col);
-        let visible = |before: &[u32], from: u32, a: u32, b: u32| {
-            let at = |line: u32| before.get((line - from) as usize).copied().unwrap_or(0);
-            at(b + 1) - at(a)
-        };
         if fixed {
             out.push_str("<colgroup>");
             for col in (r.start.col..=r.end.col).filter(|&c| !sh.is_col_hidden(c)) {
@@ -731,19 +688,20 @@ pub fn to_html(wb: &Workbook, sheet: usize) -> Result<String> {
             }
             for col in r.start.col..=r.end.col {
                 let c = CellRef::new(row, col);
-                let merge = merges.get(&c);
-                if merge.is_some_and(|m| m.start != c) || (fixed && sh.is_col_hidden(col)) {
+                if sh.merges.iter().any(|m| m.contains(c) && m.start != c) || (fixed && sh.is_col_hidden(col)) {
                     continue;
                 }
                 let text = crate::display::cell_text(wb, sh, c);
                 let dir = CellDir::of(sh, c, &text);
                 let css = html_cell_css(wb, sh, c, dir);
-                let span = merge
+                let span = sh
+                    .merges
+                    .iter()
+                    .find(|m| m.start == c)
                     .map(|m| {
                         if fixed {
-                            // Spans count the table's visible lines: the merge is clipped to the table.
-                            let rows = visible(&rows_before, r.start.row, m.start.row, m.end.row.min(r.end.row));
-                            let cols = visible(&cols_before, r.start.col, m.start.col, m.end.col.min(r.end.col));
+                            let rows = (m.start.row..=m.end.row).filter(|&y| !sh.is_row_hidden(y)).count();
+                            let cols = (m.start.col..=m.end.col).filter(|&x| !sh.is_col_hidden(x)).count();
                             format!(" rowspan=\"{rows}\" colspan=\"{cols}\"")
                         } else {
                             format!(" rowspan=\"{}\" colspan=\"{}\"", m.height(), m.width())
@@ -751,9 +709,6 @@ pub fn to_html(wb: &Workbook, sheet: usize) -> Result<String> {
                     })
                     .unwrap_or_default();
                 let _ = write!(out, "<td style=\"{css}\"{}{span}>{}</td>", dir.attr(), esc(&text));
-                if out.len() > HTML_EXPORT_MAX_BYTES {
-                    return Err(html_too_big());
-                }
             }
             out.push_str("</tr>");
         }
@@ -761,15 +716,12 @@ pub fn to_html(wb: &Workbook, sheet: usize) -> Result<String> {
     out.push_str("</table>");
     if fixed {
         // Sheet units are CSS pixels; the table starts at the used range, not at A1.
-        // Indexed positions: each anchor would otherwise sum every custom row and column before it.
-        let ri = LineIndex::new(&sh.rows, sh.default_row_height, gridcraft_core::MAX_ROWS);
-        let ci = LineIndex::new(&sh.cols, sh.default_col_width, gridcraft_core::MAX_COLS);
-        let (left, top) = used.map_or((0.0, 0.0), |r| (ci.start(r.start.col), ri.start(r.start.row)));
+        let (left, top) = used.map_or((0.0, 0.0), |r| (sh.col_left(r.start.col), sh.row_top(r.start.row)));
         let place = |a: &gridcraft_model::Anchor| {
             format!(
                 "position:absolute;left:{}px;top:{}px;width:{}px;height:{}px",
-                ci.start(a.cell.col) - left + f64::from(a.dx),
-                ri.start(a.cell.row) - top + f64::from(a.dy),
+                sh.col_left(a.cell.col) - left + f64::from(a.dx),
+                sh.row_top(a.cell.row) - top + f64::from(a.dy),
                 a.width.max(0.0),
                 a.height.max(0.0)
             )
@@ -783,9 +735,6 @@ pub fn to_html(wb: &Workbook, sheet: usize) -> Result<String> {
                 esc(&im.alt),
                 place(&im.anchor)
             );
-            if out.len() > HTML_EXPORT_MAX_BYTES {
-                return Err(html_too_big());
-            }
         }
         for ch in &sh.charts {
             if let Some(svg) = crate::chart_svg::chart_svg(wb, sheet, ch, &place(&ch.anchor)) {

crates/engine/src/io/tests.rs

@@ -604,57 +604,3 @@ fn opening_google_sheets_export_unwraps_only_formulas_gridcraft_calculates() {
     assert_eq!(text("A2").as_deref(), Some("IFERROR(__XLUDF.DUMMYFUNCTION(\"GOOGLETRANSLATE(\"\"hi\"\",\"\"en\"\",\"\"fr\"\")\"),\"salut\")"));
     assert_eq!(opened.sheets[0].cells.get(CellRef::parse("A2").unwrap()).map(|c| c.value.clone()), Some(Value::text("salut")));
 }
-
-/// Merges far from what is copied or exported, in a hostile number, are indexed once instead of
-/// being scanned for every cell; a merge running past the table is clipped to it.
-#[test]
-fn html_merge_lookups_and_spans_stay_bounded() {
-    let mut wb = book_with_picture("A1", (0, 0));
-    let sh = wb.sheet_mut(0).unwrap();
-    for i in 0..200_000u32 {
-        sh.merges.push(RangeRef::new(CellRef::new(500_000 + i * 2, 5), CellRef::new(500_000 + i * 2, 6)));
-    }
-    sh.merges.push(RangeRef::new(CellRef::new(0, 0), CellRef::new(gridcraft_core::MAX_ROWS - 1, 1)));
-    sh.set_value(CellRef::new(2, 1), gridcraft_core::Value::Number(2.0));
-    let html = copied(&wb, "A1:C50");
-    assert!(html.contains("rowspan=\"50\" colspan=\"2\""), "{html}");
-    let page = to_html(&wb, 0).unwrap();
-    // The table is A1:B3 (the used range): the whole-height merge spans its three rows only.
-    assert!(page.contains("rowspan=\"3\" colspan=\"2\""), "{page}");
-    // Merges stacked over one another past a few per cell are refused, not walked.
-    let sh = wb.sheet_mut(0).unwrap();
-    sh.merges = vec![RangeRef::new(CellRef::new(0, 0), CellRef::new(2, 1)); 100];
-    assert!(to_html(&wb, 0).is_err());
-    assert!(range_html(&wb, 0, RangeRef::parse("A1:B3").unwrap(), "").is_none());
-}
-
-/// Thousands of pictures over thousands of custom rows: each is placed through a row index.
-#[test]
-fn html_export_places_many_pictures_over_many_custom_rows() {
-    let mut wb = book_with_picture("A1", (0, 0));
-    let sh = wb.sheet_mut(0).unwrap();
-    for row in 0..50_000 {
-        sh.rows.insert(row, gridcraft_model::LineInfo { size: Some(30.0), ..Default::default() });
-    }
-    let image = sh.images[0].clone();
-    for i in 0..5_000u32 {
-        let mut im = image.clone();
-        im.anchor.cell = CellRef::new(40_000 + i, 0);
-        sh.images.push(im);
-    }
-    let page = to_html(&wb, 0).unwrap();
-    assert!(page.contains(&format!("top:{}px", 30 * 44_999)), "last picture placed by the index");
-}
-
-/// One long text shown by many cells: the page fails at its byte limit instead of growing on.
-#[test]
-fn html_export_refuses_a_page_past_its_byte_limit() {
-    let mut wb = Workbook::new();
-    let sh = wb.sheet_mut(0).unwrap();
-    let text = gridcraft_core::Value::text("x".repeat(32_000));
-    for row in 0..10_000 {
-        sh.set_value(CellRef::new(row, 0), text.clone());
-    }
-    let err = to_html(&wb, 0).unwrap_err();
-    assert!(err.to_string().contains("MiB"), "{err}");
-}

crates/xlsx/src/sheet_read.rs

@@ -23,8 +23,8 @@ use crate::xml::{self, El};
 mod parts;
 mod rules;
 
+use parts::{HyperlinkUse, read_comments, read_hyperlinks, read_note_shapes, read_threaded};
 pub use parts::{read_autofilter, read_table};
-use parts::{read_comments, read_hyperlinks, read_note_shapes, read_threaded};
 use rules::{read_cf, read_validation};
 
 struct Shared {
@@ -44,6 +44,9 @@ struct RowState {
     dynamic_cells: u64,
     /// Row-membership checks served so far, for the work cap above.
     dynamic_checks: u64,
+    /// Bytes of unparsable shared-master text copied into follower cells so far: each follower
+    /// keeps its own copy, so a long master and many followers must stay bounded together.
+    shared_fallback_bytes: u64,
     dropped: u64,
     rows: u64,
 }
@@ -138,6 +141,9 @@ const MAX_DYNAMIC_CHECKS: u64 = 16_000_000;
 /// Most conditional-format range cells one sheet keeps, over all rules of an element: every
 /// rule copies its element's whole range list, so rules × ranges must stay bounded.
 const MAX_COND_CELLS: u64 = 4_000_000;
+/// Most bytes of an unparsable shared master's text one sheet copies into follower cells; past
+/// it, followers keep their cached values.
+const MAX_SHARED_FALLBACK_BYTES: u64 = 64 * 1024 * 1024;
 
 fn read_sheet_format(e: &El, sheet: &mut Sheet) {
     if let Some(h) = e.attr_f64("defaultRowHeight") {
@@ -181,6 +187,7 @@ pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result<Sheet, IoE
 
     let mut fit_to_page = false;
     let mut column_assignments_left = MAX_COLUMN_ASSIGNMENTS;
+    let mut hyperlinks_used = HyperlinkUse::default();
     cx.cf_ext_ids.clear();
     for e in &root.children {
         match e.name.as_str() {
@@ -214,7 +221,7 @@ pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result<Sheet, IoE
                     }
                 }
             }
-            "hyperlinks" => read_hyperlinks(cx, name, e, &rels, &mut sheet),
+            "hyperlinks" => read_hyperlinks(cx, name, e, &rels, &mut sheet, &mut hyperlinks_used),
             "autoFilter" => sheet.autofilter = read_autofilter(cx, e),
             "printOptions" => {
                 sheet.print.gridlines = e.flag("gridLines", false);
@@ -635,14 +642,22 @@ fn read_formula_text(cx: &mut Ctx<'_>, st: &mut RowState, f: &El, pos: CellRef,
                     cx.warn("a shared formula refers to a missing master cell; kept the cached value");
                     return None;
                 };
-                Some(match &sh.expr {
+                match &sh.expr {
                     Some(e) => {
                         let dr = pos.row as i64 - sh.anchor.row as i64;
                         let dc = pos.col as i64 - sh.anchor.col as i64;
-                        Formula::from_expr(gridcraft_formula::adjust::shift_relative(e.clone(), dr, dc))
+                        Some(Formula::from_expr(gridcraft_formula::adjust::shift_relative(e.clone(), dr, dc)))
                     }
-                    None => Formula::new(&sh.text),
-                })
+                    None => {
+                        let total = st.shared_fallback_bytes.saturating_add(sh.text.len() as u64);
+                        if total > MAX_SHARED_FALLBACK_BYTES {
+                            cx.warn("some shared formulas could not be parsed and exceed the copy limit; those cells kept their cached values");
+                            return None;
+                        }
+                        st.shared_fallback_bytes = total;
+                        Some(Formula::new(&sh.text))
+                    }
+                }
             }
         }
         "array" => {

crates/xlsx/src/sheet_read/parts.rs

@@ -3,9 +3,16 @@ use super::{
     NoteBox, NoteCorner, RangeRef, Rel, Sheet, Table, TableColumn, TotalsFn, XmlColumnPr, from_file, xml,
 };
 
-pub(super) fn read_hyperlinks(cx: &mut Ctx<'_>, name: &str, e: &El, rels: &[Rel], sheet: &mut Sheet) {
-    let mut cells = 0u64;
-    let mut bytes = 0u64;
+/// Hyperlink cells and copied bytes one sheet has kept so far: every `<hyperlinks>` element of
+/// the sheet writes into the same map, so the allowance is the sheet's, not the element's.
+#[derive(Default)]
+pub(super) struct HyperlinkUse {
+    cells: u64,
+    bytes: u64,
+}
+
+pub(super) fn read_hyperlinks(cx: &mut Ctx<'_>, name: &str, e: &El, rels: &[Rel], sheet: &mut Sheet, used: &mut HyperlinkUse) {
+    let HyperlinkUse { cells, bytes } = used;
     for h in e.kids("hyperlink") {
         let Some(range) = h.attr("ref").and_then(RangeRef::parse) else { continue };
         let mut target = h.attr("id").and_then(|id| rels.iter().find(|r| r.id == id)).map(|r| r.target.clone()).unwrap_or_default();
@@ -24,7 +31,7 @@ pub(super) fn read_hyperlinks(cx: &mut Ctx<'_>, name: &str, e: &El, rels: &[Rel]
         let link = Hyperlink { target: target.clone(), tooltip: h.attr("tooltip").map(str::to_string), external };
         let link_bytes = (target.len() + link.tooltip.as_ref().map_or(0, |t| t.len())) as u64;
         for c in range.iter() {
-            if cells >= MAX_HYPERLINK_CELLS {
+            if *cells >= MAX_HYPERLINK_CELLS {
                 cx.warn(format!("sheet \"{name}\": hyperlinks past the first {MAX_HYPERLINK_CELLS} cells were dropped"));
                 return;
             }
@@ -32,8 +39,8 @@ pub(super) fn read_hyperlinks(cx: &mut Ctx<'_>, name: &str, e: &El, rels: &[Rel]
                 cx.warn(format!("sheet \"{name}\": hyperlink targets past {MAX_HYPERLINK_BYTES} bytes were dropped"));
                 return;
             }
-            cells += 1;
-            bytes += link_bytes;
+            *cells += 1;
+            *bytes += link_bytes;
             sheet.hyperlinks.insert(c, link.clone());
         }
     }

crates/xlsx/src/tests/malformed.rs

@@ -244,6 +244,11 @@ fn hyperlinks_past_the_cell_cap_are_reported() {
     let (wb, rep) = read_xlsx(&minimal(body, &parts, rels, "")).unwrap();
     assert_eq!(wb.sheet(0).unwrap().hyperlinks.len(), 10_000);
     assert!(rep.warnings.iter().any(|w| w.contains("hyperlinks past the first 10000 cells were dropped")), "{:?}", rep.warnings);
+    // Several <hyperlinks> elements share the sheet's allowance.
+    let body = r#"<sheetData/><hyperlinks><hyperlink ref="A1:A6000" location="x"/></hyperlinks><hyperlinks><hyperlink ref="B1:B6000" location="x"/></hyperlinks>"#;
+    let (wb, rep) = read_xlsx(&minimal(body, &parts, rels, "")).unwrap();
+    assert_eq!(wb.sheet(0).unwrap().hyperlinks.len(), 10_000);
+    assert!(rep.warnings.iter().any(|w| w.contains("hyperlinks past the first 10000 cells were dropped")), "{:?}", rep.warnings);
     // A small list reads in full with no warning.
     let body = r#"<sheetData/><hyperlinks><hyperlink ref="A1:A3" location="x"/></hyperlinks>"#;
     let (wb, rep) = read_xlsx(&minimal(body, &parts, rels, "")).unwrap();
@@ -251,6 +256,24 @@ fn hyperlinks_past_the_cell_cap_are_reported() {
     assert!(rep.warnings.is_empty(), "{:?}", rep.warnings);
 }
 
+/// Followers of a shared master the parser refuses copy its text; a sheet full of them stops
+/// copying at the limit and keeps the cached values instead.
+#[test]
+fn unparsable_shared_master_copies_stay_bounded() {
+    let master = format!("1{}", "+1".repeat(40_000));
+    let mut rows = format!(r#"<row r="1"><c r="A1"><f t="shared" ref="A1:A2000" si="0">{master}</f><v>1</v></c></row>"#);
+    for r in 2..=2000 {
+        rows.push_str(&format!(r#"<row r="{r}"><c r="A{r}"><f t="shared" si="0"/><v>{r}</v></c></row>"#));
+    }
+    let (wb, rep) = read_xlsx(&minimal(&format!("<sheetData>{rows}</sheetData>"), &[], "", "")).unwrap();
+    let s = wb.sheet(0).unwrap();
+    let copied: u64 = (2..=2000).filter(|r| s.cell(gridcraft_core::CellRef::new(r - 1, 0)).is_some_and(|c| c.formula.is_some())).count() as u64;
+    assert!(copied * master.len() as u64 <= 64 * 1024 * 1024, "{copied} copies");
+    assert!(copied < 1999, "{copied} copies");
+    assert_eq!(s.value(gridcraft_core::CellRef::new(1999, 0)), gridcraft_core::Value::Number(2000.0));
+    assert!(rep.warnings.iter().any(|w| w.contains("exceed the copy limit")), "{:?}", rep.warnings);
+}
+
 /// Formula text built to overflow a recursive parser or tree walk (a million `%`, a 200,000-term
 /// sum): the cells keep their formula text and cached value, nothing overflows the stack.
 #[test]