From ca408d061251956e92a751de5debb6606b65a8d5 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 12 Mar 2025 12:09:13 +0100 Subject: [PATCH] Do not extract candidates containing JS string interpolation pattern `${` (#17142) This PR fixes an issue where often people run into issues where they try to use string interpolation and it doesn't work. Even worse, it could result in crashes because we will actually generate CSS. This fix only filters out candidates with a pattern like `${`. If this occurs in a string position it is fine. Another solution would be to add a pre processor for JS/TS (and all thousand file extension combinations) but the problem is that you can also write JS in HTML files so we would have to pre process HTML as well which would not be ideal. # Test plan 1. Added tests to prove this works in arbitrary values, arbitrary variables in both utilities and variants. 2. Existing tests pass. 3. Some screenshots with before / after situations: Given this input: ```ts let color = '#0088cc'; let opacity = 0.8; let name = 'variable-name'; let classes = [ // Arbitrary Properties `[color:${color}]` `[${property}:value]`, `[--img:url('https://example.com?q=${name}')]`, // WONT WORK BUT VALID CSS // Arbitrary Values `bg-[${color}]`, // Arbitrary Variables `bg-(--my-${color})`, `bg-(--my-color,${color})`, // Arbitrary Modifier `bg-red-500/[${opacity}]`, `bg-red-500/(--my-${name})`, `bg-red-500/(--my-opacity,${opacity})`, // Arbitrary Variant `data-[state=${name}]:flex`, `supports-(--my-${name}):flex`, `[@media(width>=${value})]:flex`, ]; ``` This is the result: | Before | After | | --- | --- | | image | image | Fixes: #17054 Fixes: #15853 --- CHANGELOG.md | 4 +++ .../extractor/arbitrary_property_machine.rs | 30 +++++++++++++++++++ .../src/extractor/arbitrary_value_machine.rs | 21 +++++++++++++ .../extractor/arbitrary_variable_machine.rs | 29 ++++++++++++++++++ .../oxide/src/extractor/candidate_machine.rs | 30 +++++++++++++++++++ 5 files changed, 114 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d85a5da4b..261ecb8e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - _Experimental_: Add `user-valid` and `user-invalid` variants ([#12370](https://github.com/tailwindlabs/tailwindcss/pull/12370)) - _Experimental_: Add `wrap-anywhere`, `wrap-break-word`, and `wrap-normal` utilities ([#12128](https://github.com/tailwindlabs/tailwindcss/pull/12128)) +### Fixed + +- Do not extract candidates with JS string interpolation `${` ([#17142](https://github.com/tailwindlabs/tailwindcss/pull/17142)) + ## [4.0.13] - 2025-03-11 ### Fixed diff --git a/crates/oxide/src/extractor/arbitrary_property_machine.rs b/crates/oxide/src/extractor/arbitrary_property_machine.rs index 947073c2f..bf5373a3d 100644 --- a/crates/oxide/src/extractor/arbitrary_property_machine.rs +++ b/crates/oxide/src/extractor/arbitrary_property_machine.rs @@ -226,6 +226,11 @@ impl Machine for ArbitraryPropertyMachine { // URLs are not allowed Class::Slash if start_of_value_pos == cursor.pos => return self.restart(), + // String interpolation-like syntax is not allowed. E.g.: `[${x}]` + Class::Dollar if matches!(cursor.next.into(), Class::OpenCurly) => { + return self.restart() + } + // Everything else is valid _ => cursor.advance(), }; @@ -276,6 +281,9 @@ enum Class { #[bytes(b'-')] Dash, + #[bytes(b'$')] + Dollar, + #[bytes_range(b'a'..=b'z')] AlphaLower, @@ -411,4 +419,26 @@ mod tests { } } } + + #[test] + fn test_exceptions() { + for (input, expected) in [ + // JS string interpolation + // In key + ("[${x}:value]", vec![]), + // As part of the key + ("[background-${property}:value]", vec![]), + // In value + ("[key:${x}]", vec![]), + // As part of the value + ("[key:value-${x}]", vec![]), + // Allowed in strings + ("[--img:url('${x}')]", vec!["[--img:url('${x}')]"]), + ] { + assert_eq!( + ArbitraryPropertyMachine::::test_extract_all(input), + expected + ); + } + } } diff --git a/crates/oxide/src/extractor/arbitrary_value_machine.rs b/crates/oxide/src/extractor/arbitrary_value_machine.rs index c99f7f54a..b4370c2d2 100644 --- a/crates/oxide/src/extractor/arbitrary_value_machine.rs +++ b/crates/oxide/src/extractor/arbitrary_value_machine.rs @@ -95,6 +95,11 @@ impl Machine for ArbitraryValueMachine { // Any kind of whitespace is not allowed Class::Whitespace => return self.restart(), + // String interpolation-like syntax is not allowed. E.g.: `[${x}]` + Class::Dollar if matches!(cursor.next.into(), Class::OpenCurly) => { + return self.restart() + } + // Everything else is valid _ => cursor.advance(), }; @@ -133,6 +138,9 @@ enum Class { #[bytes(b' ', b'\t', b'\n', b'\r', b'\x0C')] Whitespace, + #[bytes(b'$')] + Dollar, + #[fallback] Other, } @@ -188,4 +196,17 @@ mod tests { assert_eq!(ArbitraryValueMachine::test_extract_all(input), expected); } } + + #[test] + fn test_exceptions() { + for (input, expected) in [ + // JS string interpolation + ("[${x}]", vec![]), + ("[url(${x})]", vec![]), + // Allowed in strings + ("[url('${x}')]", vec!["[url('${x}')]"]), + ] { + assert_eq!(ArbitraryValueMachine::test_extract_all(input), expected); + } + } } diff --git a/crates/oxide/src/extractor/arbitrary_variable_machine.rs b/crates/oxide/src/extractor/arbitrary_variable_machine.rs index 5c26b0659..355572769 100644 --- a/crates/oxide/src/extractor/arbitrary_variable_machine.rs +++ b/crates/oxide/src/extractor/arbitrary_variable_machine.rs @@ -252,6 +252,11 @@ impl Machine for ArbitraryVariableMachine { // Any kind of whitespace is not allowed Class::Whitespace => return self.restart(), + // String interpolation-like syntax is not allowed. E.g.: `[${x}]` + Class::Dollar if matches!(cursor.next.into(), Class::OpenCurly) => { + return self.restart() + } + // Everything else is valid _ => cursor.advance(), }; @@ -284,6 +289,9 @@ enum Class { #[bytes(b'.')] Dot, + #[bytes(b'$')] + Dollar, + #[bytes(b'\\')] Escape, @@ -380,4 +388,25 @@ mod tests { ); } } + + #[test] + fn test_exceptions() { + for (input, expected) in [ + // JS string interpolation + // As part of the variable + ("(--my-${var})", vec![]), + // As the fallback + ("(--my-variable,${var})", vec![]), + // As the fallback in strings + ( + "(--my-variable,url('${var}'))", + vec!["(--my-variable,url('${var}'))"], + ), + ] { + assert_eq!( + ArbitraryVariableMachine::::test_extract_all(input), + expected + ); + } + } } diff --git a/crates/oxide/src/extractor/candidate_machine.rs b/crates/oxide/src/extractor/candidate_machine.rs index 1d92ea153..33b67098f 100644 --- a/crates/oxide/src/extractor/candidate_machine.rs +++ b/crates/oxide/src/extractor/candidate_machine.rs @@ -327,4 +327,34 @@ mod tests { ); } } + + #[test] + fn test_js_interpolation() { + for (input, expected) in [ + // Utilities + // Arbitrary value + ("bg-[${color}]", vec![]), + // Arbitrary property + ("[color:${value}]", vec![]), + ("[${key}:value]", vec![]), + ("[${key}:${value}]", vec![]), + // Arbitrary property for CSS variables + ("[--color:${value}]", vec![]), + ("[--color-${name}:value]", vec![]), + // Arbitrary variable + ("bg-(--my-${name})", vec![]), + ("bg-(--my-variable,${fallback})", vec![]), + ( + "bg-(--my-image,url('https://example.com?q=${value}'))", + vec!["bg-(--my-image,url('https://example.com?q=${value}'))"], + ), + // Variants + ("data-[state=${state}]:flex", vec![]), + ("support-(--my-${value}):flex", vec![]), + ("support-(--my-variable,${fallback}):flex", vec![]), + ("[@media(width>=${value})]:flex", vec![]), + ] { + assert_eq!(CandidateMachine::test_extract_all(input), expected); + } + } }