From 3987b10b51ac76d18b82f94251c8fae0f6d469f5 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Bartosz=20Dziewo=C5=84ski?= <matma.rex@gmail.com>
Date: Wed, 11 Dec 2013 20:11:44 +0100
Subject: [PATCH] CSSMin: Fix remapOne() for URLs that are proto-relative or
 have query part

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

diff --git a/includes/libs/CSSMin.php b/includes/libs/CSSMin.php
index 68e30ebaaf..a2c63a108f 100644
--- a/includes/libs/CSSMin.php
+++ b/includes/libs/CSSMin.php
@@ -168,7 +168,8 @@ class CSSMin {
 
 		// 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 @@ class CSSMin {
 	 * @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 @@ class CSSMin {
 			// 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 5bbc3a52db..239cdf6ef7 100644
--- a/tests/phpunit/includes/libs/CSSMinTest.php
+++ b/tests/phpunit/includes/libs/CSSMinTest.php
@@ -153,6 +153,31 @@ class CSSMinTest extends MediaWikiTestCase {
 				'foo { background: url(http://example.org/w/foo.png); }',
 				'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); }',
@@ -218,6 +243,16 @@ class CSSMinTest extends MediaWikiTestCase {
 				'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)',
+			),
 		);
 	}
 
-- 
2.20.1