script: Don't try to evaluate xpath variable references (#39395)

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 <simon.wuelker@arcor.de>
This commit is contained in:
Simon Wülker 2025-09-20 14:36:54 +02:00 committed by GitHub
parent e2241a93fe
commit bdf630563c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 27 additions and 5 deletions

View file

@ -33,9 +33,12 @@ pub(crate) enum Error {
UnknownFunction { UnknownFunction {
name: QualName, name: QualName,
}, },
UnknownVariable { /// It is not clear where variables used in XPath expression should come from.
name: QualName, /// Firefox throws "NS_ERROR_ILLEGAL_VALUE" when using them, chrome seems to return
}, /// an empty result. We also error out.
///
/// See <https://github.com/whatwg/dom/issues/67>
CannotUseVariables,
UnknownNamespace { UnknownNamespace {
prefix: String, prefix: String,
}, },
@ -58,7 +61,7 @@ impl std::fmt::Display for Error {
Error::NotANodeset => write!(f, "expression did not evaluate to a nodeset"), Error::NotANodeset => write!(f, "expression did not evaluate to a nodeset"),
Error::InvalidPath => write!(f, "invalid path expression"), Error::InvalidPath => write!(f, "invalid path expression"),
Error::UnknownFunction { name } => write!(f, "unknown function {:?}", name), 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 } => { Error::UnknownNamespace { prefix } => {
write!(f, "unknown namespace prefix {:?}", prefix) write!(f, "unknown namespace prefix {:?}", prefix)
}, },
@ -745,7 +748,7 @@ impl Evaluatable for PrimaryExpr {
fn evaluate(&self, context: &EvaluationCtx) -> Result<Value, Error> { fn evaluate(&self, context: &EvaluationCtx) -> Result<Value, Error> {
match self { match self {
PrimaryExpr::Literal(literal) => literal.evaluate(context), PrimaryExpr::Literal(literal) => literal.evaluate(context),
PrimaryExpr::Variable(_qname) => todo!(), PrimaryExpr::Variable(_qname) => Err(Error::CannotUseVariables),
PrimaryExpr::Parenthesized(expr) => expr.evaluate(context), PrimaryExpr::Parenthesized(expr) => expr.evaluate(context),
PrimaryExpr::ContextItem => Ok(Value::Nodeset(vec![context.context_node.clone()])), PrimaryExpr::ContextItem => Ok(Value::Nodeset(vec![context.context_node.clone()])),
PrimaryExpr::Function(core_function) => core_function.evaluate(context), PrimaryExpr::Function(core_function) => core_function.evaluate(context),

View file

@ -7036,6 +7036,13 @@
] ]
}, },
"domxpath": { "domxpath": {
"variables-in-expression-crash.html": [
"d1f15a6abe0d9c8ecae08b4cdf5dd9ef3efbbbcc",
[
null,
{}
]
],
"xpath-evaluate-crash.html": [ "xpath-evaluate-crash.html": [
"5303d85ad31ca0ee230ac36231898342b07ad2c3", "5303d85ad31ca0ee230ac36231898342b07ad2c3",
[ [

View file

@ -0,0 +1,12 @@
<!DOCTYPE html>
<head>
<link rel="help" href="https://github.com/servo/servo/issues/36971">
<meta name="assert" content="Using variables in xpath expression should not crash.">
</head>
<body>
<script>
// The exact behaviour here is not defined. Firefox throws an error, Chrome doesn't.
document.evaluate("$foo", document.createElement("div"));
</script>
</body>