diff --git a/crates/js/src/xml.rs b/crates/js/src/xml.rs index 7d70fa689..1277f5402 100644 --- a/crates/js/src/xml.rs +++ b/crates/js/src/xml.rs @@ -616,22 +616,22 @@ impl Interp { } fn node_delete_at(&mut self, x: ObjId, i: usize) -> JsResult<()> { - let node = self.xml_node_mut(x)?; - if i < node.children.len() { - let c = node.children.remove(i); - let len = node.children.len(); - self.xml_node_mut(c)?.parent = None; + let len = self.children_len(x)?; + if i < len { self.xml_text_fuel(len)?; + let c = self.xml_node_mut(x)?.children.remove(i); + self.xml_node_mut(c)?.parent = None; } Ok(()) } fn node_delete_name(&mut self, x: ObjId, n: &NameRef) -> JsResult<()> { if n.attribute { + let count = self.xml_node(x)?.attrs.len(); + self.xml_take_fuel(count)?; let attrs = std::mem::take(&mut self.xml_node_mut(x)?.attrs); let mut kept = Vec::with_capacity(attrs.len()); for a in attrs { - self.use_fuel()?; let hit = self.xml_node(a)?.name.as_ref().is_some_and(|q| n.matches(q)); if hit { self.xml_node_mut(a)?.parent = None; @@ -642,10 +642,11 @@ impl Interp { self.xml_node_mut(x)?.attrs = kept; return Ok(()); } + let count = self.children_len(x)?; + self.xml_take_fuel(count)?; let children = std::mem::take(&mut self.xml_node_mut(x)?.children); let mut kept = Vec::with_capacity(children.len()); for c in children { - self.use_fuel()?; if n.matches_child(self.xml_node(c)?) { self.xml_node_mut(c)?.parent = None; } else { @@ -842,15 +843,21 @@ impl Interp { Ok(if parent == x && q < at { at - 1 } else { at }) } + /// Charges for one more child of `x`. `x` and `v` are held as roots across any collection. + fn xml_charge_child(&mut self, x: ObjId, v: ObjId) -> JsResult<()> { + self.rooted(&[Value::Object(x), Value::Object(v)], |i| i.xml_charge(XML_REF_BYTES)) + } + + /// Takes the fuel and heap for the insertion first, so a limit leaves `v` where it was. fn insert_node(&mut self, x: ObjId, at: usize, v: ObjId) -> JsResult { + self.xml_charge_child(x, v)?; + let len = self.children_len(x)?; + self.xml_text_fuel(len + 1)?; let at = self.detach(x, v, at)?; - self.xml_charge(XML_REF_BYTES)?; let node = self.xml_node_mut(x)?; let at = at.min(node.children.len()); node.children.insert(at, v); - let len = node.children.len(); self.xml_node_mut(v)?.parent = Some(x); - self.xml_text_fuel(len)?; Ok(at) } @@ -883,6 +890,15 @@ impl Interp { Ok(()) } + /// Charges for a [`Self::set_child`] of `v` at `at` that adds a child. + fn charge_set_child(&mut self, x: ObjId, at: usize, v: ObjId) -> JsResult<()> { + if at < self.children_len(x)? { + return Ok(()); + } + self.xml_charge_child(x, v) + } + + /// Puts `v` at `at`, or last when `at` is past the end, after [`Self::charge_set_child`]. fn set_child(&mut self, x: ObjId, at: usize, v: ObjId) -> JsResult<()> { let node = self.xml_node_mut(x)?; let old = match node.children.get_mut(at) { @@ -892,9 +908,6 @@ impl Interp { None } }; - if old.is_none() { - self.xml_charge(XML_REF_BYTES)?; - } if let Some(old) = old { self.xml_node_mut(old)?.parent = None; } @@ -914,6 +927,7 @@ impl Interp { if self.child_at(x, at)? == Some(*v) { return Ok(()); } + self.charge_set_child(x, at, *v)?; let at = self.detach(x, *v, at)?; self.set_child(x, at, *v) } @@ -926,11 +940,13 @@ impl Interp { let s = self.xml_node(*v)?.value.clone(); let t = self.new_text_node(s, Some(x))?; self.xml_keep(t); + self.charge_set_child(x, at, t)?; self.set_child(x, at, t) } Content::Text(s) => { let t = self.new_text_node(s.clone(), Some(x))?; self.xml_keep(t); + self.charge_set_child(x, at, t)?; self.set_child(x, at, t) } } @@ -983,10 +999,11 @@ impl Interp { self.content_to_str(c)? }; let mut found = None; + let count = self.xml_node(x)?.attrs.len(); + self.xml_take_fuel(count)?; let attrs = std::mem::take(&mut self.xml_node_mut(x)?.attrs); let mut kept = Vec::with_capacity(attrs.len()); for a in attrs { - self.use_fuel()?; let hit = self.xml_node(a)?.name.as_ref().is_some_and(|q| n.matches(q)); if hit && found.is_some() { self.xml_node_mut(a)?.parent = None; @@ -1054,6 +1071,7 @@ impl Interp { })?; self.xml_keep(y); self.check_attach_node(x, y)?; + self.charge_set_child(x, at, y)?; self.set_child(x, at, y)?; } at @@ -1061,15 +1079,22 @@ impl Interp { }; if primitive { let Some(target) = self.child_at(x, at)? else { return Ok(()) }; + let s = self.content_to_str(c)?; + let count = self.children_len(target)?; + self.xml_take_fuel(count)?; + let text = if s.len() > 0 { + let t = self.new_text_node(s, Some(target))?; + self.xml_keep(t); + self.xml_charge_child(target, t)?; + Some(t) + } else { + None + }; let children = std::mem::take(&mut self.xml_node_mut(target)?.children); for child in children { - self.use_fuel()?; self.xml_node_mut(child)?.parent = None; } - let s = self.content_to_str(c)?; - if s.len() > 0 { - self.xml_replace(target, 0, &Content::Text(s))?; - } + self.xml_node_mut(target)?.children.extend(text); Ok(()) } else { self.xml_replace(x, at, c) @@ -1621,8 +1646,8 @@ impl Interp { let (p, q) = (self.xml_node(child)?.value.clone(), self.xml_node(next)?.value.clone()); self.concat(&p, &q)? }; - self.xml_node_mut(child)?.value = joined; self.node_delete_at(x, i + 1)?; + self.xml_node_mut(child)?.value = joined; } if self.xml_node(child)?.value.len() == 0 { self.node_delete_at(x, i)?; diff --git a/crates/js/src/xml_tests.rs b/crates/js/src/xml_tests.rs index e4e7a09fc..804234bef 100644 --- a/crates/js/src/xml_tests.rs +++ b/crates/js/src/xml_tests.rs @@ -589,6 +589,109 @@ fn limits_cannot_be_caught_by_the_script() { assert_eq!(kind_of(TIGHT, "try { for (;;) ().toXMLString(); } catch (e) { 1 }"), RunErrorKind::Limit(LimitKind::Fuel)); } +/// Collects garbage and sets the heap limit to the live size plus the argument, so a script can +/// run out of heap at a chosen charge. A negative argument puts the heap over its limit, which +/// also happens when a lowered limit meets a larger heap. +fn squeeze( + i: &mut crate::interp::Interp, + _: crate::value::Value, + args: &[crate::value::Value], + _: bool, +) -> crate::interp::JsResult { + i.collect_garbage(); + if let Some(crate::value::Value::Number(spare)) = args.first() { + i.limits.heap_bytes = (i.heap.bytes as f64 + *spare) as usize; + } + Ok(crate::value::Value::Undefined) +} + +/// Runs `setup`, then the action for each step from 0 until one completes, and after each stop +/// asserts that `check` is true with the limits lifted. Returns how many runs stopped at `kind`. +fn stops_with_the_tree_intact(setup: &str, action: impl Fn(usize) -> String, check: &str, kind: LimitKind, limit: impl Fn(usize) -> Limits) -> usize { + let mut r = realm(); + let interp = r.interp(); + let global = interp.global; + assert!(crate::builtins::method(interp, global, "squeeze", squeeze, 1).is_ok()); + let mut stops = 0; + for step in 0..200_000 { + r.set_limits(Limits::default()); + r.run(setup).unwrap(); + r.set_limits(limit(step)); + let action = action(step); + let got = r.run(&action); + r.set_limits(Limits::default()); + assert_eq!(r.run(check), Ok(JsValue::Bool(true)), "{action}: {got:?}"); + match got { + Ok(_) => return stops, + Err(e) if e.kind == RunErrorKind::Limit(kind) => stops += 1, + Err(_) => {} + } + } + panic!("{check} never completed"); +} + +/// Runs `action`, whose `SPARE` is replaced by a call that leaves each number of bytes free in +/// turn, from 512 too few. Actions put it after the steps whose allocations a collection could +/// free: an argument is evaluated after the callee is looked up. +fn heap_stops(setup: &str, action: &str, check: &str) -> usize { + let action = |spare: usize| action.replace("SPARE", &format!("squeeze({})", spare as i64 - 512)); + stops_with_the_tree_intact(setup, action, check, LimitKind::Heap, |_| Limits::default()) +} + +fn fuel_stops(setup: &str, action: &str, check: &str) -> usize { + stops_with_the_tree_intact(setup, |_| action.to_string(), check, LimitKind::Fuel, |fuel| Limits { fuel: fuel as u64, ..Limits::default() }) +} + +const MOVE_SETUP: &str = "var o = ; var t = ; var n = o.b[0];"; +const MOVE_CHECK: &str = + "o.b.length() + t.b.length() === 1 && (o.b.length() === 0 || o.b[0].parent() === o) && (t.b.length() === 0 || t.b[0].parent() === t)"; + +#[test] +fn a_heap_limit_during_a_move_leaves_the_node_in_its_old_parent() { + for action in + ["t.appendChild((SPARE, n))", "t.prependChild((SPARE, n))", "t.insertChildAfter(null, (SPARE, n))", "t.insertChildBefore(null, (SPARE, n))"] + { + assert!(heap_stops(MOVE_SETUP, action, MOVE_CHECK) > 0, "{action}"); + } +} + +#[test] +fn a_fuel_limit_during_a_move_leaves_the_node_in_its_old_parent() { + for action in ["t.appendChild(n)", "t.prependChild(n)"] { + assert!(fuel_stops(MOVE_SETUP, action, MOVE_CHECK) > 0, "{action}"); + } + let replaced = "var l = t.*; l[0] = o.b[0]"; + let setup = "var o = ; var t = ;"; + assert!(fuel_stops(setup, replaced, MOVE_CHECK) > 0); +} + +#[test] +fn a_heap_limit_on_replace_past_the_end_leaves_no_child_without_a_parent() { + let check = "var n = x.*.length(); if (n === 0) { true } else { \ + var v = x.*[n - 1]; var name; try { v.appendChild(x); name = 'none' } catch (e) { name = e.name } v.parent() === x && name === 'TypeError' }"; + let action = "x.replace({ toString: function () { SPARE; return '5'; } }, v)"; + assert!(heap_stops("var x = ; var v = ;", action, check) > 0); +} + +#[test] +fn limits_during_deletes_and_puts_keep_the_other_nodes() { + let setup = "var x = old;"; + let others = "x.c.length() === 1 && x.@q == '2'"; + for action in ["delete x.b", "delete x.@p", "x.@p = 'v'", "x.@n = 'v'", "x.b = 'v'"] { + assert!(fuel_stops(setup, action, others) > 0, "{action}"); + } + let held = format!("{others} && x.b.length() === 1 && (x.b == 'old' || x.b == 'v') && x.b.*[0].parent() === x.b[0]"); + assert!(fuel_stops(setup, "x.b = 'v'", &held) > 0); + assert!(heap_stops(setup, "x.b = (SPARE, 'v')", &held) > 0); +} + +#[test] +fn a_limit_during_normalize_does_not_repeat_text() { + let setup = "var x = ; x.appendChild('p'); x.appendChild('q'); x.appendChild('r');"; + let check = "x.toString() === 'pqr'"; + assert!(fuel_stops(setup, "x.normalize()", check) > 0); +} + #[test] fn an_interrupt_stops_serialization() { let mut r = realm();