CSSMin: Correctly format 'url()' values with parentheses etc.

Introduce new static function, CSSMin::buildUrlValue.

Actually using such values in CSS does not work well because the
URL_REGEX is nowhere near good enough. :(

Change-Id: I04a7078dd0087bcb461fa5e5168c870d37c255f4

diff --git a/includes/libs/CSSMin.php b/includes/libs/CSSMin.php
index fd5bca4..3c84472 100644 (file)
--- a/includes/libs/CSSMin.php
+++ b/includes/libs/CSSMin.php
@@ -140,6 +140,24 @@ class CSSMin {
                return false;
        }
 
+       /**
+        * Build a CSS 'url()' value for the given URL, quoting parentheses (and other funny characters)
+        * and escaping quotes as necessary.
+        *
+        * @param string $url URL to process
+        * @return string 'url()' value, usually just `"url($url)"`, quoted/escaped if necessary
+        */
+       public static function buildUrlValue( $url ) {
+               // The list below has been crafted to match URLs such as:
+               //   scheme://user@domain:port/~user/fi%20le.png?query=yes&really=y+s
+               //   data:image/png;base64,R0lGODlh/+==
+               if ( preg_match( '!^[\w\d:@/~.%+;,?&=-]+$!', $url ) ) {
+                       return "url($url)";
+               } else {
+                       return 'url("' . strtr( $url, array( '\\' => '\\\\', '"' => '\\"' ) ) . '")';
+               }
+       }
+
        /**
         * Remaps CSS URL paths and automatically embeds data URIs for CSS rules or url() values
         * preceded by an / * @embed * / comment.
@@ -182,14 +200,14 @@ class CSSMin {
 
                        $ruleWithRemapped = preg_replace_callback( $pattern, function ( $match ) use ( $local, $remote ) {
                                $remapped = CSSMin::remapOne( $match['file'], $match['query'], $local, $remote, false );
-                               return "url({$remapped})";
+                               return CSSMin::buildUrlValue( $remapped );
                        }, $rule );
 
                        if ( $embedData ) {
                                $ruleWithEmbedded = preg_replace_callback( $pattern, function ( $match ) use ( $embedAll, $local, $remote ) {
                                        $embed = $embedAll || $match['embed'];
                                        $embedded = CSSMin::remapOne( $match['file'], $match['query'], $local, $remote, $embed );
-                                       return "url({$embedded})";
+                                       return CSSMin::buildUrlValue( $embedded );
                                }, $rule );
                        }
 

diff --git a/tests/phpunit/includes/libs/CSSMinTest.php b/tests/phpunit/includes/libs/CSSMinTest.php
index 94ebe60..5c0487b 100644 (file)
--- a/tests/phpunit/includes/libs/CSSMinTest.php
+++ b/tests/phpunit/includes/libs/CSSMinTest.php
@@ -183,6 +183,11 @@ class CSSMinTest extends MediaWikiTestCase {
                                'foo { background: url(/static/foo.png?query=yes); }',
                                'foo { background: url(http://doc.example.org/static/foo.png?query=yes); }',
                        ),
+                       array(
+                               'Remote URL (unnecessary quotes not preserved)',
+                               'foo { background: url("http://example.org/w/foo.png"); }',
+                               'foo { background: url(http://example.org/w/foo.png); }',
+                       ),
                        array(
                                'Embedded file',
                                'foo { /* @embed */ background: url(red.gif); }',
@@ -261,6 +266,45 @@ class CSSMinTest extends MediaWikiTestCase {
                );
        }
 
+       /**
+        * This tests basic functionality of CSSMin::buildUrlValue.
+        *
+        * @dataProvider provideBuildUrlValueCases
+        * @covers CSSMin::buildUrlValue
+        */
+       public function testBuildUrlValue( $message, $input, $expectedOutput ) {
+               $this->assertEquals(
+                       $expectedOutput,
+                       CSSMin::buildUrlValue( $input ),
+                       "CSSMin::buildUrlValue: $message"
+               );
+       }
+
+       public static function provideBuildUrlValueCases() {
+               return array(
+                       array(
+                               'Full URL',
+                               'scheme://user@domain:port/~user/fi%20le.png?query=yes&really=y+s',
+                               'url(scheme://user@domain:port/~user/fi%20le.png?query=yes&really=y+s)',
+                       ),
+                       array(
+                               'data: URI',
+                               'data:image/png;base64,R0lGODlh/+==',
+                               'url(data:image/png;base64,R0lGODlh/+==)',
+                       ),
+                       array(
+                               'URL with quotes',
+                               "https://en.wikipedia.org/wiki/Wendy's",
+                               "url(\"https://en.wikipedia.org/wiki/Wendy's\")",
+                       ),
+                       array(
+                               'URL with parentheses',
+                               'https://en.wikipedia.org/wiki/Boston_(band)',
+                               'url("https://en.wikipedia.org/wiki/Boston_(band)")',
+                       ),
+               );
+       }
+
        /**
         * Seperated because they are currently broken (bug 35492)
         *