From 9da4e4cdf7290ef22ffa31d20a9c37e38c73b74a Mon Sep 17 00:00:00 2001 From: nihilist-narwhal <240798405+nihilist-narwhal@users.noreply.github.com> Date: Sat, 10 Oct 2026 02:43:39 -0500 Subject: [PATCH] Manage Rules: don't index past the list after a delete home.manageRules took the number of rules once, before deleting, so {"delete":0,"moveUp":1} with two rules swapped past the end of the shortened list and panicked (index out of bounds; the session's guard kept the workbook). The count is now taken after each step. --- crates/engine/src/cmd/insert.rs | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index 392c5ee..6c95106 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -1420,20 +1420,20 @@ fn manage_cf(s: &mut Session, p: &Json) -> Result { } edit(s, |cx| { let sh = cx.sheet_mut(sheet)?; - let n = sh.cond_formats.len(); + // The count is taken again after each step: a delete shortens the list. if let Some(i) = u32_param(p, "delete").map(|v| v as usize) - && i < n + && i < sh.cond_formats.len() { sh.cond_formats.remove(i); } if let Some(i) = u32_param(p, "moveUp").map(|v| v as usize) && i > 0 - && i < n + && i < sh.cond_formats.len() { sh.cond_formats.swap(i, i - 1); } if let Some(i) = u32_param(p, "moveDown").map(|v| v as usize) - && i + 1 < n + && i + 1 < sh.cond_formats.len() { sh.cond_formats.swap(i, i + 1); } @@ -1466,3 +1466,23 @@ fn insert_icon(s: &mut Session, p: &Json) -> Result { Ok(json!({"shape": id})) }) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn manage_rules_delete_then_move() { + let mut s = Session::new(); + s.new_workbook(); + for v in ["1", "2"] { + let rule = json!({"type": "cellIs", "operator": "greaterThan", "value": v, "preset": "lightRedFill"}); + s.execute("home.conditionalFormat", json!({"range": "A1:A3", "rule": rule})).unwrap(); + } + // Deleting first left two rules' worth of indices for the move: it panicked. + s.execute("home.manageRules", json!({"delete": 0, "moveUp": 1})).unwrap(); + s.execute("home.manageRules", json!({"delete": 0, "moveDown": 0})).unwrap(); + let rules = s.execute("home.manageRules", json!({})).unwrap(); + assert_eq!(rules["rules"].as_array().map(Vec::len), Some(0)); + } +}