Merge pull request #6626 from dotCooCoo/fix/js-xml-copy-and-insert-fuel

JavaScript: new XML copies a one-item list's node, and insertChildAfter and insertChildBefore take fuel per child scanned

crates/js/src/xml_builtins.rs

@@ -330,7 +330,7 @@ fn xml_ctor(interp: &mut Interp, _: Value, args: &[Value], construct: bool) -> J
     let v = arg(args, 0);
     interp.rooted(std::slice::from_ref(&v), |i| {
         let x = to_xml(i, &v)?;
-        if construct && matches!(i.classify(&v), Xv::Node(_)) {
+        if construct && matches!(i.classify(&v), Xv::Node(_) | Xv::List(_)) {
             return Ok(Value::Object(i.node_copy(x)?));
         }
         Ok(Value::Object(x))
@@ -883,8 +883,15 @@ fn insert_relative(interp: &mut Interp, this: Value, args: &[Value], after: bool
         if after { 0 } else { len }
     } else {
         let Xv::Node(r) = interp.classify(&reference) else { return Ok(Value::Undefined) };
-        interp.use_fuel()?;
-        let Some(i) = interp.xml_node(x)?.children.iter().position(|&c| c == r) else { return Ok(Value::Undefined) };
+        let mut found = None;
+        for i in 0..len {
+            interp.use_fuel()?;
+            if interp.xml_node(x)?.children.get(i) == Some(&r) {
+                found = Some(i);
+                break;
+            }
+        }
+        let Some(i) = found else { return Ok(Value::Undefined) };
         if after { i + 1 } else { i }
     };
     interp.rooted(std::slice::from_ref(&v), |i| i.xml_insert(x, at, &c))?;

crates/js/src/xml_tests.rs

@@ -290,6 +290,10 @@ fn the_constructors_keep_identity_or_copy_as_specified() {
     truth("XML().nodeKind() === 'text' && XML(null) == '' && XML(undefined) == ''");
     truth("var x = <a/>; XML(x) === x && new XML(x) !== x && new XML(x) == x");
     truth("var l = <><a/></>; XML(l) === l[0]");
+    truth("var p = <p><a q='1'><c/></a></p>; var l = p.a; var y = new XML(l); y !== l[0] && y == l[0] && y.parent() === null && l[0].parent() === p");
+    truth(
+        "var p = <p><a q='1'><c/></a></p>; var l = p.a; var y = new XML(l); y.@q = '2'; y.c.@r = '3'; l[0].@q == '1' && l[0].c.@r.length() === 0 && y.c.@r == '3'",
+    );
     assert_eq!(thrown_name("XML(<><a/><b/></>)"), "TypeError");
     assert_eq!(thrown_name("XML('<a><b></a>')"), "SyntaxError");
     assert_eq!(thrown_name("XML('<a/><b/>')"), "SyntaxError");
@@ -368,7 +372,7 @@ fn the_xml_methods_behave_as_specified() {
     assert_eq!(string("(<a>1</a>).toString()"), "1");
     assert_eq!(string("(<a><b/></a>).toString()"), "<a>\n  <b/>\n</a>");
     truth("var x = <a/>; x.valueOf() === x");
-    truth("(<a/>).xpath === undefined || (<a/>).xpath.length() === 0");
+    truth("!Object.prototype.hasOwnProperty.call(XML.prototype, 'xpath') && !Object.prototype.hasOwnProperty.call(XMLList.prototype, 'xpath')");
     truth("var x = <a><b/></a>; x.b.attributes !== undefined");
 }
 
@@ -716,6 +720,26 @@ fn catastrophic_queries_end_with_the_fuel_limit() {
     assert_eq!(kind_of(limits, &serialize), RunErrorKind::Limit(LimitKind::Fuel));
 }
 
+/// The fuel that `action` takes when it runs after `setup`.
+fn fuel_of(setup: &str, action: &str) -> u64 {
+    let mut r = realm();
+    r.run(setup).unwrap();
+    r.run(action).unwrap();
+    Limits::default().fuel - r.fuel_left()
+}
+
+#[test]
+fn insert_child_before_and_after_take_fuel_for_each_child_they_scan() {
+    let narrow = "var x = <a><b/></a>; var last = x.b[0];";
+    let wide = "var x = <a/>; for (var i = 0; i < 2000; i++) x.appendChild(<b/>); var last = x.b[1999];";
+    for action in
+        ["x.insertChildAfter(<z/>, <c/>)", "x.insertChildBefore(<z/>, <c/>)", "x.insertChildAfter(last, <c/>)", "x.insertChildBefore(last, <c/>)"]
+    {
+        let (n, w) = (fuel_of(narrow, action), fuel_of(wide, action));
+        assert!(w >= n + 1999, "{action}: {n} with one child, {w} with 2000");
+    }
+}
+
 #[test]
 fn limits_cannot_be_caught_by_the_script() {
     assert_eq!(kind_of(TIGHT, "try { for (;;) (<a/>).toXMLString(); } catch (e) { 1 }"), RunErrorKind::Limit(LimitKind::Fuel));