From 1de456fd69a91d75ea4e4de667d7e7198fd4f384 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 4 Nov 2026 00:15:35 +0000 Subject: [PATCH] address review: enforce the 32,767-character limit on replacements and borrow the replacement text --- crates/engine/src/cmd/edit/find.rs | 20 ++++++++++++-------- crates/engine/src/cmd/edit/tests.rs | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/cmd/edit/find.rs b/crates/engine/src/cmd/edit/find.rs index f701e4665..eca607863 100644 --- a/crates/engine/src/cmd/edit/find.rs +++ b/crates/engine/src/cmd/edit/find.rs @@ -104,16 +104,19 @@ pub(super) fn find(s: &mut Session, p: &Json) -> Result { Ok(json!({"cell": c.a1(), "sheet": si, "value": t, "count": hits.len()})) } -/// Longest text one replacement may give a cell, in bytes: Excel's 32,767 characters, each up to -/// four bytes. Every occurrence copies the replacement text, so a short cell full of matches and a -/// long replacement would otherwise expand to gigabytes. -const REPLACE_CELL_MAX_BYTES: usize = 32_767 * 4; +/// Most characters a cell holds (Excel's limit). +const REPLACE_CELL_MAX_CHARS: usize = 32_767; +/// Longest text one replacement may give a cell, in bytes: the character limit, each up to four +/// bytes. Every occurrence copies the replacement text, so a short cell full of matches and a +/// long replacement would otherwise expand to gigabytes. The character limit is checked on what +/// fits in this. +const REPLACE_CELL_MAX_BYTES: usize = REPLACE_CELL_MAX_CHARS * 4; /// Most bytes of replaced text one Replace All builds across all its cells. const REPLACE_TOTAL_MAX_BYTES: usize = 256 * 1024 * 1024; pub(super) fn replace(s: &mut Session, p: &Json) -> Result { let what = str_param(p, "what").unwrap_or("").to_string(); - let with = str_param(p, "with").unwrap_or("").to_string(); + let with = str_param(p, "with").unwrap_or(""); if what.is_empty() { return Err(bad("edit.replace", "missing `what`")); } @@ -138,12 +141,13 @@ pub(super) fn replace(s: &mut Session, p: &Json) -> Result { continue; } let new = if whole { - Some(with.clone()).filter(|w| w.len() <= REPLACE_CELL_MAX_BYTES) + (with.len() <= REPLACE_CELL_MAX_BYTES).then(|| with.to_string()) } else if case { - replace_cs_within(&text, &what, &with, REPLACE_CELL_MAX_BYTES) + replace_cs_within(&text, &what, with, REPLACE_CELL_MAX_BYTES) } else { - replace_ci_within(&text, &what, &with, REPLACE_CELL_MAX_BYTES) + replace_ci_within(&text, &what, with, REPLACE_CELL_MAX_BYTES) }; + let new = new.filter(|n| n.chars().count() <= REPLACE_CELL_MAX_CHARS); let Some(new) = new else { return Err(EngineError::Other(format!("The replacement makes the text of {} too long.", c.a1()))); }; diff --git a/crates/engine/src/cmd/edit/tests.rs b/crates/engine/src/cmd/edit/tests.rs index 98bc438e9..130aeb878 100644 --- a/crates/engine/src/cmd/edit/tests.rs +++ b/crates/engine/src/cmd/edit/tests.rs @@ -372,6 +372,20 @@ fn replace_refuses_text_that_would_grow_past_a_cell() { assert_eq!(s.execute("cell.get", json!({"cell": "A2"})).unwrap()["value"], json!(format!("a{}c", "x".repeat(100)))); } +#[test] +fn replace_refuses_a_result_past_the_character_limit_though_it_fits_the_byte_limit() { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [["z"]]})).unwrap(); + for whole in [false, true] { + let err = s.execute("edit.replace", json!({"what": "z", "with": "x".repeat(32_768), "wholeCell": whole})).expect_err("32,768 characters"); + assert!(err.to_string().contains("too long"), "{err}"); + assert_eq!(s.execute("cell.get", json!({"cell": "A1"})).unwrap()["value"], json!("z")); + } + s.execute("edit.replace", json!({"what": "z", "with": "x".repeat(32_767)})).unwrap(); + assert_eq!(s.execute("cell.get", json!({"cell": "A1"})).unwrap()["value"], json!("x".repeat(32_767))); +} + #[test] fn bounded_replace_stops_at_its_limit() { use super::find::{replace_ci_within, replace_cs_within};