Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect comment handling in some edge-case #175

Open
holblin opened this issue Aug 15, 2023 · 0 comments
Open

Incorrect comment handling in some edge-case #175

holblin opened this issue Aug 15, 2023 · 0 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@holblin
Copy link

holblin commented Aug 15, 2023

Expected Behaviour

Comments are properly ignored in all places.

Actual Behaviour

Comment in the selector that contains a { break the parsing because the { evaluation is done before the comment escaping.

Steps to Reproduce

  • Change the file test/cases/comment/input.css by:
/* 1 */

head, /* footer, */body/*, nav */ { /* 2 */
  /* 3 */
  /**/foo: 'bar';
  /* 4 */
} /* 5 */

/* 6 */
  • Run yarn test

Logs taken while reproducing the problem

$jest
 PASS  test/parse.test.ts
 FAIL  test/cases.test.ts
  ● cases/comment › should match ast.json

    input.css:3:37: property missing ':'

      77 |
      78 |   function error(msg: string) {
    > 79 |     const err = new CssParseError(
         |                 ^
      80 |       options?.source || '',
      81 |       msg,
      82 |       lineno,

      at error (src/parse/index.ts:79:17)
      at declaration (src/parse/index.ts:263:14)
      at declarations (src/parse/index.ts:294:20)
      at rule (src/parse/index.ts:716:21)
      at rules (src/parse/index.ts:134:71)
      at stylesheet (src/parse/index.ts:98:23)
      at parse (src/parse/index.ts:720:20)
      at parseInput (test/cases.test.ts:39:19)
      at Object.<anonymous> (test/cases.test.ts:15:19)

Potential fix

  • Add a special condition in the regex for the selector to skip the comments. This should not reduce performance or introduce a dos vulnerability.
  • Change the parsing to skip the comment while doing the parsing.
@holblin holblin added bug Something isn't working good first issue Good for newcomers labels Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant