M6: form_set_props refuses unknown, mistyped and conflicting keys in calculate and validate

A notation beside op and fields set the notation and dropped the rest, a
field name that wasn't a string was left out, a misspelt key was ignored,
and validate {"min": 0, "max": "100"} wrote a min-only range. Each is
now refused and nothing changes. A null bound, as form_fields shows the
one a range doesn't have, is no bound, so its output is taken back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P6yUtvWGPsJpMLyh6pxSXG

crates/automation/src/forms.rs

@@ -94,13 +94,20 @@ fn look_arg(v: &Value) -> Result<pdfcraft_engine::FieldLookPatch> {
     })
 }
 
-/// `{"min": 0, "max": 100}` or `"none"`.
+/// `{"min": 0, "max": 100}` or `"none"`. A null bound is no bound, as `form_fields` shows it.
 fn validate_arg(v: &Value) -> Result<Validate> {
     if v.as_str() == Some("none") {
         return Ok(Validate::None);
     }
     let o = v.as_object().ok_or_else(|| bad("validate must be {\"min\": …, \"max\": …} or \"none\""))?;
-    let (min, max) = (o.get("min").and_then(Value::as_f64), o.get("max").and_then(Value::as_f64));
+    if let Some(k) = o.keys().find(|k| !matches!(k.as_str(), "min" | "max")) {
+        return Err(bad(format!("unknown key {k:?} in validate (min, max)")));
+    }
+    let bound = |k: &str| match o.get(k) {
+        None | Some(Value::Null) => Ok(None),
+        Some(x) => x.as_f64().map(Some).ok_or_else(|| bad(format!("validate {k} must be a number"))),
+    };
+    let (min, max) = (bound("min")?, bound("max")?);
     if min.is_none() && max.is_none() {
         return Err(bad("validate needs min, max or both"));
     }
@@ -117,18 +124,31 @@ fn calculate_arg(v: &Value) -> Result<Calculate> {
     if o.contains_key("script") {
         return Err(bad("a custom calculate script is set with form_set_script (event: calculate)"));
     }
-    if let Some(n) = o.get("notation").and_then(Value::as_str) {
+    if let Some(k) = o.keys().find(|k| !matches!(k.as_str(), "op" | "fields" | "notation")) {
+        return Err(bad(format!("unknown key {k:?} in calculate (op and fields, or notation)")));
+    }
+    // A null is no value, as for the tool's own arguments.
+    let get = |k: &str| o.get(k).filter(|v| !v.is_null());
+    if let Some(n) = get("notation") {
+        if get("op").is_some() || get("fields").is_some() {
+            return Err(bad("calculate takes a notation, or op and fields, not both"));
+        }
+        let n = n.as_str().ok_or_else(|| bad("notation must be a string such as \"Price * Qty\""))?;
         return Ok(Calculate::Notation(n.to_string()));
     }
-    let op = match o.get("op").and_then(Value::as_str).unwrap_or("") {
+    let op = get("op").ok_or_else(|| bad("calculate needs op and fields, or a notation"))?;
+    let op = match op.as_str().ok_or_else(|| bad("op must be one of sum, product, average, min, max"))? {
         "minimum" => CalcOp::Minimum,
         "maximum" => CalcOp::Maximum,
         x => {
             CalcOp::ALL.into_iter().find(|&op| op_name(op) == x).ok_or_else(|| bad(format!("unknown op {x:?} (sum, product, average, min, max)")))?
         }
     };
-    let fields: Vec<String> =
-        o.get("fields").and_then(Value::as_array).map(|a| a.iter().filter_map(|x| x.as_str().map(str::to_owned)).collect()).unwrap_or_default();
+    let fields = get("fields").and_then(Value::as_array).ok_or_else(|| bad("calculate fields must be a list of field names"))?;
+    let fields: Vec<String> = fields
+        .iter()
+        .map(|x| x.as_str().map(str::to_owned).ok_or_else(|| bad("calculate fields must be field names (strings)")))
+        .collect::<Result<_>>()?;
     if fields.is_empty() {
         return Err(bad("calculate needs fields"));
     }

crates/automation/src/tools.rs

@@ -492,7 +492,7 @@ pub fn tools() -> Vec<ToolDef> {
                 "rotation": { "type": "integer", "enum": [0, 90, 180, 270], "description": "Widget rotation in degrees, counterclockwise. Changing between horizontal and vertical swaps the rectangle around its center." },
                 "appearance": { "type": "object", "description": "Appearance tab: border, fill, text_color (#RRGGBB, a name or \"none\"), width (0–12 points; 1 thin, 2 medium, 3 thick), style (solid, dashed, beveled, inset, underline), font (helvetica, times, courier)." },
                 "format": { "description": "Format tab: {\"type\": \"number\", \"decimals\": 2, \"currency\": \"$\", \"separator\": 0-4, \"negative\": 0-3}, {\"type\": \"percent\"}, {\"type\": \"date\"|\"time\", \"pattern\": \"mm/dd/yyyy\"}, {\"type\": \"zip\"|\"zip4\"|\"phone\"|\"ssn\"}, {\"type\": \"mask\", \"mask\": \"AA-9999\"}, or \"none\"." },
-                "validate": { "description": "Validate tab: {\"min\": 0, \"max\": 100} (either may be left out) or \"none\"." },
+                "validate": { "description": "Validate tab: {\"min\": 0, \"max\": 100} (either may be left out, or null) or \"none\"." },
                 "calculate": { "description": "Calculate tab: {\"op\": \"sum\"|\"product\"|\"average\"|\"min\"|\"max\", \"fields\": [\"a\", \"b\"]}, {\"notation\": \"Price * Qty\"} (simplified field notation), or \"none\"." },
             }),
             &["doc"],

crates/automation/tests/it/automation.rs

@@ -5153,6 +5153,47 @@ fn every_calculation_op_can_be_read_and_given_back() {
     }
 }
 
+/// form_set_props refuses a calculate or validate it would only partly apply (a key it doesn't
+/// know, a value of the wrong type, a notation beside op and fields) and changes nothing. A
+/// validate read back from form_fields, with null for the bound it doesn't have, is taken back.
+#[test]
+fn calculate_and_validate_refuse_what_they_would_drop() {
+    let dir = workdir("calcstrict");
+    let mut a = auto(&dir);
+    let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap();
+    for (name, y) in [("a", 20), ("b", 60), ("total", 100)] {
+        ok(&mut a, "form_add_field", json!({ "doc": doc, "page": 1, "type": "text", "rect": [20, y, 180, y + 22], "name": name }));
+    }
+    let set = json!({ "op": "sum", "fields": ["a", "b"] });
+    ok(&mut a, "form_set_props", json!({ "doc": doc, "field": "total", "calculate": set, "validate": { "min": 0, "max": 10 } }));
+    let total = |a: &mut Automation| {
+        let fields = ok(a, "form_fields", json!({ "doc": doc }));
+        fields["fields"].as_array().unwrap().iter().find(|f| f["name"] == "total").unwrap().clone()
+    };
+    for (calculate, says) in [
+        (json!({ "notation": "a", "op": "sum", "fields": ["b"] }), "not both"),
+        (json!({ "op": "sum", "fields": ["a", 5] }), "field names"),
+        (json!({ "op": "sum", "fields": "a" }), "list of field names"),
+        (json!({ "notation": 5 }), "notation must be a string"),
+        (json!({ "op": 5, "fields": ["a"] }), "op must be"),
+        (json!({ "fields": ["a"] }), "needs op"),
+        (json!({ "op": "sum", "feilds": ["a"] }), "unknown key \"feilds\""),
+    ] {
+        let e = err(&mut a, "form_set_props", json!({ "doc": doc, "field": "total", "calculate": calculate }));
+        assert!(e.to_string().contains(says), "{calculate}: {e}");
+    }
+    for (validate, says) in [(json!({ "min": 0, "max": "100" }), "max must be a number"), (json!({ "min": 0, "mxa": 5 }), "unknown key \"mxa\"")] {
+        let e = err(&mut a, "form_set_props", json!({ "doc": doc, "field": "total", "validate": validate }));
+        assert!(e.to_string().contains(says), "{validate}: {e}");
+    }
+    let t = total(&mut a);
+    assert_eq!((&t["calculate"], &t["validate"]), (&set, &json!({ "min": 0.0, "max": 10.0 })), "nothing changed");
+    ok(&mut a, "form_set_props", json!({ "doc": doc, "field": "total", "validate": { "min": null, "max": 5 } }));
+    let shown = total(&mut a)["validate"].clone();
+    assert_eq!(shown, json!({ "min": null, "max": 5.0 }));
+    ok(&mut a, "form_set_props", json!({ "doc": doc, "field": "total", "validate": shown }));
+}
+
 #[test]
 fn page_tab_orders_are_read_and_set_per_page_through_tools() {
     let dir = workdir("pagetabs");