Skip to content

Commit

Permalink
fix #1803: panic with invalid "@import" conditions
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Nov 26, 2021
1 parent 366dacd commit 8569cdf
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 7 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Expand Up @@ -146,6 +146,14 @@ In addition to the breaking changes above, the following changes are also includ
.example{--some-var: var(--tw-empty, )}/*! Some legal comment */body{background-color:red}
```

* Fix panic when printing invalid CSS ([#1803](https://github.com/evanw/esbuild/issues/1803))

This release fixes a panic caused by a conditional CSS `@import` rule with a URL token. Code like this caused esbuild to enter an unexpected state because the case where tokens in the import condition with associated import records wasn't handled. This case is now handled correctly:

```css
@import "example.css" url(foo);
```

* Mark `Set` and `Map` with array arguments as pure ([#1791](https://github.com/evanw/esbuild/issues/1791))

This release introduces special behavior for references to the global `Set` and `Map` constructors that marks them as `/* @__PURE__ */` if they are known to not have any side effects. These constructors evaluate the iterator of whatever is passed to them and the iterator could have side effects, so this is only safe if whatever is passed to them is an array, since the array iterator has no side effects.
Expand Down
15 changes: 15 additions & 0 deletions internal/bundler/bundler_css_test.go
Expand Up @@ -566,6 +566,21 @@ func TestCSSAtImportConditionsBundleExternal(t *testing.T) {
})
}

func TestCSSAtImportConditionsBundleExternalConditionWithURL(t *testing.T) {
css_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.css": `
@import "https://example.com/foo.css" (foo: url("foo.png")) and (bar: url("bar.png"));
`,
},
entryPaths: []string{"/entry.css"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.css",
},
})
}

func TestCSSAtImportConditionsBundle(t *testing.T) {
css_suite.expectBundled(t, bundled{
files: map[string]string{
Expand Down
19 changes: 14 additions & 5 deletions internal/bundler/linker.go
Expand Up @@ -174,8 +174,9 @@ type chunkReprCSS struct {
}

type externalImportCSS struct {
path logger.Path
conditions []css_ast.Token
path logger.Path
conditions []css_ast.Token
conditionImportRecords []ast.ImportRecord
}

// Returns a log where "log.HasErrors()" only returns true if any errors have
Expand Down Expand Up @@ -2822,10 +2823,15 @@ func (c *linkerContext) findImportedFilesInCSSOrder(entryPoints []uint32) (exter
external.conditions = append(external.conditions, atImport.ImportConditions)
}

// Clone any import records associated with the condition tokens
conditions, conditionImportRecords := css_ast.CloneTokensWithImportRecords(
atImport.ImportConditions, repr.AST.ImportRecords, nil, nil)

externals[record.Path] = external
externalOrder = append(externalOrder, externalImportCSS{
path: record.Path,
conditions: atImport.ImportConditions,
path: record.Path,
conditions: conditions,
conditionImportRecords: conditionImportRecords,
})
}
}
Expand Down Expand Up @@ -4868,9 +4874,12 @@ func (c *linkerContext) generateChunkCSS(chunks []chunkInfo, chunkIndex int, chu
// Insert all external "@import" rules at the front. In CSS, all "@import"
// rules must come first or the browser will just ignore them.
for _, external := range chunkRepr.externalImportsInOrder {
var conditions []css_ast.Token
conditions, tree.ImportRecords = css_ast.CloneTokensWithImportRecords(
external.conditions, external.conditionImportRecords, conditions, tree.ImportRecords)
tree.Rules = append(tree.Rules, css_ast.Rule{Data: &css_ast.RAtImport{
ImportRecordIndex: uint32(len(tree.ImportRecords)),
ImportConditions: external.conditions,
ImportConditions: conditions,
}})
tree.ImportRecords = append(tree.ImportRecords, ast.ImportRecord{
Kind: ast.ImportAt,
Expand Down
7 changes: 7 additions & 0 deletions internal/bundler/snapshots/snapshots_css.txt
Expand Up @@ -93,6 +93,13 @@ TestCSSAtImportConditionsBundleExternal

/* entry.css */

================================================================================
TestCSSAtImportConditionsBundleExternalConditionWithURL
---------- /out.css ----------
@import "https://example.com/foo.css" (foo: url(foo.png)) and (bar: url(bar.png));

/* entry.css */

================================================================================
TestCSSAtImportConditionsNoBundle
---------- /out.css ----------
Expand Down
25 changes: 25 additions & 0 deletions internal/css_ast/css_ast.go
Expand Up @@ -231,6 +231,31 @@ func (t Token) IsAngle() bool {
return false
}

func CloneTokensWithImportRecords(
tokensIn []Token, importRecordsIn []ast.ImportRecord,
tokensOut []Token, importRecordsOut []ast.ImportRecord,
) ([]Token, []ast.ImportRecord) {
for _, t := range tokensIn {
// If this is a URL token, also clone the import record
if t.Kind == css_lexer.TURL {
importRecordIndex := uint32(len(importRecordsOut))
importRecordsOut = append(importRecordsOut, importRecordsIn[t.ImportRecordIndex])
t.ImportRecordIndex = importRecordIndex
}

// Also search for URL tokens in this token's children
if t.Children != nil {
var children []Token
children, importRecordsOut = CloneTokensWithImportRecords(*t.Children, importRecordsIn, children, importRecordsOut)
t.Children = &children
}

tokensOut = append(tokensOut, t)
}

return tokensOut, importRecordsOut
}

type Rule struct {
Loc logger.Loc
Data R
Expand Down
8 changes: 7 additions & 1 deletion internal/css_parser/css_parser.go
Expand Up @@ -647,9 +647,15 @@ func (p *parser) parseAtRule(context atRuleContext) css_ast.Rule {
p.eat(css_lexer.TWhitespace)
if path, r, ok := p.expectURLOrString(); ok {
importConditionsStart := p.index
for p.current().Kind != css_lexer.TSemicolon && p.current().Kind != css_lexer.TEndOfFile {
for {
if kind := p.current().Kind; kind == css_lexer.TSemicolon || kind == css_lexer.TOpenBrace || kind == css_lexer.TEndOfFile {
break
}
p.parseComponentValue()
}
if p.current().Kind == css_lexer.TOpenBrace {
break // Avoid parsing an invalid "@import" rule
}
importConditions := p.convertTokens(p.tokens[importConditionsStart:p.index])
kind := ast.ImportAt

Expand Down
3 changes: 2 additions & 1 deletion internal/css_parser/css_parser_test.go
Expand Up @@ -836,7 +836,8 @@ func TestAtImport(t *testing.T) {
<stdin>: warning: Expected ";" but found end of file
`)

expectParseError(t, "@import \"foo.css\" {}", "<stdin>: warning: Expected \";\" but found end of file\n")
expectParseError(t, "@import \"foo.css\" {}", "<stdin>: warning: Expected \";\"\n")
expectPrinted(t, "@import \"foo\"\na { color: red }\nb { color: blue }", "@import \"foo\" a { color: red }\nb {\n color: blue;\n}\n")
}

func TestLegalComment(t *testing.T) {
Expand Down

0 comments on commit 8569cdf

Please sign in to comment.