Merge "CSSMin: Don't match empty string as remappable url"

diff --git a/includes/libs/CSSMin.php b/includes/libs/CSSMin.php
index 3d1c8b8..1cbcbde 100644 (file)
--- a/includes/libs/CSSMin.php
+++ b/includes/libs/CSSMin.php
@@ -424,11 +424,11 @@ class CSSMin {
                        //   is only supported in PHP 5.6. Use a getter method for now.
                        $urlRegex = '(' .
                                // Unquoted url
-                               'url\(\s*(?P<file0>[^\'"][^\?\)]*?)(?P<query0>\?[^\)]*?|)\s*\)' .
+                               'url\(\s*(?P<file0>[^\'"][^\?\)]+?)(?P<query0>\?[^\)]*?|)\s*\)' .
                                // Single quoted url
-                               '|url\(\s*\'(?P<file1>[^\?\']*?)(?P<query1>\?[^\']*?|)\'\s*\)' .
+                               '|url\(\s*\'(?P<file1>[^\?\']+?)(?P<query1>\?[^\']*?|)\'\s*\)' .
                                // Double quoted url
-                               '|url\(\s*"(?P<file2>[^\?"]*?)(?P<query2>\?[^"]*?|)"\s*\)' .
+                               '|url\(\s*"(?P<file2>[^\?"]+?)(?P<query2>\?[^"]*?|)"\s*\)' .
                                ')';
                }
                return $urlRegex;
@@ -446,6 +446,9 @@ class CSSMin {
                                $match['file'] = $match['file1'];
                                $match['query'] = $match['query1'];
                        } else {
+                               if ( !isset( $match['file2'] ) || $match['file2'][1] === -1 ) {
+                                       throw new Exception( 'URL must be non-empty' );
+                               }
                                $match['file'] = $match['file2'];
                                $match['query'] = $match['query2'];
                        }
@@ -457,6 +460,9 @@ class CSSMin {
                                $match['file'] = $match['file1'];
                                $match['query'] = $match['query1'];
                        } else {
+                               if ( !isset( $match['file2'] ) || $match['file2'] === '' ) {
+                                       throw new Exception( 'URL must be non-empty' );
+                               }
                                $match['file'] = $match['file2'];
                                $match['query'] = $match['query2'];
                        }

diff --git a/tests/phpunit/includes/libs/CSSMinTest.php b/tests/phpunit/includes/libs/CSSMinTest.php
index 667eb0a..f2d5ef3 100644 (file)
--- a/tests/phpunit/includes/libs/CSSMinTest.php
+++ b/tests/phpunit/includes/libs/CSSMinTest.php
@@ -253,6 +253,36 @@ class CSSMinTest extends MediaWikiTestCase {
                ];
        }
 
+       /**
+        * Cases with empty url() for CSSMin::remap.
+        *
+        * Regression test for T191237.
+   *
+        * @dataProvider provideRemapEmptyUrl
+        * @covers CSSMin
+        */
+       public function testRemapEmptyUrl( $params, $expected ) {
+               $remapped = call_user_func_array( 'CSSMin::remap', $params );
+               $this->assertEquals( $expected, $remapped, 'Ignore empty url' );
+       }
+
+       public static function provideRemapEmptyUrl() {
+               return [
+                       'Empty' => [
+                               [ "background-image: url();", false, '/example', false ],
+                               "background-image: url();",
+                       ],
+                       'Single quote' => [
+                               [ "background-image: url('');", false, '/example', false ],
+                               "background-image: url('');",
+                       ],
+                       'Double quote' => [
+                               [ 'background-image: url("");', false, '/example', false ],
+                               'background-image: url("");',
+                       ],
+               ];
+       }
+
        /**
         * This tests the basic functionality of CSSMin::remap.
         *