From c5e68dd2c0c3bbc9742b4411738581741c004ede Mon Sep 17 00:00:00 2001 From: Sergey Shevchenko Date: Mon, 1 Nov 2021 14:16:00 +1300 Subject: [PATCH 1/8] ENH: resolve relative references in CSS files when combining --- src/View/Requirements_Backend.php | 32 +++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/View/Requirements_Backend.php b/src/View/Requirements_Backend.php index 5c5c9ddda..9d6c79494 100644 --- a/src/View/Requirements_Backend.php +++ b/src/View/Requirements_Backend.php @@ -1395,6 +1395,10 @@ MESSAGE throw new InvalidArgumentException("Combined file {$file} does not exist"); } $fileContent = file_get_contents($filePath ?? ''); + if ($type == 'css') { + // resolve relative paths for css files + $fileContent = $this->resolveCSSReferences($fileContent, $file); + } // Use configured minifier if ($minify) { $fileContent = $this->minifier->minify($fileContent, $type, $file); @@ -1421,6 +1425,34 @@ MESSAGE return $combinedURL; } + /** + * Resolves relative paths in CSS files which are lost when combining them + * + * @param string $content + * @param string $filePath + * @return string New content with paths resolved + */ + protected function resolveCSSReferences($content, $filePath) + { + $fileUrl = Injector::inst()->get(ResourceURLGenerator::class)->urlForResource($filePath); + $fileUrlDir = dirname($fileUrl); + $content = preg_replace_callback('#([\.]{1,2}/)+#', function ($a) use ($fileUrlDir) { + $full = $fileUrlDir . '/' . $a[0]; + $full = preg_replace('#/{2,}#', '/', $full); // ensure there's no repeated slashes + while (strpos($full, './') > 0) { + $post = $full; + $post = preg_replace('#([^/\.]+)/\.\./#', '', $post); // erase 'something/../' with the predecessor + $post = preg_replace('#([^/\.]+)/\./#', '\\1/', $post); // erase './' + if ($post == $full) { + break; // nothing changed + } + $full = $post; + } + return $full; + }, $content); + return $content; + } + /** * Given a filename and list of files, generate a new filename unique to these files * From 8370ffc2a0abb469b0e97ef1e38e81e54faf4f8b Mon Sep 17 00:00:00 2001 From: Sergey Shevchenko Date: Tue, 9 Nov 2021 00:40:20 +1300 Subject: [PATCH 2/8] ENH Test for Requirements_Backend::resolveCSSReferences() --- tests/php/View/RequirementsTest.php | 70 +++++++++++++++++++ .../css/deep/deeper/RequirementsTest_p.css | 5 ++ 2 files changed, 75 insertions(+) create mode 100644 tests/php/View/SSViewerTest/public/css/deep/deeper/RequirementsTest_p.css diff --git a/tests/php/View/RequirementsTest.php b/tests/php/View/RequirementsTest.php index 8276294db..d3d357380 100644 --- a/tests/php/View/RequirementsTest.php +++ b/tests/php/View/RequirementsTest.php @@ -77,6 +77,76 @@ class RequirementsTest extends SapphireTest $this->assertStringContainsString('http://www.mydomain.com:3000/test.css', $html, 'Load external with port'); } + public function testResolveCSSReferences() + { + /** @var Requirements_Backend $backend */ + $backend = Injector::inst()->create(Requirements_Backend::class); + $this->setupRequirements($backend); + $backend->setCombinedFilesFolder('_combinedfiles'); + + $backend->combineFiles( + 'RequirementsTest_pc.css', + [ + 'css/RequirementsTest_d.css', + 'css/deep/deeper/RequirementsTest_p.css' + ] + ); + + $backend->includeInHTML(self::$html_template); + + // we get the file path here + $allCSS = $backend->getCSS(); + $this->assertTrue(count($allCSS) == 1, 'only one combined file'); + $files = array_keys($allCSS); + $combinedFileName = $files[0]; + $combinedFileName = str_replace('/' . ASSETS_DIR . '/', '/', $combinedFileName); + + $combinedFilePath = TestAssetStore::base_path() . $combinedFileName; + + /* COMBINED JAVASCRIPT FILE EXISTS */ + $this->assertTrue( + file_exists($combinedFilePath), + 'combined css file exists' + ); + + $content = file_get_contents($combinedFilePath); + + /* COMBINED CSS DECODED ONE DOT OKAY */ + $this->assertStringContainsString( + ".p0 { background: url(/css/deep/deeper/zero.gif); }", + $content, + 'combined css decoded one dot okay' + ); + + /* COMBINED CSS DECODED ONE DOUBLE-DOT OKAY */ + $this->assertStringContainsString( + ".p1 { background: url(/css/deep/one.gif); }", + $content, + 'combined css decoded 1 double-dot okay' + ); + + /* COMBINED CSS DECODED 2 DOUBLE-DOT OKAY */ + $this->assertStringContainsString( + ".p2 { background: url(/css/two.gif); }", + $content, + 'combined css decoded 2 double-dot okay' + ); + + /* COMBINED CSS DECODED 3 DOUBLE-DOT OKAY */ + $this->assertStringContainsString( + ".p3 { background: url(/three.gif); }", + $content, + 'combined css decoded 3 double-dot okay' + ); + + /* COMBINED CSS DECODED 4 DOUBLE-DOT OKAY */ + $this->assertStringContainsString( + ".p4 { background: url(/../four.gif); }", + $content, + 'combined css decoded 4 double-dot okay' + ); + } + /** * Setup new backend * diff --git a/tests/php/View/SSViewerTest/public/css/deep/deeper/RequirementsTest_p.css b/tests/php/View/SSViewerTest/public/css/deep/deeper/RequirementsTest_p.css new file mode 100644 index 000000000..1e1dc91e4 --- /dev/null +++ b/tests/php/View/SSViewerTest/public/css/deep/deeper/RequirementsTest_p.css @@ -0,0 +1,5 @@ +.p0 { background: url(./zero.gif); } +.p1 { background: url(../one.gif); } +.p2 { background: url(../../two.gif); } +.p3 { background: url(../../../three.gif); } +.p4 { background: url(../../../../four.gif); } From a2906cd02c01d59e021202289e9835bb3cce3331 Mon Sep 17 00:00:00 2001 From: Sergey Shevchenko Date: Tue, 9 Nov 2021 14:15:12 +1300 Subject: [PATCH 3/8] ENH Requirements_Backend::resolveCSSReferences(): Tests, config, doc, safety. * Changed to ignore absolute paths altogether * Improve tests * Added config flag * Changed docs --- src/View/Requirements_Backend.php | 25 ++- tests/php/View/RequirementsTest.php | 142 ++++++++++++++++-- .../css/deep/deeper/RequirementsTest_p.css | 17 +++ 3 files changed, 168 insertions(+), 16 deletions(-) diff --git a/src/View/Requirements_Backend.php b/src/View/Requirements_Backend.php index 9d6c79494..7b2ad48db 100644 --- a/src/View/Requirements_Backend.php +++ b/src/View/Requirements_Backend.php @@ -52,6 +52,20 @@ class Requirements_Backend */ private static $combine_in_dev = false; + /** + * Determine if relative urls in the combined files should be converted to absolute. + * + * By default combined files will be parsed for relative urls to image/font assets and those + * utls will be changed to absolute to accomodate to the fact that the combined css is placed + * in a totally different folder than the source css files. + * + * Turn this off if you see some unexpected results. + * + * @config + * @var bool + */ + private static $resolve_relative_css_refs = true; + /** * Paths to all required JavaScript files relative to docroot * @@ -1434,10 +1448,15 @@ MESSAGE */ protected function resolveCSSReferences($content, $filePath) { + $doResolving = Config::inst()->get(__CLASS__, 'resolve_relative_css_refs'); + if (!$doResolving) { + return $content; + } $fileUrl = Injector::inst()->get(ResourceURLGenerator::class)->urlForResource($filePath); $fileUrlDir = dirname($fileUrl); - $content = preg_replace_callback('#([\.]{1,2}/)+#', function ($a) use ($fileUrlDir) { - $full = $fileUrlDir . '/' . $a[0]; + $content = preg_replace_callback('#([\s\'"\(])((:?[\.]{1,2}/)+)#', function ($a) use ($fileUrlDir) { + [ $_, $prefix, $match ] = $a; + $full = $fileUrlDir . '/' . $match; $full = preg_replace('#/{2,}#', '/', $full); // ensure there's no repeated slashes while (strpos($full, './') > 0) { $post = $full; @@ -1448,7 +1467,7 @@ MESSAGE } $full = $post; } - return $full; + return $prefix . $full; }, $content); return $content; } diff --git a/tests/php/View/RequirementsTest.php b/tests/php/View/RequirementsTest.php index d3d357380..2728209b3 100644 --- a/tests/php/View/RequirementsTest.php +++ b/tests/php/View/RequirementsTest.php @@ -13,6 +13,7 @@ use Silverstripe\Assets\Dev\TestAssetStore; use SilverStripe\View\Requirements_Backend; use SilverStripe\Core\Manifest\ResourceURLGenerator; use SilverStripe\Control\SimpleResourceURLGenerator; +use SilverStripe\Core\Config\Config; use SilverStripe\View\SSViewer; use SilverStripe\View\ThemeResourceLoader; @@ -77,12 +78,12 @@ class RequirementsTest extends SapphireTest $this->assertStringContainsString('http://www.mydomain.com:3000/test.css', $html, 'Load external with port'); } - public function testResolveCSSReferences() + public function testResolveCSSReferencesDisabled() { /** @var Requirements_Backend $backend */ $backend = Injector::inst()->create(Requirements_Backend::class); $this->setupRequirements($backend); - $backend->setCombinedFilesFolder('_combinedfiles'); + Config::forClass(get_class($backend))->set('resolve_relative_css_refs', false); $backend->combineFiles( 'RequirementsTest_pc.css', @@ -96,7 +97,59 @@ class RequirementsTest extends SapphireTest // we get the file path here $allCSS = $backend->getCSS(); - $this->assertTrue(count($allCSS) == 1, 'only one combined file'); + $this->assertCount( + 1, + $allCSS, + 'only one combined file' + ); + + $files = array_keys($allCSS); + $combinedFileName = $files[0]; + $combinedFileName = str_replace('/' . ASSETS_DIR . '/', '/', $combinedFileName); + + $combinedFilePath = TestAssetStore::base_path() . $combinedFileName; + + $content = file_get_contents($combinedFilePath); + + /* DISABLED COMBINED CSS URL RESOLVER IGNORED ONE DOT */ + $this->assertStringContainsString( + ".p0 { background: url(./zero.gif); }", + $content, + 'disabled combined css url resolver ignored one dot' + ); + + /* DISABLED COMBINED CSS URL RESOLVER IGNORED DOUBLE-DOT */ + $this->assertStringContainsString( + ".p1 { background: url(../one.gif); }", + $content, + 'disabled combined css url resolver ignored double-dot' + ); + } + + public function testResolveCSSReferences() + { + /** @var Requirements_Backend $backend */ + $backend = Injector::inst()->create(Requirements_Backend::class); + $this->setupRequirements($backend); + Config::forClass(get_class($backend))->set('resolve_relative_css_refs', true); + + $backend->combineFiles( + 'RequirementsTest_pc.css', + [ + 'css/RequirementsTest_d.css', + 'css/deep/deeper/RequirementsTest_p.css' + ] + ); + + $backend->includeInHTML(self::$html_template); + + // we get the file path here + $allCSS = $backend->getCSS(); + $this->assertCount( + 1, + $allCSS, + 'only one combined file' + ); $files = array_keys($allCSS); $combinedFileName = $files[0]; $combinedFileName = str_replace('/' . ASSETS_DIR . '/', '/', $combinedFileName); @@ -111,39 +164,102 @@ class RequirementsTest extends SapphireTest $content = file_get_contents($combinedFilePath); - /* COMBINED CSS DECODED ONE DOT OKAY */ + /* COMBINED CSS URL RESOLVER DECODED ONE DOT */ $this->assertStringContainsString( ".p0 { background: url(/css/deep/deeper/zero.gif); }", $content, - 'combined css decoded one dot okay' + 'combined css url resolver decoded one dot' ); - /* COMBINED CSS DECODED ONE DOUBLE-DOT OKAY */ + /* COMBINED CSS URL RESOLVER DECODED ONE DOT WITH SINGLE QUOTES */ + $this->assertStringContainsString( + ".p0sq { background: url('/css/deep/deeper/zero-sq.gif'); }", + $content, + 'combined css url resolver decoded one dot with single quotes' + ); + + /* COMBINED CSS URL RESOLVER DECODED ONE DOT WITH DOUBLE QUOTES */ + $this->assertStringContainsString( + ".p0dq { background: url(\"/css/deep/deeper/zero-dq.gif\"); }", + $content, + 'combined css url resolver decoded one dot with double quotes' + ); + + /* COMBINED CSS URL RESOLVER DECODED ONE DOT WITH DOUBLE QUOTES AND SPACES NEW LINE */ + $this->assertStringContainsString( + "\n \"/css/deep/deeper/zero-dq-nls.gif\"\n", + $content, + 'combined css url resolver decoded one dot with double quotes and spaces new line' + ); + + /* COMBINED CSS URL RESOLVER DECODED ONE DOT WITH DOUBLE QUOTES NEW LINE */ + $this->assertStringContainsString( + "\"/css/deep/deeper/zero-dq-nl.gif\"", + $content, + 'combined css url resolver decoded one dot with double quotes new line' + ); + + /* COMBINED CSS URL RESOLVER DECODED ONE DOT WITH DOUBLE QUOTES NEW LINE WITH SPACES */ + $this->assertStringContainsString( + "\"/css/deep/deeper/zero-dq-nls.gif\"", + $content, + 'combined css url resolver decoded one dot with double quotes new line with spaces' + ); + + /* COMBINED CSS URL RESOLVER DECODED 1 DOUBLE-DOT */ $this->assertStringContainsString( ".p1 { background: url(/css/deep/one.gif); }", $content, - 'combined css decoded 1 double-dot okay' + 'combined css url resolver decoded 1 double-dot' ); - /* COMBINED CSS DECODED 2 DOUBLE-DOT OKAY */ + /* COMBINED CSS URL RESOLVER DECODED 2 DOUBLE-DOT */ $this->assertStringContainsString( ".p2 { background: url(/css/two.gif); }", $content, - 'combined css decoded 2 double-dot okay' + 'combined css url resolver decoded 2 double-dot' ); - /* COMBINED CSS DECODED 3 DOUBLE-DOT OKAY */ + /* COMBINED CSS URL RESOLVER DECODED 2 DOUBLE-DOT SINGLE QUOTES */ + $this->assertStringContainsString( + ".p2sq { background: url('/css/two-sq.gif'); }", + $content, + 'combined css url resolver decoded 2 double-dot single quotes' + ); + + /* COMBINED CSS URL RESOLVER DECODED 2 DOUBLE-DOT DOUBLE QUOTES */ + $this->assertStringContainsString( + ".p2dq { background: url(\"/css/two-dq.gif\"); }", + $content, + 'combined css url resolver decoded 2 double-dot double quotes' + ); + + /* COMBINED CSS URL RESOLVER SHOULD NOT TOUCH ABSOLUTE PATH */ + $this->assertStringContainsString( + ".p2abs { background: url(/foo/bar/../../two-abs.gif); }", + $content, + 'combined css url resolver should not touch absolute path' + ); + + /* COMBINED CSS URL RESOLVER SHOULD NOT TOUCH ABSOLUTE PATH ON NEW LINE */ + $this->assertStringContainsString( + "\n /foo/bar/../../two-abs-ln.gif\n", + $content, + 'combined css url resolver should not touch absolute path on new line' + ); + + /* COMBINED CSS URL RESOLVER DECODED 3 DOUBLE-DOT */ $this->assertStringContainsString( ".p3 { background: url(/three.gif); }", $content, - 'combined css decoded 3 double-dot okay' + 'combined css url resolver decoded 3 double-dot' ); - /* COMBINED CSS DECODED 4 DOUBLE-DOT OKAY */ + /* COMBINED CSS URL RESOLVER DECODED 4 DOUBLE-DOT */ $this->assertStringContainsString( ".p4 { background: url(/../four.gif); }", $content, - 'combined css decoded 4 double-dot okay' + 'combined css url resolver decoded 4 double-dot' ); } diff --git a/tests/php/View/SSViewerTest/public/css/deep/deeper/RequirementsTest_p.css b/tests/php/View/SSViewerTest/public/css/deep/deeper/RequirementsTest_p.css index 1e1dc91e4..291c8ea52 100644 --- a/tests/php/View/SSViewerTest/public/css/deep/deeper/RequirementsTest_p.css +++ b/tests/php/View/SSViewerTest/public/css/deep/deeper/RequirementsTest_p.css @@ -1,5 +1,22 @@ .p0 { background: url(./zero.gif); } +.p0-nl { background: url( + ./zero-nl.gif ); +} +.p0sq { background: url('./zero-sq.gif'); } +.p0dq { background: url("./zero-dq.gif"); } +.p0dq-nl { background: url( +"./zero-dq-nl.gif" +); } +.p0dq-nls { background: url( + "./zero-dq-nls.gif" +); } .p1 { background: url(../one.gif); } .p2 { background: url(../../two.gif); } +.p2abs { background: url(/foo/bar/../../two-abs.gif); } /* don't touch this */ +.p2abs-ln { background: url( + /foo/bar/../../two-abs-ln.gif +); } /* don't touch this */ +.p2sq { background: url('../../two-sq.gif'); } +.p2dq { background: url("../../two-dq.gif"); } .p3 { background: url(../../../three.gif); } .p4 { background: url(../../../../four.gif); } From 9854e48cfc48a9167a12e706979e00d44da6c9f6 Mon Sep 17 00:00:00 2001 From: Sergey Shevchenko Date: Tue, 9 Nov 2021 15:12:04 +1300 Subject: [PATCH 4/8] Update Requirements_Backend.php --- src/View/Requirements_Backend.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/View/Requirements_Backend.php b/src/View/Requirements_Backend.php index 7b2ad48db..aa6015ecd 100644 --- a/src/View/Requirements_Backend.php +++ b/src/View/Requirements_Backend.php @@ -55,8 +55,8 @@ class Requirements_Backend /** * Determine if relative urls in the combined files should be converted to absolute. * - * By default combined files will be parsed for relative urls to image/font assets and those - * utls will be changed to absolute to accomodate to the fact that the combined css is placed + * By default combined files will be parsed for relative URLs to image/font assets and those + * URLs will be changed to absolute to accomodate the fact that the combined css is placed * in a totally different folder than the source css files. * * Turn this off if you see some unexpected results. From bc9a323418e8a0cb199281afd6d540806451a3c5 Mon Sep 17 00:00:00 2001 From: Sergey Shevchenko Date: Tue, 2 Aug 2022 21:39:31 +1200 Subject: [PATCH 5/8] fix: more tests, improved paths detection, readability --- src/View/Requirements_Backend.php | 18 ++++------ tests/php/View/RequirementsTest.php | 34 +++++++++++++++++-- .../css/deep/deeper/RequirementsTest_p.css | 4 +++ 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/View/Requirements_Backend.php b/src/View/Requirements_Backend.php index aa6015ecd..d0bde0851 100644 --- a/src/View/Requirements_Backend.php +++ b/src/View/Requirements_Backend.php @@ -19,6 +19,7 @@ use SilverStripe\Dev\Debug; use SilverStripe\Dev\Deprecation; use SilverStripe\i18n\i18n; use SilverStripe\ORM\FieldType\DBField; +use Symfony\Component\Filesystem\Path as FilesystemPath; class Requirements_Backend { @@ -1454,19 +1455,12 @@ MESSAGE } $fileUrl = Injector::inst()->get(ResourceURLGenerator::class)->urlForResource($filePath); $fileUrlDir = dirname($fileUrl); - $content = preg_replace_callback('#([\s\'"\(])((:?[\.]{1,2}/)+)#', function ($a) use ($fileUrlDir) { - [ $_, $prefix, $match ] = $a; - $full = $fileUrlDir . '/' . $match; - $full = preg_replace('#/{2,}#', '/', $full); // ensure there's no repeated slashes - while (strpos($full, './') > 0) { - $post = $full; - $post = preg_replace('#([^/\.]+)/\.\./#', '', $post); // erase 'something/../' with the predecessor - $post = preg_replace('#([^/\.]+)/\./#', '\\1/', $post); // erase './' - if ($post == $full) { - break; // nothing changed - } - $full = $post; + $content = preg_replace_callback('#(url\([\n\r\s\'"]*)([^\s\)\?\'"]+)#i', function ($match) use ($fileUrlDir) { + [ $_, $prefix, $relativePath ] = $match; + if ($relativePath[0] === '/' || false !== strpos($relativePath, '://')) { + return $prefix . $relativePath; } + $full = FilesystemPath::canonicalize($fileUrlDir . '/' . $relativePath); return $prefix . $full; }, $content); return $content; diff --git a/tests/php/View/RequirementsTest.php b/tests/php/View/RequirementsTest.php index 2728209b3..727d26eb0 100644 --- a/tests/php/View/RequirementsTest.php +++ b/tests/php/View/RequirementsTest.php @@ -164,6 +164,13 @@ class RequirementsTest extends SapphireTest $content = file_get_contents($combinedFilePath); + /* COMBINED CSS URL RESOLVER IGNORE FULL URLS */ + $this->assertStringContainsString( + ".url { background: url(http://example.com/zero.gif); }", + $content, + 'combined css url resolver ignore full urls' + ); + /* COMBINED CSS URL RESOLVER DECODED ONE DOT */ $this->assertStringContainsString( ".p0 { background: url(/css/deep/deeper/zero.gif); }", @@ -171,6 +178,20 @@ class RequirementsTest extends SapphireTest 'combined css url resolver decoded one dot' ); + /* COMBINED CSS URL RESOLVER DECODED NO DOTS */ + $this->assertStringContainsString( + ".p0-plain { background: url(/css/deep/deeper/zero.gif); }", + $content, + 'combined css url resolver decoded no dots' + ); + + /* COMBINED CSS URL RESOLVER DAMAGED A QUERYSTRING */ + $this->assertStringContainsString( + ".p0-qs { background: url(/css/deep/deeper/zero.gif?some=param); }", + $content, + 'combined css url resolver damaged a querystring' + ); + /* COMBINED CSS URL RESOLVER DECODED ONE DOT WITH SINGLE QUOTES */ $this->assertStringContainsString( ".p0sq { background: url('/css/deep/deeper/zero-sq.gif'); }", @@ -255,11 +276,18 @@ class RequirementsTest extends SapphireTest 'combined css url resolver decoded 3 double-dot' ); - /* COMBINED CSS URL RESOLVER DECODED 4 DOUBLE-DOT */ + /* COMBINED CSS URL RESOLVER DECODED 4 DOUBLE-DOT WHEN ONLY 3 LEVELS AVAILABLE*/ $this->assertStringContainsString( - ".p4 { background: url(/../four.gif); }", + ".p4 { background: url(/four.gif); }", $content, - 'combined css url resolver decoded 4 double-dot' + 'combined css url resolver decoded 4 double-dot when only 4 levels available' + ); + + /* COMBINED CSS URL RESOLVER MODIFIED AN ARBITRARY VALUE */ + $this->assertStringContainsString( + ".weird { content: \"./keepme.gif\"; }", + $content, + 'combined css url resolver modified an arbitrary value' ); } diff --git a/tests/php/View/SSViewerTest/public/css/deep/deeper/RequirementsTest_p.css b/tests/php/View/SSViewerTest/public/css/deep/deeper/RequirementsTest_p.css index 291c8ea52..a499064ee 100644 --- a/tests/php/View/SSViewerTest/public/css/deep/deeper/RequirementsTest_p.css +++ b/tests/php/View/SSViewerTest/public/css/deep/deeper/RequirementsTest_p.css @@ -1,4 +1,7 @@ +.url { background: url(http://example.com/zero.gif); } /* don't touch this */ .p0 { background: url(./zero.gif); } +.p0-plain { background: url(zero.gif); } +.p0-qs { background: url(./zero.gif?some=param); } /* keep the query string */ .p0-nl { background: url( ./zero-nl.gif ); } @@ -20,3 +23,4 @@ .p2dq { background: url("../../two-dq.gif"); } .p3 { background: url(../../../three.gif); } .p4 { background: url(../../../../four.gif); } +.weird { content: "./keepme.gif"; } From ebb1601d5de2f782769b297642727649577bac07 Mon Sep 17 00:00:00 2001 From: Sergey Shevchenko Date: Fri, 5 Aug 2022 15:34:59 +1200 Subject: [PATCH 6/8] fix: misc suggested changes * disable resolve_relative_css_refs by default * variable naming * using proper path joiner * test comment typo --- src/View/Requirements_Backend.php | 6 +++--- tests/php/View/RequirementsTest.php | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/View/Requirements_Backend.php b/src/View/Requirements_Backend.php index d0bde0851..1316ae123 100644 --- a/src/View/Requirements_Backend.php +++ b/src/View/Requirements_Backend.php @@ -65,7 +65,7 @@ class Requirements_Backend * @config * @var bool */ - private static $resolve_relative_css_refs = true; + private static $resolve_relative_css_refs = false; /** * Paths to all required JavaScript files relative to docroot @@ -1456,11 +1456,11 @@ MESSAGE $fileUrl = Injector::inst()->get(ResourceURLGenerator::class)->urlForResource($filePath); $fileUrlDir = dirname($fileUrl); $content = preg_replace_callback('#(url\([\n\r\s\'"]*)([^\s\)\?\'"]+)#i', function ($match) use ($fileUrlDir) { - [ $_, $prefix, $relativePath ] = $match; + [ $fullMatch, $prefix, $relativePath ] = $match; if ($relativePath[0] === '/' || false !== strpos($relativePath, '://')) { return $prefix . $relativePath; } - $full = FilesystemPath::canonicalize($fileUrlDir . '/' . $relativePath); + $full = FilesystemPath::canonicalize(FilesystemPath::join($fileUrlDir, $relativePath)); return $prefix . $full; }, $content); return $content; diff --git a/tests/php/View/RequirementsTest.php b/tests/php/View/RequirementsTest.php index 727d26eb0..69144ea5b 100644 --- a/tests/php/View/RequirementsTest.php +++ b/tests/php/View/RequirementsTest.php @@ -280,7 +280,7 @@ class RequirementsTest extends SapphireTest $this->assertStringContainsString( ".p4 { background: url(/four.gif); }", $content, - 'combined css url resolver decoded 4 double-dot when only 4 levels available' + 'combined css url resolver decoded 4 double-dot when only 3 levels available' ); /* COMBINED CSS URL RESOLVER MODIFIED AN ARBITRARY VALUE */ From 6975815513caa81dce15134fe9a3b5c5d12d60e8 Mon Sep 17 00:00:00 2001 From: Sergey Shevchenko Date: Fri, 5 Aug 2022 21:26:45 +1200 Subject: [PATCH 7/8] config: add symfony/filesystem to base dependencies --- composer.json | 1 + 1 file changed, 1 insertion(+) diff --git a/composer.json b/composer.json index 9034f16a4..e71bcc6cc 100644 --- a/composer.json +++ b/composer.json @@ -40,6 +40,7 @@ "swiftmailer/swiftmailer": "^6.2", "symfony/cache": "^3.4 || ^4.0", "symfony/config": "^3.4 || ^4.0", + "symfony/filesystem": "^5.4 || ^6.0", "symfony/translation": "^3.4 || ^4.0", "symfony/yaml": "^3.4 || ^4.0", "php": "^7.4 || ^8.0", From 49948447291782f5ada953470154fe04e46487d7 Mon Sep 17 00:00:00 2001 From: Sergey Shevchenko Date: Fri, 5 Aug 2022 21:27:36 +1200 Subject: [PATCH 8/8] refactor: variable naming in Requirements_Backend::resolveCSSReferences() --- src/View/Requirements_Backend.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/View/Requirements_Backend.php b/src/View/Requirements_Backend.php index 1316ae123..a80d9345d 100644 --- a/src/View/Requirements_Backend.php +++ b/src/View/Requirements_Backend.php @@ -1458,10 +1458,10 @@ MESSAGE $content = preg_replace_callback('#(url\([\n\r\s\'"]*)([^\s\)\?\'"]+)#i', function ($match) use ($fileUrlDir) { [ $fullMatch, $prefix, $relativePath ] = $match; if ($relativePath[0] === '/' || false !== strpos($relativePath, '://')) { - return $prefix . $relativePath; + return $fullMatch; } - $full = FilesystemPath::canonicalize(FilesystemPath::join($fileUrlDir, $relativePath)); - return $prefix . $full; + $substitute = FilesystemPath::canonicalize(FilesystemPath::join($fileUrlDir, $relativePath)); + return $prefix . $substitute; }, $content); return $content; }