diff --git a/crates/engine/src/cmd/datamerge/fill.rs b/crates/engine/src/cmd/datamerge/fill.rs index ae0b4e6b4..a7b4a0ba6 100644 --- a/crates/engine/src/cmd/datamerge/fill.rs +++ b/crates/engine/src/cmd/datamerge/fill.rs @@ -334,13 +334,18 @@ fn typed_reps( fn find_markers(text: &str) -> Vec<(usize, usize, String)> { let mut out = Vec::new(); let mut i = 0; + // The next `>>` at or after the `<<` being looked at: found once, kept until passed, so the + // text is scanned once however many `<<` it holds (not again from every one of them). + let mut close: Option = Some(0); while i < text.len() { - if text[i..].starts_with("<<") - && let Some(rel) = text[i + 2..].find(">>") - { - let name = &text[i + 2..i + 2 + rel]; + if text[i..].starts_with("<<") { + if close.is_some_and(|c| c < i + 2) { + close = text[i + 2..].find(">>").map(|rel| i + 2 + rel); + } + let Some(c) = close else { break }; + let name = &text[i + 2..c]; if !name.is_empty() && !name.contains(['<', '>', '\n']) { - let end = i + 2 + rel + 2; + let end = c + 2; out.push((i, end, name.to_string())); i = end; continue; @@ -570,6 +575,25 @@ fn cell_of<'a>(fields: &[DataField], row: &'a [String], name: &str) -> &'a str { fields.iter().position(|f| f.name == name).and_then(|i| row.get(i)).map(String::as_str).unwrap_or("") } +#[cfg(test)] +mod marker_tests { + use super::find_markers; + + #[test] + fn markers_are_found_in_one_pass() { + let found = find_markers; + assert_eq!(found("a <> b <>"), [(2, 10, "Name".to_string()), (13, 18, "x".to_string())]); + assert_eq!(found("<<>> <> <>"), [(13, 19, "ok".to_string())]); + assert_eq!(found("<<<>"), [(2, 7, "a".to_string())]); + // Many `<<` with no `>>` to close them: linear, not a search from each of them. + let open = "<<".repeat(200_000); + let t = std::time::Instant::now(); + assert!(found(&open).is_empty()); + assert!(found(&format!("{open}>>")).is_empty(), "a name made of `<` isn't one"); + assert!(t.elapsed() < std::time::Duration::from_secs(5), "{:?}", t.elapsed()); + } +} + #[cfg(test)] mod link_tests { use super::*; diff --git a/crates/engine/src/cmd/variables.rs b/crates/engine/src/cmd/variables.rs index e2bf23e9a..1bbe39849 100644 --- a/crates/engine/src/cmd/variables.rs +++ b/crates/engine/src/cmd/variables.rs @@ -361,16 +361,21 @@ fn unvar(st: &mut designcraft_doc::Story, i: usize, n: usize, value: &str, depth return; } if st.text.chars().any(|c| designcraft_doc::vars::var_index(c).is_some_and(|k| k >= i)) { + // The instances as they are now, replaced last first so earlier positions stay valid: + // text put in isn't searched again (a value holding the marker would never end). + let instances = |text: &str, target: &str| { + let mut at: Vec = text.match_indices(target).map(|(pos, _)| pos).collect(); + at.reverse(); + at + }; let target = var_char(i).unwrap_or(' ').to_string(); - let mut occurrences: Vec = st.text.match_indices(&target).map(|(pos, _)| pos).collect(); - occurrences.reverse(); - for pos in occurrences { + for pos in instances(&st.text, &target) { st.replace(pos..pos + target.len(), value); on_replace(pos, target.len()); } for k in i + 1..n { let (from, to) = (var_char(k).unwrap_or(' ').to_string(), var_char(k - 1).unwrap_or(' ').to_string()); - while let Some(pos) = st.text.find(&from) { + for pos in instances(&st.text, &from) { st.replace(pos..pos + from.len(), &to); } } diff --git a/crates/engine/src/cmd/variables/tests.rs b/crates/engine/src/cmd/variables/tests.rs index 6a2291ccb..e89573bb5 100644 --- a/crates/engine/src/cmd/variables/tests.rs +++ b/crates/engine/src/cmd/variables/tests.rs @@ -290,6 +290,22 @@ fn delete_variable_reaches_table_cells() { assert_eq!(t.cells[0].text.text, format!("Alpha|{}", var_char(nb).unwrap())); } +/// A custom variable whose text holds its own instance marker: deleting it puts that text in +/// once per instance and ends (the text put in isn't searched for instances again). +#[test] +fn delete_variable_whose_value_holds_its_marker_ends() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + s.execute("variables.define", &json!({"name": "A", "type": "custom", "text": "a"})).unwrap(); + let a = s.doc().unwrap().doc.variable("A").unwrap(); + let marker = var_char(a).unwrap(); + s.execute("variables.define", &json!({"name": "A", "type": "custom", "text": format!("<{marker}>")})).unwrap(); + s.execute("frame.create", &json!({"rect": [72, 72, 400, 300], "content": "text", "text": format!("x{marker}y{marker}z")})).unwrap(); + s.execute("variables.delete", &json!({"name": "A"})).unwrap(); + let d = &s.doc().unwrap().doc; + assert_eq!(d.stories.values().next().unwrap().text, format!("x<{marker}>y<{marker}>z")); +} + /// Deleting a variable flattens its instances in footnotes and endnotes too and moves the /// instances of later variables down. #[test] diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 1bec28c75..f87371531 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -392,6 +392,10 @@ pub struct Session { active: Option, /// An undoable command is running: the commands it runs are part of its undo step. in_undo_step: bool, + /// Scripts running one inside another, and the steps the outermost may still run + /// ([`script::MAX_SCRIPT_DEPTH`], [`script::MAX_SCRIPT_STEPS`]). + pub(crate) script_depth: usize, + pub(crate) script_steps: usize, pub prefs: Prefs, pub cache: Arc, pub(crate) tool: Box, @@ -461,6 +465,8 @@ impl Session { docs: vec![], active: None, in_undo_step: false, + script_depth: 0, + script_steps: 0, prefs: Prefs::default(), cache: Arc::new(Cache::new()), tool: designcraft_tools::create("selection"), diff --git a/crates/engine/src/script.rs b/crates/engine/src/script.rs index 2f8394f34..25c548d04 100644 --- a/crates/engine/src/script.rs +++ b/crates/engine/src/script.rs @@ -110,6 +110,10 @@ fn is_reference(r: &str) -> bool { head_ok && parts.all(|p| !p.is_empty() && p.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'_' || b == b'-')) } +/// Longest string a `${…}` interpolation may build: a step that pastes an earlier result into a +/// story, then interpolates that story into the next, could otherwise double its size each step. +pub const MAX_INTERPOLATED_BYTES: usize = 16 << 20; + /// Replace references in `params` with values from earlier `results`. pub fn resolve(params: &Value, results: &[Value]) -> Result { Ok(match params { @@ -134,6 +138,10 @@ pub fn resolve(params: &Value, results: &[Value]) -> Result { Value::String(t) => out.push_str(&t), other => out.push_str(&other.to_string()), } + if out.len() > MAX_INTERPOLATED_BYTES { + return Err(format!("the interpolated text of {} is longer than {MAX_INTERPOLATED_BYTES} bytes", s.get(..40).unwrap_or(s)) + .replace('\n', " ")); + } rest = &inner[end + 1..]; } else { out.push('$'); @@ -182,10 +190,33 @@ pub fn run(steps: &[Step], mut exec: impl FnMut(&str, Value) -> Result Report { - run(steps, |c, p| self.execute(c, &p).map_err(|e| e.to_string())) + if self.script_depth >= MAX_SCRIPT_DEPTH { + let command = steps.first().map(|s| s.command.clone()).unwrap_or_default(); + return Report { + results: vec![], + failed: Some((0, command, format!("scripts nest deeper than {MAX_SCRIPT_DEPTH} (a script running a script…)"))), + }; + } + if self.script_depth == 0 { + self.script_steps = MAX_SCRIPT_STEPS; + } + self.script_depth += 1; + let report = run(steps, |c, p| { + self.script_steps = + self.script_steps.checked_sub(1).ok_or_else(|| format!("the script and the scripts it runs exceed {MAX_SCRIPT_STEPS} steps"))?; + self.execute(c, &p).map_err(|e| e.to_string()) + }); + self.script_depth -= 1; + report } } @@ -275,6 +306,35 @@ mod tests { assert_eq!(r.results[3]["text"], "a\u{a0}v\u{a0}Praze"); } + /// A script that runs scripts stops at the depth limit, and the steps of all of them count + /// against one budget; the session goes on afterwards. + #[test] + fn nested_scripts_are_bounded() { + let mut s = crate::Session::new(); + let steps = parse("file.new\ndocument.inspect\ndocument.inspect").unwrap(); + s.script_depth = MAX_SCRIPT_DEPTH; + let e = s.run_script(&steps).failed.expect("must fail").2; + assert!(e.contains("nest deeper"), "{e}"); + s.script_depth = 1; + s.script_steps = 2; + let r = s.run_script(&steps); + assert_eq!(r.results.len(), 2); + assert!(r.failed.as_ref().is_some_and(|f| f.2.contains("exceed")), "{r:?}"); + s.script_depth = 0; + let r = s.run_script(&steps); + assert!(r.failed.is_none(), "{r:?}"); + assert_eq!(s.script_depth, 0); + } + + #[test] + fn interpolation_is_capped() { + let big = "x".repeat(MAX_INTERPOLATED_BYTES / 2 + 1); + let results = vec![json!({"text": big})]; + assert!(resolve(&json!("${0.text}"), &results).is_ok()); + let e = resolve(&json!("${0.text}${0.text}"), &results).unwrap_err(); + assert!(e.contains("longer than"), "{e}"); + } + #[test] fn runs_on_a_session_with_references() { let mut s = crate::Session::new();