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

@@ -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) {
+        $content = preg_replace_callback('#([\s\'"\(])((:?[\.]{1,2}/)+)#', function ($a) use ($fileUrlDir) {
-            $full = $fileUrlDir . '/' . $a[0];
+            [ $_, $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;
     }

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'
         );
     }
 

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); }