Bartosz Dziewoński has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/100824


Change subject: CSSMin: Fix remapOne() for URLs that are proto-relative or have 
query part
......................................................................

CSSMin: Fix remapOne() for URLs that are proto-relative or have query part

Bug: 58338
Change-Id: I836a2c054ae3edc07895b2388f4ec8663223347a
---
M includes/libs/CSSMin.php
M tests/phpunit/includes/libs/CSSMinTest.php
2 files changed, 54 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/24/100824/5

diff --git a/includes/libs/CSSMin.php b/includes/libs/CSSMin.php
index 68e30eb..a2c63a1 100644
--- a/includes/libs/CSSMin.php
+++ b/includes/libs/CSSMin.php
@@ -168,7 +168,8 @@
 
                // 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.
-               $pattern = '/[;{]\K[^;}]*' . CSSMin::URL_REGEX . 
'[^;}]*(?=[;}])/';
+               // 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( $pattern, function ( $matchOuter 
) use ( $local, $remote, $embedData ) {
                        $rule = $matchOuter[0];
 
@@ -218,36 +219,40 @@
         * @return string Remapped/embedded URL data
         */
        public static function remapOne( $file, $query, $local, $remote, $embed 
) {
-               // Skip fully-qualified URLs and data URIs
-               $urlScheme = parse_url( $file, PHP_URL_SCHEME );
+               // The full URL possibly with query, as passed to the 'url()' 
value in CSS
+               $url = $file . $query;
+
+               // Skip fully-qualified and protocol-relative URLs and data URIs
+               $urlScheme = substr( $url, 0, 2 ) === '//' || parse_url( $url, 
PHP_URL_SCHEME );
                if ( $urlScheme ) {
-                       return $file;
+                       return $url;
                }
 
                // URLs with absolute paths like /w/index.php need to be 
expanded
                // to absolute URLs but otherwise left alone
-               if ( $file !== '' && $file[0] === '/' ) {
+               if ( $url !== '' && $url[0] === '/' ) {
                        // Replace the file path with an expanded (possibly 
protocol-relative) URL
                        // ...but only if wfExpandUrl() is even available.
                        // This will not be the case if we're running outside 
of MW
                        if ( function_exists( 'wfExpandUrl' ) ) {
-                               return wfExpandUrl( $file, PROTO_RELATIVE );
+                               return wfExpandUrl( $url, PROTO_RELATIVE );
                        } else {
-                               return $file;
+                               return $url;
                        }
                }
 
+               // We drop the query part here and instead make the path 
relative to $remote
                $url = "{$remote}/{$file}";
-               $file = "{$local}/{$file}";
+               // Path to the actual file on the filesystem
+               $localFile = "{$local}/{$file}";
 
-               $replacement = false;
+               if ( $local !== false && file_exists( $localFile ) ) {
 
-               if ( $local !== false && file_exists( $file ) ) {
                        // Add version parameter as a time-stamp in ISO 8601 
format,
                        // using Z for the timezone, meaning GMT
-                       $url .= '?' . gmdate( 'Y-m-d\TH:i:s\Z', round( 
filemtime( $file ), -2 ) );
+                       $url .= '?' . gmdate( 'Y-m-d\TH:i:s\Z', round( 
filemtime( $localFile ), -2 ) );
                        if ( $embed ) {
-                               $data = self::encodeImageAsDataURI( $file );
+                               $data = self::encodeImageAsDataURI( $localFile 
);
                                if ( $data !== false ) {
                                        return $data;
                                } else {
@@ -261,7 +266,8 @@
                        // Assume that all paths are relative to $remote, and 
make them absolute
                        return $url . $query;
                } else {
-                       return $file;
+                       // $local is truthy, but the file is missing
+                       return $localFile;
                }
        }
 
diff --git a/tests/phpunit/includes/libs/CSSMinTest.php 
b/tests/phpunit/includes/libs/CSSMinTest.php
index 5bbc3a5..239cdf6 100644
--- a/tests/phpunit/includes/libs/CSSMinTest.php
+++ b/tests/phpunit/includes/libs/CSSMinTest.php
@@ -154,6 +154,31 @@
                                'foo { background: 
url(http://example.org/w/foo.png); }',
                        ),
                        array(
+                               'Protocol-relative remote URL',
+                               'foo { background: 
url(//example.org/w/foo.png); }',
+                               'foo { background: 
url(//example.org/w/foo.png); }',
+                       ),
+                       array(
+                               'Remote URL with query',
+                               'foo { background: 
url(http://example.org/w/foo.png?query=yes); }',
+                               'foo { background: 
url(http://example.org/w/foo.png?query=yes); }',
+                       ),
+                       array(
+                               'Protocol-relative remote URL with query',
+                               'foo { background: 
url(//example.org/w/foo.png?query=yes); }',
+                               'foo { background: 
url(//example.org/w/foo.png?query=yes); }',
+                       ),
+                       array(
+                               'Domain-relative URL',
+                               'foo { background: url(/static/foo.png); }',
+                               'foo { background: 
url(http://doc.example.org/static/foo.png); }',
+                       ),
+                       array(
+                               'Domain-relative URL with query',
+                               'foo { background: 
url(/static/foo.png?query=yes); }',
+                               'foo { background: 
url(http://doc.example.org/static/foo.png?query=yes); }',
+                       ),
+                       array(
                                'Embedded file',
                                'foo { /* @embed */ background: url(red.gif); 
}',
                                "foo { background: url($red); background: 
url(http://localhost/w/red.gif?timestamp)!ie; }",
@@ -218,6 +243,16 @@
                                'foo { background: url( red.gif ); }',
                                'foo { background: 
url(http://localhost/w/red.gif?timestamp); }',
                        ),
+                       array(
+                               '@import rule to local file (should we remap 
this?)',
+                               '@import url(/styles.css)',
+                               '@import 
url(http://doc.example.org/styles.css)',
+                       ),
+                       array(
+                               '@import rule to URL (should we remap this?)',
+                               '@import url(//localhost/styles.css?query=yes)',
+                               '@import url(//localhost/styles.css?query=yes)',
+                       ),
                );
        }
 

-- 
To view, visit https://gerrit.wikimedia.org/r/100824
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I836a2c054ae3edc07895b2388f4ec8663223347a
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Bartosz Dziewoński <matma....@gmail.com>
Gerrit-Reviewer: Bartosz Dziewoński <matma....@gmail.com>
Gerrit-Reviewer: Catrope <roan.katt...@gmail.com>
Gerrit-Reviewer: jenkins-bot

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to