From ff77dea87fa96794af03eec24f45b706cdbeff2d Mon Sep 17 00:00:00 2001
From: Brandon Thomas <echelon@gmail.com>
Date: Sat, 10 Oct 2026 12:16:09 -0400
Subject: [PATCH] Review fix-up: bound Switch Row/Column on whole-column chart
 refs

Clip a file chart's series block to the sheet's used range and refuse blocks over the
insert.chart cell cap, so whole-column references can't build ~1M series.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 crates/engine/src/cmd/insert.rs | 16 ++++++++++++++--
 crates/engine/src/tests.rs      | 31 +++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs
index b922b4c..d3491bc 100644
--- a/crates/engine/src/cmd/insert.rs
+++ b/crates/engine/src/cmd/insert.rs
@@ -527,9 +527,12 @@ pub fn series_from_range(sh: &Sheet, r: RangeRef, by_rows: bool) -> Vec<Series>
     out
 }
 
+/// Most cells a chart's source block may span (bounds the series it builds).
+const MAX_CHART_CELLS: u64 = 1_000_000;
+
 fn insert_chart(s: &mut Session, p: &Json) -> Result<Json> {
     let r = block(s, p)?;
-    if r.count() > 1_000_000 {
+    if r.count() > MAX_CHART_CELLS {
         return Err(bad("insert.chart", "range too large for a chart"));
     }
     let sheet = target_sheet(s, p)?;
@@ -658,8 +661,17 @@ fn chart_switch(s: &mut Session, p: &Json) -> Result<Json> {
         let Some((src, by_rows)) = c.source.clone().map(|s| (s, c.by_rows)).or_else(|| series_block(&c.series)) else { return Ok(Json::Null) };
         let (sheet_name, body) = split_sheet(&src);
         let src_sheet = sheet_name.and_then(|n| cx.wb.sheet_index(&n)).unwrap_or(si);
-        let Some(r) = RangeRef::parse(body) else { return Ok(Json::Null) };
+        let Some(mut r) = RangeRef::parse(body) else { return Ok(Json::Null) };
         let Some(ssh) = cx.wb.sheet(src_sheet) else { return Ok(Json::Null) };
+        // Whole-column/row references (common in files) span a million cells: clip the block's
+        // far edges to the cells in use, then refuse anything still too large to chart.
+        if let Some(used) = ssh.used_range() {
+            r.end.row = r.end.row.min(used.end.row).max(r.start.row);
+            r.end.col = r.end.col.min(used.end.col).max(r.start.col);
+        }
+        if r.count() > MAX_CHART_CELLS {
+            return Err(bad("chart.switchRowColumn", "range too large for a chart"));
+        }
         let by_rows = !by_rows;
         let series = series_from_range(ssh, r, by_rows);
         if let Some(c) = cx.sheet_mut(si)?.charts.get_mut(ci) {
diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs
index 5a31208..34a18b6 100644
--- a/crates/engine/src/tests.rs
+++ b/crates/engine/src/tests.rs
@@ -629,6 +629,37 @@ fn switch_row_column_on_a_chart_read_from_a_file() {
     assert_eq!(chart(&s).series.len(), 4);
 }
 
+#[test]
+fn switch_row_column_with_whole_column_refs_is_bounded() {
+    let mut s = s();
+    s.execute("range.setValues", json!({"range": "A1", "values": [["", "North", "South"], ["Mon", 10, 14], ["Tue", 12, 15], ["Wed", 14, 16]]}))
+        .unwrap();
+    s.execute("insert.chart", json!({"type": "column", "range": "A1:C4"})).unwrap();
+    // As read from a file: no source block, series over whole columns.
+    let whole = |s: &mut Session| {
+        let c = &mut std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb).sheet_mut(0).unwrap().charts[0];
+        c.source = None;
+        c.by_rows = false;
+        c.series.truncate(2);
+        for (i, col) in ["B", "C"].iter().enumerate() {
+            c.series[i].name = None;
+            c.series[i].categories = Some("Sheet1!$A:$A".into());
+            c.series[i].values = format!("Sheet1!${col}:${col}");
+        }
+    };
+    whole(&mut s);
+    s.execute("chart.switchRowColumn", json!({})).unwrap();
+    let c = s.doc().unwrap().wb.active().unwrap().charts[0].clone();
+    assert!(c.by_rows);
+    // Clipped to the used rows, not a million series.
+    assert_eq!(c.series.len(), 3);
+    // A used range that is itself enormous is refused rather than charted.
+    s.execute("cell.set", json!({"cell": "C1048576", "input": "1"})).unwrap();
+    whole(&mut s);
+    assert!(s.execute("chart.switchRowColumn", json!({})).is_err());
+    assert_eq!(s.doc().unwrap().wb.active().unwrap().charts[0].series.len(), 2);
+}
+
 #[test]
 fn ink_strokes_and_ink_to_shape() {
     let mut s = s();