Skip to content

Commit

Permalink
fix #2276: avoid parser hang on invalid css
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed May 29, 2022
1 parent 9f5b78a commit 538c3d5
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 38 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

* Fix a parser hang on invalid CSS ([#2276](https://github.com/evanw/esbuild/issues/2276))

Previously invalid CSS with unbalanced parentheses could cause esbuild's CSS parser to hang. An example of such an input is the CSS file `:x(`. This hang has been fixed.

## 0.14.41

* Fix a minification regression in 0.14.40 ([#2270](https://github.com/evanw/esbuild/issues/2270), [#2271](https://github.com/evanw/esbuild/issues/2271), [#2273](https://github.com/evanw/esbuild/pull/2273))
Expand Down
61 changes: 40 additions & 21 deletions internal/css_parser/css_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ func (p *parser) eat(kind css_lexer.T) bool {
}

func (p *parser) expect(kind css_lexer.T) bool {
return p.expectWithMatchingLoc(kind, logger.Loc{Start: -1})
}

func (p *parser) expectWithMatchingLoc(kind css_lexer.T, matchingLoc logger.Loc) bool {
if p.eat(kind) {
return true
}
Expand All @@ -123,6 +127,7 @@ func (p *parser) expect(kind css_lexer.T) bool {

var text string
var suggestion string
var notes []logger.MsgData

expected := kind.String()
if strings.HasPrefix(expected, "\"") && strings.HasSuffix(expected, "\"") {
Expand All @@ -133,6 +138,11 @@ func (p *parser) expect(kind css_lexer.T) bool {
// Have a nice error message for forgetting a trailing semicolon or colon
text = fmt.Sprintf("Expected %s", expected)
t = p.at(p.index - 1)
} else if (kind == css_lexer.TCloseBrace || kind == css_lexer.TCloseBracket || kind == css_lexer.TCloseParen) && matchingLoc.Start != -1 {
// Have a nice error message for forgetting a closing brace/bracket/parenthesis
c := p.source.Contents[matchingLoc.Start : matchingLoc.Start+1]
text = fmt.Sprintf("Expected %s to go with %q", expected, c)
notes = append(notes, p.tracker.MsgData(logger.Range{Loc: matchingLoc, Len: 1}, fmt.Sprintf("The unbalanced %q is here:", c)))
} else {
switch t.Kind {
case css_lexer.TEndOfFile, css_lexer.TWhitespace:
Expand All @@ -148,7 +158,7 @@ func (p *parser) expect(kind css_lexer.T) bool {
if t.Range.Loc.Start > p.prevError.Start {
data := p.tracker.MsgData(t.Range, text)
data.Location.Suggestion = suggestion
p.log.AddMsg(logger.Msg{Kind: logger.Warning, Data: data})
p.log.AddMsg(logger.Msg{Kind: logger.Warning, Data: data, Notes: notes})
p.prevError = t.Range.Loc
}
return false
Expand Down Expand Up @@ -594,10 +604,11 @@ func (p *parser) parseURLOrString() (string, logger.Range, bool) {

case css_lexer.TFunction:
if p.decoded() == "url" {
matchingLoc := logger.Loc{Start: p.current().Range.End() - 1}
p.advance()
t = p.current()
text := p.decoded()
if p.expect(css_lexer.TString) && p.expect(css_lexer.TCloseParen) {
if p.expect(css_lexer.TString) && p.expectWithMatchingLoc(css_lexer.TCloseParen, matchingLoc) {
return text, t.Range, true
}
}
Expand Down Expand Up @@ -829,6 +840,7 @@ abortRuleParser:
p.eat(css_lexer.TWhitespace)
blockStart := p.index

matchingLoc := p.current().Range.Loc
if p.expect(css_lexer.TOpenBrace) {
var blocks []css_ast.KeyframeBlock

Expand Down Expand Up @@ -866,7 +878,18 @@ abortRuleParser:
continue

case css_lexer.TOpenBrace:
blockMatchingLoc := p.current().Range.Loc
p.advance()
rules := p.parseListOfDeclarations()
p.expectWithMatchingLoc(css_lexer.TCloseBrace, blockMatchingLoc)

// "@keyframes { from {} to { color: red } }" => "@keyframes { to { color: red } }"
if !p.options.MinifySyntax || len(rules) > 0 {
blocks = append(blocks, css_ast.KeyframeBlock{
Selectors: selectors,
Rules: rules,
})
}
break selectors

case css_lexer.TCloseBrace, css_lexer.TEndOfFile:
Expand Down Expand Up @@ -907,25 +930,14 @@ abortRuleParser:
break badSyntax
}
}

rules := p.parseListOfDeclarations()
p.expect(css_lexer.TCloseBrace)

// "@keyframes { from {} to { color: red } }" => "@keyframes { to { color: red } }"
if !p.options.MinifySyntax || len(rules) > 0 {
blocks = append(blocks, css_ast.KeyframeBlock{
Selectors: selectors,
Rules: rules,
})
}
}
}

// Otherwise, finish parsing the body and return an unknown rule
for !p.peek(css_lexer.TCloseBrace) && !p.peek(css_lexer.TEndOfFile) {
p.parseComponentValue()
}
p.expect(css_lexer.TCloseBrace)
p.expectWithMatchingLoc(css_lexer.TCloseBrace, matchingLoc)
prelude := p.convertTokens(p.tokens[preludeStart:blockStart])
block, _ := p.convertTokensHelper(p.tokens[blockStart:p.index], css_lexer.TEndOfFile, convertTokensOpts{allowImports: true})
return css_ast.Rule{Loc: atRange.Loc, Data: &css_ast.RUnknownAt{AtToken: atToken, Prelude: prelude, Block: block}}
Expand Down Expand Up @@ -978,11 +990,12 @@ abortRuleParser:
}

// Read the optional block
matchingLoc := p.current().Range.Loc
if len(names) <= 1 && p.eat(css_lexer.TOpenBrace) {
rules := p.parseListOfRules(ruleContext{
parseSelectors: true,
})
p.expect(css_lexer.TCloseBrace)
p.expectWithMatchingLoc(css_lexer.TCloseBrace, matchingLoc)
return css_ast.Rule{Loc: atRange.Loc, Data: &css_ast.RAtLayer{Names: names, Rules: rules}}
}

Expand Down Expand Up @@ -1064,13 +1077,15 @@ prelude:

case atRuleDeclarations:
// Parse known rules whose blocks always consist of declarations
matchingLoc := p.current().Range.Loc
p.expect(css_lexer.TOpenBrace)
rules := p.parseListOfDeclarations()
p.expect(css_lexer.TCloseBrace)
p.expectWithMatchingLoc(css_lexer.TCloseBrace, matchingLoc)
return css_ast.Rule{Loc: atRange.Loc, Data: &css_ast.RKnownAt{AtToken: atToken, Prelude: prelude, Rules: rules}}

case atRuleInheritContext:
// Parse known rules whose blocks consist of whatever the current context is
matchingLoc := p.current().Range.Loc
p.expect(css_lexer.TOpenBrace)
var rules []css_ast.Rule
if context.isDeclarationList {
Expand All @@ -1080,15 +1095,16 @@ prelude:
parseSelectors: true,
})
}
p.expect(css_lexer.TCloseBrace)
p.expectWithMatchingLoc(css_lexer.TCloseBrace, matchingLoc)
return css_ast.Rule{Loc: atRange.Loc, Data: &css_ast.RKnownAt{AtToken: atToken, Prelude: prelude, Rules: rules}}

case atRuleQualifiedOrEmpty:
matchingLoc := p.current().Range.Loc
if p.eat(css_lexer.TOpenBrace) {
rules := p.parseListOfRules(ruleContext{
parseSelectors: true,
})
p.expect(css_lexer.TCloseBrace)
p.expectWithMatchingLoc(css_lexer.TCloseBrace, matchingLoc)
return css_ast.Rule{Loc: atRange.Loc, Data: &css_ast.RKnownAt{AtToken: atToken, Prelude: prelude, Rules: rules}}
}
p.expect(css_lexer.TSemicolon)
Expand Down Expand Up @@ -1471,9 +1487,10 @@ func (p *parser) parseSelectorRuleFrom(preludeStart int, opts parseSelectorOpts)
Selectors: list,
HasAtNest: opts.atNestRange.Len != 0,
}
matchingLoc := p.current().Range.Loc
if p.expect(css_lexer.TOpenBrace) {
selector.Rules = p.parseListOfDeclarations()
p.expect(css_lexer.TCloseBrace)
p.expectWithMatchingLoc(css_lexer.TCloseBrace, matchingLoc)

// Minify "@nest" when possible
if p.options.MinifySyntax && selector.HasAtNest {
Expand Down Expand Up @@ -1529,9 +1546,10 @@ loop:
Prelude: p.convertTokens(p.tokens[preludeStart:p.index]),
}

matchingLoc := p.current().Range.Loc
if p.eat(css_lexer.TOpenBrace) {
qualified.Rules = p.parseListOfDeclarations()
p.expect(css_lexer.TCloseBrace)
p.expectWithMatchingLoc(css_lexer.TCloseBrace, matchingLoc)
} else if !opts.isAlreadyInvalid {
p.expect(css_lexer.TOpenBrace)
}
Expand Down Expand Up @@ -1653,10 +1671,11 @@ func (p *parser) parseComponentValue() {
}

func (p *parser) parseBlock(open css_lexer.T, close css_lexer.T) {
matchingLoc := p.current().Range.Loc
if p.expect(open) {
for !p.eat(close) {
if p.peek(css_lexer.TEndOfFile) {
p.expect(close)
p.expectWithMatchingLoc(close, matchingLoc)
return
}

Expand Down
12 changes: 9 additions & 3 deletions internal/css_parser/css_parser_selector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ subclassSelectors:
p.expect(css_lexer.TIdent)

case css_lexer.TOpenBracket:
p.advance()
attr, good := p.parseAttributeSelector()
if !good {
return
Expand Down Expand Up @@ -231,6 +230,9 @@ subclassSelectors:
}

func (p *parser) parseAttributeSelector() (attr css_ast.SSAttribute, ok bool) {
matchingLoc := p.current().Range.Loc
p.advance()

// Parse the namespaced name
switch p.current().Kind {
case css_lexer.TDelimBar, css_lexer.TDelimAsterisk:
Expand Down Expand Up @@ -314,7 +316,7 @@ func (p *parser) parseAttributeSelector() (attr css_ast.SSAttribute, ok bool) {
}
}

p.expect(css_lexer.TCloseBracket)
p.expectWithMatchingLoc(css_lexer.TCloseBracket, matchingLoc)
ok = true
return
}
Expand All @@ -324,9 +326,10 @@ func (p *parser) parsePseudoClassSelector() css_ast.SSPseudoClass {

if p.peek(css_lexer.TFunction) {
text := p.decoded()
matchingLoc := logger.Loc{Start: p.current().Range.End() - 1}
p.advance()
args := p.convertTokens(p.parseAnyValue())
p.expect(css_lexer.TCloseParen)
p.expectWithMatchingLoc(css_lexer.TCloseParen, matchingLoc)
return css_ast.SSPseudoClass{Name: text, Args: args}
}

Expand Down Expand Up @@ -367,6 +370,9 @@ loop:

case css_lexer.TOpenBrace:
p.stack = append(p.stack, css_lexer.TCloseBrace)

case css_lexer.TEndOfFile:
break loop
}

p.advance()
Expand Down
37 changes: 23 additions & 14 deletions internal/css_parser/css_parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,22 +273,26 @@ func TestString(t *testing.T) {
expectParseError(t, "a:after { content: '\r' }",
`<stdin>: ERROR: Unterminated string token
<stdin>: ERROR: Unterminated string token
<stdin>: WARNING: Expected "}" but found end of file
<stdin>: WARNING: Expected "}" to go with "{"
<stdin>: NOTE: The unbalanced "{" is here:
`)
expectParseError(t, "a:after { content: '\n' }",
`<stdin>: ERROR: Unterminated string token
<stdin>: ERROR: Unterminated string token
<stdin>: WARNING: Expected "}" but found end of file
<stdin>: WARNING: Expected "}" to go with "{"
<stdin>: NOTE: The unbalanced "{" is here:
`)
expectParseError(t, "a:after { content: '\f' }",
`<stdin>: ERROR: Unterminated string token
<stdin>: ERROR: Unterminated string token
<stdin>: WARNING: Expected "}" but found end of file
<stdin>: WARNING: Expected "}" to go with "{"
<stdin>: NOTE: The unbalanced "{" is here:
`)
expectParseError(t, "a:after { content: '\r\n' }",
`<stdin>: ERROR: Unterminated string token
<stdin>: ERROR: Unterminated string token
<stdin>: WARNING: Expected "}" but found end of file
<stdin>: WARNING: Expected "}" to go with "{"
<stdin>: NOTE: The unbalanced "{" is here:
`)

expectPrinted(t, "a:after { content: '\\1010101' }", "a:after {\n content: \"\U001010101\";\n}\n")
Expand Down Expand Up @@ -596,9 +600,9 @@ func TestSelector(t *testing.T) {
expectPrinted(t, "a[b] {}", "a[b] {\n}\n")
expectPrinted(t, "a [b] {}", "a [b] {\n}\n")
expectParseError(t, "[] {}", "<stdin>: WARNING: Expected identifier but found \"]\"\n")
expectParseError(t, "[b {}", "<stdin>: WARNING: Expected \"]\" but found \"{\"\n")
expectParseError(t, "[b {}", "<stdin>: WARNING: Expected \"]\" to go with \"[\"\n<stdin>: NOTE: The unbalanced \"[\" is here:\n")
expectParseError(t, "[b]] {}", "<stdin>: WARNING: Unexpected \"]\"\n")
expectParseError(t, "a[b {}", "<stdin>: WARNING: Expected \"]\" but found \"{\"\n")
expectParseError(t, "a[b {}", "<stdin>: WARNING: Expected \"]\" to go with \"[\"\n<stdin>: NOTE: The unbalanced \"[\" is here:\n")
expectParseError(t, "a[b]] {}", "<stdin>: WARNING: Unexpected \"]\"\n")

expectPrinted(t, "[|b]{}", "[b] {\n}\n") // "[|b]" is equivalent to "[b]"
Expand All @@ -618,24 +622,24 @@ func TestSelector(t *testing.T) {
expectPrinted(t, "[b$=\"c\"] {}", "[b$=c] {\n}\n")
expectPrinted(t, "[b*=\"c\"] {}", "[b*=c] {\n}\n")
expectPrinted(t, "[b|=\"c\"] {}", "[b|=c] {\n}\n")
expectParseError(t, "[b?=\"c\"] {}", "<stdin>: WARNING: Expected \"]\" but found \"?\"\n")
expectParseError(t, "[b?=\"c\"] {}", "<stdin>: WARNING: Expected \"]\" to go with \"[\"\n<stdin>: NOTE: The unbalanced \"[\" is here:\n")

expectPrinted(t, "[b = \"c\"] {}", "[b=c] {\n}\n")
expectPrinted(t, "[b ~= \"c\"] {}", "[b~=c] {\n}\n")
expectPrinted(t, "[b ^= \"c\"] {}", "[b^=c] {\n}\n")
expectPrinted(t, "[b $= \"c\"] {}", "[b$=c] {\n}\n")
expectPrinted(t, "[b *= \"c\"] {}", "[b*=c] {\n}\n")
expectPrinted(t, "[b |= \"c\"] {}", "[b|=c] {\n}\n")
expectParseError(t, "[b ?= \"c\"] {}", "<stdin>: WARNING: Expected \"]\" but found \"?\"\n")
expectParseError(t, "[b ?= \"c\"] {}", "<stdin>: WARNING: Expected \"]\" to go with \"[\"\n<stdin>: NOTE: The unbalanced \"[\" is here:\n")

expectPrinted(t, "[b = \"c\" i] {}", "[b=c i] {\n}\n")
expectPrinted(t, "[b = \"c\" I] {}", "[b=c I] {\n}\n")
expectPrinted(t, "[b = \"c\" s] {}", "[b=c s] {\n}\n")
expectPrinted(t, "[b = \"c\" S] {}", "[b=c S] {\n}\n")
expectParseError(t, "[b i] {}", "<stdin>: WARNING: Expected \"]\" but found \"i\"\n<stdin>: WARNING: Unexpected \"]\"\n")
expectParseError(t, "[b I] {}", "<stdin>: WARNING: Expected \"]\" but found \"I\"\n<stdin>: WARNING: Unexpected \"]\"\n")
expectParseError(t, "[b s] {}", "<stdin>: WARNING: Expected \"]\" but found \"s\"\n<stdin>: WARNING: Unexpected \"]\"\n")
expectParseError(t, "[b S] {}", "<stdin>: WARNING: Expected \"]\" but found \"S\"\n<stdin>: WARNING: Unexpected \"]\"\n")
expectParseError(t, "[b i] {}", "<stdin>: WARNING: Expected \"]\" to go with \"[\"\n<stdin>: NOTE: The unbalanced \"[\" is here:\n<stdin>: WARNING: Unexpected \"]\"\n")
expectParseError(t, "[b I] {}", "<stdin>: WARNING: Expected \"]\" to go with \"[\"\n<stdin>: NOTE: The unbalanced \"[\" is here:\n<stdin>: WARNING: Unexpected \"]\"\n")
expectParseError(t, "[b s] {}", "<stdin>: WARNING: Expected \"]\" to go with \"[\"\n<stdin>: NOTE: The unbalanced \"[\" is here:\n<stdin>: WARNING: Unexpected \"]\"\n")
expectParseError(t, "[b S] {}", "<stdin>: WARNING: Expected \"]\" to go with \"[\"\n<stdin>: NOTE: The unbalanced \"[\" is here:\n<stdin>: WARNING: Unexpected \"]\"\n")

expectPrinted(t, "|b {}", "|b {\n}\n")
expectPrinted(t, "|* {}", "|* {\n}\n")
Expand Down Expand Up @@ -665,6 +669,11 @@ func TestSelector(t *testing.T) {
expectPrinted(t, "a:b(:c) {}", "a:b(:c) {\n}\n")
expectPrinted(t, "a: b {}", "a: b {\n}\n")

// These test cases previously caused a hang (see https://github.com/evanw/esbuild/issues/2276)
expectParseError(t, ":x(", "<stdin>: WARNING: Unexpected end of file\n")
expectParseError(t, ":x( {}", "<stdin>: WARNING: Expected \")\" to go with \"(\"\n<stdin>: NOTE: The unbalanced \"(\" is here:\n")
expectParseError(t, ":x(, :y() {}", "<stdin>: WARNING: Expected \")\" to go with \"(\"\n<stdin>: NOTE: The unbalanced \"(\" is here:\n")

expectPrinted(t, "#id {}", "#id {\n}\n")
expectPrinted(t, "#--0 {}", "#--0 {\n}\n")
expectPrinted(t, "#\\-0 {}", "#\\-0 {\n}\n")
Expand Down Expand Up @@ -964,7 +973,7 @@ func TestAtImport(t *testing.T) {
expectParseError(t, "@import;", "<stdin>: WARNING: Expected URL token but found \";\"\n")
expectParseError(t, "@import ;", "<stdin>: WARNING: Expected URL token but found \";\"\n")
expectParseError(t, "@import \"foo.css\"", "<stdin>: WARNING: Expected \";\" but found end of file\n")
expectParseError(t, "@import url(\"foo.css\";", "<stdin>: WARNING: Expected \")\" but found \";\"\n")
expectParseError(t, "@import url(\"foo.css\";", "<stdin>: WARNING: Expected \")\" to go with \"(\"\n<stdin>: NOTE: The unbalanced \"(\" is here:\n")
expectParseError(t, "@import noturl(\"foo.css\");", "<stdin>: WARNING: Expected URL token but found \"noturl(\"\n")
expectParseError(t, "@import url(", `<stdin>: WARNING: Expected URL token but found bad URL token
<stdin>: ERROR: Expected ")" to end URL token
Expand Down Expand Up @@ -1041,7 +1050,7 @@ func TestAtKeyframes(t *testing.T) {
expectParseError(t, "@keyframes name { 1% }", "<stdin>: WARNING: Expected \"{\" but found \"}\"\n")
expectParseError(t, "@keyframes name { 1%", "<stdin>: WARNING: Expected \"{\" but found end of file\n")
expectParseError(t, "@keyframes name { 1%,,2% {} }", "<stdin>: WARNING: Expected percentage but found \",\"\n")
expectParseError(t, "@keyframes name {", "<stdin>: WARNING: Expected \"}\" but found end of file\n")
expectParseError(t, "@keyframes name {", "<stdin>: WARNING: Expected \"}\" to go with \"{\"\n<stdin>: NOTE: The unbalanced \"{\" is here:\n")

expectPrinted(t, "@keyframes x { 1%, {} } @keyframes z { 1% {} }", "@keyframes x { 1%, {} }\n@keyframes z {\n 1% {\n }\n}\n")
expectPrinted(t, "@keyframes x { .y {} } @keyframes z { 1% {} }", "@keyframes x { .y {} }\n@keyframes z {\n 1% {\n }\n}\n")
Expand Down

0 comments on commit 538c3d5

Please sign in to comment.