M6: a document script refused before running is skipped and reported too (#3940)

CodeRabbit on #3985: a document script over the size or nesting limit made `run`
return before any script ran, so the field script failed as before. It is now
skipped (never evaluated, as the limits require), reported in its place in
doc_errors, and the other scripts and the field script still run. The field
script's own limits still refuse it outright.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/js/src/lib.rs

@@ -1207,16 +1207,31 @@ pub fn run(script: &str, event: &Event, doc: &DocInfo, fields: &[FieldState], do
 /// script can observe uncommitted writes. Read-only calculations avoid copying the whole form.
 pub fn run_shared(script: &str, event: &Event, doc: &DocInfo, fields: Arc<Vec<FieldState>>, doc_scripts: &[String], limits: Limits) -> Outcome {
     let failed = |why: String| Outcome { rc: true, value: event.value.clone(), change: event.change.clone(), error: Some(why), ..Default::default() };
-    if let Some(why) = std::iter::once(script).chain(doc_scripts.iter().map(String::as_str)).find_map(refuse) {
+    if let Some(why) = refuse(script) {
         return failed(why);
     }
+    // A document script too long or too deeply nested to run safely is skipped and reported,
+    // as one that throws is; the others and the script still run (#3940).
+    let mut skipped = Vec::new();
+    let kept: Vec<(usize, &str)> = doc_scripts
+        .iter()
+        .enumerate()
+        .filter_map(|(i, d)| match refuse(d) {
+            Some(why) => {
+                skipped.push((i, why));
+                None
+            }
+            None => Some((i, d.as_str())),
+        })
+        .collect();
+    let (kept, skipped) = (&kept, &skipped);
     #[cfg(not(target_arch = "wasm32"))]
     {
         std::thread::scope(|scope| {
             let spawned = std::thread::Builder::new()
                 .name("pdfcraft-js".into())
                 .stack_size(SCRIPT_STACK)
-                .spawn_scoped(scope, || run_here(script, event, doc, fields, doc_scripts, limits));
+                .spawn_scoped(scope, || run_here(script, event, doc, fields, kept, skipped, limits));
             match spawned {
                 Ok(thread) => thread.join().unwrap_or_else(|_| failed("the script stopped with an internal error".into())),
                 Err(e) => failed(format!("the script engine could not start: {e}")),
@@ -1224,7 +1239,7 @@ pub fn run_shared(script: &str, event: &Event, doc: &DocInfo, fields: Arc<Vec<Fi
         })
     }
     #[cfg(target_arch = "wasm32")]
-    run_here(script, event, doc, fields, doc_scripts, limits)
+    run_here(script, event, doc, fields, kept, skipped, limits)
 }
 
 /// Host hooks that refuse to compile code from strings (`eval`, `Function` and its generator
@@ -1259,7 +1274,22 @@ impl HostHooks for CheckedCodeFromStrings {
     }
 }
 
-fn run_here(script: &str, event: &Event, doc: &DocInfo, fields: Arc<Vec<FieldState>>, doc_scripts: &[String], limits: Limits) -> Outcome {
+/// How [`Outcome::doc_errors`] names a failing document script: by its 1-based place.
+fn doc_error(index: usize, error: &dyn std::fmt::Display) -> String {
+    format!("document script {}: {error}", index.saturating_add(1))
+}
+
+/// `doc_scripts` are the document scripts to run with their places among all of them;
+/// `skipped` the ones refused before running, with the reason.
+fn run_here(
+    script: &str,
+    event: &Event,
+    doc: &DocInfo,
+    fields: Arc<Vec<FieldState>>,
+    doc_scripts: &[(usize, &str)],
+    skipped: &[(usize, String)],
+    limits: Limits,
+) -> Outcome {
     let mut ctx = match Context::builder().host_hooks(std::rc::Rc::new(NoCodeFromStrings)).build() {
         Ok(ctx) => ctx,
         Err(e) => {
@@ -1280,6 +1310,7 @@ fn run_here(script: &str, event: &Event, doc: &DocInfo, fields: Arc<Vec<FieldSta
         dropped: 0,
     }));
     let mut out = Outcome { rc: true, value: event.value.clone(), change: event.change.clone(), ..Default::default() };
+    let mut doc_errors: Vec<(usize, String)> = skipped.iter().map(|(i, why)| (*i, doc_error(*i, why))).collect();
     let result = (|| -> JsResult<()> {
         install(&mut ctx)?;
         let ev = event_object(&mut ctx, event);
@@ -1287,9 +1318,9 @@ fn run_here(script: &str, event: &Event, doc: &DocInfo, fields: Arc<Vec<FieldSta
         // One document script that throws (calling a method PdfCraft lacks, say) must not keep
         // every field script in the form from running: what it defined before the error, and
         // the other scripts, stay available (#3940).
-        for (i, d) in doc_scripts.iter().enumerate() {
+        for &(i, d) in doc_scripts {
             if let Err(e) = ctx.eval(Source::from_bytes(d)) {
-                out.doc_errors.push(format!("document script {}: {e}", i.saturating_add(1)));
+                doc_errors.push((i, doc_error(i, &e)));
             }
         }
         let r = ctx.eval(Source::from_bytes(script))?;
@@ -1307,6 +1338,8 @@ fn run_here(script: &str, event: &Event, doc: &DocInfo, fields: Arc<Vec<FieldSta
     if let Err(e) = result {
         out.error = Some(e.to_string());
     }
+    doc_errors.sort_by_key(|(i, _)| *i);
+    out.doc_errors = doc_errors.into_iter().map(|(_, e)| e).collect();
     if let Some(h) = ctx.remove_data::<Shared>() {
         let h = h.into_inner();
         out.changed = h.changed.iter().map(|i| h.fields[*i].clone()).collect();

crates/js/src/tests.rs

@@ -325,6 +325,33 @@ fn a_failing_document_script_does_not_stop_the_field_script() {
     assert!(go("event.value = 1;", &Event::field("Calculate", "total", "")).doc_errors.is_empty());
 }
 
+/// A document script refused before running (too long, or nested too deeply to run safely) is
+/// skipped like one that throws: never run, reported in its place, the rest still run.
+#[test]
+fn a_document_script_too_big_to_run_is_skipped_and_reported() {
+    let docs = [
+        "this.getField('qty').buttonSetIcon(null);".to_string(),
+        format!("var big = 1{};", "+1".repeat(MAX_SCRIPT_BYTES / 2)),
+        "function double(x) { return x * 2; }".to_string(),
+        format!("var deep = {}1{};", "(".repeat(10_000), ")".repeat(10_000)),
+    ];
+    let o = run(
+        "event.value = double(getField('qty').value) + typeof big;",
+        &Event::field("Calculate", "total", ""),
+        &doc(),
+        &fields(),
+        &docs,
+        Limits::default(),
+    );
+    assert_eq!((o.error.as_deref(), o.value.as_str()), (None, "8undefined"));
+    let places: Vec<&str> = o.doc_errors.iter().map(|e| e.split(':').next().unwrap_or_default()).collect();
+    assert_eq!(places, ["document script 1", "document script 2", "document script 4"], "{:?}", o.doc_errors);
+    assert!(o.doc_errors[1].contains("too long") && o.doc_errors[2].contains("nested too deeply"), "{:?}", o.doc_errors);
+    // The field script's own limits still refuse it outright.
+    let o = run(&format!("1{}", "+1".repeat(MAX_SCRIPT_BYTES)), &Event::field("Calculate", "total", ""), &doc(), &fields(), &docs, Limits::default());
+    assert!(o.error.is_some_and(|e| e.contains("too long")) && o.doc_errors.is_empty());
+}
+
 #[test]
 fn errors_and_runaway_scripts_are_contained() {
     let o = go("event.value = nosuch.thing;", &Event::field("Calculate", "total", "keep"));
@@ -364,9 +391,11 @@ fn deeply_nested_scripts_fail_instead_of_crashing() {
         let o = go(s, &Event::doc("Open"));
         assert!(o.error.is_some(), "{}…", &s[..20]);
     }
-    // A hostile document-level script is refused too, before the field script runs.
+    // A hostile document-level script is refused too: it never runs, and is reported, while
+    // the field script still does (#3940).
     let o = run("event.value = 'ran';", &Event::field("Calculate", "a", ""), &doc(), &fields(), &[hostile[0].clone()], Limits::default());
-    assert!(o.error.is_some());
+    assert_eq!((o.error, o.value.as_str()), (None, "ran"));
+    assert!(o.doc_errors.len() == 1 && o.doc_errors[0].contains("nested too deeply"), "{:?}", o.doc_errors);
 }
 
 #[test]