From e36dcfa4c6e821bce7ecdcec3000ebf308e587f3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bartosz=20Dziewo=C5=84ski?= Date: Wed, 11 Dec 2013 21:21:36 +0100 Subject: [PATCH] CSSMin: Change behavior for missing files We would previously return the path to the local file on the filesystem, which is useless in all cases and possibly a security issue in some. Now we return the URL at which the file would be accessible had it existed. Also reordered the code around that part to make the control flow clearer and added a test. Change-Id: I1d5befb2ea385ae4d316c5d8c5d1fc092b64c4ff --- includes/libs/CSSMin.php | 45 ++++++++++------------ tests/phpunit/includes/libs/CSSMinTest.php | 5 +++ 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/includes/libs/CSSMin.php b/includes/libs/CSSMin.php index a2c63a108f..fd5bca45aa 100644 --- a/includes/libs/CSSMin.php +++ b/includes/libs/CSSMin.php @@ -241,33 +241,28 @@ class CSSMin { } } - // We drop the query part here and instead make the path relative to $remote - $url = "{$remote}/{$file}"; - // Path to the actual file on the filesystem - $localFile = "{$local}/{$file}"; - - if ( $local !== false && file_exists( $localFile ) ) { - - // 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( $localFile ), -2 ) ); - if ( $embed ) { - $data = self::encodeImageAsDataURI( $localFile ); - if ( $data !== false ) { - return $data; - } else { - return $url; - } - } else { - // Assume that all paths are relative to $remote, and make them absolute - return $url; - } - } elseif ( $local === false ) { + if ( $local === false ) { // Assume that all paths are relative to $remote, and make them absolute - return $url . $query; + return $remote . '/' . $url; } else { - // $local is truthy, but the file is missing - return $localFile; + // We drop the query part here and instead make the path relative to $remote + $url = "{$remote}/{$file}"; + // Path to the actual file on the filesystem + $localFile = "{$local}/{$file}"; + if ( file_exists( $localFile ) ) { + // 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( $localFile ), -2 ) ); + if ( $embed ) { + $data = self::encodeImageAsDataURI( $localFile ); + if ( $data !== false ) { + return $data; + } + } + } + // If any of these conditions failed (file missing, we don't want to embed it + // or it's not embeddable), return the URL (possibly with ?timestamp part) + return $url; } } diff --git a/tests/phpunit/includes/libs/CSSMinTest.php b/tests/phpunit/includes/libs/CSSMinTest.php index 239cdf6ef7..94ebe60f56 100644 --- a/tests/phpunit/includes/libs/CSSMinTest.php +++ b/tests/phpunit/includes/libs/CSSMinTest.php @@ -148,6 +148,11 @@ class CSSMinTest extends MediaWikiTestCase { 'foo { background: url(red.gif); }', 'foo { background: url(http://localhost/w/red.gif?timestamp); }', ), + array( + 'Regular file (missing)', + 'foo { background: url(theColorOfHerHair.gif); }', + 'foo { background: url(http://localhost/w/theColorOfHerHair.gif); }', + ), array( 'Remote URL', 'foo { background: url(http://example.org/w/foo.png); }', -- 2.20.1