From 3063e8524865e469c1dd89f06a544c49485a3c57 Mon Sep 17 00:00:00 2001 From: Ori Livneh Date: Sun, 16 Aug 2015 19:15:40 -0700 Subject: [PATCH] CSSMin: version URLs based on content, not mtime The content of these files is more stable than their mtimes, which change every time we roll out a new branch. Because MD5 avalanches well[0], using the first five hexadecimal digits is sufficient to ensure that the chance of two successive versions colliding is improbably small (roughly one in a million). [0]: https://en.wikipedia.org/wiki/Avalanche_effect Change-Id: I1bdf94c58786d2545311b238476b48217a5a60af --- includes/libs/CSSMin.php | 6 +-- tests/phpunit/includes/libs/CSSMinTest.php | 53 ++++++++++------------ 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/includes/libs/CSSMin.php b/includes/libs/CSSMin.php index f415c9b6e7..b386a92755 100644 --- a/includes/libs/CSSMin.php +++ b/includes/libs/CSSMin.php @@ -406,9 +406,9 @@ class CSSMin { // 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 ) ); + // Add version parameter as the first five hex digits + // of the MD5 hash of the file's contents. + $url .= '?' . substr( md5_file( $localFile ), 0, 5 ); if ( $embed ) { $data = self::encodeImageAsDataURI( $localFile ); if ( $data !== false ) { diff --git a/tests/phpunit/includes/libs/CSSMinTest.php b/tests/phpunit/includes/libs/CSSMinTest.php index 6142f9675b..22ad6ce0f7 100644 --- a/tests/phpunit/includes/libs/CSSMinTest.php +++ b/tests/phpunit/includes/libs/CSSMinTest.php @@ -133,12 +133,7 @@ class CSSMinTest extends MediaWikiTestCase { $remotePath = 'http://localhost/w/'; $realOutput = CSSMin::remap( $input, $localPath, $remotePath ); - - $this->assertEquals( - $expectedOutput, - preg_replace( '/\d+-\d+-\d+T\d+:\d+:\d+Z/', 'timestamp', $realOutput ), - "CSSMin::remap: $message" - ); + $this->assertEquals( $expectedOutput, $realOutput, "CSSMin::remap: $message" ); } public static function provideIsRemoteUrl() { @@ -197,7 +192,7 @@ class CSSMinTest extends MediaWikiTestCase { array( 'Regular file', 'foo { background: url(red.gif); }', - 'foo { background: url(http://localhost/w/red.gif?timestamp); }', + 'foo { background: url(http://localhost/w/red.gif?34ac6); }', ), array( 'Regular file (missing)', @@ -242,12 +237,12 @@ class CSSMinTest extends MediaWikiTestCase { array( 'Embedded file', 'foo { /* @embed */ background: url(red.gif); }', - "foo { background: url($red); background: url(http://localhost/w/red.gif?timestamp)!ie; }", + "foo { background: url($red); background: url(http://localhost/w/red.gif?34ac6)!ie; }", ), array( 'Embedded file, other comments before the rule', "foo { /* Bar. */ /* @embed */ background: url(red.gif); }", - "foo { /* Bar. */ background: url($red); /* Bar. */ background: url(http://localhost/w/red.gif?timestamp)!ie; }", + "foo { /* Bar. */ background: url($red); /* Bar. */ background: url(http://localhost/w/red.gif?34ac6)!ie; }", ), array( 'Can not re-embed data: URIs', @@ -268,12 +263,12 @@ class CSSMinTest extends MediaWikiTestCase { 'Embedded file (inline @embed)', 'foo { background: /* @embed */ url(red.gif); }', "foo { background: url($red); " - . "background: url(http://localhost/w/red.gif?timestamp)!ie; }", + . "background: url(http://localhost/w/red.gif?34ac6)!ie; }", ), array( 'Can not embed large files', 'foo { /* @embed */ background: url(large.png); }', - "foo { background: url(http://localhost/w/large.png?timestamp); }", + "foo { background: url(http://localhost/w/large.png?e3d1f); }", ), array( 'SVG files are embedded without base64 encoding and unnecessary IE 6 and 7 fallback', @@ -283,55 +278,55 @@ class CSSMinTest extends MediaWikiTestCase { array( 'Two regular files in one rule', 'foo { background: url(red.gif), url(green.gif); }', - 'foo { background: url(http://localhost/w/red.gif?timestamp), ' - . 'url(http://localhost/w/green.gif?timestamp); }', + 'foo { background: url(http://localhost/w/red.gif?34ac6), ' + . 'url(http://localhost/w/green.gif?13651); }', ), array( 'Two embedded files in one rule', 'foo { /* @embed */ background: url(red.gif), url(green.gif); }', "foo { background: url($red), url($green); " - . "background: url(http://localhost/w/red.gif?timestamp), " - . "url(http://localhost/w/green.gif?timestamp)!ie; }", + . "background: url(http://localhost/w/red.gif?34ac6), " + . "url(http://localhost/w/green.gif?13651)!ie; }", ), array( 'Two embedded files in one rule (inline @embed)', 'foo { background: /* @embed */ url(red.gif), /* @embed */ url(green.gif); }', "foo { background: url($red), url($green); " - . "background: url(http://localhost/w/red.gif?timestamp), " - . "url(http://localhost/w/green.gif?timestamp)!ie; }", + . "background: url(http://localhost/w/red.gif?34ac6), " + . "url(http://localhost/w/green.gif?13651)!ie; }", ), array( 'Two embedded files in one rule (inline @embed), one too large', 'foo { background: /* @embed */ url(red.gif), /* @embed */ url(large.png); }', - "foo { background: url($red), url(http://localhost/w/large.png?timestamp); " - . "background: url(http://localhost/w/red.gif?timestamp), " - . "url(http://localhost/w/large.png?timestamp)!ie; }", + "foo { background: url($red), url(http://localhost/w/large.png?e3d1f); " + . "background: url(http://localhost/w/red.gif?34ac6), " + . "url(http://localhost/w/large.png?e3d1f)!ie; }", ), array( 'Practical example with some noise', 'foo { /* @embed */ background: #f9f9f9 url(red.gif) 0 0 no-repeat; }', "foo { background: #f9f9f9 url($red) 0 0 no-repeat; " - . "background: #f9f9f9 url(http://localhost/w/red.gif?timestamp) 0 0 no-repeat!ie; }", + . "background: #f9f9f9 url(http://localhost/w/red.gif?34ac6) 0 0 no-repeat!ie; }", ), array( 'Does not mess with other properties', 'foo { color: red; background: url(red.gif); font-size: small; }', - 'foo { color: red; background: url(http://localhost/w/red.gif?timestamp); font-size: small; }', + 'foo { color: red; background: url(http://localhost/w/red.gif?34ac6); font-size: small; }', ), array( 'Spacing and miscellanea not changed (1)', 'foo { background: url(red.gif); }', - 'foo { background: url(http://localhost/w/red.gif?timestamp); }', + 'foo { background: url(http://localhost/w/red.gif?34ac6); }', ), array( 'Spacing and miscellanea not changed (2)', 'foo {background:url(red.gif)}', - 'foo {background:url(http://localhost/w/red.gif?timestamp)}', + 'foo {background:url(http://localhost/w/red.gif?34ac6)}', ), array( 'Spaces within url() parentheses are ignored', 'foo { background: url( red.gif ); }', - 'foo { background: url(http://localhost/w/red.gif?timestamp); }', + 'foo { background: url(http://localhost/w/red.gif?34ac6); }', ), array( '@import rule to local file (should we remap this?)', @@ -351,22 +346,22 @@ class CSSMinTest extends MediaWikiTestCase { array( 'Simple case with comments after url', 'foo { prop: url(red.gif)/* some {funny;} comment */ ; }', - 'foo { prop: url(http://localhost/w/red.gif?timestamp)/* some {funny;} comment */ ; }', + 'foo { prop: url(http://localhost/w/red.gif?34ac6)/* some {funny;} comment */ ; }', ), array( 'Embedded file with comment before url', 'foo { /* @embed */ background: /* some {funny;} comment */ url(red.gif); }', - "foo { background: /* some {funny;} comment */ url($red); background: /* some {funny;} comment */ url(http://localhost/w/red.gif?timestamp)!ie; }", + "foo { background: /* some {funny;} comment */ url($red); background: /* some {funny;} comment */ url(http://localhost/w/red.gif?34ac6)!ie; }", ), array( 'Embedded file with comments inside and outside the rule', 'foo { /* @embed */ background: url(red.gif) /* some {foo;} comment */; /* some {bar;} comment */ }', - "foo { background: url($red) /* some {foo;} comment */; background: url(http://localhost/w/red.gif?timestamp) /* some {foo;} comment */!ie; /* some {bar;} comment */ }", + "foo { background: url($red) /* some {foo;} comment */; background: url(http://localhost/w/red.gif?34ac6) /* some {foo;} comment */!ie; /* some {bar;} comment */ }", ), array( 'Embedded file with comment outside the rule', 'foo { /* @embed */ background: url(red.gif); /* some {funny;} comment */ }', - "foo { background: url($red); background: url(http://localhost/w/red.gif?timestamp)!ie; /* some {funny;} comment */ }", + "foo { background: url($red); background: url(http://localhost/w/red.gif?34ac6)!ie; /* some {funny;} comment */ }", ), array( 'Rule with two urls, each with comments', -- 2.20.1