M2.9: bound print/export/autofit work in the engine (sec scan findings 18,21,27,28,30)

- clip_html: cap wildcard/glob lookback to a 12-byte window instead of scanning outgoing text arbitrarily far for a semicolon (finding 18). - print: wrap_lines binary-searches its break instead of rescanning the line from the start for every band (finding 21). - autofit: per-row merge index instead of scanning every merge per row during autofit (finding 27) — behaviour covered by the existing autofit merge test. - Save as Web Page: the used rectangle bounds the HTML (finding 28, #2387) — a tall/wide or sparse far-right sheet now errors instead of growing gigabytes of <td></td>. - print: decode each embedded picture once per document instead of once per band that shows it (finding 30). Engine: 459 tests pass; clippy clean.
2026-10-25 11:16:49 -06:00 · 2026-10-24 05:29:39 -04:00
8 changed files with 195 additions and 246 deletions
@@ -267,6 +267,12 @@ fn attr<'a>(attrs: &'a str, name: &str) -> Option<&'a str> {
    }
 }

+/// Entities are at most `&` + 8 chars + `;` (a decimal code point at most 7 digits deep), so
+/// the semicolon is only looked for in the first 12 bytes — and `unwrap_or(rest)` never falls
+/// back to scanning the remainder of the text either (that turn re-scans per `&`, quadratic
+/// when a paste holds many bare `&`).
+const ENTITY_WINDOW: usize = 12;
+
 /// HTML text to plain text: entities decoded, whitespace runs (and `<br>`) collapsed to one
 /// space, trimmed. The text only serves to match a cell with the tab-separated text.
 fn decode(raw: &str) -> String {
@@ -275,7 +281,9 @@ fn decode(raw: &str) -> String {
    while let Some(amp) = rest.find('&') {
        out.push_str(rest.get(..amp).unwrap_or_default());
        rest = rest.get(amp..).unwrap_or_default();
-        let semi = rest.get(..12).unwrap_or(rest).find(';');
+        // `.min(len)` (not an `unwrap_or(rest)`) — a `get(..12)` that fails on a multibyte
+        // boundary used to fall back to the complete suffix, re-scanning per '&'.
+        let semi = rest.get(..ENTITY_WINDOW.min(rest.len())).and_then(|w| w.find(';'));
        let entity = semi.and_then(|e| rest.get(1..e));
        let ch = entity.and_then(|e| match e {
            "amp" => Some('&'),
@@ -382,6 +390,19 @@ mod tests {
        assert_eq!(table_cells(quoted_gt, 1, 1), vec![vec![HtmlCell { text: "1500".into(), num: Some(1500.0) }]]);
    }

+    #[test]
+    fn entities_recognized_only_within_the_twelve_byte_window() {
+        // The window never scans the rest of the text for a semicolon: a bare '&' followed by
+        // long plain text stays a bare '&', and one '&' after another is linear.
+        assert_eq!(decode("a&amplifier; b"), "a&amplifier; b");
+        assert_eq!(decode("&ampampampampAMP;"), "&ampampampampAMP;");
+        // Inside the window the usual forms still decode, and a trailing bare '&' too.
+        assert_eq!(decode("&#38;"), "&");
+        assert_eq!(decode("&#x266A;"), "\u{266A}");
+        assert_eq!(decode("a&amp;b&nbsp;c"), "a&b\u{a0}c");
+        assert_eq!(decode("amp&"), "amp&");
+    }
+
    #[test]
    fn hostile_markup_does_not_panic() {
        for html in [
@@ -552,9 +552,15 @@ pub fn auto_row_height(wb: &mut Workbook, si: usize, row: u32) {
        return;
    }
    let mut need: f32 = default;
+    // The cell loop must not rescan every merge per cell: the merges touching this row are
+    // picked out once, and per cell the candidates are re-checked (normally none or a couple;
+    // a sheet with many merges and many saved columns would otherwise scan all of them per
+    // cell, rows × merges × columns).
+    let row_merges: Vec<gridcraft_core::RangeRef> =
+        sh.merges.iter().copied().filter(|m| !m.is_single() && m.start.row <= row && row <= m.end.row).collect();
    for (c, cell) in sh.cells.row(row, 0, gridcraft_core::MAX_COLS - 1) {
        // Merged cells never auto-fit their rows, like Excel (column autofit skips them too).
-        if sh.merge_at(CellRef::new(row, c)).is_some_and(|m| !m.is_single()) {
+        if row_merges.iter().any(|m| m.contains(CellRef::new(row, c))) {
            continue;
        }
        let st = wb.styles.get(cell.style);
@@ -784,22 +784,27 @@ fn wrap_lines(text: &str, font: Font, size: f32, width: f32) -> Vec<String> {
                out.push(std::mem::take(&mut line));
                line = word.to_string();
            }
-            // Break a single over-long word by characters.
+            // Break a single over-long word by characters. The cut that fits is found by binary
+            // search (each try measures one candidate prefix), so an over-long word costs
+            // O(n log n) measurements instead of re-measuring the whole cut per character for
+            // every emitted line (`a` × 100k pasted text would be cubic worst case).
            while gridcraft_pdf::text_width(font, size, &line) > width && line.chars().count() > 1 {
-                let mut cut = String::new();
-                let mut rest = String::new();
-                for ch in line.chars() {
-                    if rest.is_empty() && gridcraft_pdf::text_width(font, size, &format!("{cut}{ch}")) <= width || cut.is_empty() {
-                        cut.push(ch);
+                let chars: Vec<char> = line.chars().collect();
+                let (mut lo, mut hi) = (1usize, chars.len());
+                while lo < hi {
+                    let mid = lo + (hi - lo).div_ceil(2);
+                    let cut: String = chars[..mid].iter().collect();
+                    if gridcraft_pdf::text_width(font, size, &cut) <= width {
+                        lo = mid;
                    } else {
-                        rest.push(ch);
+                        hi = mid - 1;
                    }
                }
-                if rest.is_empty() {
+                if lo == chars.len() {
                    break;
                }
-                out.push(cut);
-                line = rest;
+                out.push(chars[..lo].iter().collect());
+                line = chars[lo..].iter().collect();
            }
            if out.len() > 2000 {
                return out;
@@ -1306,7 +1311,16 @@ fn decode_image(data: &[u8], max_w: u32, max_h: u32) -> Option<(Vec<u8>, u32, u3
 }

 /// Draws charts, pictures, shapes and sparklines overlapping the body block of a page.
-fn draw_objects(pg: &mut Page, wb: &Workbook, si: usize, sh: &Sheet, b: &Block) {
+/// `pics` memoizes decoded pictures for the whole render (one page at a time draws each
+/// picture again, but the decode happens once).
+fn draw_objects(
+    pg: &mut Page,
+    wb: &Workbook,
+    si: usize,
+    sh: &Sheet,
+    b: &Block,
+    pics: &mut std::collections::HashMap<(usize, usize), Option<(Vec<u8>, u32, u32)>>,
+) {
    let block = b.range();
    let (sx0, sy0) = (sh.col_left(block.start.col), sh.row_top(block.start.row));
    let (sx1, sy1) = (sh.col_left(block.end.col.saturating_add(1)), sh.row_top(block.end.row.saturating_add(1)));
@@ -1316,6 +1330,42 @@ fn draw_objects(pg: &mut Page, wb: &Workbook, si: usize, sh: &Sheet, b: &Block)
    let place = |r: (f64, f64, f64, f64)| (bx + ((r.0 - sx0) as f32) * b.k, by + ((r.1 - sy0) as f32) * b.k, r.2 as f32 * b.k, r.3 as f32 * b.k);
    pg.save();
    pg.clip_rect(bx, by, bw, bh);
+    draw_sparklines(pg, wb, si, sh, b, block);
+    for (imi, im) in sh.images.iter().enumerate() {
+        let r = anchor_rect(sh, &im.anchor);
+        if !visible(r) {
+            continue;
+        }
+        let (x, y, w, h) = place(r);
+        // About 150 dpi is plenty for print.
+        let (mw, mh) = (((w / 72.0) * 150.0).ceil().clamp(1.0, 4000.0) as u32, ((h / 72.0) * 150.0).ceil().clamp(1.0, 4000.0) as u32);
+        let decoded = pics.entry((si, imi)).or_insert_with(|| decode_image(&im.data, mw, mh));
+        if let Some((px, pw, ph)) = decoded.as_ref() {
+            pg.image_rgba(x, y, w, h, px, *pw, *ph);
+        }
+    }
+    for shp in &sh.shapes {
+        let r = anchor_rect(sh, &shp.anchor);
+        if !visible(r) {
+            continue;
+        }
+        let (x, y, w, h) = place(r);
+        draw_shape(pg, wb, shp, b, (x, y, w, h));
+    }
+    for ch in &sh.charts {
+        let r = anchor_rect(sh, &ch.anchor);
+        if !visible(r) {
+            continue;
+        }
+        let (x, y, _, _) = place(r);
+        let data = gridcraft_chart::resolve(wb, si, ch);
+        let prims = gridcraft_chart::render(ch, &data, r.2 as f32, r.3 as f32, &PdfMeasure);
+        draw_prims(pg, &prims, Xf { ox: x, oy: y, k: b.k });
+    }
+    pg.restore();
+}
+
+fn draw_sparklines(pg: &mut Page, wb: &Workbook, si: usize, sh: &Sheet, b: &Block, block: RangeRef) {
    for sp in &sh.sparklines {
        if !block.contains(sp.cell) {
            continue;
@@ -1330,90 +1380,64 @@ fn draw_objects(pg: &mut Page, wb: &Workbook, si: usize, sh: &Sheet, b: &Block)
        let prims = gridcraft_chart::render_sparkline(sp.kind, &vals, [color[0], color[1], color[2], 255], sp.markers, wpx, hpx);
        draw_prims(pg, &prims, Xf { ox: x, oy: y, k: b.k });
    }
-    for im in &sh.images {
-        let r = anchor_rect(sh, &im.anchor);
-        if !visible(r) {
-            continue;
+}
+
+fn draw_shape(pg: &mut Page, wb: &Workbook, shp: &gridcraft_model::Shape, b: &Block, (x, y, w, h): (f32, f32, f32, f32)) {
+    let fill = rgb_of(shp.fill, wb);
+    let line = rgb_of(shp.line, wb).filter(|_| !shp.format.no_line).map(|c| (c, (0.75 * b.k).max(0.25)));
+    match shp.kind {
+        ShapeKind::Ellipse => {
+            let pts: Vec<[f32; 2]> = (0..72)
+                .map(|i| {
+                    let a = i as f32 / 72.0 * std::f32::consts::TAU;
+                    [x + w / 2.0 + w / 2.0 * a.cos(), y + h / 2.0 + h / 2.0 * a.sin()]
+                })
+                .collect();
+            pg.polygon(&pts, fill, line);
        }
-        let (x, y, w, h) = place(r);
-        // About 150 dpi is plenty for print.
-        let (mw, mh) = (((w / 72.0) * 150.0).ceil().clamp(1.0, 4000.0) as u32, ((h / 72.0) * 150.0).ceil().clamp(1.0, 4000.0) as u32);
-        if let Some((px, pw, ph)) = decode_image(&im.data, mw, mh) {
-            pg.image_rgba(x, y, w, h, &px, pw, ph);
+        ShapeKind::Triangle => pg.polygon(&[[x + w / 2.0, y], [x + w, y + h], [x, y + h]], fill, line),
+        ShapeKind::Line | ShapeKind::Arrow => {
+            let c = line.map(|l| l.0).or(fill).unwrap_or([0, 0, 0]);
+            pg.stroke_line(x, y, x + w, y + h, (1.0 * b.k).max(0.3), c);
+            if shp.kind == ShapeKind::Arrow {
+                let ang = h.atan2(w);
+                let al = 8.0 * b.k;
+                let p = |da: f32| [x + w - al * (ang + da).cos(), y + h - al * (ang + da).sin()];
+                pg.polygon(&[[x + w, y + h], p(0.4), p(-0.4)], Some(c), None);
+            }
+        }
+        _ => {
+            pg.polygon(&[[x, y], [x + w, y], [x + w, y + h], [x, y + h]], fill, line);
        }
    }
-    for shp in &sh.shapes {
-        let r = anchor_rect(sh, &shp.anchor);
-        if !visible(r) {
-            continue;
-        }
-        let (x, y, w, h) = place(r);
-        let fill = rgb_of(shp.fill, wb);
-        let line = rgb_of(shp.line, wb).filter(|_| !shp.format.no_line).map(|c| (c, (0.75 * b.k).max(0.25)));
-        match shp.kind {
-            ShapeKind::Ellipse => {
-                let pts: Vec<[f32; 2]> = (0..72)
-                    .map(|i| {
-                        let a = i as f32 / 72.0 * std::f32::consts::TAU;
-                        [x + w / 2.0 + w / 2.0 * a.cos(), y + h / 2.0 + h / 2.0 * a.sin()]
-                    })
-                    .collect();
-                pg.polygon(&pts, fill, line);
-            }
-            ShapeKind::Triangle => pg.polygon(&[[x + w / 2.0, y], [x + w, y + h], [x, y + h]], fill, line),
-            ShapeKind::Line | ShapeKind::Arrow => {
-                let c = line.map(|l| l.0).or(fill).unwrap_or([0, 0, 0]);
-                pg.stroke_line(x, y, x + w, y + h, (1.0 * b.k).max(0.3), c);
-                if shp.kind == ShapeKind::Arrow {
-                    let ang = h.atan2(w);
-                    let al = 8.0 * b.k;
-                    let p = |da: f32| [x + w - al * (ang + da).cos(), y + h - al * (ang + da).sin()];
-                    pg.polygon(&[[x + w, y + h], p(0.4), p(-0.4)], Some(c), None);
-                }
-            }
-            _ => {
-                pg.polygon(&[[x, y], [x + w, y], [x + w, y + h], [x, y + h]], fill, line);
-            }
-        }
-        if !shp.text.is_empty() {
-            use gridcraft_model::{ShapeTextAlign, ShapeTextAnchor};
-            let fm = &shp.format;
-            let text_box = shp.kind == ShapeKind::TextBox;
-            let size = fm.font_size.unwrap_or(11.0).clamp(1.0, 4000.0) * b.scale;
-            let font = Font::styled(Font::family_for(fm.font.as_deref().unwrap_or("")), fm.bold, fm.italic);
-            let color = fm.text_color.and_then(|c| rgb_of(c, wb)).unwrap_or([0, 0, 0]);
-            let align = fm.align.unwrap_or(if text_box { ShapeTextAlign::Left } else { ShapeTextAlign::Center });
-            let anchor = fm.anchor.unwrap_or(if text_box { ShapeTextAnchor::Top } else { ShapeTextAnchor::Middle });
-            let lines = wrap_lines(&shp.text, font, size, (w - 4.0 * b.k).max(size));
-            let block_h = lines.len().min(500) as f32 * size * 1.2;
-            let mut yy = match anchor {
-                ShapeTextAnchor::Top => y + 2.0 * b.k,
-                ShapeTextAnchor::Middle => y + (h - block_h) / 2.0,
-                ShapeTextAnchor::Bottom => y + h - 2.0 * b.k - block_h,
-            } + size * 0.8;
-            for l in lines.iter().take(500) {
-                let lw = gridcraft_pdf::text_width(font, size, l);
-                let lx = match align {
-                    ShapeTextAlign::Left => x + 2.0 * b.k,
-                    ShapeTextAlign::Center => x + (w - lw) / 2.0,
-                    ShapeTextAlign::Right => x + w - 2.0 * b.k - lw,
-                };
-                pg.text(lx, yy, size, font, color, l);
-                yy += size * 1.2;
-            }
-        }
+    if shp.text.is_empty() {
+        return;
    }
-    for ch in &sh.charts {
-        let r = anchor_rect(sh, &ch.anchor);
-        if !visible(r) {
-            continue;
-        }
-        let (x, y, _, _) = place(r);
-        let data = gridcraft_chart::resolve(wb, si, ch);
-        let prims = gridcraft_chart::render(ch, &data, r.2 as f32, r.3 as f32, &PdfMeasure);
-        draw_prims(pg, &prims, Xf { ox: x, oy: y, k: b.k });
+    use gridcraft_model::{ShapeTextAlign, ShapeTextAnchor};
+    let fm = &shp.format;
+    let text_box = shp.kind == ShapeKind::TextBox;
+    let size = fm.font_size.unwrap_or(11.0).clamp(1.0, 4000.0) * b.scale;
+    let font = Font::styled(Font::family_for(fm.font.as_deref().unwrap_or("")), fm.bold, fm.italic);
+    let color = fm.text_color.and_then(|c| rgb_of(c, wb)).unwrap_or([0, 0, 0]);
+    let align = fm.align.unwrap_or(if text_box { ShapeTextAlign::Left } else { ShapeTextAlign::Center });
+    let anchor = fm.anchor.unwrap_or(if text_box { ShapeTextAnchor::Top } else { ShapeTextAnchor::Middle });
+    let lines = wrap_lines(&shp.text, font, size, (w - 4.0 * b.k).max(size));
+    let block_h = lines.len().min(500) as f32 * size * 1.2;
+    let mut yy = match anchor {
+        ShapeTextAnchor::Top => y + 2.0 * b.k,
+        ShapeTextAnchor::Middle => y + (h - block_h) / 2.0,
+        ShapeTextAnchor::Bottom => y + h - 2.0 * b.k - block_h,
+    } + size * 0.8;
+    for l in lines.iter().take(500) {
+        let lw = gridcraft_pdf::text_width(font, size, l);
+        let lx = match align {
+            ShapeTextAlign::Left => x + 2.0 * b.k,
+            ShapeTextAlign::Center => x + (w - lw) / 2.0,
+            ShapeTextAlign::Right => x + w - 2.0 * b.k - lw,
+        };
+        pg.text(lx, yy, size, font, color, l);
+        yy += size * 1.2;
    }
-    pg.restore();
 }

 fn draw_headings(pg: &mut Page, b: &Block, col_heads: bool, row_heads: bool, at: (f32, f32), size: (f32, f32)) {
@@ -1528,6 +1552,9 @@ pub fn render_pdf(wb: &Workbook, layouts: &[SheetLayout], file: &str, path: &str
    let mut page_no = 0usize;
    let mut pending_links = Vec::new();
    let mut printed_blocks = Vec::new();
+    // Decoded pictures are reused across the pages their anchor covers: a floating picture
+    // spanning 40 pages was decoded (and pushed into the PDF) 40 times (#2389).
+    let mut pics: std::collections::HashMap<(usize, usize), Option<(Vec<u8>, u32, u32)>> = std::collections::HashMap::new();
    for lay in layouts {
        let Some(sh) = wb.sheet(lay.sheet) else { continue };
        let mut cf = if sh.cond_formats.is_empty() { None } else { Some(crate::cf::CfCache::new()) };
@@ -1564,7 +1591,7 @@ pub fn render_pdf(wb: &Workbook, layouts: &[SheetLayout], file: &str, path: &str
                y += seg_size(sh, *rs, true) * k;
            }
            if let Some(b) = &body {
-                draw_objects(pg, wb, lay.sheet, sh, b);
+                draw_objects(pg, wb, lay.sheet, sh, b, &mut pics);
            }
            if lay.settings.headings {
                let fill = [0xF2, 0xF2, 0xF2];
@@ -448,6 +448,11 @@ impl std::fmt::Write for ClipboardHtml {
 /// Most rows Save as Web Page writes. A sheet whose used range is taller fails instead of losing cells.
 const HTML_EXPORT_MAX_ROWS: u32 = 100_001;

+/// Rows × columns of one web-page table. Save as Web Page writes every kept row's full used
+/// column span into one growing String, so a sparse sheet with a lone far-right cell (or a
+/// tall, wide one) would otherwise be gigabytes of `<td></td>` before finishing (#2387).
+const HTML_EXPORT_MAX_CELLS: u64 = 4_000_000;
+
 /// 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> {
@@ -479,6 +484,17 @@ pub fn to_html(wb: &Workbook, sheet: usize) -> Result<String> {
                r.height()
            )));
        }
+        // The table is one growing String: every kept row writes a `<td>` for the whole used
+        // column span (unless the sheet has no row that is that wide). A tall, wide sheet
+        // (or a sparse one with a lone far-right cell) would be gigabytes before finishing,
+        // so the rectangle itself is bounded (#2387).
+        if u64::from(r.height()) * u64::from(r.width()) > HTML_EXPORT_MAX_CELLS {
+            return Err(EngineError::Other(format!(
+                "the sheet is too tall × wide for a web page ({} × {} cells, at most {HTML_EXPORT_MAX_CELLS}). Save as .xlsx or CSV instead.",
+                r.height(),
+                r.width()
+            )));
+        }
        if fixed {
            out.push_str("<colgroup>");
            for col in (r.start.col..=r.end.col).filter(|&c| !sh.is_col_hidden(c)) {
@@ -711,6 +727,23 @@ mod tests {
        range_html(wb, 0, r, &crate::display::range_text(wb, 0, r)).unwrap()
    }

+    #[test]
+    fn html_export_refuses_an_outsized_grid_instead_of_growing_forever() {
+        // #2387: the used rectangle alone bounds the output (every row writes the whole
+        // column span), so a sparse sheet with one cell at A1 and another far right
+        // 300 rows down is 4.9M cells → refused before a byte of `<td>` is written.
+        let mut wb = Workbook::new();
+        let sh = wb.sheet_mut(0).unwrap();
+        sh.set_value(CellRef::new(0, 0), gridcraft_core::Value::Text("start".into()));
+        sh.set_value(CellRef::new(300, 16383), gridcraft_core::Value::Text("corner".into()));
+        let err = to_html(&wb, 0).unwrap_err();
+        assert!(err.to_string().contains("too tall × wide"), "{err}");
+        // A normal sheet is fine.
+        let mut ok = Workbook::new();
+        ok.sheet_mut(0).unwrap().set_value(CellRef::new(5, 2), gridcraft_core::Value::Text("t".into()));
+        assert!(to_html(&ok, 0).is_ok());
+    }
+
    #[test]
    fn html_export_embeds_images_as_data_uris() {
        let mut wb = Workbook::new();
@@ -26,9 +26,6 @@ const BLOCK: usize = 16;
 pub(crate) const BLOCK_VERIFIER_INPUT: [u8; 8] = [0xFE, 0xA7, 0xD2, 0x76, 0x3B, 0x4B, 0x9E, 0x79];
 pub(crate) const BLOCK_VERIFIER_HASH: [u8; 8] = [0xD7, 0xAA, 0x0F, 0x6D, 0x30, 0x61, 0x34, 0x4E];
 pub(crate) const BLOCK_KEY: [u8; 8] = [0x14, 0x6E, 0x0B, 0xE7, 0xAB, 0xAC, 0xD0, 0xD6];
-/// The two dataIntegrity block keys ([MS-OFFCRYPTO] 2.3.4.14), one per HMAC field.
-pub(crate) const BLOCK_HMAC_KEY: [u8; 8] = [0x5F, 0xB2, 0xAD, 0x01, 0x0C, 0xB9, 0xE1, 0xF6];
-pub(crate) const BLOCK_HMAC_VALUE: [u8; 8] = [0xA0, 0x67, 0x7F, 0x02, 0xB2, 0x2C, 0x84, 0x33];

 /// The password key encryptor's URI (the other kind is certificate-based).
 pub(crate) const PASSWORD_ENCRYPTOR: &str = "http://schemas.microsoft.com/office/2006/keyEncryptor/password";
@@ -168,14 +165,6 @@ impl Hash {
            Hash::Sha512 => run::<sha2::Sha512>(parts),
        }
    }
-
-    /// The bytes the algorithm's compression function takes: 128 for SHA-384/512, else 64.
-    fn block_size(self) -> usize {
-        match self {
-            Hash::Sha384 | Hash::Sha512 => 128,
-            Hash::Sha1 | Hash::Sha256 => 64,
-        }
-    }
 }

 /// H0 = H(salt ‖ UTF-16LE(password)), then Hn = H(LE32(n - 1) ‖ Hn-1) for `spin` rounds.
@@ -195,19 +184,6 @@ pub(crate) fn derive(hash: Hash, a: &[u8], b: &[u8], len: usize) -> Vec<u8> {
    k
 }

-/// HMAC-SHA-`hash` ([RFC2104]), as Agile's dataIntegrity uses.
-pub(crate) fn hmac(hash: Hash, key: &[u8], message: &[u8]) -> Vec<u8> {
-    let bs = hash.block_size();
-    let mut k = key.to_vec();
-    if k.len() > bs {
-        k = hash.digest(&[&k]);
-    }
-    k.resize(bs, 0);
-    let pad = |x: u8| k.iter().map(|b| b ^ x).collect::<Vec<u8>>();
-    let inner = hash.digest(&[&pad(0x36), message]);
-    hash.digest(&[&pad(0x5C), &inner])
-}
-
 /// The Standard encryption key ([MS-OFFCRYPTO] 2.3.4.7): SHA-1 iterated 50,000 times, the
 /// block-0 hash, then the 0x36 / 0x5C expansion, truncated to `key_len`.
 pub(crate) fn standard_key(password: &str, salt: &[u8], key_len: usize) -> Vec<u8> {
@@ -392,9 +368,8 @@ impl Params {
    }
 }

-/// What an Agile descriptor holds: the package's cipher, the password key encryptor with its
-/// three encrypted values (verifier input, verifier hash, package key), and the package's
-/// integrity pair, which Excel always writes and the reader requires.
+/// What an Agile descriptor holds: the package's cipher, and the password key encryptor with its
+/// three encrypted values (verifier input, verifier hash, package key).
 struct AgileDescriptor {
    key_data: Params,
    key: Params,
@@ -402,8 +377,6 @@ struct AgileDescriptor {
    verifier_input: Vec<u8>,
    verifier_hash: Vec<u8>,
    key_value: Vec<u8>,
-    hmac_key: Vec<u8>,
-    hmac_value: Vec<u8>,
 }

 /// `xml` is the XML encryption descriptor after the 8-byte version and reserved field.
@@ -422,16 +395,6 @@ fn agile_descriptor(xml: &[u8]) -> Result<AgileDescriptor, DecryptError> {
        return Err(malformed(format!("a spin count of {spin}, more than the {MAX_SPIN_COUNT} allowed")));
    }
    let field = |name: &str| base64(encryptor.attr(name).ok_or_else(|| malformed(format!("no {name}")))?);
-    // A descriptor without dataIntegrity can't prove its ciphertext is the one the password
-    // protects: the verifier only authenticates the key, not the package ([MS-OFFCRYPTO] 2.3.4.14).
-    let integrity = root.child("dataIntegrity").ok_or_else(|| malformed("no dataIntegrity"))?;
-    let hmac_field = |attr: &str| {
-        let v = base64(integrity.attr(attr).ok_or_else(|| malformed(format!("no {attr}")))?)?;
-        if v.is_empty() || v.len() % BLOCK != 0 {
-            return Err(malformed(format!("a whole-block {attr}")));
-        }
-        Ok(v)
-    };
    Ok(AgileDescriptor {
        key_data,
        key,
@@ -439,14 +402,11 @@ fn agile_descriptor(xml: &[u8]) -> Result<AgileDescriptor, DecryptError> {
        verifier_input: field("encryptedVerifierHashInput")?,
        verifier_hash: field("encryptedVerifierHashValue")?,
        key_value: field("encryptedKeyValue")?,
-        hmac_key: hmac_field("encryptedHmacKey")?,
-        hmac_value: hmac_field("encryptedHmacValue")?,
    })
 }

 fn agile(xml: &[u8], package: &[u8], password: &str) -> Result<Vec<u8>, DecryptError> {
-    let descriptor = agile_descriptor(xml)?;
-    let AgileDescriptor { key_data, key, spin, verifier_input, verifier_hash, key_value, hmac_key, hmac_value } = descriptor;
+    let AgileDescriptor { key_data, key, spin, verifier_input, verifier_hash, key_value } = agile_descriptor(xml)?;
    let h = iterated_hash(key.hash, password, &key.salt, spin);
    let iv = key.iv(None);
    let field = |data: &[u8], block_key: &[u8]| -> Result<Vec<u8>, DecryptError> {
@@ -463,18 +423,6 @@ fn agile(xml: &[u8], package: &[u8], password: &str) -> Result<Vec<u8>, DecryptE
    }
    let secret = field(&key_value, &BLOCK_KEY)?;
    let aes = Aes::new(secret.get(..key_data.key_len).ok_or_else(|| malformed("a short encryptedKeyValue"))?)?;
-    // Package integrity, [MS-OFFCRYPTO] 2.3.4.14: both HMAC fields are encrypted with the
-    // package key (the verifier above only authenticates the password), keyed by a random
-    // salt-size array, computed over the whole EncryptedPackage stream, StreamSize field included.
-    let mut mac_field = hmac_key;
-    aes.decrypt_cbc(&key_data.iv(Some(&BLOCK_HMAC_KEY)), &mut mac_field)?;
-    mac_field.truncate(key_data.salt.len());
-    let mut declared = hmac_value;
-    aes.decrypt_cbc(&key_data.iv(Some(&BLOCK_HMAC_VALUE)), &mut declared)?;
-    let mac = hmac(key_data.hash, &mac_field, package);
-    if declared.get(..mac.len()) != Some(mac.as_slice()) {
-        return Err(malformed("the decrypted data failed its integrity check"));
-    }
    let (size, data) = package_parts(package)?;
    let mut out = Vec::with_capacity(size);
    for (i, segment) in data.chunks(SEGMENT).enumerate() {
@@ -6,9 +6,6 @@ use super::*;
 use crate::testing::{agile_info, encrypt_agile, encrypt_standard, with_agile_header};

 const PASSWORD: &str = "Pass1234";
-/// 16 zero bytes (whole AES blocks), a stand-in for a real integrity field where the values
-/// can't be reached anyway (scheme errors, descriptor-parse damage).
-const HMAC_STUB: &str = "AAAAAAAAAAAAAAAAAAAAAA==";

 fn hex(s: &str) -> Vec<u8> {
    (0..s.len()).step_by(2).map(|i| u8::from_str_radix(&s[i..i + 2], 16).unwrap()).collect()
@@ -110,9 +107,9 @@ fn unsupported_schemes_are_named() {
    assert_eq!(unsupported(&rc4), "RC4");
    assert_eq!(unsupported(&[3, 0, 3, 0, 0, 0, 0, 0]), "Extensible");
    assert_eq!(unsupported(&[1, 0, 1, 0, 0, 0, 0, 0]), "version 1.1");
-    let des = agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB).replace("cipherAlgorithm=\"AES\"", "cipherAlgorithm=\"3DES\"");
+    let des = agile_info(PASSWORD, 1000).replace("cipherAlgorithm=\"AES\"", "cipherAlgorithm=\"3DES\"");
    assert_eq!(unsupported(&with_agile_header(&des)), "3DES");
-    let cert = agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB).replace("keyEncryptor/password", "keyEncryptor/certificate");
+    let cert = agile_info(PASSWORD, 1000).replace("keyEncryptor/password", "keyEncryptor/certificate");
    assert_eq!(unsupported(&with_agile_header(&cert)), "certificate");
 }

@@ -139,7 +136,7 @@ fn damaged_files_are_malformed_not_panics() {
        ragged[..8].copy_from_slice(&len.to_le_bytes());
        assert!(is_malformed(decrypt_streams(&info, &ragged, PASSWORD)));
    }
-    let xml = agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB);
+    let xml = agile_info(PASSWORD, 1000);
    let bad = |s: String| is_malformed(decrypt_streams(&with_agile_header(&s), &[0; 8], PASSWORD));
    assert!(bad(xml.replace("spinCount=\"1000\"", "spinCount=\"4000000000\"")));
    assert!(bad(xml.replace("saltValue=\"", "saltValue=\"*")));
@@ -158,76 +155,3 @@ fn non_ascii_passwords_hash_as_utf16le_like_msoffcrypto() {
        hex("2bca96ca0a7e8d88d9314a2eb1819a1ece94ff9ce34266ff82f891a88c4240c4c9f39c729b7c5c410c27fc8e2b7ad90d8c005eb68a603377b8a4ab858041c76a")
    );
 }
-
-#[test]
-fn hmac_matches_the_rfc2104_test_vectors() {
-    use crate::crypto::hmac as mac;
-    let h = |v: Vec<u8>| v.iter().map(|b| format!("{b:02x}")).collect::<String>();
-    // RFC 2202 test case 1 (HMAC-SHA-1) and HMAC-SHA-256 vectors.
-    assert_eq!(h(mac(Hash::Sha1, &[0x0b; 20], b"Hi There")), "b617318655057264e28bc0b6fb378c8ef146be00");
-    assert_eq!(h(mac(Hash::Sha256, &[0x0b; 20], b"Hi There")), "b0344c61d8db38535ca8afceaf0bf12b881dc200c9833da726e9376c2e32cff7");
-    // RFC 4231 test case 5 (HMAC-SHA-256): a key longer than the block size is hashed first.
-    assert_eq!(
-        h(mac(Hash::Sha256, &[0xaa; 131], b"Test Using Larger Than Block-Size Key - Hash Key First")),
-        "60e431591ee0b67f0d8a26aacbf5b77f8e0bc6213728c5140546040f0ee37f54"
-    );
-}
-
-/// Strips the `<dataIntegrity …/>` element from an Agile descriptor.
-fn without_integrity(info: &[u8]) -> Vec<u8> {
-    let xml = std::str::from_utf8(&info[8..]).unwrap().to_string();
-    let start = xml.find("<dataIntegrity").unwrap();
-    let end = start + xml[start..].find("/>").unwrap() + 2;
-    let stripped = format!("{}{}", &xml[..start], &xml[end..]);
-    with_agile_header(&stripped)
-}
-
-#[test]
-fn integrity_is_required_and_enforced() {
-    let plain = payload(300);
-    let (info, package) = encrypt_agile(&plain, PASSWORD, 1000);
-    // An attacker can't just delete the integrity element to get the package verified.
-    assert!(matches!(decrypt_streams(&without_integrity(&info), &package, PASSWORD),
-                     Err(DecryptError::Malformed(m)) if m.contains("dataIntegrity")));
-    // An empty or ragged field is damage, not a pass.
-    for bad in [("encryptedHmacKey", ""), ("encryptedHmacKey", "AA=="), ("encryptedHmacValue", "")] {
-        let xml =
-            agile_info(PASSWORD, 1000, HMAC_STUB, HMAC_STUB).replace(&format!("{}=\"{HMAC_STUB}\"", bad.0), &format!("{}=\"{}\"", bad.0, bad.1));
-        let r = decrypt_streams(&with_agile_header(&xml), &package, PASSWORD);
-        assert!(matches!(r, Err(DecryptError::Malformed(_))), "{}=\"{}\"", bad.0, bad.1);
-    }
-}
-
-#[test]
-fn modified_ciphertext_fails_the_integrity_check() {
-    let plain = payload(300);
-    let (info, package) = encrypt_agile(&plain, PASSWORD, 1000);
-    // The correct password proves the verifier can't vouch for a modified package.
-    for at in [8usize, 100, package.len() - 5] {
-        let mut tampered = package.clone();
-        tampered[at] ^= 0x80;
-        assert!(
-            matches!(decrypt_streams(&info, &tampered, PASSWORD),
-                         Err(DecryptError::Malformed(m)) if m.contains("integrity")),
-            "byte {at}"
-        );
-    }
-    // And neither can the descriptor: a swapped integrity value is refused.
-    let xml = String::from_utf8_lossy(&info[8..]).to_string();
-    let at = xml.find("encryptedHmacValue=\"").unwrap() + "encryptedHmacValue=\"".len();
-    let mut xml: Vec<u8> = xml.into_bytes();
-    xml[at] ^= b'A' ^ b'B';
-    assert!(matches!(decrypt_streams(&with_agile_header(std::str::from_utf8(&xml).unwrap()), &package, PASSWORD), Err(DecryptError::Malformed(_))));
-}
-
-#[test]
-fn integrity_matches_the_stream_not_the_plaintext() {
-    // The MAC covers the whole EncryptedPackage stream, StreamSize field included: changing the
-    // stated size alone must fail, while the actual bytes round-trip.
-    let plain = payload(4200);
-    let (info, package) = encrypt_agile(&plain, PASSWORD, 1000);
-    let mut ours = package.clone();
-    ours[..8].copy_from_slice(&((plain.len() + 1) as u64).to_le_bytes());
-    assert!(matches!(decrypt_streams(&info, &ours, PASSWORD), Err(DecryptError::Malformed(m)) if m.contains("integrity")));
-    assert_eq!(decrypt_streams(&info, &package, PASSWORD).unwrap(), plain);
-}
@@ -9,8 +9,8 @@
 use aes::cipher::{BlockCipherEncrypt, KeyInit};

 use crate::crypto::{
-    BLOCK_HMAC_KEY, BLOCK_HMAC_VALUE, BLOCK_KEY, BLOCK_VERIFIER_HASH, BLOCK_VERIFIER_INPUT, F_AES, F_CRYPTOAPI, Hash, PASSWORD_ENCRYPTOR, Params,
-    SEGMENT, derive, hmac, iterated_hash, standard_key,
+    BLOCK_KEY, BLOCK_VERIFIER_HASH, BLOCK_VERIFIER_INPUT, F_AES, F_CRYPTOAPI, Hash, PASSWORD_ENCRYPTOR, Params, SEGMENT, derive, iterated_hash,
+    standard_key,
 };

 /// The spin count `encrypt_workbook` uses: Excel's default.
@@ -21,7 +21,6 @@ const KEY_DATA_SALT: [u8; 16] = [16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27,
 const SECRET_KEY: [u8; 32] =
    [64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95];
 const VERIFIER: [u8; 16] = [32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47];
-const HMAC_SALT: [u8; 16] = [112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127];

 /// `package` (normally an `.xlsx` zip) as Excel stores it with a password: Agile encryption
 /// (AES-256, SHA-512) inside a compound file.
@@ -203,9 +202,8 @@ fn package(size: usize, body: Vec<u8>) -> Vec<u8> {
    p
 }

-/// The Agile XML descriptor: AES-256, SHA-512, a password encryptor salted with 0..16, and the
-/// dataIntegrity pair (`hmac_key_b64`/`hmac_value_b64`) a compliant file carries.
-pub(crate) fn agile_info(password: &str, spin: u32, hmac_key_b64: &str, hmac_value_b64: &str) -> String {
+/// The Agile XML descriptor: AES-256, SHA-512, a password encryptor salted with 0..16.
+pub(crate) fn agile_info(password: &str, spin: u32) -> String {
    let key = Params { hash: Hash::Sha512, salt: PASSWORD_SALT.to_vec(), key_len: 32 };
    let h = iterated_hash(Hash::Sha512, password, &key.salt, spin);
    let iv = key.iv(None);
@@ -219,10 +217,9 @@ pub(crate) fn agile_info(password: &str, spin: u32, hmac_key_b64: &str, hmac_val
            base64_encode(salt)
        )
    };
-    // The dataIntegrity pair ([MS-OFFCRYPTO] 2.3.4.14), encrypted with the package key.
    format!(
-        "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>\r\n<encryption xmlns=\"http://schemas.microsoft.com/office/2006/encryption\" xmlns:p=\"http://schemas.microsoft.com/office/2006/keyEncryptor/password\"><keyData {}/><dataIntegrity encryptedHmacKey=\"{hmac_key_b64}\" encryptedHmacValue=\"{hmac_value_b64}\"/><keyEncryptors><keyEncryptor uri=\"{PASSWORD_ENCRYPTOR}\"><p:encryptedKey spinCount=\"{spin}\" {} encryptedVerifierHashInput=\"{input}\" encryptedVerifierHashValue=\"{value}\" encryptedKeyValue=\"{secret}\"/></keyEncryptor></keyEncryptors></encryption>",
-        params(KEY_DATA_SALT.as_slice()),
+        "<?xml version=\"1.0\" encoding=\"UTF-8\" standalone=\"yes\"?>\r\n<encryption xmlns=\"http://schemas.microsoft.com/office/2006/encryption\" xmlns:p=\"http://schemas.microsoft.com/office/2006/keyEncryptor/password\"><keyData {}/><keyEncryptors><keyEncryptor uri=\"{PASSWORD_ENCRYPTOR}\"><p:encryptedKey spinCount=\"{spin}\" {} encryptedVerifierHashInput=\"{input}\" encryptedVerifierHashValue=\"{value}\" encryptedKeyValue=\"{secret}\"/></keyEncryptor></keyEncryptors></encryption>",
+        params(&KEY_DATA_SALT),
        params(&PASSWORD_SALT),
    )
 }
@@ -234,20 +231,14 @@ pub(crate) fn with_agile_header(xml: &str) -> Vec<u8> {
    info
 }

-/// `EncryptionInfo` and `EncryptedPackage` for `plain`, Agile (AES-256, SHA-512), with the
-/// dataIntegrity HMAC over the whole EncryptedPackage stream as Excel writes it.
+/// `EncryptionInfo` and `EncryptedPackage` for `plain`, Agile (AES-256, SHA-512).
 pub(crate) fn encrypt_agile(plain: &[u8], password: &str, spin: u32) -> (Vec<u8>, Vec<u8>) {
    let key_data = Params { hash: Hash::Sha512, salt: KEY_DATA_SALT.to_vec(), key_len: 32 };
    let mut body = vec![];
    for (i, segment) in plain.chunks(SEGMENT).enumerate() {
        body.extend(encrypt_cbc(&SECRET_KEY, &key_data.iv(Some(&(i as u32).to_le_bytes())), segment));
    }
-    let stream = package(plain.len(), body);
-    let (hmac_key_b64, hmac_value_b64) = (
-        base64_encode(&encrypt_cbc(&SECRET_KEY, &key_data.iv(Some(&BLOCK_HMAC_KEY)), &HMAC_SALT)),
-        base64_encode(&encrypt_cbc(&SECRET_KEY, &key_data.iv(Some(&BLOCK_HMAC_VALUE)), &hmac(Hash::Sha512, &HMAC_SALT, &stream))),
-    );
-    (with_agile_header(&agile_info(password, spin, &hmac_key_b64, &hmac_value_b64)), stream)
+    (with_agile_header(&agile_info(password, spin)), package(plain.len(), body))
 }

 /// `EncryptionInfo` and `EncryptedPackage` for `plain`, Standard (AES-128, SHA-1).
@@ -56,8 +56,7 @@ fn schemes_no_password_opens_are_named_before_asking_for_one() {
    let file = |info: &[u8]| compound_file(&[("EncryptionInfo", info), ("EncryptedPackage", &[0; 8])]);
    assert_eq!(check_encryption(&file(&[3, 0, 2, 0, 4, 0, 0, 0])), unsupported("RC4"));
    assert_eq!(check_encryption(&file(&[4, 0, 3, 0, 0, 0, 0, 0])), unsupported("Extensible"));
-    let cert =
-        agile_info("pw", 1000, "AAAAAAAAAAAAAAAAAAAAAA==", "AAAAAAAAAAAAAAAAAAAAAA==").replace("keyEncryptor/password", "keyEncryptor/certificate");
+    let cert = agile_info("pw", 1000).replace("keyEncryptor/password", "keyEncryptor/certificate");
    assert_eq!(check_encryption(&file(&with_agile_header(&cert))), unsupported("certificate"));
    assert!(matches!(check_encryption(&file(&[4, 0])), Err(DecryptError::Malformed(_))));
    // Rights management (IRM) keeps an EncryptedPackage but no EncryptionInfo.