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

diff --git a/includes/libs/CSSMin.php b/includes/libs/CSSMin.php
index f415c9b..b386a92 100644 (file)
--- 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 6142f96..22ad6ce 100644 (file)
--- 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',