Fix CSSMin url() remapping when comments in CSS contain curly braces

Remapping of url()s was thrown off if the CSS contained comments
with curly braces. To fix, replace all block comments (except @embed
directives) with placeholders before url() processing and swap them
back in afterwards. Patch also includes tests.

Bug: 60077
Change-Id: If18a93c17ea9bcd529f6a664aa2dcc51d4a37f38

diff --git a/includes/libs/CSSMin.php b/includes/libs/CSSMin.php
index 10277e6..6b10ae4 100644 (file)
--- a/includes/libs/CSSMin.php
+++ b/includes/libs/CSSMin.php
@@ -40,6 +40,7 @@ class CSSMin {
        const EMBED_SIZE_LIMIT = 24576;
        const URL_REGEX = 'url\(\s*[\'"]?(?P<file>[^\?\)\'"]*?)(?P<query>\?[^\)\'"]*?|)[\'"]?\s*\)';
        const EMBED_REGEX = '\/\*\s*\@embed\s*\*\/';
+       const COMMENT_REGEX = '\/\*.*?\*\/';
 
        /* Protected Static Members */
 
@@ -203,13 +204,31 @@ class CSSMin {
                        $remote = substr( $remote, 0, -1 );
                }
 
+               // Replace all comments by a placeholder so they will not interfere
+               // with the remapping
+               // Warning: This will also catch on anything looking like the start of
+               // a comment between quotation marks (e.g. "foo /* bar").
+               $comments = array();
+               $placeholder = uniqid( '', true );
+
+               $pattern = '/(?!' . CSSMin::EMBED_REGEX . ')(' . CSSMin::COMMENT_REGEX . ')/s';
+
+               $source = preg_replace_callback(
+                       $pattern,
+                       function ( $match ) use ( &$comments, $placeholder ) {
+                               $comments[] = $match[ 0 ];
+                               return $placeholder . ( count( $comments ) - 1 ) . 'x';
+                       },
+                       $source
+               );
+
                // Note: This will not correctly handle cases where ';', '{' or '}'
                // appears in the rule itself, e.g. in a quoted string. You are advised
                // not to use such characters in file names. We also match start/end of
                // the string to be consistent in edge-cases ('@import url(…)').
                $pattern = '/(?:^|[;{])\K[^;{}]*' . CSSMin::URL_REGEX . '[^;}]*(?=[;}]|$)/';
 
-               return preg_replace_callback(
+               $source = preg_replace_callback(
                        $pattern,
                        function ( $matchOuter ) use ( $local, $remote, $embedData ) {
                                $rule = $matchOuter[0];
@@ -262,6 +281,15 @@ class CSSMin {
                                        return $ruleWithRemapped;
                                }
                        }, $source );
+
+               // Re-insert comments
+               $pattern = '/' . $placeholder . '(\d+)x/';
+               $source = preg_replace_callback( $pattern, function( $match ) use ( &$comments ) {
+                       return $comments[ $match[1] ];
+               }, $source );
+
+               return $source;
+
        }
 
        /**

diff --git a/tests/phpunit/includes/libs/CSSMinTest.php b/tests/phpunit/includes/libs/CSSMinTest.php
index bb5e398..2b4d60d 100644 (file)
--- a/tests/phpunit/includes/libs/CSSMinTest.php
+++ b/tests/phpunit/includes/libs/CSSMinTest.php
@@ -291,6 +291,41 @@ class CSSMinTest extends MediaWikiTestCase {
                                '@import url(//localhost/styles.css?query=yes)',
                                '@import url(//localhost/styles.css?query=yes)',
                        ),
+                       array(
+                               'Simple case with comments before url',
+                               'foo { prop: /* some {funny;} comment */ url(bar.png); }',
+                               'foo { prop: /* some {funny;} comment */ url(http://localhost/w/bar.png); }',
+                       ),
+                       array(
+                               'Simple case with comments after url',
+                               'foo { prop: url(red.gif)/* some {funny;} comment */ ; }',
+                               'foo { prop: url(http://localhost/w/red.gif?timestamp)/* some {funny;} comment */ ; }',
+                       ),
+                       array(
+                               'Embedded file with comment before url',
+                               'foo { /* @embed */ background: /* some {funny;} comment */ url(red.gif); }',
+                               "foo { background: /* some {funny;} comment */ url($red); background: /* some {funny;} comment */ url(http://localhost/w/red.gif?timestamp)!ie; }",
+                       ),
+                       array(
+                               'Embedded file with comments inside and outside the rule',
+                               'foo { /* @embed */ background: url(red.gif) /* some {foo;} comment */; /* some {bar;} comment */ }',
+                               "foo { background: url($red) /* some {foo;} comment */; background: url(http://localhost/w/red.gif?timestamp) /* some {foo;} comment */!ie; /* some {bar;} comment */ }",
+                       ),
+                       array(
+                               'Embedded file with comment outside the rule',
+                               'foo { /* @embed */ background: url(red.gif); /* some {funny;} comment */ }',
+                               "foo { background: url($red); background: url(http://localhost/w/red.gif?timestamp)!ie; /* some {funny;} comment */ }",
+                       ),
+                       array(
+                               'Rule with two urls, each with comments',
+                               '{ background: /*asd*/ url(something.png); background: /*jkl*/ url(something.png); }',
+                               '{ background: /*asd*/ url(http://localhost/w/something.png); background: /*jkl*/ url(http://localhost/w/something.png); }',
+                       ),
+                       array(
+                               'Sanity check for offending line from jquery.ui.theme.css (bug 60077)',
+                               '.ui-state-default, .ui-widget-content .ui-state-default, .ui-widget-header .ui-state-default { border: 1px solid #d3d3d3/*{borderColorDefault}*/; background: #e6e6e6/*{bgColorDefault}*/ url(images/ui-bg_glass_75_e6e6e6_1x400.png)/*{bgImgUrlDefault}*/ 50%/*{bgDefaultXPos}*/ 50%/*{bgDefaultYPos}*/ repeat-x/*{bgDefaultRepeat}*/; font-weight: normal/*{fwDefault}*/; color: #555555/*{fcDefault}*/; }',
+                               '.ui-state-default, .ui-widget-content .ui-state-default, .ui-widget-header .ui-state-default { border: 1px solid #d3d3d3/*{borderColorDefault}*/; background: #e6e6e6/*{bgColorDefault}*/ url(http://localhost/w/images/ui-bg_glass_75_e6e6e6_1x400.png)/*{bgImgUrlDefault}*/ 50%/*{bgDefaultXPos}*/ 50%/*{bgDefaultYPos}*/ repeat-x/*{bgDefaultRepeat}*/; font-weight: normal/*{fwDefault}*/; color: #555555/*{fcDefault}*/; }',
+                       ),
                );
        }