From c45212ac688e0118d7656668062e8fe08e662786 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Mon, 12 Oct 2026 08:27:27 +0200 Subject: [PATCH] Refuse inserting cells that would push values off the worksheet (#580) --- crates/engine/src/cmd/sheet.rs | 9 +++++++++ crates/engine/src/tests.rs | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index f05b77d91..14bb37f87 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -427,6 +427,15 @@ fn insert_cells(s: &mut Session, p: &Json) -> Result { let sheet = target_sheet(s, p)?; if let Some(sh) = s.doc()?.wb.sheet(sheet) { check_table_shift(sh, &e)?; + // Refuse to push data off the sheet: cells in the last `width`/`height` positions would be dropped. + let doomed = if shift == "right" { + RangeRef::new(CellRef::new(r.start.row, r.start.col.max(MAX_COLS.saturating_sub(r.width()))), CellRef::new(r.end.row, MAX_COLS - 1)) + } else { + RangeRef::new(CellRef::new(r.start.row.max(MAX_ROWS.saturating_sub(r.height())), r.start.col), CellRef::new(MAX_ROWS - 1, r.end.col)) + }; + if sh.cells.iter_range(doomed).any(|(_, x)| x.formula.is_some() || !x.value.is_empty()) { + return Err(EngineError::Other("To prevent possible loss of data, Excel cannot shift nonblank cells off of the worksheet.".into())); + } } let right = shift == "right"; let out = edit(s, |cx| { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 33fdc113e..c1173fff4 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1121,6 +1121,24 @@ fn a_whole_column_table_without_headers_stays_on_the_sheet() { assert!(e.to_string().contains("pushed off the end"), "{e}"); } +#[test] +fn inserting_cells_refuses_to_push_values_off_the_sheet() { + for (shift, edge, last_fit, last_ok) in [("right", "XFD1", "XFC1", "XFD1"), ("down", "A1048576", "A1048575", "A1048576")] { + let mut sess = s(); + sess.execute("cell.set", json!({"cell": "A1", "input": "1"})).unwrap(); + sess.execute("cell.set", json!({"cell": edge, "input": "2"})).unwrap(); + let e = sess.execute("home.insertCells", json!({"range": "A1", "shift": shift})).unwrap_err(); + assert!(e.to_string().contains("loss of data"), "{e}"); + assert_eq!(v(&sess, "A1"), Value::Number(1.0)); + assert_eq!(v(&sess, edge), Value::Number(2.0)); + // A value that still fits moves to the last valid cell. + let mut sess = s(); + sess.execute("cell.set", json!({"cell": last_fit, "input": "2"})).unwrap(); + sess.execute("home.insertCells", json!({"range": "A1", "shift": shift})).unwrap(); + assert_eq!(v(&sess, last_ok), Value::Number(2.0)); + } +} + #[test] fn autosum_over_whole_rows_is_one_undo_step_within_the_data() { // Found by the command-sequence fuzzer: AutoSum over rows `3:5` ran one `cell.set` per column