From fd2e13f3320b048a614f04d12f3327f4bb559c40 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 21:35:52 +0000 Subject: [PATCH] M8: XFA instance changes reserve their effect before changing the snapshot --- crates/js/src/tests.rs | 16 ++++++++++++++ crates/js/src/xfa.rs | 47 ++++++++++++++++++++++++++++++++---------- 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/crates/js/src/tests.rs b/crates/js/src/tests.rs index 3201641b8..5e9512789 100644 --- a/crates/js/src/tests.rs +++ b/crates/js/src/tests.rs @@ -686,6 +686,22 @@ mod xfa_model { assert!(matches!(&o.effects[0], XfaEffect::MessageBox(m) if m.chars().count() <= 4_097)); } + /// Past the byte budget, adding or removing a row is left out of the snapshot as well as the + /// effects, so the script never counts rows the form will not get. + #[test] + fn rows_changed_past_the_byte_budget_are_left_out_of_the_snapshot_too() { + let o = go( + "var s = 'x'; while (s.length < 4096) s += s; for (var i = 0; i < 9000; i++) xfa.host.gotoURL(s); \ + table._row.removeInstance(0); var added = table._row.addInstance(1); (added === null) + ':' + table._row.count", + "form1[0].page1[0].go[0]", + "click", + ); + assert_eq!(o.error, None, "{o:?}"); + assert!(o.notes.iter().any(|n| n.contains("MiB")), "{:?}", o.notes); + assert!(!o.effects.iter().any(|e| matches!(e, XfaEffect::AddInstance { .. } | XfaEffect::RemoveInstance { .. }))); + assert_eq!(o.result.as_deref(), Some("true:2")); + } + #[test] fn urls_and_reset_lists_are_clipped_and_deduplicated() { let o = go( diff --git a/crates/js/src/xfa.rs b/crates/js/src/xfa.rs index 99a1dd45c..b8440454a 100644 --- a/crates/js/src/xfa.rs +++ b/crates/js/src/xfa.rs @@ -321,6 +321,17 @@ impl XHost { } } + /// Room for one more effect holding `bytes` of text, charged now: a change to the snapshot + /// is made only when the effect that applies it will be kept, so a script never sees a change + /// its effects leave out. A drop is counted when there is no room. + fn reserve(&mut self, bytes: usize) -> bool { + if self.effects.len() >= MAX_EFFECTS { + self.dropped_effects += 1; + return false; + } + self.room(bytes) + } + /// `e` with its text cut to the lengths an effect keeps. fn clip_effect(&mut self, e: XfaEffect) -> XfaEffect { match e { @@ -668,13 +679,7 @@ fn renumber(h: &mut XHost, idx: usize, depth: usize) { let parent_som = n.parent.and_then(|p| h.nodes.get(p)).map(|p| p.som.clone()).unwrap_or_default(); (parent_som, n.name.clone(), n.index, n.children.clone()) }; - let som = if name.starts_with('#') || name.is_empty() { - parent_som - } else if parent_som.is_empty() { - format!("{name}[{index}]") - } else { - format!("{parent_som}.{name}[{index}]") - }; + let som = child_som(parent_som, &name, index); if let Some(n) = h.nodes.get_mut(idx) { n.som = som; } @@ -683,6 +688,17 @@ fn renumber(h: &mut XHost, idx: usize, depth: usize) { } } +/// The SOM of instance `index` of the node `name` under a parent at `parent_som`. +fn child_som(parent_som: String, name: &str, index: usize) -> String { + if name.starts_with('#') || name.is_empty() { + parent_som + } else if parent_som.is_empty() { + format!("{name}[{index}]") + } else { + format!("{parent_som}.{name}[{index}]") + } +} + /// A copy of `src`'s subtree (values cleared) under `parent`, returning the copy's index. fn clone_subtree(h: &mut XHost, src: usize, parent: Option, depth: usize) -> Option { if depth > MAX_DEPTH || h.nodes.len() >= MAX_NODES { @@ -750,7 +766,14 @@ impl XHost { let live = instances(h, parent, name); let &first = live.first()?; let (repeatable, max) = h.nodes.get(first).map(|n| (n.repeatable, n.occur_max)).unwrap_or((false, Some(1))); - if !repeatable || max.is_some_and(|m| live.len() >= m) || h.added >= MAX_ADDED { + if !repeatable || max.is_some_and(|m| live.len() >= m) || h.added >= MAX_ADDED || h.nodes.len() >= MAX_NODES { + return None; + } + // The copy's SOM, as `renumber` will give it, so its effect is reserved before the copy. + let name = h.nodes.get(first).map(|n| n.name.clone()).unwrap_or_default(); + let parent_som = h.nodes.get(parent).map(|p| p.som.clone()).unwrap_or_default(); + let som = child_som(parent_som, &name, live.len()); + if !h.reserve(som.len()) { return None; } let new = clone_subtree(h, first, Some(parent), 0)?; @@ -765,8 +788,7 @@ impl XHost { } renumber(h, new, 0); h.added += 1; - let som = h.nodes.get(new).map(|n| n.som.clone()).unwrap_or_default(); - h.push(XfaEffect::AddInstance { som }); + h.record(XfaEffect::AddInstance { som }); Some(new) } @@ -803,6 +825,9 @@ impl XHost { return; } let som = h.nodes.get(gone).map(|n| n.som.clone()).unwrap_or_default(); + if !h.reserve(som.len()) { + return; + } if let Some(n) = h.nodes.get_mut(gone) { n.gone = true; } @@ -813,7 +838,7 @@ impl XHost { } renumber(h, inst, 0); } - h.push(XfaEffect::RemoveInstance { som }); + h.record(XfaEffect::RemoveInstance { som }); } }