Skip to content

Commit af64de5

Browse files
nicolas-grekasnicolas-grekas
committed
[AssetMapper] Fix parsing @import that don't use url()
1 parent d13b9ae commit af64de5

File tree

3 files changed

+111
-9
lines changed

3 files changed

+111
-9
lines changed

‎src/Symfony/Component/AssetMapper/Compiler/CssAssetUrlCompiler.php‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,16 @@
2424
*/
2525
final class CssAssetUrlCompiler implements AssetCompilerInterface
2626
{
27-
// https://regex101.com/r/BOJ3vG/1
28-
public const ASSET_URL_PATTERN = '/url\(\s*["\']?(?!(?:\/|\#|%23|data|http|\/\/))([^"\'\s?#)]+)([#?][^"\')]+)?\s*["\']?\)/';
27+
// https://regex101.com/r/BOJ3vG/2
28+
public const ASSET_URL_PATTERN = <<<'REGEX'
29+
{
30+
(?|
31+
(url\()\s*+["']?(?!(?:/|\#|%23|data|http|//))([^"')\s?#]++)(?:[?#][^"')]++)?["']?\s*+(\))
32+
|
33+
(@import\s++)["'](?!(?:/|\#|%23|data|http|//))([^"')\s?#]++)(?:[?#][^"')]++)?["']
34+
)
35+
}x
36+
REGEX;
2937

3038
public function __construct(
3139
private readonly string $missingImportMode = self::MISSING_IMPORT_WARN,
@@ -62,16 +70,16 @@ public function compile(string $content, MappedAsset $asset, AssetMapperInterfac
6270
}
6371

6472
try {
65-
$resolvedSourcePath = Path::join(\dirname($asset->sourcePath), $matches[1]);
73+
$resolvedSourcePath = Path::join(\dirname($asset->sourcePath), $matches[2][0]);
6674
} catch (RuntimeException $e) {
6775
$this->handleMissingImport(\sprintf('Error processing import in "%s": ', $asset->sourcePath).$e->getMessage(), $e);
6876

69-
return $matches[0];
77+
return $matches[0][0];
7078
}
7179
$dependentAsset = $assetMapper->getAssetFromSourcePath($resolvedSourcePath);
7280

7381
if (null === $dependentAsset) {
74-
$message = \sprintf('Unable to find asset "%s" referenced in "%s". The file "%s" ', $matches[1], $asset->sourcePath, $resolvedSourcePath);
82+
$message = \sprintf('Unable to find asset "%s" referenced in "%s". The file "%s" ', $matches[2][0], $asset->sourcePath, $resolvedSourcePath);
7583
if (is_file($resolvedSourcePath)) {
7684
$message .= 'exists, but it is not in a mapped asset path. Add it to the "paths" config.';
7785
} else {
@@ -80,14 +88,14 @@ public function compile(string $content, MappedAsset $asset, AssetMapperInterfac
8088
$this->handleMissingImport($message);
8189

8290
// return original, unchanged path
83-
return $matches[0];
91+
return $matches[0][0];
8492
}
8593

8694
$asset->addDependency($dependentAsset);
8795
$relativePath = Path::makeRelative($dependentAsset->publicPath, \dirname($asset->publicPathWithoutDigest));
8896

89-
return 'url("'.$relativePath.'")';
90-
}, $content);
97+
return $matches[1][0].'"'.$relativePath.'"'.($matches[3][0] ?? '');
98+
}, $content, -1, $count, \PREG_OFFSET_CAPTURE);
9199
}
92100

93101
public function supports(MappedAsset $asset): bool

‎src/Symfony/Component/AssetMapper/ImportMap/Resolver/JsDelivrEsmResolver.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ private function makeImportsBare(string $content, array &$dependencies, array &$
318318
}
319319

320320
preg_match_all(CssAssetUrlCompiler::ASSET_URL_PATTERN, $content, $matches);
321-
foreach ($matches[1] as $path) {
321+
foreach ($matches[2] as $path) {
322322
if (str_starts_with($path, 'data:')) {
323323
continue;
324324
}

‎src/Symfony/Component/AssetMapper/Tests/Compiler/CssAssetUrlCompilerTest.php‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,82 @@ public static function provideCompileTests(): iterable
144144
}',
145145
'expectedDependencies' => ['images/foo.png'],
146146
];
147+
148+
yield 'import_without_url_double_quotes' => [
149+
'input' => '@import "more-styles.css"',
150+
'expectedOutput' => '@import "more-styles.abcd123.css"',
151+
'expectedDependencies' => ['more-styles.css'],
152+
];
153+
154+
yield 'import_without_url_single_quotes' => [
155+
'input' => "@import 'more-styles.css'",
156+
'expectedOutput' => '@import "more-styles.abcd123.css"',
157+
'expectedDependencies' => ['more-styles.css'],
158+
];
159+
160+
yield 'import_without_url_with_media_query' => [
161+
'input' => '@import "more-styles.css" screen',
162+
'expectedOutput' => '@import "more-styles.abcd123.css" screen',
163+
'expectedDependencies' => ['more-styles.css'],
164+
];
165+
166+
yield 'import_without_url_with_media_query_single_quotes' => [
167+
'input' => "@import 'more-styles.css' screen and (max-width: 600px)",
168+
'expectedOutput' => '@import "more-styles.abcd123.css" screen and (max-width: 600px)',
169+
'expectedDependencies' => ['more-styles.css'],
170+
];
171+
172+
yield 'import_without_url_with_dot_slash' => [
173+
'input' => '@import "./more-styles.css"',
174+
'expectedOutput' => '@import "more-styles.abcd123.css"',
175+
'expectedDependencies' => ['more-styles.css'],
176+
];
177+
178+
yield 'import_without_url_with_dot_dot_slash' => [
179+
'input' => '@import "../assets/more-styles.css"',
180+
'expectedOutput' => '@import "more-styles.abcd123.css"',
181+
'expectedDependencies' => ['more-styles.css'],
182+
];
183+
184+
yield 'import_without_url_absolute_path_ignored' => [
185+
'input' => '@import "/path/to/more-styles.css"',
186+
'expectedOutput' => '@import "/path/to/more-styles.css"',
187+
'expectedDependencies' => [],
188+
];
189+
190+
yield 'import_without_url_http_url_ignored' => [
191+
'input' => '@import "https://cdn.io/more-styles.css"',
192+
'expectedOutput' => '@import "https://cdn.io/more-styles.css"',
193+
'expectedDependencies' => [],
194+
];
195+
196+
yield 'import_without_url_data_uri_ignored' => [
197+
'input' => '@import "data:text/css;base64,LmJvZHkgeyBiYWNrZ3JvdW5kOiByZWQ7IH0="',
198+
'expectedOutput' => '@import "data:text/css;base64,LmJvZHkgeyBiYWNrZ3JvdW5kOiByZWQ7IH0="',
199+
'expectedDependencies' => [],
200+
];
201+
202+
yield 'import_without_url_in_comments_ignored' => [
203+
'input' => 'body { background: url("images/foo.png"); } /* @import "more-styles.css" */',
204+
'expectedOutput' => 'body { background: url("images/foo.123456.png"); } /* @import "more-styles.css" */',
205+
'expectedDependencies' => ['images/foo.png'],
206+
];
207+
208+
yield 'mixed_url_and_import_without_url' => [
209+
'input' => <<<EOF
210+
@import "more-styles.css";
211+
body { background: url("images/foo.png"); }
212+
@import url(./more-styles.css);
213+
EOF
214+
,
215+
'expectedOutput' => <<<EOF
216+
@import "more-styles.abcd123.css";
217+
body { background: url("images/foo.123456.png"); }
218+
@import url("more-styles.abcd123.css");
219+
EOF
220+
,
221+
'expectedDependencies' => ['more-styles.css', 'images/foo.png', 'more-styles.css'],
222+
];
147223
}
148224

149225
public function testCompileFindsRelativeFilesViaSourcePath()
@@ -223,5 +299,23 @@ public static function provideStrictModeTests(): iterable
223299
'input' => "background-image: url(\'data:image/png;base64,iVBORw0KG\')",
224300
'expectedExceptionMessage' => null,
225301
];
302+
303+
yield 'importing_non_existent_file_without_url_throws_exception' => [
304+
'sourceLogicalName' => 'styles.css',
305+
'input' => '@import "non-existent.css"',
306+
'expectedExceptionMessage' => 'Unable to find asset "non-existent.css" referenced in "/path/to/styles.css".',
307+
];
308+
309+
yield 'importing_absolute_file_path_without_url_is_ignored' => [
310+
'sourceLogicalName' => 'styles.css',
311+
'input' => '@import "/path/to/non-existent.css"',
312+
'expectedExceptionMessage' => null,
313+
];
314+
315+
yield 'importing_a_url_without_url_is_ignored' => [
316+
'sourceLogicalName' => 'styles.css',
317+
'input' => '@import "https://cdn.io/non-existent.css"',
318+
'expectedExceptionMessage' => null,
319+
];
226320
}
227321
}

0 commit comments

Comments
 (0)