From bdf630563c177bc3ba56898162e18be0b19e1c6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20W=C3=BClker?= Date: Sat, 20 Sep 2025 14:36:54 +0200 Subject: [PATCH] script: Don't try to evaluate xpath variable references (#39395) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Variables in xpath via the javascript bindings are a bit mysterious, as there is no way that a variable can be specified. We currently panic when encountering a variable, which is not good. Instead we now throw an error. We keep parsing the variables because the code is already there and it seems realistic that their behaviour will be specified in the future. I'm fine with removing them too if that is preferred. Testing: This behaviour is unspecified and different browser produce different results. There is no "correct" way to do this, but we should not crash Part of: https://github.com/servo/servo/issues/34527 --------- Signed-off-by: Simon Wülker --- components/script/xpath/eval.rs | 13 ++++++++----- tests/wpt/meta/MANIFEST.json | 7 +++++++ .../domxpath/variables-in-expression-crash.html | 12 ++++++++++++ 3 files changed, 27 insertions(+), 5 deletions(-) create mode 100644 tests/wpt/tests/domxpath/variables-in-expression-crash.html diff --git a/components/script/xpath/eval.rs b/components/script/xpath/eval.rs index 9291602b70d..57a35e96cff 100644 --- a/components/script/xpath/eval.rs +++ b/components/script/xpath/eval.rs @@ -33,9 +33,12 @@ pub(crate) enum Error { UnknownFunction { name: QualName, }, - UnknownVariable { - name: QualName, - }, + /// It is not clear where variables used in XPath expression should come from. + /// Firefox throws "NS_ERROR_ILLEGAL_VALUE" when using them, chrome seems to return + /// an empty result. We also error out. + /// + /// See + CannotUseVariables, UnknownNamespace { prefix: String, }, @@ -58,7 +61,7 @@ impl std::fmt::Display for Error { Error::NotANodeset => write!(f, "expression did not evaluate to a nodeset"), Error::InvalidPath => write!(f, "invalid path expression"), Error::UnknownFunction { name } => write!(f, "unknown function {:?}", name), - Error::UnknownVariable { name } => write!(f, "unknown variable {:?}", name), + Error::CannotUseVariables => write!(f, "cannot use variables"), Error::UnknownNamespace { prefix } => { write!(f, "unknown namespace prefix {:?}", prefix) }, @@ -745,7 +748,7 @@ impl Evaluatable for PrimaryExpr { fn evaluate(&self, context: &EvaluationCtx) -> Result { match self { PrimaryExpr::Literal(literal) => literal.evaluate(context), - PrimaryExpr::Variable(_qname) => todo!(), + PrimaryExpr::Variable(_qname) => Err(Error::CannotUseVariables), PrimaryExpr::Parenthesized(expr) => expr.evaluate(context), PrimaryExpr::ContextItem => Ok(Value::Nodeset(vec![context.context_node.clone()])), PrimaryExpr::Function(core_function) => core_function.evaluate(context), diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index 7903ca7500f..bd748da4133 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -7036,6 +7036,13 @@ ] }, "domxpath": { + "variables-in-expression-crash.html": [ + "d1f15a6abe0d9c8ecae08b4cdf5dd9ef3efbbbcc", + [ + null, + {} + ] + ], "xpath-evaluate-crash.html": [ "5303d85ad31ca0ee230ac36231898342b07ad2c3", [ diff --git a/tests/wpt/tests/domxpath/variables-in-expression-crash.html b/tests/wpt/tests/domxpath/variables-in-expression-crash.html new file mode 100644 index 00000000000..d1f15a6abe0 --- /dev/null +++ b/tests/wpt/tests/domxpath/variables-in-expression-crash.html @@ -0,0 +1,12 @@ + + + + + + + + +