From fef8effc1296d9e909bca48db40a20751181a527 Mon Sep 17 00:00:00 2001 From: Rizwan Saleem Date: Fri, 7 Aug 2026 20:11:48 +0100 Subject: [PATCH] fix: attribute selector with no valid attribute name `attribute()` read `next[TOKEN.TYPE]` in four places without checking that a next token existed, so an attribute selector whose last token before `]` was `*`, `$`, `^`, `~` or `|` threw a raw TypeError out of the parser. Two other reads of `next` in the same file already guarded with `next &&`. Guarding those four alone moved the failure rather than fixing it: with no attribute name captured, `Attribute#toString` interpolated the missing value and emitted a selector containing the literal text `undefined`. That path was already reachable without any crash, for example `[ * ]` produced `[ *|undefined]`. So the token loop now also rejects a bracket pair that supplied no attribute name, which matches the error `[*]` already produced. Across 43,200 generated attribute selectors compared against 7.1.5: 341 raw TypeErrors and 596 outputs containing `undefined` are both eliminated, and no input that previously parsed produces different output. Refs #334 Co-Authored-By: Claude Opus 5 --- src/__tests__/attributes.mjs | 19 +++++++++++++++++++ src/__tests__/exceptions.mjs | 31 +++++++++++++++++++++++++++++++ src/parser.js | 16 ++++++++++++---- 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/__tests__/attributes.mjs b/src/__tests__/attributes.mjs index a83d17d..8398b97 100644 --- a/src/__tests__/attributes.mjs +++ b/src/__tests__/attributes.mjs @@ -1,4 +1,6 @@ import { test, nodeVersionAtLeast, nodeVersionBefore } from "./util/helpers.mjs"; +import ava from "./util/runner.mjs"; +import parser from "../../dist/index.js"; test("attribute selector", "[href]", (t, tree) => { t.deepEqual(tree.nodes[0].nodes[0].attribute, "href"); @@ -556,3 +558,20 @@ testDeprecation("set Attribute#quoteMark", "[data-foo=bar]", (t, tree) => { attr.quoteMark = '"'; t.deepEqual(attr.toString(), '[data-foo="has space"]'); }); + +// A trailing `* $ ^ ~ |` immediately before the closing bracket used to look +// ahead past the end of the token stream and throw a raw TypeError. None of +// them is a valid operator without a following `=`, so the token is dropped, +// which matches how `[href=]` already drops an operator that has no value. +// Asserted with `ava` rather than `test` because the output is intentionally +// not a round-trip, and pinning it with `test` would assert the lossy form is +// correct. +for (const trailing of ["*", "$", "^", "~", "|"]) { + ava(`attribute name followed by a trailing ${trailing}`, (t) => { + const tree = parser().astSync(`[href${trailing}]`); + const attr = tree.nodes[0].nodes[0]; + t.deepEqual(attr.attribute, "href"); + t.deepEqual(attr.operator, undefined); + t.deepEqual(tree.toString(), "[href]"); + }); +} diff --git a/src/__tests__/exceptions.mjs b/src/__tests__/exceptions.mjs index 62d0169..358382a 100644 --- a/src/__tests__/exceptions.mjs +++ b/src/__tests__/exceptions.mjs @@ -29,6 +29,37 @@ throws( "Expected a closing parenthesis.", ); +// An attribute selector whose last token before `]` is one of `* $ ^ ~ |`. +// The `attribute` token loop looked ahead to `next` without checking it +// existed, so these threw a raw TypeError. Asserted by message for the same +// reason as the cases above. +throws( + "namespaced attribute name is an asterisk", + "[ns|*]", + "Expected an attribute.", +); +throws("default namespace with asterisk name", "[|*]", "Expected an attribute."); +throws("any namespace with asterisk name", "[*|*]", "Expected an attribute."); +throws( + "namespaced attribute ending in a dollar", + "[ns|$]", + "Expected an attribute.", +); +throws( + "namespaced attribute ending in a caret", + "[ns|^]", + "Expected an attribute.", +); + +// A bracket pair that contains no attribute name at all. `toString` used to +// interpolate the string "undefined" into the output. +throws("universal selector as an attribute", "[ * ]", "Expected an attribute."); +throws( + "asterisk name with trailing space", + "[ns|* ]", + "Expected an attribute.", +); + throws("no opening parenthesis", ")"); throws("no opening parenthesis (2)", ":global.foo)"); throws("no opening parenthesis (3)", "h1:not(h2:not(h3)))"); diff --git a/src/parser.js b/src/parser.js index bcec5ac..78a45a1 100644 --- a/src/parser.js +++ b/src/parser.js @@ -219,7 +219,7 @@ export default class Parser { } break; case tokens.asterisk: - if (next[TOKEN.TYPE] === tokens.equals) { + if (next && next[TOKEN.TYPE] === tokens.equals) { node.operator = content; lastAdded = "operator"; } else if ( @@ -256,14 +256,14 @@ export default class Parser { } // Falls through case tokens.caret: - if (next[TOKEN.TYPE] === tokens.equals) { + if (next && next[TOKEN.TYPE] === tokens.equals) { node.operator = content; lastAdded = "operator"; } spaceAfterMeaningfulToken = false; break; case tokens.combinator: - if (content === "~" && next[TOKEN.TYPE] === tokens.equals) { + if (content === "~" && next && next[TOKEN.TYPE] === tokens.equals) { node.operator = content; lastAdded = "operator"; } @@ -271,7 +271,7 @@ export default class Parser { spaceAfterMeaningfulToken = false; break; } - if (next[TOKEN.TYPE] === tokens.equals) { + if (next && next[TOKEN.TYPE] === tokens.equals) { node.operator = content; lastAdded = "operator"; } else if (!node.namespace && !node.attribute) { @@ -415,6 +415,14 @@ export default class Parser { } pos++; } + if (!node.attribute) { + // Every token inside the brackets was consumed without one of them + // supplying an attribute name. Stringifying now would interpolate the + // string "undefined" into the output, so report it as a parse error + // instead. Points at the opening bracket, which is where the author + // needs to look. + return this.expected("attribute", startingToken[TOKEN.START_POS]); + } unescapeProp(node, "attribute"); unescapeProp(node, "namespace"); this.newNode(new Attribute(node));