From 01673695afb3e9d4e9cc3d1999548168f4617e38 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Mon, 9 Nov 2026 07:47:29 -0800 Subject: [PATCH] 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 | 6 ++- crates/js/src/bytecode.rs | 2 +- crates/js/src/compiler.rs | 5 ++- crates/js/src/parser/xml.rs | 38 ++++++++++--------- crates/js/src/xml.rs | 74 ++++++++++++++++++++++++++++++------- crates/js/src/xml_tests.rs | 74 +++++++++++++++++++++++++++++++++++++ 6 files changed, 164 insertions(+), 35 deletions(-) diff --git a/crates/js/src/ast.rs b/crates/js/src/ast.rs index ca991645b..5a4363108 100644 --- a/crates/js/src/ast.rs +++ b/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. diff --git a/crates/js/src/bytecode.rs b/crates/js/src/bytecode.rs index 895d91171..ed99aa95d 100644 --- a/crates/js/src/bytecode.rs +++ b/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 { diff --git a/crates/js/src/compiler.rs b/crates/js/src/compiler.rs index f0f171d59..9c07113cc 100644 --- a/crates/js/src/compiler.rs +++ b/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, })); } } diff --git a/crates/js/src/parser/xml.rs b/crates/js/src/parser/xml.rs index 0b29bf879..e91691a58 100644 --- a/crates/js/src/parser/xml.rs +++ b/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 { - 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 { 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 { + 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 { diff --git a/crates/js/src/xml.rs b/crates/js/src/xml.rs index 6cfe962a1..5a02ccdc1 100644 --- a/crates/js/src/xml.rs +++ b/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 { @@ -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 { + 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("e) { + 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 { - 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)) } diff --git a/crates/js/src/xml_tests.rs b/crates/js/src/xml_tests.rs index 86b813d25..58c984ad4 100644 --- a/crates/js/src/xml_tests.rs +++ b/crates/js/src/xml_tests.rs @@ -120,6 +120,60 @@ fn embedded_expressions_are_escaped_by_position() { truth("({1 < 2}) == 'true'"); } +#[test] +fn a_computed_tag_name_cannot_add_markup() { + for src in [ + r#"(<{'a x="1"'}/>)"#, + "(<{'a>)", + "(t)", + "(<{''}/>)", + "(<{'1a'}/>)", + "()", + r#"()"#, + ] { + assert_eq!(thrown_name(src), "SyntaxError", "{src}"); + } +} + +#[test] +fn a_computed_name_may_be_any_xml_name() { + assert_eq!(string("(<{'a.b-c'}/>).toXMLString()"), ""); + assert_eq!(string("(<{'tag'}>t).toXMLString()"), "t"); + assert_eq!(string("var n = 'n:b'; (<{n}/>).toXMLString()"), "\n \n"); +} + +#[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}"; + assert_eq!(string(&format!("{vars} ({literal}).toXMLString()")), "Fred"); + assert_eq!(string(r#"var a = 'id'; ().toXMLString()"#), ""); +} + +#[test] +fn a_computed_attribute_run_cannot_add_markup() { + for src in [ + r#"()"#, + r#"(<>)"#, + "()", + "()", + r#"()"#, + r#"()"#, + ] { + 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 == '1' && x.@b == '>' && x.attributes().length() === 2 && x.children().length() === 0"#, + ); + assert_eq!(string("().toXMLString()"), ""); + assert_eq!(string("().toXMLString()"), ""); +} + // ---- selectors and updates ---- #[test] @@ -953,9 +1007,29 @@ fn tree_markup(shape: &[u8]) -> String { out } +const TAG_PIECES: [&str; 11] = ["a", "b", " ", "=", "\"", "'", "<", "/", "<", "a=\"1\"", ">; v.children().length() === 0 && v.attributes().length() === 0"), + format!("var v = ; v.children().length() === 0 && v.name().localName === 'x'"), + format!("(<>).length() === 1"), + format!("var v = ; 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();