From 533c0dadafe4eefe246d0a3e0355bbd688698b39 Mon Sep 17 00:00:00 2001 From: Hector Villarreal Date: Tue, 3 Nov 2026 04:19:49 -0600 Subject: [PATCH] #2225: a field that only looks quoted keeps the tabs and line breaks in it Addresses CodeRabbit review on #3231, "Reparse consumed separators when a field is not quoted": parse_tsv read "ab"x as one quoted field, then fell back to its raw text without the separator it had consumed, so the cells "a, b"x and c pasted back as two. The new copy (which no longer quotes a cell holding only a quote) makes that text. A field whose closing quote doesn't end it, or that never closes, is now read again as unquoted text, so its tabs and line breaks separate cells. The text read again is bounded by the input's length, so hostile input stays linear. Test: a_field_that_is_not_really_quoted_keeps_its_separators. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/edit/clipboard.rs | 44 +++++++++++++++++++++---- crates/engine/src/cmd/edit/tests.rs | 22 +++++++++++++ 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/cmd/edit/clipboard.rs b/crates/engine/src/cmd/edit/clipboard.rs index 1490b082c..0cd814ba0 100644 --- a/crates/engine/src/cmd/edit/clipboard.rs +++ b/crates/engine/src/cmd/edit/clipboard.rs @@ -226,24 +226,54 @@ pub(crate) fn parse_tsv(text: &str) -> Vec> { let mut row = Vec::new(); let mut field = String::new(); let mut chars = text.chars().peekable(); + // Text read again as unquoted, ahead of the rest of the input: a field that opened with a + // quote but turned out not to be quoted. + let mut replay: std::collections::VecDeque = std::collections::VecDeque::new(); + // Text is replayed at most as much as the input is long; past that, quotes are text, so + // hostile input full of fields that only look quoted reads in linear time. + let mut replay_budget = text.len(); let mut quoted = false; let mut at_start = true; // A quoted field's text as it stands in the input, for a field that turns out not to be one. let mut raw = String::new(); - while let Some(c) = chars.next() { + loop { + let c = match replay.pop_front().or_else(|| chars.next()) { + Some(c) => c, + // A quote that never closes: the field is its text as written. + None if quoted => { + quoted = false; + field.clear(); + field.push('"'); + replay_budget = replay_budget.saturating_sub(raw.len()); + replay.extend(raw.chars().skip(1)); + raw.clear(); + continue; + } + None => break, + }; if quoted { raw.push(c); if c == '"' { - if chars.peek() == Some(&'"') { + let next = replay.front().copied().or_else(|| chars.peek().copied()); + if next == Some('"') { field.push('"'); raw.push('"'); - chars.next(); + if replay.pop_front().is_none() { + chars.next(); + } } else { quoted = false; // A field is quoted only when its closing quote ends it, as Excel reads it: - // `"hi" there` is that text, quotes included. - if !matches!(chars.peek(), None | Some('\t' | '\n' | '\r')) { - field = std::mem::take(&mut raw); + // `"hi" there` is that text, quotes included, and a tab or line break inside + // it still ends the cell, as if the leading quote weren't there. + if !matches!(next, None | Some('\t' | '\n' | '\r')) { + field.clear(); + field.push('"'); + replay_budget = replay_budget.saturating_sub(raw.len()); + let rest: Vec = std::mem::take(&mut raw).chars().skip(1).collect(); + for ch in rest.into_iter().rev() { + replay.push_front(ch); + } } } } else { @@ -252,7 +282,7 @@ pub(crate) fn parse_tsv(text: &str) -> Vec> { continue; } match c { - '"' if at_start => { + '"' if at_start && replay_budget > 0 => { quoted = true; at_start = false; raw = String::from('"'); diff --git a/crates/engine/src/cmd/edit/tests.rs b/crates/engine/src/cmd/edit/tests.rs index 5034646cd..020592c56 100644 --- a/crates/engine/src/cmd/edit/tests.rs +++ b/crates/engine/src/cmd/edit/tests.rs @@ -390,3 +390,25 @@ fn copy_quotes_only_cells_with_tabs_or_line_breaks() { let cells: Vec<&str> = rows.iter().map(|r| r[0].as_str()).collect(); assert_eq!(cells, ["say \"hi\"", "plain", "\"hi\" there", "x \"q\"\ny", "tab\tinside"]); } + +#[test] +fn a_field_that_is_not_really_quoted_keeps_its_separators() { + // A field opening with a quote is quoted only if a closing quote ends it; when it doesn't (or + // never closes), the text is read as written, and the tabs and line breaks inside it still + // separate cells, as they would have without the leading quote. + assert_eq!(parse_tsv("\"a\tb\"x\tc"), vec![vec!["\"a", "b\"x", "c"]]); + assert_eq!(parse_tsv("\"a\tb\nc"), vec![vec!["\"a", "b"], vec!["c"]]); + assert_eq!(parse_tsv("\"a\tb\tc\n"), vec![vec!["\"a", "b", "c"]]); + // Cells copied without quotes (they hold no tab or line break) read back as the same cells. + let mut s = Session::new(); + s.new_workbook(); + for (cell, input) in [("A1", "'\"a"), ("B1", "b\"x"), ("C1", "c")] { + s.execute("cell.set", json!({"cell": cell, "input": input})).unwrap(); + } + let text = s.execute("edit.copy", json!({"range": "A1:C1"})).unwrap()["text"].as_str().unwrap().to_string(); + assert_eq!(parse_tsv(&text), vec![vec!["\"a", "b\"x", "c"]], "{text:?}"); + // Many fields that only look quoted: each is replayed at most once (the replay budget is the + // input's length), and every one is still its own cell. + let hostile = "\"x\t".repeat(50_000); + assert_eq!(parse_tsv(&hostile).first().map(Vec::len), Some(50_001)); +}