diff --git a/crates/js/src/lib.rs b/crates/js/src/lib.rs index 81633d597..f6d5a076c 100644 --- a/crates/js/src/lib.rs +++ b/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>, 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>, 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>, + 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 = 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() { let h = h.into_inner(); out.changed = h.changed.iter().map(|i| h.fields[*i].clone()).collect(); diff --git a/crates/js/src/tests.rs b/crates/js/src/tests.rs index aec0631fd..7124af144 100644 --- a/crates/js/src/tests.rs +++ b/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]