merge main into fix/sec-chart-geometry-budgets

- sheet.moveColumns resolved its target sheet from `sheet`, or a
  `Sheet!` range or cell, but its two protection checks were passed
  only the `sheet` parameter, so a qualified range moved columns of
  a protected sheet while the checks authorized the active one. Both
  checks now get the resolved sheet index.
- check_protection compared only cell content, so a sort or duplicate
  removal moving an unlocked cell with equal content onto a locked one
  left the content as it was but made the cell editable. The commit
  check now also refuses a change that leaves a previously locked cell
  unlocked (effective lock, row and column formats included).

apps/gridcraft-cli/src/check/calc.rs

@@ -31,20 +31,41 @@ fn is_volatile(e: &Expr, volatile_names: &HashSet<String>) -> bool {
 }
 
 /// Upper-cased defined names whose formula is volatile, directly or through other names.
+///
+/// Volatility flows backwards along name references: each name is parsed and walked once, and a
+/// worklist carries volatility from a name to the names that use it. Rescanning every name until
+/// nothing changes was quadratic: a file listing its names so that each pass marked only one more
+/// did one full pass per name.
 pub(super) fn volatile_names(wb: &Workbook) -> HashSet<String> {
-    let parsed: Vec<(String, Option<Expr>)> =
-        wb.names.iter().map(|n| (n.name.to_ascii_uppercase(), gridcraft_engine::formula::parse(&n.formula).ok())).collect();
     let mut set = HashSet::new();
-    // Each round adds at least one name or stops, so `len + 1` rounds reach the fixed point.
-    for _ in 0..=parsed.len() {
-        let before = set.len();
-        for (name, e) in &parsed {
-            if !set.contains(name) && e.as_ref().is_some_and(|e| is_volatile(e, &set)) {
-                set.insert(name.clone());
+    // Upper-cased name -> the names whose formulas refer to it.
+    let mut users: HashMap<String, Vec<String>> = HashMap::new();
+    let mut queue = Vec::new();
+    let none = HashSet::new();
+    for n in &wb.names {
+        let name = n.name.to_ascii_uppercase();
+        let Ok(e) = gridcraft_engine::formula::parse(&n.formula) else { continue };
+        if is_volatile(&e, &none) {
+            if set.insert(name.clone()) {
+                queue.push(name.clone());
             }
+            continue;
         }
-        if set.len() == before {
-            break;
+        let mut used = BTreeSet::new();
+        e.walk(&mut |x| {
+            if let Expr::Name(r) = x {
+                used.insert(r.to_ascii_uppercase());
+            }
+        });
+        for r in used {
+            users.entry(r).or_default().push(name.clone());
+        }
+    }
+    while let Some(v) = queue.pop() {
+        for user in users.remove(&v).unwrap_or_default() {
+            if set.insert(user.clone()) {
+                queue.push(user);
+            }
         }
     }
     set

apps/gridcraft-cli/src/check/tests.rs

@@ -178,6 +178,28 @@ fn volatile_names_propagate() {
     assert_eq!(check(&wb, &Options::default()).calc.volatile, 1);
 }
 
+#[test]
+fn volatile_names_propagate_through_a_long_chain_in_one_walk() {
+    // Chain0 uses Chain1, Chain1 uses Chain2, ... and only the last is volatile: listed in this order, a rescan
+    // until nothing changes marked one name per pass (quadratic). Every name in the chain is
+    // volatile; a name off the chain is not.
+    const LEN: usize = 5_000;
+    let mut wb = book(&[("A1", "=1", n(1.0))]);
+    let name =
+        |name: String, formula: String| gridcraft_engine::model::DefinedName { name, scope: None, formula, comment: String::new(), hidden: false };
+    for i in 0..LEN {
+        let formula = if i + 1 == LEN { "NOW()".to_string() } else { format!("Chain{}+1", i + 1) };
+        wb.names.push(name(format!("Chain{i}"), formula));
+    }
+    wb.names.push(name("Plain".into(), "Chain0*0+Other".into()));
+    wb.names.push(name("Other".into(), "1".into()));
+    let set = volatile_names(&wb);
+    assert!((0..LEN).all(|i| set.contains(&format!("CHAIN{i}"))));
+    assert!(set.contains("PLAIN"));
+    assert!(!set.contains("OTHER"));
+    assert_eq!(set.len(), LEN + 1);
+}
+
 #[test]
 fn round_trip_of_a_simple_book_is_clean() {
     let mut wb = book(&[("A1", "2", n(2.0)), ("B1", "=A1*3", n(6.0)), ("A2", "x", Value::text("x"))]);

crates/chart/src/raster.rs

@@ -7,6 +7,12 @@ use crate::{Prim, Rgba};
 /// Largest image we allocate (pixels).
 const MAX_PIXELS: u64 = 64 * 1024 * 1024;
 const SUB: usize = 4;
+/// Scan-conversion work one [`rasterize`] call may do: edge tests (an active edge at one
+/// sub-sample) plus covered pixels blended. Edges are only tested on the rows they cross, but a
+/// caller can still pass enough geometry to keep the rasteriser busy for minutes on a small
+/// image; once the allowance is spent the remaining fills are skipped. 2^28 is about a second,
+/// far beyond any chart.
+const MAX_SCAN_WORK: u64 = 1 << 28;
 
 type Path = Vec<Vec<[f32; 2]>>;
 
@@ -15,6 +21,8 @@ struct Canvas {
     h: usize,
     buf: Vec<u8>,
     acc: Vec<f32>,
+    /// Scan-conversion work left (see [`MAX_SCAN_WORK`]).
+    work: u64,
 }
 
 impl Canvas {
@@ -41,7 +49,8 @@ impl Canvas {
 
     /// Fills a multi-contour path with the even-odd rule.
     fn fill(&mut self, path: &Path, color: Rgba) {
-        let mut edges: Vec<[f32; 4]> = Vec::new();
+        // Each edge as [top y, bottom y, x0, y0, x1, y1].
+        let mut edges: Vec<[f32; 6]> = Vec::new();
         let (mut minx, mut miny, mut maxx, mut maxy) = (f32::INFINITY, f32::INFINITY, f32::NEG_INFINITY, f32::NEG_INFINITY);
         for c in path {
             if c.len() < 3 || !c.iter().all(|p| p[0].is_finite() && p[1].is_finite()) {
@@ -54,7 +63,7 @@ impl Canvas {
                 miny = miny.min(p[1]);
                 maxy = maxy.max(p[1]);
                 if p[1] != q[1] {
-                    edges.push([p[0], p[1], q[0], q[1]]);
+                    edges.push([p[1].min(q[1]), p[1].max(q[1]), p[0], p[1], q[0], q[1]]);
                 }
             }
         }
@@ -72,8 +81,26 @@ impl Canvas {
         if self.acc.len() < bw + 1 {
             self.acc.resize(bw + 1, 0.0);
         }
+        // Active-edge list: edges join when the scan reaches their top and leave past their
+        // bottom, so each row tests only the edges crossing it instead of every edge.
+        edges.sort_by(|a, b| a[0].total_cmp(&b[0]));
+        let mut pending = edges.iter().peekable();
+        let mut active: Vec<[f32; 6]> = Vec::new();
         let mut xs: Vec<f32> = Vec::new();
         for py in y0..y1 {
+            while let Some(e) = pending.next_if(|e| e[0] < py as f32 + 1.0) {
+                active.push(*e);
+            }
+            active.retain(|e| e[1] > py as f32);
+            if active.is_empty() {
+                continue;
+            }
+            let tests = (active.len() * SUB) as u64;
+            if tests > self.work {
+                self.work = 0;
+                return;
+            }
+            self.work -= tests;
             for v in self.acc.iter_mut().take(bw) {
                 *v = 0.0;
             }
@@ -81,10 +108,9 @@ impl Canvas {
             for s in 0..SUB {
                 let sy = py as f32 + (s as f32 + 0.5) / SUB as f32;
                 xs.clear();
-                for e in &edges {
-                    let (ya, yb) = if e[1] < e[3] { (e[1], e[3]) } else { (e[3], e[1]) };
-                    if sy >= ya && sy < yb {
-                        xs.push(e[0] + (sy - e[1]) * (e[2] - e[0]) / (e[3] - e[1]));
+                for e in &active {
+                    if sy >= e[0] && sy < e[1] {
+                        xs.push(e[2] + (sy - e[3]) * (e[4] - e[2]) / (e[5] - e[3]));
                     }
                 }
                 xs.sort_by(f32::total_cmp);
@@ -110,6 +136,7 @@ impl Canvas {
             if !any {
                 continue;
             }
+            self.work = self.work.saturating_sub(bw as u64);
             for px in x0..x1 {
                 let cov = self.acc.get(px - x0).copied().unwrap_or(0.0);
                 if cov > 0.0 {
@@ -212,7 +239,7 @@ pub fn rasterize(prims: &[Prim], w: u32, h: u32, background: Rgba) -> Vec<u8> {
     for _ in 0..w * h {
         buf.extend_from_slice(&background);
     }
-    let mut cv = Canvas { w, h, buf, acc: Vec::new() };
+    let mut cv = Canvas { w, h, buf, acc: Vec::new(), work: MAX_SCAN_WORK };
     for p in prims {
         match p {
             Prim::Rect { x, y, w, h, fill, stroke, radius } => {
@@ -274,6 +301,31 @@ pub fn rasterize(prims: &[Prim], w: u32, h: u32, background: Rgba) -> Vec<u8> {
 mod tests {
     use super::*;
 
+    /// Security: each row tests only the edges crossing it. A polygon of 100,000 tiny edges over
+    /// 4,000 rows tested every edge at every sub-sample of every row (1.6 billion tests).
+    #[test]
+    fn many_short_edges_scan_only_their_rows() {
+        let mut pts: Vec<[f32; 2]> = (0..100_000).map(|i| [if i % 2 == 0 { 0.0 } else { 0.5 }, i as f32 * 0.04]).collect();
+        pts.push([2.0, 4_000.0]);
+        pts.push([2.0, 0.0]);
+        let img = rasterize(&[Prim::Polygon { pts, fill: [255, 0, 0, 255], stroke: None }], 2, 4_000, [0, 0, 0, 0]);
+        // The right-hand pixel of every row is inside the polygon.
+        assert_eq!(img.get(4..8), Some(&[255, 0, 0, 255][..]));
+        assert_eq!(img.get(4 * (2 * 3_999 + 1)..4 * (2 * 3_999 + 2)), Some(&[255, 0, 0, 255][..]));
+    }
+
+    /// Security: scan conversion stops once the call's work allowance is spent.
+    #[test]
+    fn fills_stop_when_the_work_allowance_is_spent() {
+        let square = vec![vec![[0.0, 0.0], [4.0, 0.0], [4.0, 4.0], [0.0, 4.0]]];
+        let mut cv = Canvas { w: 4, h: 4, buf: vec![0; 64], acc: Vec::new(), work: 2 * SUB as u64 * 4 + 16 };
+        cv.fill(&square, [255; 4]);
+        assert!(cv.buf.iter().all(|&b| b == 255));
+        cv.buf = vec![0; 64];
+        cv.fill(&square, [255; 4]);
+        assert!(cv.buf.iter().all(|&b| b == 0));
+    }
+
     #[test]
     fn dashed_geometry_stays_within_the_dash_budget() {
         // Forty very long dashed lines, each allowed 10_000 dashes per segment: before the

crates/chart/src/render/lines.rs

@@ -136,7 +136,13 @@ pub(super) fn decimate_area(top: Vec<[f32; 2]>, base: Vec<[f32; 2]>, max: usize)
 
 /// Draws a polyline split at gaps (`None`), decimated when long. With `smooth` (the plot box),
 /// each run is interpolated with a Catmull-Rom spline so the line curves through the data points.
+///
+/// The points interpolation adds are budgeted across all the runs of the line, not per run:
+/// short runs between gaps escape decimation, and 100 sub-segments each would turn a
+/// 100,000-point series of three-point runs into millions of points. Past the budget the
+/// remaining runs are drawn through their data points only.
 pub(super) fn draw_runs(ctx: &mut Ctx, pts: &[Option<[f32; 2]>], color: Rgba, width: f32, smooth: Option<Bx>) {
+    let mut budget = MAX_LINE_PTS * 2;
     let mut run: Vec<[f32; 2]> = Vec::new();
     for p in pts.iter().chain(std::iter::once(&None)) {
         match p {
@@ -145,7 +151,11 @@ pub(super) fn draw_runs(ctx: &mut Ctx, pts: &[Option<[f32; 2]>], color: Rgba, wi
                 if run.len() >= 2 {
                     let r = decimate(std::mem::take(&mut run), MAX_LINE_PTS);
                     let r = match smooth {
-                        Some(plot) => smooth_run(r, plot),
+                        Some(plot) => {
+                            let r = smooth_run(r, plot, budget);
+                            budget = budget.saturating_sub(r.len());
+                            r
+                        }
                         None => r,
                     };
                     ctx.line(r, color, width);
@@ -158,15 +168,15 @@ pub(super) fn draw_runs(ctx: &mut Ctx, pts: &[Option<[f32; 2]>], color: Rgba, wi
 }
 
 /// Catmull-Rom spline through `pts`: every segment gets the same number of sub-segments (up to
-/// 100), chosen so the whole curve stays within `MAX_LINE_PTS * 2` points. Endpoints are
+/// 100), chosen so the whole curve stays within `cap` points (one per data point when `cap` is
+/// smaller than that). Endpoints are
 /// duplicated so the curve passes through the first and last points, and the curve is clamped
 /// to the `plot` box (a spline overshoots its data near sharp turns).
-fn smooth_run(pts: Vec<[f32; 2]>, plot: Bx) -> Vec<[f32; 2]> {
+fn smooth_run(pts: Vec<[f32; 2]>, plot: Bx, cap: usize) -> Vec<[f32; 2]> {
     let n = pts.len();
     if n < 3 {
         return pts;
     }
-    let cap = MAX_LINE_PTS * 2;
     let steps = (cap / n).clamp(1, 100);
     let mut out: Vec<[f32; 2]> = Vec::with_capacity(n.saturating_mul(steps).min(cap) + 1);
     let at = |i: isize| -> [f32; 2] {
@@ -219,7 +229,7 @@ mod smooth_tests {
     fn long_runs_are_smoothed_uniformly_within_the_cap() {
         let plot = Bx { x: 0.0, y: 0.0, w: 3000.0, h: 100.0 };
         let pts: Vec<[f32; 2]> = (0..1500).map(|i| [i as f32 * 2.0, if i % 2 == 0 { 20.0 } else { 80.0 }]).collect();
-        let out = smooth_run(pts, plot);
+        let out = smooth_run(pts, plot, MAX_LINE_PTS * 2);
         assert!(out.len() <= MAX_LINE_PTS * 2 + 1, "{}", out.len());
         // Two sub-segments per segment all the way to the end, not 100 each and then a straight tail.
         assert_eq!(out.len(), 1499 * 2 + 1);
@@ -231,11 +241,29 @@ mod smooth_tests {
         let plot = Bx { x: 0.0, y: 10.0, w: 300.0, h: 100.0 };
         // A step right at the plot edges: Catmull-Rom overshoots past both.
         let pts = vec![[0.0, 110.0], [100.0, 110.0], [110.0, 10.0], [200.0, 10.0], [300.0, 10.0]];
-        let out = smooth_run(pts, plot);
+        let out = smooth_run(pts, plot, MAX_LINE_PTS * 2);
         assert!(out.len() > 5);
         assert!(out.iter().all(|p| (0.0..=300.0).contains(&p[0]) && (10.0..=110.0).contains(&p[1])));
     }
 
+    /// Security: smoothing budgets its added points across every run of a line. A series of
+    /// three-point runs between gaps used to get 100 sub-segments per run, 67 times its data.
+    #[test]
+    fn gap_separated_runs_share_one_smoothing_budget() {
+        let plot = Bx { x: 0.0, y: 0.0, w: 1e6, h: 100.0 };
+        let pts: Vec<Option<[f32; 2]>> =
+            (0..100_000).map(|i| if i % 4 == 3 { None } else { Some([i as f32, if i % 2 == 0 { 20.0 } else { 80.0 }]) }).collect();
+        let m = crate::ApproxMeasure;
+        let mut ctx = Ctx { out: Vec::new(), m: &m };
+        draw_runs(&mut ctx, &pts, WHITE, 1.0, Some(plot));
+        let total: usize = ctx.out.iter().map(|p| if let Prim::Line { pts, .. } = p { pts.len() } else { 0 }).sum();
+        // 75,000 data points, plus at most the budget added by interpolation.
+        assert!(total <= 75_000 + MAX_LINE_PTS * 2, "{total}");
+        assert_eq!(ctx.out.len(), 25_000);
+        // The first runs are still smoothed.
+        assert!(matches!(ctx.out.first(), Some(Prim::Line { pts, .. }) if pts.len() > 3));
+    }
+
     #[test]
     fn decimate_area_preserves_paired_vertical_strips() {
         let top: Vec<[f32; 2]> = (0..1000).map(|i| [i as f32 * 0.25, ((i * 37) % 100) as f32]).collect();

crates/chart/src/resolve.rs

@@ -133,6 +133,46 @@ fn value_text(v: &Value) -> String {
     }
 }
 
+/// Characters kept of one category label: an axis label never shows more (labels are fitted to
+/// the axis), and copying more only costs memory.
+const MAX_LABEL_CHARS: usize = 255;
+/// Bytes of category label text kept per series. One long text broadcast by a short formula
+/// (`IF(SEQUENCE(100000), long_text)`) is a single shared value, but turning every element into
+/// its own label copied it 100,000 times (gigabytes). Labels past the budget are empty.
+const MAX_CATEGORY_BYTES: usize = 8 << 20;
+
+/// At most [`MAX_LABEL_CHARS`] characters and `room` bytes of `s`, cut at a character boundary.
+fn clip_label(s: &str, room: usize) -> String {
+    let mut end = 0;
+    for (n, (i, c)) in s.char_indices().enumerate() {
+        if n >= MAX_LABEL_CHARS || i + c.len_utf8() > room {
+            break;
+        }
+        end = i + c.len_utf8();
+    }
+    s.get(..end).unwrap_or_default().to_string()
+}
+
+/// The label of category `i` (value `v`) in at most `room` bytes: formatted with its source
+/// cell's number format when it is a number.
+fn category_label(wb: &Workbook, src: Option<(&gridcraft_model::Sheet, RangeRef)>, i: usize, v: &Value, room: usize) -> String {
+    if let (Some((s, r)), Value::Number(_)) = (src, v) {
+        let w = r.width().max(1) as usize;
+        let at = r.start.offset((i / w) as i64, (i % w) as i64);
+        if let Some(c) = at {
+            let code = &wb.styles.get(s.style_id(c)).num_fmt.0;
+            if !code.eq_ignore_ascii_case("general") {
+                return clip_label(&format_value(v, &NumberFormat::parse_cached(code), wb.date_system).text, room);
+            }
+        }
+    }
+    // Text is clipped before it is copied: the source may be one long shared string.
+    match v {
+        Value::Text(t) => clip_label(t, room),
+        _ => clip_label(&value_text(v), room),
+    }
+}
+
 /// Category labels, formatted with each source cell's number format (dates, currency…), and
 /// the outer levels of a multi-level axis: a category range wider than a column of values (or
 /// taller than a row of values) groups its innermost labels, year beside month. Each outer level
@@ -141,21 +181,14 @@ fn category_table(wb: &Workbook, sheet: usize, formula: &str, values: &str) -> (
     let vals = eval_list(wb, sheet, formula);
     let cells: Option<(usize, RangeRef)> = parse_ref(wb, sheet, formula);
     let src = cells.and_then(|(sh, r)| wb.sheet(sh).map(|s| (s, r)));
+    let mut room = MAX_CATEGORY_BYTES;
     let texts: Vec<String> = vals
         .iter()
         .enumerate()
         .map(|(i, v)| {
-            if let (Some((s, r)), Value::Number(_)) = (src, v) {
-                let w = r.width().max(1) as usize;
-                let at = r.start.offset((i / w) as i64, (i % w) as i64);
-                if let Some(c) = at {
-                    let code = &wb.styles.get(s.style_id(c)).num_fmt.0;
-                    if !code.eq_ignore_ascii_case("general") {
-                        return format_value(v, &NumberFormat::parse_cached(code), wb.date_system).text;
-                    }
-                }
-            }
-            value_text(v)
+            let label = category_label(wb, src, i, v, room);
+            room = room.saturating_sub(label.len());
+            label
         })
         .collect();
     let Some((_, r)) = src else { return (texts, vec![]) };
@@ -276,3 +309,23 @@ pub fn resolve(wb: &Workbook, sheet: usize, chart: &Chart) -> ChartData {
     let palette = default_palette(&wb.theme, n.min(PALETTE_CYCLE));
     ChartData { categories, category_levels, series, palette }
 }
+
+#[cfg(test)]
+mod label_budget_tests {
+    use gridcraft_model::Workbook;
+
+    /// Security: category labels are clipped before they are copied, with one byte budget per
+    /// series. A formula broadcasting one long text to 100,000 categories used to copy it
+    /// 100,000 times (3.3 GB).
+    #[test]
+    fn category_labels_share_one_byte_budget() {
+        let wb = Workbook::new();
+        let (texts, _) = super::category_table(&wb, 0, r#"=IF(SEQUENCE(100000),REPT("x",32767))"#, "");
+        assert_eq!(texts.len(), 100_000);
+        assert_eq!(texts.first().map(String::len), Some(super::MAX_LABEL_CHARS));
+        assert!(texts.iter().map(String::len).sum::<usize>() <= super::MAX_CATEGORY_BYTES);
+        // Labels are cut at character boundaries.
+        assert_eq!(super::clip_label("aé", 2), "a");
+        assert_eq!(super::clip_label("aé", 3), "aé");
+    }
+}

crates/engine/src/cmd.rs

@@ -459,8 +459,11 @@ pub(crate) const TOUCH_LOG_LIMIT: usize = 1_000_000;
 pub(crate) const PROTECTED: &str = "The cell or chart you're trying to change is on a protected sheet.";
 
 /// Refuses an edit that changed what a locked cell on a protected sheet holds (its value or
-/// formula), whichever command made it. Sheets are matched by name, so moving, renaming or
-/// deleting a sheet isn't an edit of its cells.
+/// formula), or that unlocked it, whichever command made it. Sheets are matched by name, so
+/// moving, renaming or deleting a sheet isn't an edit of its cells. A sort or duplicate removal
+/// that moves an unlocked cell with equal content onto a locked one changes no content but would
+/// leave the cell editable, so the effective lock (row and column formats included) is compared
+/// as well.
 fn check_protection(old: &Workbook, new: &Workbook, unlocked: &[(String, String)]) -> Result<()> {
     for sh in &old.sheets {
         if !sh.is_protected() {
@@ -471,7 +474,8 @@ fn check_protection(old: &Workbook, new: &Workbook, unlocked: &[(String, String)
             continue;
         }
         for c in sh.cells.diff(&after.cells) {
-            if !same_content(sh.cell(c), after.cell(c))
+            let unlocked_now = !new.styles.get(after.style_id(c)).protection.locked;
+            if (unlocked_now || !same_content(sh.cell(c), after.cell(c)))
                 && old.styles.get(sh.style_id(c)).protection.locked
                 && let Some(m) = locked_cell_error(sh, c, unlocked)
             {

crates/engine/src/cmd/sheet.rs

@@ -398,10 +398,10 @@ fn move_columns(s: &mut Session, p: &Json) -> Result<Json> {
     {
         return Err(EngineError::Other("To prevent possible loss of data, columns cannot be shifted off of the worksheet.".into()));
     }
-    // The checks must authorize the sheet this command modifies (`target_sheet`), not whatever
-    // sheet is active: both carry the command's own `sheet` parameter on.
-    check_lines_protection(s, &json!({"cols": RangeRef::cols(before, before).a1(), "sheet": p.get("sheet")}), Axis::Cols, before, count, true)?;
-    check_lines_protection(s, &json!({"cols": RangeRef::cols(from, end - 1).a1(), "sheet": p.get("sheet")}), Axis::Cols, from, count, false)?;
+    // The checks must authorize the sheet this command modifies, not whatever sheet is active:
+    // both get the index `target_sheet` resolved (from `sheet`, or a `Sheet!` range or cell).
+    check_lines_protection(s, &json!({"cols": RangeRef::cols(before, before).a1(), "sheet": sheet}), Axis::Cols, before, count, true)?;
+    check_lines_protection(s, &json!({"cols": RangeRef::cols(from, end - 1).a1(), "sheet": sheet}), Axis::Cols, from, count, false)?;
 
     let moved_from = if before <= from { from + count } else { from };
     if moved_from.checked_add(count).is_none_or(|end| end > MAX_COLS) {

crates/engine/src/tests.rs

@@ -5297,6 +5297,42 @@ fn move_columns_checks_the_protection_of_the_named_sheet() {
     assert_eq!(v(&s, "A1"), Value::text("A"));
 }
 
+#[test]
+fn move_columns_checks_the_protection_of_a_sheet_named_by_its_range() {
+    // A `Sheet!` range or cell picks the sheet the move modifies; the protection checks must
+    // authorize that sheet too, not the active one.
+    let mut s = s();
+    s.execute("home.insertSheet", json!({"name": "P"})).unwrap();
+    s.execute("range.setValues", json!({"range": "A1", "values": [[1, 2, 3, 4]]})).unwrap();
+    s.execute("review.protectSheet", json!({})).unwrap();
+    s.execute("sheet.activate", json!({"sheet": 0})).unwrap();
+    for key in ["range", "cell"] {
+        let err = s.execute("sheet.moveColumns", json!({key: "P!A1", "cols": "B:C", "before": 4})).expect_err("moving columns on a protected sheet");
+        assert!(err.to_string().contains("protected"), "{err}");
+    }
+    let d = s.doc().unwrap();
+    let sh = &d.wb.sheets[d.wb.sheet_index("P").unwrap()];
+    let pv: Vec<Value> = (0..4).map(|c| sh.value(CellRef::new(0, c))).collect();
+    assert_eq!(pv, vec![Value::number(1.0), Value::number(2.0), Value::number(3.0), Value::number(4.0)]);
+}
+
+#[test]
+fn sorting_cannot_unlock_a_locked_cell_by_moving_equal_content_onto_it() {
+    // An unlocked 2 sorted onto a locked 2 leaves the content as it was, but would make the cell
+    // editable; the sort is refused instead.
+    let mut s = s();
+    s.execute("range.setValues", json!({"range": "A1", "values": [[2], [2], [1]]})).unwrap();
+    s.execute("home.lockCell", json!({"range": "A1", "on": false})).unwrap();
+    s.execute("home.lockCell", json!({"range": "A3", "on": false})).unwrap();
+    s.execute("review.protectSheet", json!({"sort": true})).unwrap();
+    let before = s.doc().unwrap().wb.clone();
+    let err =
+        s.execute("data.sort", json!({"range": "A1:A3", "header": false, "keys": [{"column": "A", "order": "asc"}]})).expect_err("sort unlocking A2");
+    assert!(err.to_string().contains("protected sheet"), "{err}");
+    assert_eq!(*s.doc().unwrap().wb, *before);
+    assert!(s.execute("cell.set", json!({"cell": "A2", "input": "9"})).is_err(), "A2 stays locked");
+}
+
 #[test]
 fn a_script_cannot_switch_windows() {
     // Stored scripts are scoped to their workbook: a step that activates another window would

crates/xlsx/src/styles.rs

@@ -59,14 +59,11 @@ fn bool_prop(e: &El, name: &str) -> bool {
 
 const DEFAULT_FONT_FAMILY: u8 = 2;
 const MAX_FONT_FAMILY: u8 = 14;
-/// Longest font name kept, in characters (Excel's own limit). Every cell format copies its
-/// font, so a long name repeated by many formats would otherwise multiply in memory.
-const MAX_FONT_NAME: usize = 31;
 
 pub fn read_font(e: &El, palette: &[u32]) -> Font {
     let mut f = Font::default();
     if let Some(n) = e.child("name").or_else(|| e.child("rFont")).and_then(|n| n.attr("val")) {
-        f.name = n.chars().take(MAX_FONT_NAME).collect();
+        f.name = n.to_string();
     }
     if let Some(sz) = e.child("sz").and_then(|s| s.attr_f64("val")) {
         f.size = sz.clamp(1.0, 409.0) as f32;
@@ -280,14 +277,14 @@ fn read_protection(p: &El) -> Protection {
     Protection { locked: p.flag("locked", true), hidden: p.flag("hidden", false) }
 }
 
-fn num_fmt_code(id: u32, custom: &HashMap<u32, NumFmt>) -> NumFmt {
+fn num_fmt_code(id: u32, custom: &HashMap<u32, String>) -> NumFmt {
     if let Some(c) = custom.get(&id) {
-        return c.clone();
+        return NumFmt::new(c);
     }
     NumFmt::new(builtin_format(id).unwrap_or("General"))
 }
 
-fn read_xf(xf: &El, fonts: &[Font], fills: &[Fill], borders: &[Borders], fmts: &HashMap<u32, NumFmt>, checkboxes: &[bool]) -> Style {
+fn read_xf(xf: &El, fonts: &[Font], fills: &[Fill], borders: &[Borders], fmts: &HashMap<u32, String>, checkboxes: &[bool]) -> Style {
     let mut s = Style::default();
     if let Some(f) = xf.attr_u32("fontId").and_then(|i| fonts.get(i as usize)) {
         s.font = f.clone();
@@ -339,8 +336,6 @@ pub fn read_styles(root: &El, checkboxes: &[bool], minor_font: &str) -> StylesIn
             }
         }
     }
-    // One shared copy of each custom code, which every format using it points to.
-    let shared_fmts: HashMap<u32, NumFmt> = fmts.iter().map(|(id, code)| (*id, NumFmt::new(code))).collect();
     // The scheme link only means something on a font in the body face; elsewhere it stays as a new font has it.
     let body_font = |e: &El| {
         let mut f = read_font(e, &palette);
@@ -350,14 +345,10 @@ pub fn read_styles(root: &El, checkboxes: &[bool], minor_font: &str) -> StylesIn
     let fonts: Vec<Font> = root.child("fonts").map(|f| f.kids("font").map(body_font).collect()).unwrap_or_default();
     let fills: Vec<Fill> = root.child("fills").map(|f| f.kids("fill").map(|e| read_fill(e, &palette, false)).collect()).unwrap_or_default();
     let borders: Vec<Borders> = root.child("borders").map(|f| f.kids("border").map(|e| read_border(e, &palette)).collect()).unwrap_or_default();
-    let xfs = root
-        .child("cellXfs")
-        .map(|x| x.kids("xf").map(|xf| read_xf(xf, &fonts, &fills, &borders, &shared_fmts, checkboxes)).collect())
-        .unwrap_or_default();
-    let style_xfs: Vec<Style> = root
-        .child("cellStyleXfs")
-        .map(|x| x.kids("xf").map(|xf| read_xf(xf, &fonts, &fills, &borders, &shared_fmts, &[])).collect())
-        .unwrap_or_default();
+    let xfs =
+        root.child("cellXfs").map(|x| x.kids("xf").map(|xf| read_xf(xf, &fonts, &fills, &borders, &fmts, checkboxes)).collect()).unwrap_or_default();
+    let style_xfs: Vec<Style> =
+        root.child("cellStyleXfs").map(|x| x.kids("xf").map(|xf| read_xf(xf, &fonts, &fills, &borders, &fmts, &[])).collect()).unwrap_or_default();
     let mut named = Vec::new();
     if let Some(cs) = root.child("cellStyles") {
         for c in cs.kids("cellStyle") {
@@ -368,12 +359,12 @@ pub fn read_styles(root: &El, checkboxes: &[bool], minor_font: &str) -> StylesIn
             }
         }
     }
-    let dxfs = root.child("dxfs").map(|d| d.kids("dxf").map(|x| read_dxf(x, &palette, &shared_fmts)).collect()).unwrap_or_default();
+    let dxfs = root.child("dxfs").map(|d| d.kids("dxf").map(|x| read_dxf(x, &palette, &fmts)).collect()).unwrap_or_default();
     StylesIn { xfs, dxfs, palette, named, num_fmts: fmts }
 }
 
 /// One differential format (`dxf`): the style it sets over a cell's own.
-fn read_dxf(x: &El, palette: &[u32], fmts: &HashMap<u32, NumFmt>) -> Style {
+fn read_dxf(x: &El, palette: &[u32], fmts: &HashMap<u32, String>) -> Style {
     let mut s = Style::default();
     if let Some(e) = x.child("font") {
         let f = read_font(e, palette);

crates/xlsx/src/tests/malformed.rs

@@ -385,22 +385,3 @@ fn outline_summary_directions_round_trip() {
     let xml = String::from_utf8(crate::package::Package::open(&bytes).unwrap().read("xl/worksheets/sheet1.xml").unwrap().unwrap()).unwrap();
     assert!(!xml.contains("outlinePr"), "{xml}");
 }
-
-/// Many cell formats naming one long font and one long custom number format: the code is shared
-/// by every format and the font name is cut to Excel's 31 characters, so neither is copied per
-/// format.
-#[test]
-fn cell_formats_share_their_number_format_and_keep_short_font_names() {
-    let xml = format!(
-        r#"<styleSheet><numFmts><numFmt numFmtId="164" formatCode="{}"/></numFmts><fonts><font><name val="{}"/></font></fonts><cellXfs>{}</cellXfs></styleSheet>"#,
-        "0".repeat(100_000),
-        "F".repeat(100_000),
-        r#"<xf numFmtId="164" fontId="0"/>"#.repeat(20_000)
-    );
-    let root = crate::xml::parse(xml.as_bytes()).unwrap();
-    let st = crate::styles::read_styles(&root, &[], "Calibri");
-    assert_eq!(st.xfs.len(), 20_000);
-    assert!(st.xfs.iter().all(|s| std::sync::Arc::ptr_eq(&s.num_fmt.0, &st.xfs[0].num_fmt.0)));
-    assert_eq!(st.xfs[0].num_fmt.as_str().len(), 100_000);
-    assert!(st.xfs.iter().all(|s| s.font.name.chars().count() == 31));
-}