From 350d667577206dfa65fd812bc8bb088eeec0b797 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Thu, 30 Jan 2025 20:29:53 -0500 Subject: [PATCH 1/6] Discard invalid declarations when parsing CSS --- packages/tailwindcss/src/css-parser.test.ts | 28 +++++++++++++++++++ packages/tailwindcss/src/css-parser.ts | 31 ++++++++++++++------- 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/packages/tailwindcss/src/css-parser.test.ts b/packages/tailwindcss/src/css-parser.test.ts index a4b123b28fde..0ff921a2ed63 100644 --- a/packages/tailwindcss/src/css-parser.test.ts +++ b/packages/tailwindcss/src/css-parser.test.ts @@ -1041,6 +1041,34 @@ describe.each(['Unix', 'Windows'])('Line endings: %s', (lineEndings) => { }, ]) }) + + // TODO: These should probably be errors in some cases? + it('discards invalid declarations', () => { + // This shouldn't parse as a custom property declaration + expect(parse(`--foo`)).toEqual([]) + + // This shouldn't parse as a rule followed by a declaration + expect(parse(`@plugin "foo" {};`)).toEqual([ + { + kind: 'at-rule', + name: '@plugin', + params: '"foo"', + nodes: [], + }, + ]) + + // This shouldn't parse as consecutive declarations + expect(parse(`;;;`)).toEqual([]) + + // This shouldn't parse as a rule with a declaration inside of it + expect(parse(`.foo { bar }`)).toEqual([ + { + kind: 'rule', + selector: '.foo', + nodes: [], + }, + ]) + }) }) describe('errors', () => { diff --git a/packages/tailwindcss/src/css-parser.ts b/packages/tailwindcss/src/css-parser.ts index c12c63051cee..8990c30f87df 100644 --- a/packages/tailwindcss/src/css-parser.ts +++ b/packages/tailwindcss/src/css-parser.ts @@ -286,10 +286,12 @@ export function parse(input: string) { } let declaration = parseDeclaration(buffer, colonIdx) - if (parent) { - parent.nodes.push(declaration) - } else { - ast.push(declaration) + if (declaration) { + if (parent) { + parent.nodes.push(declaration) + } else { + ast.push(declaration) + } } buffer = '' @@ -337,10 +339,12 @@ export function parse(input: string) { closingBracketStack[closingBracketStack.length - 1] !== ')' ) { let declaration = parseDeclaration(buffer) - if (parent) { - parent.nodes.push(declaration) - } else { - ast.push(declaration) + if (declaration) { + if (parent) { + parent.nodes.push(declaration) + } else { + ast.push(declaration) + } } buffer = '' @@ -435,7 +439,10 @@ export function parse(input: string) { // Attach the declaration to the parent. if (parent) { - parent.nodes.push(parseDeclaration(buffer, colonIdx)) + let node = parseDeclaration(buffer, colonIdx) + if (node) { + parent.nodes.push(node) + } } } } @@ -543,7 +550,11 @@ export function parseAtRule(buffer: string, nodes: AstNode[] = []): AtRule { return atRule(buffer.trim(), '', nodes) } -function parseDeclaration(buffer: string, colonIdx: number = buffer.indexOf(':')): Declaration { +function parseDeclaration( + buffer: string, + colonIdx: number = buffer.indexOf(':'), +): Declaration | null { + if (colonIdx === -1) return null let importantIdx = buffer.indexOf('!important', colonIdx + 1) return decl( buffer.slice(0, colonIdx).trim(), From 650bfe5f420969acc82cb5bdcdf0789daaf84b0e Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Thu, 30 Jan 2025 20:33:21 -0500 Subject: [PATCH 2/6] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff0ec536b880..114412e42d3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Vite: Transform `` blocks in HTML files ([#16069](https://github.com/tailwindlabs/tailwindcss/pull/16069)) - Prevent camelCasing CSS custom properties added by JavaScript plugins ([#16103](https://github.com/tailwindlabs/tailwindcss/pull/16103)) - Do not emit `@keyframes` in `@theme reference` ([#16120](https://github.com/tailwindlabs/tailwindcss/pull/16120)) +- Discard invalid declarations when parsing CSS ([#16093](https://github.com/tailwindlabs/tailwindcss/pull/16093)) ## [4.0.1] - 2025-01-29 From 6f58ccc936b2f81ec45a8df649de3ce70fa4a7c8 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 31 Jan 2025 10:49:01 +0100 Subject: [PATCH 3/6] turn some unexpected values into errors --- packages/tailwindcss/src/css-parser.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/tailwindcss/src/css-parser.ts b/packages/tailwindcss/src/css-parser.ts index 8990c30f87df..9a76dee07d63 100644 --- a/packages/tailwindcss/src/css-parser.ts +++ b/packages/tailwindcss/src/css-parser.ts @@ -292,6 +292,8 @@ export function parse(input: string) { } else { ast.push(declaration) } + } else { + throw new Error(`Invalid custom property, expected a value`) } buffer = '' @@ -345,6 +347,20 @@ export function parse(input: string) { } else { ast.push(declaration) } + } else if (input.charCodeAt(i - 1) === SEMICOLON || input.charCodeAt(i + 1) === SEMICOLON) { + let startSemiColonIdx = i + while (input.charCodeAt(startSemiColonIdx - 1) === SEMICOLON) { + startSemiColonIdx-- + } + let endSemiColonIdx = i + while (input.charCodeAt(endSemiColonIdx) === SEMICOLON) { + endSemiColonIdx++ + } + throw new Error(`Unexpected: \`${input.slice(startSemiColonIdx, endSemiColonIdx)}\``) + } else if (buffer.length == 0) { + throw new Error('Unexpected semicolon') + } else { + throw new Error(`Invalid declaration: \`${buffer.trim()}\``) } buffer = '' @@ -442,6 +458,8 @@ export function parse(input: string) { let node = parseDeclaration(buffer, colonIdx) if (node) { parent.nodes.push(node) + } else { + throw new Error(`Invalid declaration: \`${buffer.trim()}\``) } } } From 857c7e368334d4a7b2ed079680d3e1eb25b5e80d Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 31 Jan 2025 11:48:16 +0100 Subject: [PATCH 4/6] move tests to error describe block --- packages/tailwindcss/src/css-parser.test.ts | 84 ++++++++++++++------- 1 file changed, 56 insertions(+), 28 deletions(-) diff --git a/packages/tailwindcss/src/css-parser.test.ts b/packages/tailwindcss/src/css-parser.test.ts index 0ff921a2ed63..723e2a8425d1 100644 --- a/packages/tailwindcss/src/css-parser.test.ts +++ b/packages/tailwindcss/src/css-parser.test.ts @@ -329,6 +329,28 @@ describe.each(['Unix', 'Windows'])('Line endings: %s', (lineEndings) => { ]) }) + it('should parse a custom property with an empty value', () => { + expect(parse('--foo:;')).toEqual([ + { + kind: 'declaration', + property: '--foo', + value: '', + important: false, + }, + ]) + }) + + it('should parse a custom property with a space value', () => { + expect(parse('--foo: ;')).toEqual([ + { + kind: 'declaration', + property: '--foo', + value: '', + important: false, + }, + ]) + }) + it('should parse a custom property with a block including nested "css"', () => { expect( parse(css` @@ -1041,34 +1063,6 @@ describe.each(['Unix', 'Windows'])('Line endings: %s', (lineEndings) => { }, ]) }) - - // TODO: These should probably be errors in some cases? - it('discards invalid declarations', () => { - // This shouldn't parse as a custom property declaration - expect(parse(`--foo`)).toEqual([]) - - // This shouldn't parse as a rule followed by a declaration - expect(parse(`@plugin "foo" {};`)).toEqual([ - { - kind: 'at-rule', - name: '@plugin', - params: '"foo"', - nodes: [], - }, - ]) - - // This shouldn't parse as consecutive declarations - expect(parse(`;;;`)).toEqual([]) - - // This shouldn't parse as a rule with a declaration inside of it - expect(parse(`.foo { bar }`)).toEqual([ - { - kind: 'rule', - selector: '.foo', - nodes: [], - }, - ]) - }) }) describe('errors', () => { @@ -1125,5 +1119,39 @@ describe.each(['Unix', 'Windows'])('Line endings: %s', (lineEndings) => { `), ).toThrowErrorMatchingInlineSnapshot(`[Error: Unterminated string: "Hello world!;"]`) }) + + it('should error when incomplete custom properties are used', () => { + expect(() => parse('--foo')).toThrowErrorMatchingInlineSnapshot( + `[Error: Invalid custom property, expected a value]`, + ) + }) + + it('should error when incomplete custom properties are used inside rules', () => { + expect(() => parse('.foo { --bar }')).toThrowErrorMatchingInlineSnapshot( + `[Error: Invalid custom property, expected a value]`, + ) + }) + + it('should error when a declaration is incomplete', () => { + expect(() => parse('.foo { bar }')).toThrowErrorMatchingInlineSnapshot( + `[Error: Invalid declaration: \`bar\`]`, + ) + }) + + it('should error when a semicolon exists after an at-rule with a body', () => { + expect(() => parse('@plugin "foo" {} ;')).toThrowErrorMatchingInlineSnapshot( + `[Error: Unexpected semicolon]`, + ) + }) + + it('should error when consecutive semicolons exist', () => { + expect(() => parse(';;;')).toThrowErrorMatchingInlineSnapshot(`[Error: Unexpected: \`;;;\`]`) + }) + + it('should error when consecutive semicolons exist after a declaration', () => { + expect(() => parse('.foo { color: red;;; }')).toThrowErrorMatchingInlineSnapshot( + `[Error: Unexpected: \`;;;\`]`, + ) + }) }) }) From 368fb26ebacfa9d63f51d48f9ae63dcb197859ec Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 31 Jan 2025 15:29:44 +0100 Subject: [PATCH 5/6] cleanup error handling Let's us some early returns and simplify some error messages. --- packages/tailwindcss/src/css-parser.ts | 47 +++++++++----------------- 1 file changed, 16 insertions(+), 31 deletions(-) diff --git a/packages/tailwindcss/src/css-parser.ts b/packages/tailwindcss/src/css-parser.ts index 9a76dee07d63..d80bb3e79807 100644 --- a/packages/tailwindcss/src/css-parser.ts +++ b/packages/tailwindcss/src/css-parser.ts @@ -286,14 +286,12 @@ export function parse(input: string) { } let declaration = parseDeclaration(buffer, colonIdx) - if (declaration) { - if (parent) { - parent.nodes.push(declaration) - } else { - ast.push(declaration) - } + if (!declaration) throw new Error(`Invalid custom property, expected a value`) + + if (parent) { + parent.nodes.push(declaration) } else { - throw new Error(`Invalid custom property, expected a value`) + ast.push(declaration) } buffer = '' @@ -341,28 +339,17 @@ export function parse(input: string) { closingBracketStack[closingBracketStack.length - 1] !== ')' ) { let declaration = parseDeclaration(buffer) - if (declaration) { - if (parent) { - parent.nodes.push(declaration) - } else { - ast.push(declaration) - } - } else if (input.charCodeAt(i - 1) === SEMICOLON || input.charCodeAt(i + 1) === SEMICOLON) { - let startSemiColonIdx = i - while (input.charCodeAt(startSemiColonIdx - 1) === SEMICOLON) { - startSemiColonIdx-- - } - let endSemiColonIdx = i - while (input.charCodeAt(endSemiColonIdx) === SEMICOLON) { - endSemiColonIdx++ - } - throw new Error(`Unexpected: \`${input.slice(startSemiColonIdx, endSemiColonIdx)}\``) - } else if (buffer.length == 0) { - throw new Error('Unexpected semicolon') - } else { + if (!declaration) { + if (buffer.length === 0) throw new Error('Unexpected semicolon') throw new Error(`Invalid declaration: \`${buffer.trim()}\``) } + if (parent) { + parent.nodes.push(declaration) + } else { + ast.push(declaration) + } + buffer = '' } @@ -456,11 +443,9 @@ export function parse(input: string) { // Attach the declaration to the parent. if (parent) { let node = parseDeclaration(buffer, colonIdx) - if (node) { - parent.nodes.push(node) - } else { - throw new Error(`Invalid declaration: \`${buffer.trim()}\``) - } + if (!node) throw new Error(`Invalid declaration: \`${buffer.trim()}\``) + + parent.nodes.push(node) } } } From 7d8c054ce0b922904c93d6aae8518c912128ee7f Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 31 Jan 2025 15:30:06 +0100 Subject: [PATCH 6/6] update tests with simpler error message --- packages/tailwindcss/src/css-parser.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/tailwindcss/src/css-parser.test.ts b/packages/tailwindcss/src/css-parser.test.ts index 723e2a8425d1..25d5b3a454ca 100644 --- a/packages/tailwindcss/src/css-parser.test.ts +++ b/packages/tailwindcss/src/css-parser.test.ts @@ -1145,12 +1145,12 @@ describe.each(['Unix', 'Windows'])('Line endings: %s', (lineEndings) => { }) it('should error when consecutive semicolons exist', () => { - expect(() => parse(';;;')).toThrowErrorMatchingInlineSnapshot(`[Error: Unexpected: \`;;;\`]`) + expect(() => parse(';;;')).toThrowErrorMatchingInlineSnapshot(`[Error: Unexpected semicolon]`) }) it('should error when consecutive semicolons exist after a declaration', () => { expect(() => parse('.foo { color: red;;; }')).toThrowErrorMatchingInlineSnapshot( - `[Error: Unexpected: \`;;;\`]`, + `[Error: Unexpected semicolon]`, ) }) })