From 0ce545fcb82d8223dfcf85af34d13e10e0fe7e03 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 22 Oct 2026 18:46:51 +0200 Subject: [PATCH] Reject invalid row/col index in table row and column commands Found while testing crates/engine/src/cmd/table.rs:234-238 (at), via crates/engine/src/cmd/mod.rs:318 (usize_param) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/table.rs | 16 ++++++++++------ crates/engine/src/tests.rs | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/table.rs b/crates/engine/src/cmd/table.rs index 8ab95549..a65b61e7 100644 --- a/crates/engine/src/cmd/table.rs +++ b/crates/engine/src/cmd/table.rs @@ -231,10 +231,11 @@ fn cell_param(p: &Value, key: &str) -> Option<(usize, usize)> { } /// The table's row/col to act on: params, the edited cell, or the selected cells. -fn at(s: &Session, p: &Value) -> (usize, usize) { +/// A present but invalid `row`/`col` is an error; only an absent one falls back. +fn at(s: &Session, p: &Value, cmd: &str) -> Result<(usize, usize)> { let st = s.active(); let cell = st.and_then(|d| d.selection.text.as_ref().and_then(|t| t.cell)).or_else(|| st.and_then(|d| d.selection.cells.map(|c| c.1))); - (usize_param(p, "row").or(cell.map(|c| c.0)).unwrap_or(0), usize_param(p, "col").or(cell.map(|c| c.1)).unwrap_or(0)) + Ok((opt_usize_param(p, "row", cmd)?.or(cell.map(|c| c.0)).unwrap_or(0), opt_usize_param(p, "col", cmd)?.or(cell.map(|c| c.1)).unwrap_or(0))) } fn with_table(s: &mut Session, p: &Value, cmd: &str, f: impl Fn(&mut Table) -> Result<()>) -> Result { @@ -283,7 +284,7 @@ fn with_table(s: &mut Session, p: &Value, cmd: &str, f: impl Fn(&mut Table) -> R } fn rows_cols(s: &mut Session, p: &Value, op: &str) -> Result { - let (r, c) = at(s, p); + let (r, c) = at(s, p, &format!("table.{op}"))?; let op = op.to_string(); with_table(s, p, "table", move |t| { match op.as_str() { @@ -319,7 +320,10 @@ fn merge(s: &mut Session, p: &Value) -> Result { } fn split(s: &mut Session, p: &Value) -> Result { - let cell = cell_param(p, "cell").unwrap_or_else(|| at(s, p)); + let cell = match cell_param(p, "cell") { + Some(c) => c, + None => at(s, p, "table.split")?, + }; with_table(s, p, "table.split", move |t| if t.split(cell.0, cell.1) { Ok(()) } else { Err(bad("table.split", "that cell isn't merged")) }) } @@ -531,7 +535,7 @@ fn distribute(s: &mut Session, p: &Value, rows: bool) -> Result { } fn column_width(s: &mut Session, p: &Value) -> Result { - let (_, c) = at(s, p); + let (_, c) = at(s, p, "table.columnWidth")?; let w = f64_param(p, "width").ok_or_else(|| bad("table.columnWidth", "missing `width`"))?.clamp(4.0, 4000.0); let keep_total = bool_or(p, "keepTotal", false); with_table(s, p, "table.columnWidth", move |t| { @@ -560,7 +564,7 @@ fn column_width(s: &mut Session, p: &Value) -> Result { } fn row_height(s: &mut Session, p: &Value) -> Result { - let (r, _) = at(s, p); + let (r, _) = at(s, p, "table.rowHeight")?; let h = f64_param(p, "height").ok_or_else(|| bad("table.rowHeight", "missing `height`"))?.clamp(4.0, 4000.0); with_table(s, p, "table.rowHeight", move |t| { if let Some(x) = t.rows.get_mut(r) { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index dd64b08c..6aaf51ae 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3672,6 +3672,26 @@ fn table_insert_below_or_right_of_a_huge_index_appends_at_the_end() { assert_eq!(t.cell(0, 1).unwrap().text.text(), "b", "the original columns keep their place"); } +#[test] +fn table_row_col_commands_reject_an_invalid_index() { + let mut s = session(); + let id = table_at(&mut s, &[100.0, 100.0], json!([["a", "b"], ["c", "d"], ["e", "f"]])); + for (cmd, key) in [ + ("table.deleteRow", "row"), + ("table.deleteColumn", "col"), + ("table.insertRowAbove", "row"), + ("table.insertColumnLeft", "col"), + ("table.split", "row"), + ] { + for bad in [json!(-1), json!(1.5), json!("1")] { + let r = s.execute(cmd, &json!({"id": id.0, key: bad})); + assert!(r.is_err(), "{cmd} {key}={bad} should be rejected, got {r:?}"); + let t = table_of(&s, id); + assert_eq!((t.n_rows(), t.n_cols()), (3, 2), "{cmd} {key}={bad} must leave the table unchanged"); + } + } +} + #[test] fn table_column_width_of_a_huge_column_is_ignored() { let mut s = session();