Also simplify external links paths

For normal files, @import & url() syntax is unified already (e.g. removing quotes around the path, where not needed), but this was not done for external paths. This also simplifies the huge regex a little
matthiasmullie · Jan 25, 2017 · cd15113 · cd15113
1 parent bc46e8b
commit cd15113
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 39 deletions.
diff --git a/src/CSS.php b/src/CSS.php
@@ -122,10 +122,10 @@ protected function combineImports($source, $content, $parents)
                         # fetch path
                         (?P<path>
 
-                            # do not fetch data uris or external sources
+                            # do not fetch data uris, external sources or absolute paths
                             (?!(
                                 ["\']?
-                                (data|https?):
+                                (data:|https?:\\/\\/|\\/)
                             ))
 
                             .+?
@@ -166,10 +166,10 @@ protected function combineImports($source, $content, $parents)
                     # fetch path
                     (?P<path>
 
-                        # do not fetch data uris or external sources
+                        # do not fetch data uris, external sources or absolute paths
                         (?!(
                             ["\']?
-                            (data|https?):
+                            (data:|https?:\\/\\/|\\/)
                         ))
 
                         .+?
@@ -380,17 +380,7 @@ protected function move(Converter $converter, $content)
                 (?P<quotes>["\'])?
 
                     # fetch path
-                    (?P<path>
-
-                        # do not fetch data uris or external sources
-                        (?!(
-                            \s?
-                            ["\']?
-                            (data|https?):
-                        ))
-
-                        .+?
-                    )
+                    (?P<path>.+?)
 
                 # close path enclosure
                 (?(quotes)(?P=quotes))
@@ -417,16 +407,7 @@ protected function move(Converter $converter, $content)
                 (?P<quotes>["\'])
 
                     # fetch path
-                    (?P<path>
-
-                        # do not fetch data uris or external sources
-                        (?!(
-                            ["\']?
-                            (data|https?):
-                        ))
-
-                        .+?
-                    )
+                    (?P<path>.+?)
 
                 # close path enclosure
                 (?P=quotes)
@@ -450,29 +431,30 @@ protected function move(Converter $converter, $content)
             // determine if it's a url() or an @import match
             $type = (strpos($match[0], '@import') === 0 ? 'import' : 'url');
 
-            // attempting to interpret GET-params makes no sense, so let's discard them for awhile
-            $params = strrchr($match['path'], '?');
-            $url = $params ? substr($match['path'], 0, -strlen($params)) : $match['path'];
+            $url = $match['path'];
+            if ($this->canImportByPath($url)) {
+                // attempting to interpret GET-params makes no sense, so let's discard them for awhile
+                $params = strrchr($url, '?');
+                $url = $params ? substr($url, 0, -strlen($params)) : $url;
 
-            // fix relative url
-            $url = $converter->convert($url);
+                // fix relative url
+                $url = $converter->convert($url);
 
-            // now that the path has been converted, re-apply GET-params
-            $url .= $params;
+                // now that the path has been converted, re-apply GET-params
+                $url .= $params;
+            }
 
             // build replacement
             $search[] = $match[0];
-            if ($type == 'url') {
+            if ($type === 'url') {
                 $replace[] = 'url('.$url.')';
-            } elseif ($type == 'import') {
+            } elseif ($type === 'import') {
                 $replace[] = '@import "'.$url.'"';
             }
         }
 
         // replace urls
-        $content = str_replace($search, $replace, $content);
-
-        return $content;
+        return str_replace($search, $replace, $content);
     }
 
     /**
@@ -671,4 +653,16 @@ protected function canImportBySize($path)
     {
         return ($size = @filesize($path)) && $size <= $this->maxImportSize * 1024;
     }
+
+    /**
+     * Check if file a file can be imported, going by the path.
+     *
+     * @param string $path
+     *
+     * @return bool
+     */
+    protected function canImportByPath($path)
+    {
+        return preg_match('/^(data:|https?:|\\/)/', $path) === 0;
+    }
 }
diff --git a/tests/css/CSSTest.php b/tests/css/CSSTest.php
@@ -504,7 +504,7 @@ public function dataProvider()
 body{
      background: white;
 }',
-            '@import "./css1.css";@import url(\'https://www.google.com/main.css\');body{background:white}',
+            '@import "./css1.css";@import url(https://www.google.com/main.css);body{background:white}',
         );
 
         // https://github.com/matthiasmullie/minify/commit/3253a81d07cd01afcb651e309900d8ad58a052da#commitcomment-19223603
@@ -584,7 +584,7 @@ public function dataProviderPaths()
         $tests[] = array(
             $source.'/issue29.css',
             $target.'/issue29.css',
-            "@import url('http://myurl.de');",
+            "@import url(http://myurl.de);",
         );
 
         // https://github.com/matthiasmullie/minify/issues/38