fix(js): bound array text, month parsing, name registration and list work (#5883)

* fix(js): bound array text, month parsing, name registration and list work

- strings() (field value and colour setters, resetForm) copied every array
  entry into its own String with no total: one long string listed a million
  times was copied a million times. The copies together are now held to the
  run's MAX_PAYLOAD byte budget.
- FormCalc parse_date copied and lowercased the rest of the text for every
  month token of the picture, quadratic in both; it reads at most the longest
  month name now.
- XFA global registration checked each child name against a Vec of every
  name defined so far, quadratic in a wide form; it is a HashSet.
- FormCalc gather charged one step for a whole collection and list_push
  charged nothing for a duplicate it dropped, so .all on same-named siblings
  did quadratic work for one step a pair; every node visited costs a step.
- add_instance reserved only the new root's SOM before copying a subtree;
  the copy's nodes, names and SOMs are charged to the byte budget first.

* address review: gather charges each call; check an array entry's length before copying it

* Hold an array entry to its widest escaped size before copying it

The pre-copy check compared the payload budget against a JS string's UTF-16
length, which is shorter than the escaped UTF-8 form it is copied to: a
string of quotes or non-ASCII text could allocate up to six times the
budget before the post-check rejected it.

The check now uses the widest escaped width, so the copy cannot exceed the
budget it is checked against.

* review: measure an array entry's escaped width before refusing it

* address review: count an array entry's escaped width without copying it

* address review: charge retained field text and colours to a run-wide budget

* fmt

* fmt

* merge main, keep both
This commit is contained in:
Dylan M. Taylor
2026-11-11 15:50:29 -05:00
committed by GitHub
parent 77078864f4
commit abec4e3fee
8 changed files with 126 additions and 15 deletions
+8 -3
View File
@@ -1,4 +1,6 @@
use super::host::{FieldKey, arg, colour, error, function, host, read, s, set_value, strings, text, text_line, value_of, write};
use super::host::{
FieldKey, arg, charge, colour, error, function, host, kind_of, read, s, strings, text, text_bytes, text_line, value_for, value_of, write,
};
use super::model::{DISPLAY_HIDDEN, DISPLAY_VISIBLE, FieldType, Request};
use super::{Attribute, Context, JsArray, JsObject, JsResult, JsString, JsValue, NativeFunction, ObjectInitializer, js_string};
@@ -41,8 +43,9 @@ fn field_object_new(ctx: &mut Context, name: &str, id: Option<u64>, token: u64)
let value_get = getter!(ctx, n, |f, c| value_of(&f, c));
let value_set = {
fn set(_: &JsValue, args: &[JsValue], name: &FieldKey, c: &mut Context) -> JsResult<JsValue> {
let vals = strings(&arg(args, 0), c)?;
write(c, name, |f| set_value(f, vals))?;
let vals = value_for(kind_of(c, name)?, strings(&arg(args, 0), c)?);
charge(c, name, |f| text_bytes(&f.value), text_bytes(&vals))?;
write(c, name, |f| f.value = vals)?;
Ok(JsValue::undefined())
}
function(ctx, NativeFunction::from_copy_closure_with_captures(set, n.clone()))
@@ -104,6 +107,7 @@ fn field_object_new(ctx: &mut Context, name: &str, id: Option<u64>, token: u64)
let text_color_set = {
fn set(_: &JsValue, args: &[JsValue], name: &FieldKey, c: &mut Context) -> JsResult<JsValue> {
let v = strings(&arg(args, 0), c)?;
charge(c, name, |f| f.text_color.as_deref().map_or(0, text_bytes), text_bytes(&v))?;
write(c, name, |f| f.text_color = Some(v.into()))?;
Ok(JsValue::undefined())
}
@@ -113,6 +117,7 @@ fn field_object_new(ctx: &mut Context, name: &str, id: Option<u64>, token: u64)
let fill_color_set = {
fn set(_: &JsValue, args: &[JsValue], name: &FieldKey, c: &mut Context) -> JsResult<JsValue> {
let v = strings(&arg(args, 0), c)?;
charge(c, name, |f| f.fill_color.as_deref().map_or(0, text_bytes), text_bytes(&v))?;
write(c, name, |f| f.fill_color = Some(v.into()))?;
Ok(JsValue::undefined())
}
+9 -4
View File
@@ -33,9 +33,12 @@ impl Interp<'_> {
Ok(())
}
/// `more` nodes into `out`, each once, never past [`MAX_LIST`].
/// `more` nodes into `out`, each once, never past [`MAX_LIST`]. Every node visited costs a
/// step, kept or not, and the call itself one more: a search that matches nothing still
/// costs work when it is repeated over a long list.
pub(super) fn gather(&mut self, out: &mut Vec<usize>, seen: &mut HashSet<usize>, more: impl IntoIterator<Item = usize>) -> R<()> {
for i in more {
self.step()?;
if seen.insert(i) {
out.push(i);
if out.len() > MAX_LIST {
@@ -43,7 +46,7 @@ impl Interp<'_> {
}
}
}
self.charge(1)
self.step()
}
/// The nodes of `v`, each once.
@@ -294,8 +297,10 @@ impl Interp<'_> {
}
}
/// `v` onto `out`: a node only once, never past [`MAX_LIST`].
/// `v` onto `out`: a node only once, never past [`MAX_LIST`]. A duplicate costs its step
/// too: `all` on a list of same-named siblings makes one for every pair.
fn list_push(&mut self, out: &mut Vec<Val>, seen: &mut HashSet<usize>, v: Val) -> R<()> {
self.step()?;
if let Val::Node(i) = v
&& !seen.insert(i)
{
@@ -305,7 +310,7 @@ impl Interp<'_> {
if out.len() > MAX_LIST {
return Err(LIST_TOO_LONG.into());
}
self.charge(1)
Ok(())
}
/// Property `name` of form object `i` as FormCalc reads it: its value, attributes,
+6 -2
View File
@@ -56,6 +56,9 @@ const CENTURY_SPLIT: i64 = 30;
const MONTHS: [&str; 12] = ["January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"];
/// Characters of the longest month name: all a month token needs to read of the text.
const LONGEST_MONTH: usize = 9;
/// Parse `text` as a date by `picture` (`YYYY-MM-DD`, `MM/DD/YYYY`, `MMM D, YYYY`, …).
pub(super) fn parse_date(text: &str, picture: &str) -> Option<(i64, u32, u32)> {
let pic: Vec<char> = picture.chars().collect();
@@ -71,8 +74,9 @@ pub(super) fn parse_date(text: &str, picture: &str) -> Option<(i64, u32, u32)> {
}
i += run;
if c == 'M' && run >= 3 {
// Month name.
let rest: String = txt.get(j..).unwrap_or(&[]).iter().collect();
// Month name: only as much of the text as a name can be, so a picture of many
// month tokens does not copy the rest of a long text for each of them.
let rest: String = txt.get(j..).unwrap_or(&[]).iter().take(LONGEST_MONTH).collect();
let lower = rest.to_ascii_lowercase();
let found = MONTHS
.iter()
+37 -3
View File
@@ -27,6 +27,10 @@ pub(crate) struct Host {
pub(crate) reset_limit_exceeded: bool,
/// Bytes of text kept in alerts, console lines and requests, against [`xfa::MAX_PAYLOAD`].
pub(crate) kept: usize,
/// Bytes of text the script has added to the fields' values and colours, net of what it
/// replaced, against [`xfa::MAX_PAYLOAD`]: each assignment alone is capped, so without this
/// many fields could each hold a maximal array.
pub(crate) retained: usize,
/// Alerts, console lines and requests the limits left out.
pub(crate) dropped: usize,
}
@@ -262,12 +266,42 @@ pub(crate) fn value_of(f: &FieldState, ctx: &mut Context) -> JsValue {
}
}
pub(crate) fn set_value(f: &mut FieldState, vals: Vec<String>) {
f.value = match f.kind {
/// Bytes of text in `v`, counting each string's heap size and its handle.
pub(crate) fn text_bytes(v: &[String]) -> usize {
v.iter().fold(0usize, |n, t| n.saturating_add(t.len()).saturating_add(std::mem::size_of::<String>()))
}
/// Charge the replacement of the text `old` selects from field `name` with `new_bytes` of text to
/// the run's retained-text budget; an assignment the budget has no room for is an error and
/// charges nothing.
pub(crate) fn charge(ctx: &Context, name: &FieldKey, old: impl FnOnce(&FieldState) -> usize, new_bytes: usize) -> JsResult<()> {
let i = field_index(ctx, name)?;
let mut h = host(ctx)?.borrow_mut();
let freed = old(&h.fields[i]);
let total = h.retained.saturating_sub(freed).saturating_add(new_bytes);
if total > xfa::MAX_PAYLOAD {
return Err(error(format!("the fields' values and colours hold more than {} bytes of text", xfa::MAX_PAYLOAD)));
}
h.retained = total;
Ok(())
}
pub(crate) fn kind_of(ctx: &Context, name: &FieldKey) -> JsResult<FieldType> {
let i = field_index(ctx, name)?;
Ok(host(ctx)?.borrow().fields[i].kind)
}
/// What a field of `kind` keeps of the values assigned to it.
pub(crate) fn value_for(kind: FieldType, vals: Vec<String>) -> Vec<String> {
match kind {
FieldType::CheckBox | FieldType::RadioButton => vals.into_iter().filter(|v| v != "Off" && !v.is_empty()).take(1).collect(),
FieldType::ListBox => vals,
_ => vals.into_iter().take(1).collect(),
};
}
}
pub(crate) fn set_value(f: &mut FieldState, vals: Vec<String>) {
f.value = value_for(f.kind, vals);
}
#[cfg(test)]
+1
View File
@@ -270,6 +270,7 @@ pub(crate) fn run_in_context(
reset_calls: setup.reset_calls,
reset_limit_exceeded: false,
kept: 0,
retained: 0,
dropped: 0,
}));
let mut out = Outcome { rc: true, value: event.value.clone(), change: event.change.clone(), ..Default::default() };
+46
View File
@@ -87,6 +87,17 @@ fn calculate_scripts_read_fields_and_set_the_value() {
assert!(o.rc);
}
/// An array of field values copies each entry: one long string listed many times is held to
/// the run's byte budget, not copied once per listing.
#[test]
fn an_array_of_values_stops_at_the_byte_budget() {
let fill = "var s = 'x'; while (s.length < 65536) s += s; var a = []; for (var i = 0; i < N; i++) a.push(s); getField('qty').value = a;";
let o = go(&fill.replace('N', "600"), &Event::field("Calculate", "total", ""));
assert!(o.error.as_deref().is_some_and(|e| e.contains("byte limit exceeded")), "{:?}", o.error);
let o = go(&fill.replace('N', "400"), &Event::field("Calculate", "total", ""));
assert_eq!(o.error, None);
}
/// `event.value` reads and writes as a plain value, numbers included, and the outcome says whether
/// the script assigned it, even to the value it already had.
#[test]
@@ -528,6 +539,7 @@ fn app_alert_titles_are_clipped_and_charged() {
reset_calls: 0,
reset_limit_exceeded: false,
kept: 0,
retained: 0,
dropped: 0,
};
h.alert(Message { title: Some("Order form".into()), ..Message::plain("Saved") });
@@ -1113,6 +1125,24 @@ mod xfa_model {
assert_eq!(o.result.as_deref(), Some("7"));
}
/// A copy of a subform costs its nodes, names and SOMs: instances of one with a long name
/// stop at the run's byte budget, well before the instance cap.
#[test]
fn instances_of_a_subform_with_long_names_stop_at_the_byte_budget() {
let mut f = form();
let table = &mut f.children[0].children[4];
assert_eq!(table.name, "table");
for row in &mut table.children {
row.children.push(field(&"n".repeat(1 << 20), "", "", false));
}
let ev = XfaEvent { activity: "click".into(), target: "form1[0].page1[0].go[0]".into(), ..Default::default() };
let o = run_xfa("for (var i = 0; i < 100; i++) table._row.addInstance(1); table._row.count", &ev, &XfaDoc::default(), &f, Limits::default());
assert_eq!(o.error, None, "{:?}", o.error);
let count: usize = o.result.as_deref().unwrap_or_default().parse().expect("a count");
assert!((3..50).contains(&count), "{count} instances");
assert_eq!(o.effects.iter().filter(|e| matches!(e, XfaEffect::AddInstance { .. })).count(), count - 2);
}
/// On screen every page is a sheet of its own: `xfa.layout.sheet` counts it from 0, as
/// `absPage` does (forms show `sheet(this) + 1`), and `sheetCount` is the page count.
#[test]
@@ -1625,6 +1655,10 @@ mod formcalc {
#[test]
fn date2num_rejects_days_past_the_end_of_the_month() {
// A month name is read from the text's next few characters only, whatever follows.
let serial = calc(r#"Date2Num("2024-09-01")"#);
assert_eq!(calc(r#"Date2Num("September 1, 2024, and a note that runs on", "MMMM D, YYYY")"#), serial);
assert_eq!(calc(r#"Date2Num("Sep 1, 2024", "MMM D, YYYY")"#), serial);
assert_eq!(calc(r#"Date2Num("2023-02-30")"#), "0");
assert_eq!(calc(r#"Date2Num("31/04/2024", "DD/MM/YYYY")"#), "0");
assert_eq!(calc(r#"Date2Num("2023-02-29")"#), "0");
@@ -2641,6 +2675,18 @@ fn a_plain_form_sees_the_xfa_version_but_not_installed() {
assert_eq!(o.alerts, [Message::plain("old viewer")]);
}
/// Arrays assigned to several fields are held to one run-wide budget, and replacing a field's
/// array frees its charge.
#[test]
fn retained_array_text_is_charged_across_fields() {
let fill = "var s = 'x'; while (s.length < 65536) s += s; var a = []; for (var i = 0; i < 320; i++) a.push(s);";
let one = |tail: &str| go(&format!("{fill} {tail}"), &Event::field("Calculate", "total", ""));
let o = one("getField('qty').textColor = a; getField('qty').textColor = a; getField('zip').textColor = ['RGB'];");
assert_eq!(o.error, None, "replacing an array frees its charge: {:?}", o.error);
let o = one("getField('qty').textColor = a; getField('zip').fillColor = a;");
assert!(o.error.as_deref().is_some_and(|e| e.contains("hold more than")), "{:?}", o.error);
}
#[test]
fn string_arrays_refuse_aggregate_text_before_request_clipping() {
let o = go("resetForm(new Array(65).fill('x'.repeat(1 << 19)));", &Event::doc("Console"));
+4 -2
View File
@@ -1,3 +1,5 @@
use std::collections::HashSet;
use super::host::host;
use super::instances::instance_manager;
use super::model::{MAX_DEPTH, XfaDoc, XfaEffect, XfaEvent, clip, payload, reset_names};
@@ -379,7 +381,7 @@ pub(super) fn install(ctx: &mut Context, event: &XfaEvent, doc: &XfaDoc, current
"DataView",
"Intl",
];
let mut defined: Vec<String> = RESERVED.iter().map(ToString::to_string).collect();
let mut defined: HashSet<String> = RESERVED.iter().map(ToString::to_string).collect();
for scope in scopes {
let kids: Vec<(usize, String, bool)> = {
let h = host(ctx)?.borrow();
@@ -398,7 +400,7 @@ pub(super) fn install(ctx: &mut Context, event: &XfaEvent, doc: &XfaDoc, current
let im = instance_manager(ctx, scope, &name)?;
ctx.register_global_property(JsString::from(format!("_{name}").as_str()), im, rw)?;
}
defined.push(name);
defined.insert(name);
}
}
Ok(())
+15 -1
View File
@@ -137,6 +137,17 @@ fn child_som(parent_som: String, name: &str, index: usize) -> String {
}
}
/// Bytes a copy of `src`'s subtree keeps: a node, its name and its SOM (the copy's SOMs differ
/// in their indices only) for every node [`clone_subtree`] copies.
fn subtree_bytes(h: &XHost, src: usize, depth: usize) -> usize {
let Some(n) = h.nodes.get(src).filter(|_| depth <= MAX_DEPTH) else { return 0 };
let own = std::mem::size_of::<HNode>().saturating_add(n.name.len()).saturating_add(n.som.len());
n.children
.iter()
.filter(|&&k| !h.nodes.get(k).is_some_and(|c| c.gone || c.repeatable && c.index > 0))
.fold(own, |sum, &k| sum.saturating_add(subtree_bytes(h, k, depth + 1)))
}
/// 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<usize>, depth: usize) -> Option<usize> {
if depth > MAX_DEPTH || h.nodes.len() >= MAX_NODES {
@@ -214,7 +225,10 @@ impl XHost {
let name = h.nodes.get(first).map(|n| n.name.to_string()).unwrap_or_default();
let parent_som = h.nodes.get(parent).map(|p| p.som.to_string()).unwrap_or_default();
let som = child_som(parent_som, &name, live.len());
if !h.reserve(&XfaEffect::AddInstance { som: String::new() }, som.len()) {
// The copy's nodes, their names and SOMs, are charged to the run's byte budget before
// they are made: a subtree with long names must not be copied past it.
let copied = subtree_bytes(h, first, 0);
if !h.reserve(&XfaEffect::AddInstance { som: String::new() }, som.len().saturating_add(copied)) {
return None;
}
let new = clone_subtree(h, first, Some(parent), 0)?;