JavaScript: a computed XML name or attribute run cannot add markup

An embedded expression used as a tag or attribute name must now be one XML
name, and one used as a run of attributes must hold only name="value"
attributes separated by white space. Anything else throws SyntaxError, so a
script string cannot add an attribute, close the tag or add a child.

{name}={value} in a tag now reads as a computed attribute name, as in the
example in ECMA-357 section 11.1.4.

crates/js/src/ast.rs

@@ -210,8 +210,10 @@ pub enum XmlPart {
     Content(ExprId),
     /// `{expression}` as an attribute's value.
     Attribute(ExprId),
-    /// `{expression}` as a name or a run of attributes, its string used as markup.
-    Raw(ExprId),
+    /// `{expression}` as an element or attribute name, its string checked as an XML name.
+    Name(ExprId),
+    /// `{expression}` as a run of attributes, its string checked as `name="value"` attributes.
+    Attributes(ExprId),
 }
 
 /// What stands before `::` in a qualified name.

crates/js/src/bytecode.rs

@@ -153,7 +153,7 @@ pub(crate) enum Op {
     /// `object name` → the descendants that match.
     XmlDescendants,
     /// `value` → the string an embedded expression puts in a literal (0 content, 1 attribute
-    /// value, 2 name).
+    /// value, 2 name, 3 run of attributes).
     XmlEscape(u8),
     /// The `parts` strings → the XML or XMLList they spell.
     XmlLiteral {

crates/js/src/compiler.rs

@@ -861,12 +861,13 @@ impl<'a> FnCompiler<'a> {
                     let i = self.string(units);
                     self.emit(Op::String(i));
                 }
-                XmlPart::Content(e) | XmlPart::Attribute(e) | XmlPart::Raw(e) => {
+                XmlPart::Content(e) | XmlPart::Attribute(e) | XmlPart::Name(e) | XmlPart::Attributes(e) => {
                     self.expr(*e)?;
                     self.emit(Op::XmlEscape(match part {
                         XmlPart::Content(_) => 0,
                         XmlPart::Attribute(_) => 1,
-                        _ => 2,
+                        XmlPart::Name(_) => 2,
+                        _ => 3,
                     }));
                 }
             }

crates/js/src/parser/xml.rs

@@ -8,7 +8,7 @@ use super::{Parser, Result, describe};
 use crate::ast::{Expr, ExprId, Stmt, StmtId, XmlNamePart, XmlPart, XmlQualifier};
 use crate::error::TOO_DEEP;
 use crate::token::{Keyword, Punct, Span, TokenKind};
-use crate::xml::{is_name_part, is_name_start, is_xml_space, scan_reference, valid_xml_units};
+use crate::xml::{is_name_start, is_xml_space, scan_name, scan_reference};
 
 const LT: u16 = b'<' as u16;
 const GT: u16 = b'>' as u16;
@@ -277,17 +277,7 @@ impl Parser<'_> {
 
     /// Reads a static name at `pos`, which must be one, and returns the offset after it.
     fn xml_static_name(&self, pos: usize) -> Result<usize> {
-        if !self.unit(pos).is_some_and(is_name_start) {
-            return Err(self.xml_error(pos, "invalid XML name"));
-        }
-        let mut at = pos + 1;
-        while self.unit(at).is_some_and(is_name_part) {
-            at += 1;
-        }
-        if !valid_xml_units(self.src.get(pos..at).unwrap_or_default()) {
-            return Err(self.xml_error(pos, "invalid XML name"));
-        }
-        Ok(at)
+        scan_name(self.src, pos).ok_or_else(|| self.xml_error(pos, "invalid XML name"))
     }
 
     fn xml_skip_space(&self, mut pos: usize, scan: &mut Scan) -> usize {
@@ -308,7 +298,7 @@ impl Parser<'_> {
         let name = if self.unit(at) == Some(LBRACE) {
             scan.flush();
             let (e, after) = self.xml_embedded(at)?;
-            scan.parts.push(XmlPart::Raw(e));
+            scan.parts.push(XmlPart::Name(e));
             at = after;
             None
         } else {
@@ -338,8 +328,17 @@ impl Parser<'_> {
                     }
                     scan.flush();
                     let (e, after) = self.xml_embedded(at)?;
-                    scan.parts.push(XmlPart::Raw(e));
-                    at = after;
+                    let mut next = after;
+                    while self.unit(next).is_some_and(is_xml_space) {
+                        next += 1;
+                    }
+                    if self.unit(next) == Some(EQ) {
+                        scan.parts.push(XmlPart::Name(e));
+                        at = self.xml_attribute_value(after, scan)?;
+                    } else {
+                        scan.parts.push(XmlPart::Attributes(e));
+                        at = after;
+                    }
                 }
                 Some(_) if at == before => return Err(self.xml_error(at, "expected white space before an attribute")),
                 Some(_) => at = self.xml_attribute(at, scan)?,
@@ -351,7 +350,12 @@ impl Parser<'_> {
     fn xml_attribute(&mut self, pos: usize, scan: &mut Scan) -> Result<usize> {
         let end = self.xml_static_name(pos)?;
         self.xml_copy(pos, end, scan);
-        let at = self.xml_skip_space(end, scan);
+        self.xml_attribute_value(end, scan)
+    }
+
+    /// Reads the `=` and value of an attribute whose name ends at `pos` and returns the offset after them.
+    fn xml_attribute_value(&mut self, pos: usize, scan: &mut Scan) -> Result<usize> {
+        let at = self.xml_skip_space(pos, scan);
         if self.unit(at) != Some(EQ) {
             return Err(self.xml_error(at, "expected '=' after an attribute name"));
         }
@@ -397,7 +401,7 @@ impl Parser<'_> {
         let name = if self.unit(at) == Some(LBRACE) {
             scan.flush();
             let (e, after) = self.xml_embedded(at)?;
-            scan.parts.push(XmlPart::Raw(e));
+            scan.parts.push(XmlPart::Name(e));
             at = after;
             None
         } else {

crates/js/src/xml.rs

@@ -18,7 +18,7 @@ use crate::heap::{
     XmlListData, XmlName, XmlNode, XmlNs, key, string_bytes,
 };
 use crate::interp::{Abrupt, ErrorType, HeldBytes, Interp, JsResult};
-use crate::string_builtins::UNITS_PER_FUEL;
+use crate::string_builtins::{UNITS_PER_FUEL, flat_charged};
 use crate::value::{Str, Value, index_key};
 
 fn parser_units(s: &str) -> Vec<u16> {
@@ -123,6 +123,53 @@ pub(crate) fn is_name_part(c: u16) -> bool {
     is_name_start(c) || matches!(c, 0x2D | 0x2E | 0xB7) || (c != 0x24 && is_id_part(u32::from(c)))
 }
 
+/// The offset after the XML name that starts at `src[pos]`, when one does.
+pub(crate) fn scan_name(src: &[u16], pos: usize) -> Option<usize> {
+    if !src.get(pos).copied().is_some_and(is_name_start) {
+        return None;
+    }
+    let mut end = pos + 1;
+    while src.get(end).copied().is_some_and(is_name_part) {
+        end += 1;
+    }
+    valid_xml_units(src.get(pos..end).unwrap_or_default()).then_some(end)
+}
+
+fn skip_xml_space(src: &[u16], mut pos: usize) -> usize {
+    while src.get(pos).copied().is_some_and(is_xml_space) {
+        pos += 1;
+    }
+    pos
+}
+
+/// Whether `units` holds only `name="value"` attributes separated by white space.
+fn is_attribute_run(units: &[u16]) -> bool {
+    let mut at = skip_xml_space(units, 0);
+    while at < units.len() {
+        let Some(name_end) = scan_name(units, at) else { return false };
+        let eq = skip_xml_space(units, name_end);
+        if units.get(eq) != Some(&EQ) {
+            return false;
+        }
+        let open = skip_xml_space(units, eq + 1);
+        let quote = match units.get(open).copied() {
+            Some(q @ (DQUOTE | SQUOTE)) => q,
+            _ => return false,
+        };
+        let value = units.get(open + 1..).unwrap_or_default();
+        let Some(len) = value.iter().position(|&c| c == quote || c == LT) else { return false };
+        if value.get(len) != Some(&quote) {
+            return false;
+        }
+        let end = open + len + 2;
+        at = skip_xml_space(units, end);
+        if at == end && at < units.len() {
+            return false;
+        }
+    }
+    true
+}
+
 /// An XML name without a prefix (ECMA-357 section 13.1.2.1).
 pub(crate) fn is_ncname(units: &[u16]) -> bool {
     let Some((&first, rest)) = units.split_first() else { return false };
@@ -2145,17 +2192,7 @@ impl Interp {
     }
 
     fn read_name(&mut self, src: &[u16], pos: usize) -> JsResult<usize> {
-        if !src.get(pos).copied().is_some_and(is_name_start) {
-            return Err(self.syntax("invalid XML name"));
-        }
-        let mut end = pos + 1;
-        while src.get(end).copied().is_some_and(is_name_part) {
-            end += 1;
-        }
-        if !valid_xml_units(src.get(pos..end).unwrap_or_default()) {
-            return Err(self.syntax("invalid XML name"));
-        }
-        Ok(end)
+        scan_name(src, pos).ok_or_else(|| self.syntax("invalid XML name"))
     }
 
     fn check_units(&mut self, src: &[u16], from: usize, to: usize) -> JsResult<()> {
@@ -2605,7 +2642,18 @@ impl Interp {
                 let Value::String(s) = out.finish(self) else { return Err(Abrupt::Internal("not a string".into())) };
                 s
             }
-            _ => self.to_str(&v)?,
+            _ => {
+                let text = self.to_str(&v)?;
+                let units = flat_charged(self, &text)?;
+                self.xml_take_fuel(units.len() / UNITS_PER_FUEL + 1)?;
+                if kind == 2 && scan_name(&units, 0) != Some(units.len()) {
+                    return Err(self.syntax("an embedded expression used as an XML name does not give a name"));
+                }
+                if kind != 2 && !is_attribute_run(&units) {
+                    return Err(self.syntax("an embedded expression used as XML attributes does not give only attributes"));
+                }
+                text
+            }
         };
         self.replace(1, Value::String(result))
     }

crates/js/src/xml_tests.rs

@@ -120,6 +120,60 @@ fn embedded_expressions_are_escaped_by_position() {
     truth("(<a>{1 < 2}</a>) == 'true'");
 }
 
+#[test]
+fn a_computed_tag_name_cannot_add_markup() {
+    for src in [
+        r#"(<{'a x="1"'}/>)"#,
+        "(<{'a><c/'}></{'a'}>)",
+        "(<a><b>t</{'b><c/'}></a>)",
+        "(<{''}/>)",
+        "(<{'1a'}/>)",
+        "(<a></{'a '}>)",
+        r#"(<x {'a="1" b'}="2"/>)"#,
+    ] {
+        assert_eq!(thrown_name(src), "SyntaxError", "{src}");
+    }
+}
+
+#[test]
+fn a_computed_name_may_be_any_xml_name() {
+    assert_eq!(string("(<{'a.b-c'}/>).toXMLString()"), "<a.b-c/>");
+    assert_eq!(string("(<{'tag'}>t</{'tag'}>).toXMLString()"), "<tag>t</tag>");
+    assert_eq!(string("var n = 'n:b'; (<a xmlns:n=\"u\"><{n}/></a>).toXMLString()"), "<a xmlns:n=\"u\">\n  <n:b/>\n</a>");
+}
+
+#[test]
+fn a_computed_attribute_name_matches_the_standard_example() {
+    let vars = "var tagname = 'name', attributename = 'id', attributevalue = 5, content = 'Fred';";
+    let literal = "<{tagname} {attributename}={attributevalue}>{content}</{tagname}>";
+    assert_eq!(string(&format!("{vars} ({literal}).toXMLString()")), "<name id=\"5\">Fred</name>");
+    assert_eq!(string(r#"var a = 'id'; (<x {a} = "1" b="2"/>).toXMLString()"#), "<x id=\"1\" b=\"2\"/>");
+}
+
+#[test]
+fn a_computed_attribute_run_cannot_add_markup() {
+    for src in [
+        r#"(<x {'a="1"><y/'}></x>)"#,
+        r#"(<><x {'a="1"/><y b="2"'}/></>)"#,
+        "(<x {'b'}/>)",
+        "(<x {'a=1'}/>)",
+        r#"(<x {'a="1'}/>)"#,
+        r#"(<x {'a="1"b="2"'}/>)"#,
+        r#"(<x {'a="<"'}/>)"#,
+    ] {
+        assert_eq!(thrown_name(src), "SyntaxError", "{src}");
+    }
+}
+
+#[test]
+fn a_computed_attribute_run_may_hold_any_attributes() {
+    truth(
+        r#"var a = " a=\"1\"\n b = '>' "; var x = <x {a}/>; x.@a == '1' && x.@b == '>' && x.attributes().length() === 2 && x.children().length() === 0"#,
+    );
+    assert_eq!(string("(<x {''}/>).toXMLString()"), "<x/>");
+    assert_eq!(string("(<x {' '} c=\"3\"/>).toXMLString()"), "<x c=\"3\"/>");
+}
+
 // ---- selectors and updates ----
 
 #[test]
@@ -953,9 +1007,29 @@ fn tree_markup(shape: &[u8]) -> String {
     out
 }
 
+const TAG_PIECES: [&str; 11] = ["a", "b", " ", "=", "\"", "'", "<", "/", "&lt;", "a=\"1\"", "><y/"];
+
 proptest! {
     #![proptest_config(ProptestConfig::with_cases(256))]
 
+    #[test]
+    fn computed_tag_parts_give_no_child_or_throw(parts in proptest::collection::vec(0..TAG_PIECES.len(), 0..5)) {
+        let text: String = parts.iter().map(|&i| TAG_PIECES[i]).collect();
+        let quoted = format!("{text:?}");
+        for src in [
+            format!("var v = <{{{quoted}}}/>; v.children().length() === 0 && v.attributes().length() === 0"),
+            format!("var v = <x {{{quoted}}}></x>; v.children().length() === 0 && v.name().localName === 'x'"),
+            format!("(<><x {{{quoted}}}/></>).length() === 1"),
+            format!("var v = <a><b></{{{quoted}}}></a>; v.children().length() === 1"),
+        ] {
+            let mut r = limited(Limits { fuel: 200_000, heap_bytes: 4 << 20, frames: 200, reentry: 8 });
+            match r.run(&src) {
+                Ok(v) => prop_assert_eq!(v, JsValue::Bool(true), "{}", src),
+                Err(e) => prop_assert!(e.kind != RunErrorKind::Internal, "{src:?}: {e:?}"),
+            }
+        }
+    }
+
     #[test]
     fn source_mixes_parse_or_give_a_located_error(parts in proptest::collection::vec(0..PIECES.len(), 0..40)) {
         let src: String = parts.iter().map(|&i| PIECES[i]).collect();