Skip to content

Commit

Permalink
allow unterminated strings in css
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Mar 24, 2023
1 parent b0a72c6 commit 377c6c7
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 28 deletions.
39 changes: 39 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,45 @@

Go's WebAssembly implementation returns `EINVAL` instead of `ENOTDIR` when using the `readdir` syscall on a file. This messes up esbuild's implementation of node's module resolution algorithm since encountering `ENOTDIR` causes esbuild to continue its search (since it's a normal condition) while other encountering other errors causes esbuild to fail with an I/O error (since it's an unexpected condition). You can encounter this issue in practice if you use node's legacy `NODE_PATH` feature to tell esbuild to resolve node modules in a custom directory that was not installed by npm. This release works around this problem by converting `EINVAL` into `ENOTDIR` for the `readdir` syscall.

* Unterminated strings in CSS are no longer an error

The CSS specification provides [rules for handling parsing errors](https://www.w3.org/TR/CSS22/syndata.html#parsing-errors). One of those rules is that user agents must close strings upon reaching the end of a line (i.e., before an unescaped line feed, carriage return or form feed character), but then drop the construct (declaration or rule) in which the string was found. For example:

```css
p {
color: green;
font-family: 'Courier New Times
color: red;
color: green;
}
```

...would be treated the same as:

```css
p { color: green; color: green; }
```

...because the second declaration (from `font-family` to the semicolon after `color: red`) is invalid and is dropped.

Previously using this CSS with esbuild failed to build due to a syntax error, even though the code can be interpreted by a browser. With this release, the code now produces a warning instead of an error, and esbuild prints the invalid CSS such that it stays invalid in the output:

```css
/* esbuild's new non-minified output: */
p {
color: green;
font-family: 'Courier New Times
color: red;
color: green;
}
```

```css
/* esbuild's new minified output: */
p{font-family:'Courier New Times
color: red;color:green}
```

## 0.17.12

* Fix a crash when parsing inline TypeScript decorators ([#2991](https://github.com/evanw/esbuild/issues/2991))
Expand Down
14 changes: 4 additions & 10 deletions internal/css_lexer/css_lexer.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const (
TEndOfFile T = iota

TAtKeyword
TBadString
TUnterminatedString
TBadURL
TCDC // "-->"
TCDO // "<!--"
Expand Down Expand Up @@ -785,17 +785,11 @@ func (lexer *lexer) consumeString() T {

// Otherwise, fall through to ignore the character after the backslash

case eof:
lexer.log.AddError(&lexer.tracker,
logger.Range{Loc: logger.Loc{Start: lexer.Token.Range.End()}},
"Unterminated string token")
return TBadString

case '\n', '\r', '\f':
lexer.log.AddError(&lexer.tracker,
case eof, '\n', '\r', '\f':
lexer.log.AddID(logger.MsgID_CSS_CSSSyntaxError, logger.Warning, &lexer.tracker,
logger.Range{Loc: logger.Loc{Start: lexer.Token.Range.End()}},
"Unterminated string token")
return TBadString
return TUnterminatedString

case quote:
lexer.step()
Expand Down
8 changes: 4 additions & 4 deletions internal/css_lexer/css_lexer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,10 @@ func TestComment(t *testing.T) {
}

func TestString(t *testing.T) {
test.AssertEqualWithDiff(t, lexerError("'"), "<stdin>: ERROR: Unterminated string token\n")
test.AssertEqualWithDiff(t, lexerError("\""), "<stdin>: ERROR: Unterminated string token\n")
test.AssertEqualWithDiff(t, lexerError("'\\'"), "<stdin>: ERROR: Unterminated string token\n")
test.AssertEqualWithDiff(t, lexerError("\"\\\""), "<stdin>: ERROR: Unterminated string token\n")
test.AssertEqualWithDiff(t, lexerError("'"), "<stdin>: WARNING: Unterminated string token\n")
test.AssertEqualWithDiff(t, lexerError("\""), "<stdin>: WARNING: Unterminated string token\n")
test.AssertEqualWithDiff(t, lexerError("'\\'"), "<stdin>: WARNING: Unterminated string token\n")
test.AssertEqualWithDiff(t, lexerError("\"\\\""), "<stdin>: WARNING: Unterminated string token\n")
test.AssertEqualWithDiff(t, lexerError("''"), "")
test.AssertEqualWithDiff(t, lexerError("\"\""), "")
}
Expand Down
4 changes: 2 additions & 2 deletions internal/css_parser/css_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func (p *parser) expectWithMatchingLoc(kind css_lexer.T, matchingLoc logger.Loc)
case css_lexer.TEndOfFile, css_lexer.TWhitespace:
text = fmt.Sprintf("Expected %s but found %s", expected, t.Kind.String())
t.Range.Len = 0
case css_lexer.TBadURL, css_lexer.TBadString:
case css_lexer.TBadURL, css_lexer.TUnterminatedString:
text = fmt.Sprintf("Expected %s but found %s", expected, t.Kind.String())
default:
text = fmt.Sprintf("Expected %s but found %q", expected, p.raw())
Expand All @@ -172,7 +172,7 @@ func (p *parser) unexpected() {
case css_lexer.TEndOfFile, css_lexer.TWhitespace:
text = fmt.Sprintf("Unexpected %s", t.Kind.String())
t.Range.Len = 0
case css_lexer.TBadURL, css_lexer.TBadString:
case css_lexer.TBadURL, css_lexer.TUnterminatedString:
text = fmt.Sprintf("Unexpected %s", t.Kind.String())
default:
text = fmt.Sprintf("Unexpected %q", p.raw())
Expand Down
33 changes: 21 additions & 12 deletions internal/css_parser/css_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ func expectParseError(t *testing.T, contents string, expected string, expectedLo
expectPrintedCommon(t, contents, contents, expected, &expectedLog, config.Options{})
}

func expectParseErrorMinify(t *testing.T, contents string, expected string, expectedLog string) {
t.Helper()
expectPrintedCommon(t, contents, contents, expected, &expectedLog, config.Options{
MinifyWhitespace: true,
})
}

func expectPrinted(t *testing.T, contents string, expected string) {
t.Helper()
expectPrintedCommon(t, contents, contents, expected, nil, config.Options{})
Expand Down Expand Up @@ -278,29 +285,29 @@ func TestString(t *testing.T) {
expectPrinted(t, "a:after { content: 'a\\\r\nb' }", "a:after {\n content: \"ab\";\n}\n")
expectPrinted(t, "a:after { content: 'a\\62 c' }", "a:after {\n content: \"abc\";\n}\n")

expectParseError(t, "a:after { content: '\r' }", "a:after {\n content: ' ' };\n}\n",
`<stdin>: ERROR: Unterminated string token
<stdin>: ERROR: Unterminated string token
expectParseError(t, "a:after { content: '\r' }", "a:after {\n content: '\n ' }\n ;\n}\n",
`<stdin>: WARNING: Unterminated string token
<stdin>: WARNING: Expected "}" to go with "{"
<stdin>: NOTE: The unbalanced "{" is here:
<stdin>: WARNING: Unterminated string token
`)
expectParseError(t, "a:after { content: '\n' }", "a:after {\n content: ' ' };\n}\n",
`<stdin>: ERROR: Unterminated string token
<stdin>: ERROR: Unterminated string token
expectParseError(t, "a:after { content: '\n' }", "a:after {\n content: '\n ' }\n ;\n}\n",
`<stdin>: WARNING: Unterminated string token
<stdin>: WARNING: Expected "}" to go with "{"
<stdin>: NOTE: The unbalanced "{" is here:
<stdin>: WARNING: Unterminated string token
`)
expectParseError(t, "a:after { content: '\f' }", "a:after {\n content: ' ' };\n}\n",
`<stdin>: ERROR: Unterminated string token
<stdin>: ERROR: Unterminated string token
expectParseError(t, "a:after { content: '\f' }", "a:after {\n content: '\n ' }\n ;\n}\n",
`<stdin>: WARNING: Unterminated string token
<stdin>: WARNING: Expected "}" to go with "{"
<stdin>: NOTE: The unbalanced "{" is here:
<stdin>: WARNING: Unterminated string token
`)
expectParseError(t, "a:after { content: '\r\n' }", "a:after {\n content: ' ' };\n}\n",
`<stdin>: ERROR: Unterminated string token
<stdin>: ERROR: Unterminated string token
expectParseError(t, "a:after { content: '\r\n' }", "a:after {\n content: '\n ' }\n ;\n}\n",
`<stdin>: WARNING: Unterminated string token
<stdin>: WARNING: Expected "}" to go with "{"
<stdin>: NOTE: The unbalanced "{" is here:
<stdin>: WARNING: Unterminated string token
`)

expectPrinted(t, "a:after { content: '\\1010101' }", "a:after {\n content: \"\U001010101\";\n}\n")
Expand Down Expand Up @@ -1890,4 +1897,6 @@ func TestParseErrorRecovery(t *testing.T) {
"<stdin>: ERROR: Expected \")\" to end URL token\n<stdin>: WARNING: Expected \"}\" to go with \"{\"\n<stdin>: NOTE: The unbalanced \"{\" is here:\n")
expectParseError(t, "/* @license */ x {} /* @preserve", "/* @license */\nx {\n}\n",
"<stdin>: ERROR: Expected \"*/\" to terminate multi-line comment\n<stdin>: NOTE: The multi-line comment starts here:\n")
expectParseError(t, "a { b: c; d: 'e\n f: g; h: i }", "a {\n b: c;\n d: 'e\n f: g;\n h: i;\n}\n", "<stdin>: WARNING: Unterminated string token\n")
expectParseErrorMinify(t, "a { b: c; d: 'e\n f: g; h: i }", "a{b:c;d:'e\nf: g;h:i}", "<stdin>: WARNING: Unterminated string token\n")
}
9 changes: 9 additions & 0 deletions internal/css_printer/css_printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,15 @@ func (p *printer) printTokens(tokens []css_ast.Token, opts printTokensOpts) bool
p.print(")")
p.recordImportPathForMetafile(t.ImportRecordIndex)

case css_lexer.TUnterminatedString:
// We must end this with a newline so that this string stays unterminated
p.print(t.Text)
p.print("\n")
if !p.options.MinifyWhitespace {
p.printIndent(opts.indent)
}
hasWhitespaceAfter = false

default:
p.print(t.Text)
}
Expand Down

0 comments on commit 377c6c7

Please sign in to comment.