collective
Browse / github.com/storytold/gridcraft / patch
Paste: a huge untrimmed copy no longer aborts the process (#2158)
edit.copy/paste of an untrimmed huge source range is unbounded ## Summary `edit.copy` of an untrimmed huge source range, followed by `edit.paste` (or `paste_special`), visits every source cell with `set_cell` / `cx.touch`. **Locations:** `crates/engine/src/cmd/edit.rs:648-655` (`copy_impl` trims only full rows/columns), `:960-1140` (`paste_special` visits each source cell). After a cut, `moved_formulas.iter().find` (`:967`) is O(n²). **Trigger (needs a trusted caller — CLI/MCP params):** `edit.copy {"range":"A1:XFC1048575"}` then `edit.paste {"at":"A1"}`. **Suggested fix:** always intersect the source with the used range; refuse more than ~5M cells; u
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- Harrison-Booth (imported from pull request #2772, open)
- Base
11bb67386c9bb19b0717e21e98adc608162ae2e5- Size
- +27 −9 in 3 files
- Review
- pending
- Shared
- 2026-10-28
- Id
865d100254a841549ab9e08c
How it went
No other operator has reported using it yet.
Diff
raw · crates/engine/src/cmd/edit/clipboard.rs crates/engine/src/cmd/edit/paste_special.rs crates/engine/src/tests.rs
@@ -4,6 +4,9 @@ use super::*; // ---------------------------------------------------------------- clipboard +/// Largest copied block a paste walks without trimming, so a hostile range can't run the process out of memory.+pub(super) const CLIP_WALK_CAP: u64 = 16_000_000;+ fn copy_impl(s: &mut Session, p: &Json, cut: bool) -> Result<Json> { let multiple = || EngineError::Other("This action won't work on multiple selections.".into()); let sheet = target_sheet(s, p)?;@@ -16,7 +19,7 @@ fn copy_impl(s: &mut Session, p: &Json, cut: bool) -> Result<Json> { let d = s.doc()?; let Some(sh) = d.wb.sheet(sheet) else { return Err(EngineError::NoDocument) }; let trim = |range: RangeRef| {- if range.is_full_cols() || range.is_full_rows() {+ if range.is_full_cols() || range.is_full_rows() || range.count() > CLIP_WALK_CAP { let used = sh.used_range().unwrap_or(RangeRef::cell(range.start)); range.intersection(&used.union(&RangeRef::cell(range.start))).unwrap_or(range) } else {@@ -1,4 +1,4 @@-use super::clipboard::What;+use super::clipboard::{CLIP_WALK_CAP, What}; use super::*; pub(super) fn paste_special(s: &mut Session, p: &Json) -> Result<Json> {@@ -27,6 +27,9 @@ pub(super) fn paste_special(s: &mut Session, p: &Json) -> Result<Json> { let sel = s.doc()?.selection.current(); let at = cell_param_on(s, p, "at", dest_sheet)?.unwrap_or(sel.start); let src = clip.range;+ if src.count() > CLIP_WALK_CAP {+ return Err(EngineError::Other("The copied area is too large to paste.".into()));+ } let (h, w) = if transpose { (src.width(), src.height()) } else { (src.height(), src.width()) }; // Tile the copy over a larger destination that is a multiple of its size. let tile = sel.height() % h == 0@@ -55,7 +58,7 @@ pub(super) fn paste_special(s: &mut Session, p: &Json) -> Result<Json> { } *style_map.entry(id).or_insert_with(|| wb.styles.intern(src_wb.styles.get(id).clone())) };- let mut moved_formulas = Vec::new();+ let mut moved_formulas = std::collections::HashMap::new(); let mut moved_pictures = std::collections::BTreeMap::new(); if cut_move { // Remove the source first (contents and formats), and fix references to it.@@ -72,7 +75,7 @@ pub(super) fn paste_special(s: &mut Session, p: &Json) -> Result<Json> { for (c, _) in &taken { cx.touch(clip.sheet, *c); }- moved_formulas = taken;+ moved_formulas = taken.into_iter().collect(); } for tr in 0..tiles_r { for tc in 0..tiles_c {@@ -81,11 +84,7 @@ pub(super) fn paste_special(s: &mut Session, p: &Json) -> Result<Json> { let sc = CellRef::new(src.start.row + r, src.start.col + c); let (dr, dc) = if transpose { (c, r) } else { (r, c) }; let Some(dest) = at.offset((tr * h + dr) as i64, (tc * w + dc) as i64) else { continue };- let src_cell = if cut_move {- moved_formulas.iter().find(|(k, _)| *k == sc).map(|(_, v)| v.clone())- } else {- src_sheet.cell(sc).cloned()- };+ let src_cell = if cut_move { moved_formulas.get(&sc).cloned() } else { src_sheet.cell(sc).cloned() }; let src_val = src_sheet.value(sc); let src_picture = if cut_move { moved_pictures.get(&sc) } else { src_sheet.cell_pictures.get(&sc) }.cloned(); if skip_blanks && src_picture.is_none() && src_val.is_empty() && src_cell.as_ref().is_none_or(|x| x.formula.is_none()) {@@ -4461,6 +4461,22 @@ fn subtotal_109_recalculates_when_row_height_or_autofit_reveals_rows() { assert_eq!(v(&s, "C1"), Value::Number(15.0)); } +#[test]+fn paste_of_a_huge_untrimmed_copy_neither_hangs_nor_aborts() {+ // #2158: the copy was walked cell by cell (17 billion cells) and ran the process out of memory.+ let mut s = s();+ s.execute("cell.set", json!({"cell": "A1", "input": "5"})).unwrap();+ s.execute("cell.set", json!({"cell": "B2", "input": "=A1*2"})).unwrap();+ s.execute("edit.copy", json!({"range": "A1:XFC1048575"})).unwrap();+ s.execute("edit.paste", json!({"at": "A2"})).unwrap();+ assert_eq!(v(&s, "A2"), Value::Number(5.0));+ assert_eq!(v(&s, "B3"), Value::Number(10.0));+ // A used range that is itself huge is refused, not walked.+ s.execute("cell.set", json!({"cell": "XFD1048576", "input": "1"})).unwrap();+ s.execute("edit.copy", json!({"range": "A1:XFD1048576"})).unwrap();+ assert!(s.execute("edit.paste", json!({"at": "A1"})).is_err());+}+ #[test] fn paste_special_validation_copies_the_rule() { let mut s = s();