From 884f02c46b1c675e21fa33a21bdad7df4aa8e236 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 23 May 2025 21:59:02 +0200 Subject: [PATCH] Fix "Cannot read properties of undefined" crash on malformed arbitrary value (#18133) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR fixes a crash when an arbitrary value was malformed and crashed the build. If you have a utility like `[--btn-border:var(--color-maroon)/90)]` which is malformed, it will crash the build. It might not be easy to spot but the easy is the additional `)` after the `90`. The reason this crashes is because we parse the value `var(--color-maroon)/90)` and when we see `)` we assume it's the end of a "function" which also assumes it was preceded by a `(`. This is not the case and we crash. This PR fixes that by not assuming the parsed object is available and uses `?` to be safe and only access `nodes` if it's available. I'm actually not 100% sure what the best solution is in this scenario because these candidates could (and will) be returned from Oxide so even if we throw a more descriptive error, it will still crash the build and you might not even have control over the candidate. This candidate will now eventually generate the following CSS: ```css .\[--btn-border\:var\(--color-maroon\)\/90\)\] { --btn-border: var(--color-maroon) / ; } ``` Which still looks odd, but even Lightning CSS doesn't throw an error in this case (because it's a CSS variable definition), so I think it's the best we can do. If you open your devtools you will see the weird values, so it's still debug-able. image Fixes: #17064 ## Test plan Manually tested the candidate that crashed it, and after the change generated the above CSS. Then used it in JSFiddle to proof it's fixed now. https://jsfiddle.net/z850ykew/ Couldn't use Tailwind Play because the candidate will cause a crash there as well 😅 --- CHANGELOG.md | 1 + packages/tailwindcss/src/index.test.ts | 33 ++++++++++--------- packages/tailwindcss/src/value-parser.test.ts | 16 +++++++++ packages/tailwindcss/src/value-parser.ts | 2 +- 4 files changed, 35 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c002d985e..cc106a20e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Upgrade: Improve `pnpm` workspaces support ([#18065](https://github.com/tailwindlabs/tailwindcss/pull/18065)) - Upgrade: Migrate deprecated `order-none` to `order-0` ([#18126](https://github.com/tailwindlabs/tailwindcss/pull/18126)) - Support Leptos `class:` attributes when extracting classes ([#18093](https://github.com/tailwindlabs/tailwindcss/pull/18093)) +- Fix "Cannot read properties of undefined" crash on malformed arbitrary value ([#18133](https://github.com/tailwindlabs/tailwindcss/pull/18133)) ## [4.1.7] - 2025-05-15 diff --git a/packages/tailwindcss/src/index.test.ts b/packages/tailwindcss/src/index.test.ts index 93719f083..9c6df63fc 100644 --- a/packages/tailwindcss/src/index.test.ts +++ b/packages/tailwindcss/src/index.test.ts @@ -4305,11 +4305,11 @@ it("should error when `layer(…)` is used, but it's not the first param", async describe('`@reference "…" imports`', () => { test('recursively removes styles', async () => { - let loadStylesheet = async (id: string, base: string) => { + let loadStylesheet = async (id: string, base = '/root/foo') => { if (id === './foo/baz.css') { return { + base, path: '', - base: '/root/foo', content: css` .foo { color: red; @@ -4355,11 +4355,11 @@ describe('`@reference "…" imports`', () => { }) test('does not generate utilities', async () => { - let loadStylesheet = async (id: string, base: string) => { + let loadStylesheet = async (id: string, base = '/root/foo') => { if (id === './foo/baz.css') { return { + base, path: '', - base: '/root/foo', content: css` @layer utilities { @tailwind utilities; @@ -4442,7 +4442,7 @@ describe('`@reference "…" imports`', () => { }) matchUtilities( { - 'match-utility': (value) => ({ + 'match-utility': (_value) => ({ '@keyframes match-utilities': { '100%': { opacity: '0' } }, }), }, @@ -4450,7 +4450,7 @@ describe('`@reference "…" imports`', () => { ) matchComponents( { - 'match-components': (value) => ({ + 'match-components': (_value) => ({ '@keyframes match-components': { '100%': { opacity: '0' } }, }), }, @@ -4474,12 +4474,12 @@ describe('`@reference "…" imports`', () => { }) test('emits CSS variable fallback and keyframes defined inside @reference-ed files', async () => { - let loadStylesheet = async (id: string, base: string) => { + let loadStylesheet = async (id: string, base = '/root') => { switch (id) { case './one.css': { return { + base, path: '', - base: '/root', content: css` @import './two.css' layer(two); `, @@ -4487,8 +4487,8 @@ describe('`@reference "…" imports`', () => { } case './two.css': { return { + base, path: '', - base: '/root', content: css` @import './three.css' layer(three); `, @@ -4496,8 +4496,8 @@ describe('`@reference "…" imports`', () => { } case './three.css': { return { + base, path: '', - base: '/root', content: css` .foo { color: red; @@ -5519,7 +5519,7 @@ describe('feature detection', () => { css` @import 'tailwindcss/preflight'; `, - { loadStylesheet: async (_, base) => ({ base, content: '' }) }, + { loadStylesheet: async (_, base) => ({ base, path: '', content: '' }) }, ) expect(compiler.features & Features.AtImport).toBeTruthy() @@ -5530,7 +5530,7 @@ describe('feature detection', () => { css` @import 'tailwindcss/preflight'; `, - { loadStylesheet: async (_, base) => ({ base, content: '' }) }, + { loadStylesheet: async (_, base) => ({ base, path: '', content: '' }) }, ) // There's little difference between `@reference` and `@import` on a feature @@ -5551,7 +5551,7 @@ describe('feature detection', () => { color: theme(--color-red); } `, - { loadStylesheet: async (_, base) => ({ base, content: '' }) }, + { loadStylesheet: async (_, base) => ({ base, path: '', content: '' }) }, ) expect(compiler.features & Features.ThemeFunction).toBeTruthy() @@ -5562,7 +5562,7 @@ describe('feature detection', () => { css` @plugin "./some-plugin.js"; `, - { loadModule: async (_, base) => ({ base, module: () => {} }) }, + { loadModule: async (_, base) => ({ base, path: '', module: () => {} }) }, ) expect(compiler.features & Features.JsPluginCompat).toBeTruthy() @@ -5573,7 +5573,7 @@ describe('feature detection', () => { css` @config "./some-config.js"; `, - { loadModule: async (_, base) => ({ base, module: {} }) }, + { loadModule: async (_, base) => ({ base, path: '', module: {} }) }, ) expect(compiler.features & Features.JsPluginCompat).toBeTruthy() @@ -5605,9 +5605,10 @@ describe('feature detection', () => { @reference "tailwindcss/utilities"; `, { - async loadStylesheet(id, base) { + async loadStylesheet(_id, base) { return { base, + path: '', content: css` @tailwind utilities; `, diff --git a/packages/tailwindcss/src/value-parser.test.ts b/packages/tailwindcss/src/value-parser.test.ts index e1e64832c..43110f781 100644 --- a/packages/tailwindcss/src/value-parser.test.ts +++ b/packages/tailwindcss/src/value-parser.test.ts @@ -165,6 +165,22 @@ describe('parse', () => { }, ]) }) + + it('should not error when extra `)` was passed', () => { + expect(parse('calc(1 + 2))')).toEqual([ + { + kind: 'function', + value: 'calc', + nodes: [ + { kind: 'word', value: '1' }, + { kind: 'separator', value: ' ' }, + { kind: 'word', value: '+' }, + { kind: 'separator', value: ' ' }, + { kind: 'word', value: '2' }, + ], + }, + ]) + }) }) describe('toCss', () => { diff --git a/packages/tailwindcss/src/value-parser.ts b/packages/tailwindcss/src/value-parser.ts index 5d111bce2..945b2a112 100644 --- a/packages/tailwindcss/src/value-parser.ts +++ b/packages/tailwindcss/src/value-parser.ts @@ -302,7 +302,7 @@ export function parse(input: string) { // Handle everything before the closing paren a word if (buffer.length > 0) { let node = word(buffer) - tail!.nodes.push(node) + tail?.nodes.push(node) buffer = '' }