From fd3634cc38f3859f9c784ea63812f6134fdbc258 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Sun, 25 Oct 2026 17:27:08 +0100 Subject: [PATCH] M6: form_set_props refuses a notation it couldn't store as written MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A notation is kept in the script's /** BVCALC … EVCALC **/ comment and read back from there. One holding EVCALC was stored cut short ("a EVCALC b" read back as "a", and its JavaScript translation didn't parse), one holding */ ended the comment, and an empty one became "event.value = ;". form_set_props now refuses each (af::notation_storable: it has to read back as itself). The test also pins a notation beside op alone and beside fields alone (from the review). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01P6yUtvWGPsJpMLyh6pxSXG --- crates/automation/src/forms.rs | 6 +++++- crates/automation/tests/it/automation.rs | 6 ++++++ crates/forms/src/af.rs | 2 +- crates/forms/src/af/script.rs | 9 +++++++++ crates/forms/src/af/tests.rs | 10 ++++++++++ 5 files changed, 31 insertions(+), 2 deletions(-) diff --git a/crates/automation/src/forms.rs b/crates/automation/src/forms.rs index 78da146fa0..a9add477f2 100644 --- a/crates/automation/src/forms.rs +++ b/crates/automation/src/forms.rs @@ -1,7 +1,7 @@ //! Form tools: list fields, fill them (several at once, one undo step), clear the form, and //! prepare a form (add fields one at a time or as a table, change and delete them). -use pdfcraft_engine::form_scripts::{CalcOp, Calculate, Format, Validate}; +use pdfcraft_engine::form_scripts::{CalcOp, Calculate, Format, Validate, notation_storable}; use pdfcraft_engine::{CalculationIssue, Edit, FieldProps, FieldValue, FormField, FormFieldKind, NewField, field_flags}; use serde_json::{Value, json}; @@ -134,6 +134,10 @@ fn calculate_arg(v: &Value) -> Result { 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\""))?; + // Refused rather than stored as a different formula. + if !notation_storable(n) { + return Err(bad("the notation can't be stored as written: it is empty, or holds EVCALC or */, which would end it early")); + } return Ok(Calculate::Notation(n.to_string())); } let op = get("op").ok_or_else(|| bad("calculate needs op and fields, or a notation"))?; diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index 6745c895cb..1504dee7a3 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -5172,6 +5172,12 @@ fn calculate_and_validate_refuse_what_they_would_drop() { }; for (calculate, says) in [ (json!({ "notation": "a", "op": "sum", "fields": ["b"] }), "not both"), + (json!({ "notation": "a", "op": "sum" }), "not both"), + (json!({ "notation": "a", "fields": ["b"] }), "not both"), + // A notation that wouldn't read back as itself from the script's comment. + (json!({ "notation": "a EVCALC b" }), "can't be stored"), + (json!({ "notation": "a */ x /*" }), "can't be stored"), + (json!({ "notation": " " }), "can't be stored"), (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"), diff --git a/crates/forms/src/af.rs b/crates/forms/src/af.rs index 00cfc99d12..c83ce29cf3 100644 --- a/crates/forms/src/af.rs +++ b/crates/forms/src/af.rs @@ -22,7 +22,7 @@ use calc::{Notation, eval_notation, notation_name, notation_tokens, unique}; pub use calc::{calculate, calculation_reads, notation_js, reads_built_names, rename_in_calculation}; pub use format::{DATE_PRESETS, DateTime, TIME_PRESETS, format_date, format_value, js_number, keystroke, make_number, parse_date, validate}; use script::{Arg, call}; -pub use script::{calculate_js, format_js, parse_calculate, parse_format, parse_validate, validate_js}; +pub use script::{calculate_js, format_js, notation_storable, parse_calculate, parse_format, parse_validate, validate_js}; /// The Format tab. #[derive(Clone, Debug, PartialEq, Default)] diff --git a/crates/forms/src/af/script.rs b/crates/forms/src/af/script.rs index 9aed297b45..1544258a2c 100644 --- a/crates/forms/src/af/script.rs +++ b/crates/forms/src/af/script.rs @@ -325,6 +325,15 @@ pub fn validate_js(v: &Validate) -> Option { } } +/// Whether simplified field notation `expr` can be stored: the script keeps it in a comment, +/// between `BVCALC` and `EVCALC`, and it has to read back from there as itself. An empty one, or +/// one holding `EVCALC` or `*/` (either ends it early), can't. +pub fn notation_storable(expr: &str) -> bool { + !expr.trim().is_empty() + && calculate_js(&Calculate::Notation(expr.to_owned())).and_then(|js| parse_calculate(&js)) + == Some(Calculate::Notation(expr.trim().to_owned())) +} + pub fn calculate_js(c: &Calculate) -> Option { match c { Calculate::None => None, diff --git a/crates/forms/src/af/tests.rs b/crates/forms/src/af/tests.rs index 851c5c032e..d0246d3016 100644 --- a/crates/forms/src/af/tests.rs +++ b/crates/forms/src/af/tests.rs @@ -217,6 +217,16 @@ fn hostile_scripts_are_read_in_one_pass() { assert_eq!(rename_in_calculation(&nested, "a", "b"), Some(format!("{}\"b\"", "AFSimple_Calculate(\"SUM\", new Array(".repeat(100_000)))); } +/// A notation is stored only when it reads back from the script's comment as itself. +#[test] +fn a_notation_is_stored_only_when_it_reads_back() { + assert!(notation_storable("Price * Qty")); + assert!(notation_storable(" a + b "), "the spaces around it are left out when it is read"); + for expr in ["", " ", "a EVCALC b", "a */ x /*", "a */ b"] { + assert!(!notation_storable(expr), "{expr:?}"); + } +} + /// Notation is read through one lexer: field names with escapes, numbers and operators. #[test] fn notation_is_read_by_one_lexer() {