From: Antoine Musso <hashar@free.fr>
Date: Tue, 17 Jul 2018 15:59:17 +0000 (+0200)
Subject: resourceloader: CSSMin::getLocalFileReferences now strips anchors
X-Git-Tag: 1.34.0-rc.0~4697^2
X-Git-Url: http://git.cyclocoop.org/%28%28?a=commitdiff_plain;h=4030b2cb2b077fc32d79d9bae703be6c9d0be729;p=lhc%2Fweb%2Fwiklou.git

resourceloader: CSSMin::getLocalFileReferences now strips anchors

When processing:

  url( 'foo.svg#anchors' )

getLocalFileReferences() would yield a file 'foo.svg#anchors'. It might
occur with upstream libraries, such as WikiFonts used by the Refreshed
skin.

When a path is found, strip the anchor entirely. Skip the case if that
results in an empty file.

remap() would properly skip the behavior "#default#', also skip url that
are solely an anchor such as '#other'.

Add a few test cases to cover CSSMin::getLocalFileReferences and cover
anchors usage in CSSMin::remap.

Bug: T115436
Change-Id: I1749ddc2b89021807f42d64131931ad7a99a7b43
---

diff --git a/includes/libs/CSSMin.php b/includes/libs/CSSMin.php
index 74e8b54e2a..92a4f9e6f4 100644
--- a/includes/libs/CSSMin.php
+++ b/includes/libs/CSSMin.php
@@ -72,15 +72,24 @@ class CSSMin {
 				$url = $match['file'][0];
 
 				// Skip fully-qualified and protocol-relative URLs and data URIs
-				// Also skips the rare `behavior` property specifying application's default behavior
 				if (
 					substr( $url, 0, 2 ) === '//' ||
-					parse_url( $url, PHP_URL_SCHEME ) ||
-					substr( $url, 0, 9 ) === '#default#'
+					parse_url( $url, PHP_URL_SCHEME )
 				) {
 					break;
 				}
 
+				// Strip trailing anchors - T115436
+				$anchor = strpos( $url, '#' );
+				if ( $anchor !== false ) {
+					$url = substr( $url, 0, $anchor );
+
+					// '#some-anchors' is not a file
+					if ( $url === '' ) {
+						break;
+					}
+				}
+
 				$files[] = $path . $url;
 			}
 		}
@@ -485,11 +494,11 @@ class CSSMin {
 
 		// Pass thru fully-qualified and protocol-relative URLs and data URIs, as well as local URLs if
 		// we can't expand them.
-		// Also skips the rare `behavior` property specifying application's default behavior
+		// Also skips anchors or the rare `behavior` property specifying application's default behavior
 		if (
 			self::isRemoteUrl( $url ) ||
 			self::isLocalUrl( $url ) ||
-			substr( $url, 0, 9 ) === '#default#'
+			substr( $url, 0, 1 ) === '#'
 		) {
 			return $url;
 		}
diff --git a/tests/phpunit/includes/libs/CSSMinTest.php b/tests/phpunit/includes/libs/CSSMinTest.php
index c711291483..04aecc9086 100644
--- a/tests/phpunit/includes/libs/CSSMinTest.php
+++ b/tests/phpunit/includes/libs/CSSMinTest.php
@@ -19,6 +19,30 @@ class CSSMinTest extends MediaWikiTestCase {
 		] );
 	}
 
+	/**
+	 * @dataProvider providesReferencedFiles
+	 * @covers CSSMin::getLocalFileReferences
+	 */
+	public function testGetLocalFileReferences( $input, $expected ) {
+		$output = CSSMin::getLocalFileReferences( $input, '/' );
+		$this->assertEquals(
+			$expected,
+			$output,
+			'getLocalFileReferences() must find the local file properly'
+		);
+	}
+
+	public static function providesReferencedFiles() {
+		// input, array of expected local file names
+		return [
+			[ 'url("//example.org")', [] ],
+			[ 'url("https://example.org")', [] ],
+			[ 'url("#default#")', [] ],
+			[ 'url("WikiFont-Glyphs.svg#wikiglyph")', [ '/WikiFont-Glyphs.svg' ] ],
+			[ 'url("#some-anchor")', [] ],
+		];
+	}
+
 	/**
 	 * @dataProvider provideSerializeStringValue
 	 * @covers CSSMin::serializeStringValue
@@ -292,6 +316,16 @@ class CSSMinTest extends MediaWikiTestCase {
 				[ 'foo { behavior: url(#default#bar); }', false, '/w/', false ],
 				'foo { behavior: url("#default#bar"); }',
 			],
+			[
+				'Keeps anchors',
+				[ 'url(#other)', false, '/', false ],
+				'url("#other")'
+			],
+			[
+				'Keeps anchors after a path',
+				[ 'url(images/file.svg#id)', false, '/', false ],
+				'url("/images/file.svg#id")'
+			],
 		];
 	}