From 3b1b2b0c33714e5768421ea4a87a97aa2ff8717a Mon Sep 17 00:00:00 2001 From: Stephan Gambke Date: Thu, 12 Jun 2014 23:55:33 +0200 Subject: [PATCH] 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 --- includes/libs/CSSMin.php | 30 ++++++++++++++++++- tests/phpunit/includes/libs/CSSMinTest.php | 35 ++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/includes/libs/CSSMin.php b/includes/libs/CSSMin.php index 10277e6e31..6b10ae46bf 100644 --- 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[^\?\)\'"]*?)(?P\?[^\)\'"]*?|)[\'"]?\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 bb5e398c6a..2b4d60d9a0 100644 --- 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}*/; }', + ), ); } -- 2.20.1