diff --git a/crates/engine/src/macros/mod.rs b/crates/engine/src/macros/mod.rs index 38016049d..2aa45e12c 100644 --- a/crates/engine/src/macros/mod.rs +++ b/crates/engine/src/macros/mod.rs @@ -53,9 +53,6 @@ fn check_size(m: &Macro) -> Result<(), String> { /// before the steps are deserialized. fn normalize_steps(steps: &mut Json, level: usize) -> Result<(), String> { let Some(list) = steps.as_array_mut() else { return Err("`steps` must be a list.".into()) }; - if level > gridcraft_model::MAX_MACRO_DEPTH { - return Err(format!("Repeat and If steps can nest at most {} deep.", gridcraft_model::MAX_MACRO_DEPTH)); - } for st in list { let Some(o) = st.as_object_mut() else { return Err("Every step must be an object.".into()) }; if !o.contains_key("type") @@ -64,6 +61,11 @@ fn normalize_steps(steps: &mut Json, level: usize) -> Result<(), String> { o.insert("type".into(), json!("command")); o.insert("id".into(), id); } + // A step with a body is a Repeat or an If: one at the top is nested 1 deep. + let nested = ["body", "then", "else"].iter().any(|key| o.contains_key(*key)); + if nested && level > gridcraft_model::MAX_MACRO_DEPTH { + return Err(format!("Repeat and If steps can nest at most {} deep.", gridcraft_model::MAX_MACRO_DEPTH)); + } for key in ["body", "then", "else"] { if let Some(body) = o.get_mut(key) { normalize_steps(body, level + 1)?; diff --git a/crates/engine/src/macros/tests.rs b/crates/engine/src/macros/tests.rs index 7b498f0b8..5fbc6d38c 100644 --- a/crates/engine/src/macros/tests.rs +++ b/crates/engine/src/macros/tests.rs @@ -55,16 +55,25 @@ fn macros_over_the_limits_are_refused() { let flat = |n: usize| json!({"name": "Big", "steps": vec![json!({"command": "cell.set"}); n]}); assert!(parse_macro(&flat(MAX_MACRO_STEPS)).is_ok()); assert!(parse_macro(&flat(MAX_MACRO_STEPS + 1)).unwrap_err().contains("10000 steps")); - // Nested Repeat steps: 4 levels are fine, 5 are not. - let nest = |levels: usize| { + // Nested Repeat steps: 4 Repeats one inside the other are fine (the innermost holds a + // command, or nothing), a 5th is not. + let nest = |repeats: usize| { let mut step = json!({"command": "cell.set"}); - for _ in 1..levels { + for _ in 0..repeats { step = json!({"type": "repeat", "over": "Input", "body": [step]}); } json!({"name": "Deep", "steps": [step]}) }; - assert!(parse_macro(&nest(4)).is_ok()); - assert!(parse_macro(&nest(5)).unwrap_err().contains("4 deep")); + assert!(parse_macro(&nest(MAX_MACRO_DEPTH)).is_ok()); + assert_eq!(parse_macro(&nest(MAX_MACRO_DEPTH)).unwrap().step_count(), MAX_MACRO_DEPTH + 1); + assert!(parse_macro(&nest(MAX_MACRO_DEPTH + 1)).unwrap_err().contains("4 deep")); + let mut empty = json!({"type": "repeat", "over": "Input", "body": []}); + for _ in 1..MAX_MACRO_DEPTH { + empty = json!({"type": "if", "cell": "A1", "op": "eq", "value": 1, "then": [empty], "else": []}); + } + let deep = parse_macro(&json!({"name": "Deep", "steps": [empty]})).unwrap(); + assert_eq!(deep.step_count(), MAX_MACRO_DEPTH); + assert!(deep.check_limits().is_ok(), "the model's check agrees with the parser"); // Far deeper than serde would allow is refused before it is read. assert!(parse_macro(&nest(200)).is_err()); for bad in [ diff --git a/crates/model/src/macros.rs b/crates/model/src/macros.rs index 8dc657f90..03cb03cac 100644 --- a/crates/model/src/macros.rs +++ b/crates/model/src/macros.rs @@ -81,10 +81,8 @@ impl Macro { return Err(format!("A macro description can be at most {MAX_MACRO_TEXT} characters long.")); } // Count without recursing past the depth limit (a hostile file can nest deeper). + // A Repeat or If at the top is nested 1 deep; one inside it, 2 deep. fn walk(steps: &[Step], level: usize, count: &mut usize) -> Result<(), String> { - if level > MAX_MACRO_DEPTH { - return Err(format!("Repeat and If steps can nest at most {MAX_MACRO_DEPTH} deep.")); - } for s in steps { *count = count.saturating_add(1); if *count > MAX_MACRO_STEPS { @@ -93,6 +91,9 @@ impl Macro { if s.note.chars().count() > MAX_MACRO_TEXT { return Err(format!("A step note can be at most {MAX_MACRO_TEXT} characters long.")); } + if s.kind.children().next().is_some() && level > MAX_MACRO_DEPTH { + return Err(format!("Repeat and If steps can nest at most {MAX_MACRO_DEPTH} deep.")); + } for body in s.kind.children() { walk(body, level + 1, count)?; }