Skip to content

Attribute selector with no valid attribute name throws a raw TypeError, or emits the text "undefined" #334

Description

@theRizwan

Parser#attribute looks ahead to the next token without checking that one exists, and never verifies that an attribute name was actually captured. Two symptoms follow.

1. Raw TypeError when the last token before ] is *, $, ^, ~ or |

const parser = require("postcss-selector-parser");

parser().astSync("[ns|*]");
// TypeError: Cannot read properties of undefined (reading '0')
//     at Parser.attribute (dist/parser.js:255:29)

Same crash for [a*], [a$], [a^], [a~], [a|], [|*], [*|*], [a|*], [ns|$], [ns|^].

These inputs are invalid CSS, so an error is correct, but it should be the parser's own error rather than a TypeError escaping from internals. This is the same class of defect as #329, at a different site: there the token stream ran out before a closing bracket, here it runs out before the lookahead in the token loop.

The four unguarded reads are in src/parser.js inside attribute():

case tokens.asterisk:
  if (next[TOKEN.TYPE] === tokens.equals) {        // next may be undefined
case tokens.caret:                                  // reached by dollar via fall-through
  if (next[TOKEN.TYPE] === tokens.equals) {        // next may be undefined
case tokens.combinator:
  if (content === "~" && next[TOKEN.TYPE] === tokens.equals) {   // next may be undefined
  ...
  if (next[TOKEN.TYPE] === tokens.equals) {        // next may be undefined

Two other reads of next[TOKEN.TYPE] in the same file already guard with next &&, so the pattern is established. The else if immediately below the asterisk case also guards with && next, which suggests the possibility was known at the time.

2. The literal string undefined in the output when no attribute name is captured

parser().astSync("[ * ]").toString();
// "[ *|undefined]"        <- an attribute name of "undefined", and a "|" that was never written

parser().astSync("[  *  ]").toString();
// "[  *|undefined]"

attribute() ends with this.newNode(new Attribute(node)) with no check that node.attribute was ever set. When it was not, Attribute#toString interpolates the missing value, so a selector containing the text undefined is emitted. [*] on its own already throws Expected an attribute., so the two are inconsistent.

Guarding the four lookaheads alone converts most of the crashes in the first section into this second failure instead, so the two need addressing together.

Scale

Comparing 43,200 generated attribute selectors against 7.1.5:

  • 341 produce a raw TypeError
  • 596 produce output containing the literal text undefined

Expected

An attribute selector with no valid attribute name should produce the parser's own error, consistent with [*], rather than a TypeError or a stringified undefined.

Version

Reproduced on 7.1.5 (current latest) and on main at e33e9bc.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions