From 0aae1ffeea37e9d6a86deebb9c1c594d0aa9d835 Mon Sep 17 00:00:00 2001
From: prxnvt <53838599+prxnvt@users.noreply.github.com>
Date: Sun, 25 Oct 2026 06:00:13 -0400
Subject: [PATCH] Engine: an xlsx save refuses a macro that skipped the check,
 as a JSON save does

`with_names` checked only the size of each item, so a macro built in code past the model's
limits (a name with a line break) was written to the file and dropped the next time it opened.
The xlsx save now runs `check` on every macro first.

- Test: the macro past the limits in `a_macro_built_in_code_goes_through_the_size_check_too`.
---
 crates/engine/src/macros/mod.rs   |  5 +++++
 crates/engine/src/macros/tests.rs | 14 +++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/crates/engine/src/macros/mod.rs b/crates/engine/src/macros/mod.rs
index ab692b5b7..8c23fbf0e 100644
--- a/crates/engine/src/macros/mod.rs
+++ b/crates/engine/src/macros/mod.rs
@@ -106,6 +106,11 @@ pub fn with_names(wb: &Workbook) -> crate::Result<Option<Workbook>> {
     if wb.macros.is_empty() && wb.macros_unread.is_empty() && !wb.names.iter().any(stored) {
         return Ok(None);
     }
+    // A macro that skipped `check` (built in code) fails the save, as the JSON save does, rather
+    // than being written and dropped the next time the file opens.
+    for m in wb.macros.iter() {
+        check(m).map_err(crate::EngineError::Other)?;
+    }
     let mut out = wb.clone();
     // One pass drops the items of the last save, and one writes the macros (a file can hold a
     // thousand macros and many thousands of names).
diff --git a/crates/engine/src/macros/tests.rs b/crates/engine/src/macros/tests.rs
index dc25de621..5f5531207 100644
--- a/crates/engine/src/macros/tests.rs
+++ b/crates/engine/src/macros/tests.rs
@@ -211,12 +211,16 @@ fn a_macro_built_in_code_goes_through_the_size_check_too() {
     };
     assert!(big.check_limits().is_ok(), "the model's limits say nothing about the size");
     assert!(check(&big).unwrap_err().contains("too large"));
-    let mut wb = Workbook::new();
-    wb.macros.push(big);
     // A macro that fails `check` fails the save in either format, rather than being written and
-    // dropped the next time the file opens.
-    for file in ["book.xlsx", "book.json"] {
-        assert!(crate::io::save_bytes(&wb, file).is_err(), "{file}");
+    // dropped the next time the file opens: too large, or past the model's limits.
+    let bad_name = Macro { name: "Two\nlines".into(), ..Macro::default() };
+    assert!(check(&bad_name).is_err());
+    for m in [big, bad_name] {
+        let mut wb = Workbook::new();
+        wb.macros.push(m);
+        for file in ["book.xlsx", "book.json"] {
+            assert!(crate::io::save_bytes(&wb, file).is_err(), "{file}");
+        }
     }
 }