JavaScript: a limit while a list or named match replaces XML children leaves the tree intact

Replacing a child with a list, inserting a list, and assigning or
replacing by name now check the depth and take all fuel and heap before
the first change. A limit no longer leaves the old child removed, part
of a list inserted or one of several matches gone.

appendChild and assignment past the end of a list remove their empty
placeholder when the assignment stops. A list that takes an earlier
sibling at an index keeps that node in its slot.
This commit is contained in:
dotCooCoo
2026-11-09 07:37:24 -08:00
parent 775d72ddff
commit 80ef5d5f53
3 changed files with 309 additions and 122 deletions
+212 -107
View File
@@ -654,7 +654,30 @@ impl Interp {
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.remove_child(x, i)?;
}
Ok(())
}
/// Takes the child at `i` out of `x`. The caller has checked `i` and taken the fuel.
fn remove_child(&mut self, x: ObjId, i: usize) -> JsResult<()> {
let c = self.xml_node_mut(x)?.children.remove(i);
self.xml_node_mut(c)?.parent = None;
Ok(())
}
/// Takes `gone`, children of `x` in document order, out of `x` in one pass. The caller has
/// taken the fuel.
fn remove_children(&mut self, x: ObjId, gone: &[ObjId]) -> JsResult<()> {
let mut next = gone.iter().peekable();
self.xml_node_mut(x)?.children.retain(|c| {
let hit = next.peek() == Some(&c);
if hit {
next.next();
}
!hit
});
for &c in gone {
self.xml_node_mut(c)?.parent = None;
}
Ok(())
@@ -867,17 +890,43 @@ impl Interp {
}
/// Removes `v` from its parent, so a node is never in two lists of children. Returns the
/// position in `x` to use after the removal.
/// position in `x` to use after the removal. The caller has taken [`Self::place_fuel`].
fn detach(&mut self, x: ObjId, v: ObjId, at: usize) -> JsResult<usize> {
let Some(parent) = self.xml_node(v)?.parent else { return Ok(at) };
if self.xml_kind(v)? == XmlKind::Attribute {
return Ok(at);
}
let Some(q) = self.index_in_parent(parent, v)? else { return Ok(at) };
self.node_delete_at(parent, q)?;
let Some(q) = self.xml_node(parent)?.children.iter().position(|&c| c == v) else { return Ok(at) };
self.remove_child(parent, q)?;
Ok(if parent == x && q < at { at - 1 } else { at })
}
/// Takes the fuel to put `v` among `len` children: the insertion and, when `v` has a parent,
/// the scan and removal that take it out.
fn place_fuel(&mut self, len: usize, v: ObjId) -> JsResult<()> {
self.xml_text_fuel(len + 1)?;
let node = self.xml_node(v)?;
if node.kind != XmlKind::Attribute
&& let Some(parent) = node.parent
{
let old = self.children_len(parent)?;
self.xml_text_fuel(old)?;
self.xml_text_fuel(old)?;
}
Ok(())
}
/// Moves `v` to position `at` of `x` and returns where it went. The caller has taken the fuel
/// and heap.
fn place_child(&mut self, x: ObjId, at: usize, v: ObjId) -> JsResult<usize> {
let at = self.detach(x, v, at)?;
let node = self.xml_node_mut(x)?;
let at = at.min(node.children.len());
node.children.insert(at, v);
self.xml_node_mut(v)?.parent = Some(x);
Ok(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))
@@ -887,13 +936,30 @@ impl Interp {
fn insert_node(&mut self, x: ObjId, at: usize, v: ObjId) -> JsResult<usize> {
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)?;
let node = self.xml_node_mut(x)?;
let at = at.min(node.children.len());
node.children.insert(at, v);
self.xml_node_mut(v)?.parent = Some(x);
Ok(at)
self.place_fuel(len, v)?;
self.place_child(x, at, v)
}
/// Takes the fuel and heap to put every item of `l` under `x` and returns the items. The tree
/// does not change here, so a limit leaves it as it was.
fn reserve_list_insert(&mut self, x: ObjId, l: ObjId) -> JsResult<Vec<ObjId>> {
let items = self.xml_list(l)?.items.clone();
let bytes = XML_REF_BYTES.saturating_mul(items.len());
self.rooted(&[Value::Object(x), Value::Object(l)], |i| i.xml_charge(bytes))?;
let len = self.children_len(x)?;
for (j, &item) in items.iter().enumerate() {
self.place_fuel(len + j, item)?;
}
Ok(items)
}
/// Puts `items` at `at` in order, after [`Self::reserve_list_insert`].
fn place_children(&mut self, x: ObjId, at: usize, items: &[ObjId]) -> JsResult<()> {
let mut at = at;
for &item in items {
at = self.place_child(x, at, item)? + 1;
}
Ok(())
}
/// [[Insert]] (ECMA-357 section 9.1.1.11).
@@ -915,11 +981,8 @@ impl Interp {
self.insert_node(x, at, *v)?;
}
Content::List(l) => {
let mut at = at;
for j in 0..self.list_len(*l)? {
let Some(item) = self.list_at(*l, j)? else { break };
at = self.insert_node(x, at, item)? + 1;
}
let items = self.reserve_list_insert(x, *l)?;
self.place_children(x, at, &items)?;
}
}
Ok(())
@@ -963,13 +1026,20 @@ impl Interp {
return Ok(());
}
self.charge_set_child(x, at, *v)?;
let len = self.children_len(x)?;
self.place_fuel(len, *v)?;
let at = self.detach(x, *v, at)?;
self.set_child(x, at, *v)
}
Content::List(_) => {
Content::List(l) => {
self.check_attach(x, c)?;
self.node_delete_at(x, at)?;
self.xml_insert(x, at, c)
let items = self.reserve_list_insert(x, *l)?;
let len = self.children_len(x)?;
self.xml_text_fuel(len)?;
if at < len {
self.remove_child(x, at)?;
}
self.place_children(x, at, &items)
}
Content::Node(v) => {
let s = self.xml_node(*v)?.value.clone();
@@ -987,6 +1057,21 @@ impl Interp {
}
}
/// Replaces the first of `matches`, children of `x` in document order, with `c` and removes
/// the rest; appends `c` when there are none. A limit leaves `x` as it was. The value in `c`
/// is never one of `matches`.
pub(crate) fn xml_replace_matches(&mut self, x: ObjId, matches: &[ObjId], c: &Content) -> JsResult<()> {
let len = self.children_len(x)?;
self.xml_text_fuel(len)?;
self.xml_text_fuel(len)?;
let at = match matches.first() {
Some(first) => self.xml_node(x)?.children.iter().position(|c| c == first).unwrap_or(len),
None => len,
};
self.xml_replace(x, at, c)?;
self.remove_children(x, matches.get(1..).unwrap_or_default())
}
// ---- [[Put]] ----
/// A value to assign into a tree, copied when it is XML (steps 3 and 4 of section 9.1.1.2).
@@ -1074,66 +1159,68 @@ impl Interp {
if !is_ncname(&n.local) && !n.is_star() {
return Ok(());
}
let primitive = matches!(c, Content::Text(_)) && !n.is_star();
let mut found: Option<usize> = None;
for k in (0..self.children_len(x)?).rev() {
self.use_fuel()?;
let Some(child) = self.child_at(x, k)? else { continue };
let len = self.children_len(x)?;
self.xml_take_fuel(len)?;
let mut matches = Vec::new();
for k in 0..len {
let Some(child) = self.child_at(x, k)? else { break };
if n.matches_child(self.xml_node(child)?) {
if let Some(i) = found {
self.node_delete_at(x, i)?;
}
found = Some(k);
matches.push(child);
}
}
let at = match found {
Some(i) => i,
match c {
Content::Text(s) if !n.is_star() => self.put_element_text(x, n, &matches, s),
_ => self.xml_replace_matches(x, &matches, c),
}
}
/// Makes `s` the only content of the first of `matches`, or of a new element named `n` when
/// there are none, and removes the other matches. A limit leaves `x` as it was.
fn put_element_text(&mut self, x: ObjId, n: &NameRef, matches: &[ObjId], s: &Str) -> JsResult<()> {
let at = self.children_len(x)?;
self.xml_text_fuel(at)?;
let target = match matches.first() {
Some(&first) => first,
None => {
let at = self.children_len(x)?;
if primitive {
let uri = match &n.uri {
Some(u) => u.clone(),
None => self.xml_default_uri()?,
};
let y = self.new_xml_node(XmlNode {
kind: XmlKind::Element,
name: Some(XmlName { uri: Some(uri), local: n.local.clone(), prefix: None }),
parent: Some(x),
value: Str::from(""),
attrs: Vec::new(),
children: Vec::new(),
ns: Vec::new(),
})?;
self.xml_keep(y);
self.check_attach_node(x, y)?;
self.charge_set_child(x, at, y)?;
self.set_child(x, at, y)?;
}
at
let uri = match &n.uri {
Some(u) => u.clone(),
None => self.xml_default_uri()?,
};
let y = self.new_xml_node(XmlNode {
kind: XmlKind::Element,
name: Some(XmlName { uri: Some(uri), local: n.local.clone(), prefix: None }),
parent: Some(x),
value: Str::from(""),
attrs: Vec::new(),
children: Vec::new(),
ns: Vec::new(),
})?;
self.xml_keep(y);
self.check_attach_node(x, y)?;
self.charge_set_child(x, at, y)?;
y
}
};
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.xml_node_mut(child)?.parent = None;
}
self.xml_node_mut(target)?.children.extend(text);
Ok(())
let count = self.children_len(target)?;
self.xml_take_fuel(count)?;
let text = if s.len() > 0 {
let t = self.new_text_node(s.clone(), Some(target))?;
self.xml_keep(t);
self.xml_charge_child(target, t)?;
Some(t)
} else {
self.xml_replace(x, at, c)
None
};
if matches.is_empty() {
self.set_child(x, at, target)?;
}
self.remove_children(x, matches.get(1..).unwrap_or_default())?;
let children = std::mem::take(&mut self.xml_node_mut(target)?.children);
for child in children {
self.xml_node_mut(child)?.parent = None;
}
self.xml_node_mut(target)?.children.extend(text);
Ok(())
}
fn node_put(&mut self, x: ObjId, n: &NameRef, v: &Value) -> JsResult<()> {
@@ -1210,7 +1297,6 @@ impl Interp {
}
fn list_put_index(&mut self, x: ObjId, index: usize, v: &Value) -> JsResult<()> {
let mut i = index;
let data = self.xml_list(x)?;
let target = data.target;
let target_name = data.target_name.clone();
@@ -1221,7 +1307,7 @@ impl Interp {
},
None => None,
};
if i >= self.list_len(x)? {
if index >= self.list_len(x)? {
let r = match r {
Some(r) => match self.classify(&Value::Object(r)) {
Xv::List(rl) => {
@@ -1262,29 +1348,56 @@ impl Interp {
let y =
self.new_xml_node(XmlNode { kind, name, parent: r, value: Str::from(""), attrs: Vec::new(), children: Vec::new(), ns: Vec::new() })?;
self.xml_keep(y);
i = self.list_len(x)?;
if kind == XmlKind::Attribute {
if let Some(r) = r {
self.xml_charge(XML_REF_BYTES)?;
self.xml_node_mut(r)?.attrs.push(y);
}
} else if let Some(r) = r {
let len = self.children_len(r)?;
let at = if i > 0 {
let prev = self.list_at(x, i - 1)?;
let position = match prev {
Some(prev) => self.index_in_parent(r, prev)?,
None => None,
};
position.map_or(len, |j| j + 1)
} else {
len
};
self.check_attach_node(r, y)?;
self.insert_node(r, at, y)?;
let i = self.list_len(x)?;
let put = self.list_append_new(x, i, r, kind, y).and_then(|()| self.list_put_at(x, i, v));
if put.is_err() {
self.undo_list_append(x, y)?;
}
self.list_push(x, y)?;
return put;
}
self.list_put_at(x, index, v)
}
/// Adds the new node `y` as item `i` of `x` and, when there is a parent `r`, to the tree.
fn list_append_new(&mut self, x: ObjId, i: usize, r: Option<ObjId>, kind: XmlKind, y: ObjId) -> JsResult<()> {
if kind == XmlKind::Attribute {
if let Some(r) = r {
self.xml_charge(XML_REF_BYTES)?;
self.xml_node_mut(r)?.attrs.push(y);
}
} else if let Some(r) = r {
let len = self.children_len(r)?;
let at = if i > 0 {
let prev = self.list_at(x, i - 1)?;
let position = match prev {
Some(prev) => self.index_in_parent(r, prev)?,
None => None,
};
position.map_or(len, |j| j + 1)
} else {
len
};
self.check_attach_node(r, y)?;
self.insert_node(r, at, y)?;
}
self.list_push(x, y)
}
/// Takes `y` out of its parent and of `x` after an assignment that added it stopped. It takes
/// no fuel or heap, so it runs after a limit.
fn undo_list_append(&mut self, x: ObjId, y: ObjId) -> JsResult<()> {
if let Some(parent) = self.xml_node(y)?.parent {
let node = self.xml_node_mut(parent)?;
node.children.retain(|&c| c != y);
node.attrs.retain(|&a| a != y);
self.xml_node_mut(y)?.parent = None;
}
self.xml_list_mut(x)?.items.retain(|&item| item != y);
Ok(())
}
/// Assigns `v` to item `i` of `x`.
fn list_put_at(&mut self, x: ObjId, i: usize, v: &Value) -> JsResult<()> {
let Some(item) = self.list_at(x, i)? else { return Ok(()) };
let xv = self.classify(v);
let value = match xv {
@@ -1313,21 +1426,13 @@ impl Interp {
Content::List(l) => {
let c = self.list_shallow_copy(l)?;
self.xml_keep(c);
let count = self.list_len(c)?;
let items = self.xml_list(c)?.items.clone();
self.xml_charge(XML_REF_BYTES.saturating_mul(items.len()))?;
if let Some(parent) = parent
&& let Some(q) = self.index_in_parent(parent, item)?
{
self.xml_replace(parent, q, &Content::List(c))?;
for j in 0..count {
if let Some(child) = self.child_at(parent, q + j)?
&& let Some(slot) = self.xml_list_mut(c)?.items.get_mut(j)
{
*slot = child;
}
}
}
let items = self.xml_list(c)?.items.clone();
self.xml_charge(XML_REF_BYTES.saturating_mul(items.len()))?;
self.xml_list_mut(x)?.items.splice(i..=i, items);
Ok(())
}
@@ -1348,14 +1453,14 @@ impl Interp {
replaced = self.child_at(parent, q)?;
}
let new_item = match (replaced, value) {
(Some(r), _) => r,
(None, Content::Node(n)) => *n,
(_, Content::Node(n)) => *n,
(Some(r), Content::Text(_)) => r,
(None, Content::Text(s)) => {
let t = self.new_text_node(s.clone(), None)?;
self.xml_keep(t);
t
}
(None, Content::List(_)) => return Ok(()),
(_, Content::List(_)) => return Ok(()),
};
if let Some(slot) = self.xml_list_mut(x)?.items.get_mut(i) {
*slot = new_item;
+3 -15
View File
@@ -967,23 +967,11 @@ fn replace(interp: &mut Interp, this: Value, args: &[Value], _: bool) -> JsResul
};
let matched = i.xml_select(x, &NameRef { attribute: false, ..n })?;
i.xml_keep(matched);
let count = i.list_len(matched)?;
if count == 0 {
let items = i.xml_list(matched)?.items.clone();
if items.is_empty() {
return Ok(());
}
let Some(first) = i.list_at(matched, 0)? else { return Ok(()) };
for at in (1..count).rev() {
let Some(item) = i.list_at(matched, at)? else { continue };
i.use_fuel()?;
let position = i.xml_node(x)?.children.iter().position(|&c| c == item);
if let Some(q) = position {
let removed = i.xml_node_mut(x)?.children.remove(q);
i.xml_node_mut(removed)?.parent = None;
}
}
i.use_fuel()?;
let at = i.xml_node(x)?.children.iter().position(|&c| c == first).unwrap_or(0);
i.xml_replace(x, at, &c)
i.xml_replace_matches(x, &items, &c)
})?;
Ok(this)
}
+94
View File
@@ -763,6 +763,100 @@ fn limits_during_deletes_and_puts_keep_the_other_nodes() {
assert!(heap_stops(setup, "x.b = (SPARE, 'v')", &held) > 0);
}
/// True when the markup of `tree` is `before` or `after`, ignoring white space.
fn tree_is(tree: &str, before: &str, after: &str) -> String {
format!("(function () {{ var s = {tree}.toXMLString().replace(/\\s/g, ''); return s === '{before}' || s === '{after}'; }})()")
}
#[test]
fn a_limit_while_a_list_replaces_a_child_leaves_the_children() {
let setup = "var x = <a><b/><z/></a>; var v = <><p/><q/></>;";
let replaced = tree_is("x", "<a><b/><z/></a>", "<a><p/><q/><z/></a>");
for action in ["x.replace(0, v)", "x.b = v"] {
assert!(fuel_stops(setup, action, &replaced) > 0, "{action}");
}
let spare = "x.replace({ toString: function () { SPARE; return '0'; } }, v)";
assert!(heap_stops(setup, spare, &replaced) > 0);
let set = tree_is("x", "<a><b/><z/></a>", "<a><p/><q/></a>");
assert!(fuel_stops(setup, "x.setChildren(v)", &set) > 0);
}
#[test]
fn a_limit_while_a_list_is_inserted_leaves_the_children() {
let setup = "var o = <o><p/><q/></o>; var x = <a><b/></a>; var v = o.*;";
let check = |after: &str| {
format!("{} && (o.*.length() === 0 ? x.*.length() === 3 : o.*.length() === 2 && x.*.length() === 1)", tree_is("x", "<a><b/></a>", after))
};
for (action, after) in [
("x.appendChild(v)", "<a><b/><p/><q/></a>"),
("x.prependChild(v)", "<a><p/><q/><b/></a>"),
("x.insertChildAfter(null, v)", "<a><p/><q/><b/></a>"),
("x.insertChildBefore(null, v)", "<a><b/><p/><q/></a>"),
] {
assert!(fuel_stops(setup, action, &check(after)) > 0, "{action}");
}
assert!(heap_stops(setup, "x.appendChild((SPARE, v))", &check("<a><b/><p/><q/></a>")) > 0);
}
#[test]
fn a_heap_limit_on_a_list_put_at_an_index_keeps_the_list_and_tree_in_step() {
let setup = "var x = <a><b/><z/></a>; var l = x.*; var v = <><p/><q/></>;";
let check = "var s = x.toXMLString().replace(/\\s/g, ''); \
(s === '<a><b/><z/></a>' && l.length() === 2 && l[0].name() == 'b') \
|| (s === '<a><p/><q/><z/></a>' && l.length() === 3 && l[0].name() == 'p')";
assert!(heap_stops(setup, "l[(SPARE, 0)] = v", check) > 0);
}
#[test]
fn an_index_put_of_an_earlier_sibling_keeps_that_node_in_the_list() {
let setup = "var x = <a><b/><c/><d/><e/></a>; var l = x.*;";
truth(&format!("{setup} l[2] = x.b; l[2] === x.b[0] && l[3] === x.e[0]"));
truth(&format!("{setup} l[2] = x.b[0]; l[2] === x.b[0] && l[3] === x.e[0]"));
}
/// A realm whose trees may be 32 levels deep, and a script line making `deep`, a tree of 32 levels
/// that cannot go under any element.
fn shallow_with_deep_value() -> (Realm, &'static str) {
let deep = "var s = ''; var e = ''; for (var i = 0; i < 32; i++) { s += '<d>'; e += '</d>'; } var deep = new XML(s + e);";
(Realm::with_options(Options { max_depth: 32, ..e4x() }, Limits::default()), deep)
}
#[test]
fn a_too_deep_value_leaves_every_match_in_place() {
for action in ["x.replace('b', deep)", "x.b = deep"] {
let (mut r, deep) = shallow_with_deep_value();
r.run(deep).unwrap();
r.run("var x = <a><b/><c/><b/></a>;").unwrap();
assert_eq!(r.run(action).map_err(|e| e.kind), Err(RunErrorKind::Limit(LimitKind::Heap)), "{action}");
assert_eq!(r.run("x.b.length() === 2 && x.*.length() === 3"), Ok(JsValue::Bool(true)), "{action}");
}
}
#[test]
fn a_limit_while_matches_are_replaced_keeps_every_match() {
let setup = "var x = <a><b/><c/><b/></a>; var n = <n/>; var l = <><p/><q/></>;";
let two = "<a><b/><c/><b/></a>";
for (action, after) in [
("x.replace('b', n)", "<a><n/><c/></a>"),
("x.replace('b', l)", "<a><p/><q/><c/></a>"),
("x.b = n", "<a><n/><c/></a>"),
("x.b = l", "<a><p/><q/><c/></a>"),
("x.b = 'v'", "<a><b>v</b><c/></a>"),
] {
assert!(fuel_stops(setup, action, &tree_is("x", two, after)) > 0, "{action}");
}
let by_name = "x.replace({ toString: function () { SPARE; return 'b'; } }, n)";
assert!(heap_stops(setup, by_name, &tree_is("x", two, "<a><n/><c/></a>")) > 0);
assert!(heap_stops(setup, "x.b = (SPARE, 'v')", &tree_is("x", two, "<a><b>v</b><c/></a>")) > 0);
}
#[test]
fn matches_are_replaced_in_document_order() {
truth("var x = <a><b>1</b><c/><b>2</b><d/><b>3</b></a>; x.replace('b', <n/>); x.*.length() === 3 && x.*[0].name() == 'n' && x.d.length() === 1");
truth("var x = <a><b>1</b><c/><b>2</b></a>; var old = x.b[1]; x.b = 'v'; x.b.length() === 1 && x.b == 'v' && old.parent() === null");
truth("var x = <a><c/></a>; x.b = 'v'; x.b == 'v' && x.*.length() === 2 && x.b[0].parent() === x");
}
#[test]
fn a_limit_during_normalize_does_not_repeat_text() {
let setup = "var x = <a/>; x.appendChild('p'); x.appendChild('q'); x.appendChild('r');";