diff --git a/lib/configonlylib.php b/lib/configonlylib.php index df542688628..bcdfa9bee90 100644 --- a/lib/configonlylib.php +++ b/lib/configonlylib.php @@ -205,3 +205,57 @@ function min_get_slash_argument($clean = true) { } return $relativepath; } + +/** + * Get the lowest possible currently valid revision number. + * + * This is based on the current Moodle version. + * + * @return int Unix timestamp + */ +function min_get_minimum_revision(): int { + static $timestamp = null; + + if ($timestamp === null) { + global $CFG; + require("{$CFG->dirroot}/version.php"); + // Get YYYYMMDD from version. + $datestring = floor($version / 100); + // Parse the date components. + $year = intval(substr($datestring, 0, 4)); + $month = intval(substr($datestring, 4, 2)); + $day = intval(substr($datestring, 6, 2)); + // Return converted GMT Unix timestamp. + $timestamp = gmmktime(0, 0, 0, $month, $day, $year); + } + + return $timestamp; +} + +/** + * Get the highest possible currently valid revision number. + * + * This is based on the current time, allowing for a small amount of clock skew between servers. + * + * Future values beyond the clock skew are not allowed to avoid the possibility of cache poisoning. + * + * @return int + */ +function min_get_maximum_revision(): int { + return time() + 60; +} + +/** + * Helper function to determine if the given revision number is valid. + * + * @param int $revision A numeric revision to check for validity + * @return bool Whether the revision is valid + */ +function min_is_revision_valid_and_current(int $revision): bool { + // Invalid revision. + if ($revision <= 0) { + return false; + } + // Check revision is within range. + return $revision >= min_get_minimum_revision() && $revision <= min_get_maximum_revision(); +} diff --git a/lib/editor/tiny/lang.php b/lib/editor/tiny/lang.php index 058c41fff1d..b4240c1f903 100644 --- a/lib/editor/tiny/lang.php +++ b/lib/editor/tiny/lang.php @@ -95,7 +95,8 @@ class lang { */ protected function serve_file(): void { // Attempt to send the cached langpack. - if ($this->rev > 0) { + // We only cache the file if the rev is valid. + if (min_is_revision_valid_and_current($this->rev)) { if ($this->is_candidate_file_available()) { // The send_cached_file_if_available function will exit if successful. // In theory the file could become unavailable after checking that the file exists. diff --git a/lib/editor/tiny/loader.php b/lib/editor/tiny/loader.php index 1539a6ccfb7..68baaac0ce9 100644 --- a/lib/editor/tiny/loader.php +++ b/lib/editor/tiny/loader.php @@ -114,7 +114,8 @@ class loader { */ public function serve_file(): void { // Attempt to send the cached filepathpack. - if ($this->rev > 0) { + // We only cache the file if the rev is valid. + if (min_is_revision_valid_and_current($this->rev)) { if ($this->is_candidate_file_available()) { // The send_cached_file_if_available function will exit if successful. // In theory the file could become unavailable after checking that the file exists. diff --git a/lib/javascript.php b/lib/javascript.php index 7e7869fbac1..a6b0e0940c8 100644 --- a/lib/javascript.php +++ b/lib/javascript.php @@ -47,6 +47,11 @@ if ($slashargument = min_get_slash_argument()) { $file = min_optional_param('jsfile', '', 'RAW'); // 'file' would collide with URL rewriting! } +if (!min_is_revision_valid_and_current($rev)) { + // If the rev is invalid, normalise it to -1 to disable all caching. + $rev = -1; +} + // some security first - pick only files with .js extension in dirroot $jsfiles = array(); $files = explode(',', $file); @@ -79,8 +84,7 @@ if (!$jsfiles) { $etag = sha1($rev.implode(',', $jsfiles)); -// Use the caching only for meaningful revision numbers which prevents future cache poisoning. -if ($rev > 0 and $rev < (time() + 60*60)) { +if ($rev > 0) { $candidate = $CFG->localcachedir.'/js/'.$etag; if (file_exists($candidate)) { diff --git a/lib/tests/configonlylib_test.php b/lib/tests/configonlylib_test.php index 1032e440241..01d8215aca1 100644 --- a/lib/tests/configonlylib_test.php +++ b/lib/tests/configonlylib_test.php @@ -21,7 +21,6 @@ defined('MOODLE_INTERNAL') || die(); // Global $CFG not used here intentionally to make sure it is not required inside the lib. require_once(__DIR__ . '/../configonlylib.php'); - /** * Unit tests for config only library functions. * @@ -144,4 +143,76 @@ class configonlylib_test extends \advanced_testcase { // Windows dir separators are removed, multiple ... gets collapsed to one . $this->assertSame('/standardcore/./5/u/f1', min_get_slash_argument()); } + + /** + * Test the min_get_minimum_version function. + * + * @covers ::min_get_minimum_version + */ + public function test_min_get_minimum_version(): void { + // This is fairly hard to write a test for, but we can at least check that it returns a number + // greater than the version when the feature was first introduced. + $firstintroduced = 1693612800; // Equivalent to 20230902 00:00:00 GMT. + $this->assertGreaterThanOrEqual($firstintroduced, min_get_minimum_revision()); + } + + /** + * Test the min_get_maximum_version function. + * + * @covers ::min_get_maximum_version + */ + public function test_min_get_maximum_version(): void { + // The maximum version should be set to a time in the near future. + // This is currently defined as "in the next minute". + // Note: We use a 65 second window to allow for slow test runners. + $this->assertGreaterThan(time(), min_get_maximum_revision()); + $this->assertLessThanOrEqual(time() + 65, min_get_maximum_revision()); + } + + /** + * Test the min_is_revision_valid_and_current function. + * + * @covers ::min_is_revision_valid_and_current + * @dataProvider min_is_revision_valid_and_current_provider + */ + public function test_min_is_revision_valid_and_current(int $revision, bool $expected): void { + $this->assertEquals($expected, min_is_revision_valid_and_current($revision)); + } + + /** + * Data provider for the min_is_revision_valid_and_current tests. + * + * @return array + */ + public function min_is_revision_valid_and_current_provider(): array { + return [ + 'Negative value' => [-1, false], + 'Empty value' => [0, false], + 'A time before the minimum accepted value' => [min_get_minimum_revision() - 1, false], + 'The minimum accepted value' => [min_get_minimum_revision(), true], + 'The current time' => [time(), true], + // Note: We have to be careful using time values because the data provider is run at the start of the phpunit run, + // but the test may not be run for some time. + // On a slower machine and/or database, this could be several hours. + // For a more specific time we must have a specific test function. + 'A time in the future' => [time() + DAYSECS, false] + ]; + } + + + /** + * Test the min_is_revision_valid_and_current function with close times. + * + * Note: These tests are incompatible with data providers. + * + * @covers ::min_is_revision_valid_and_current + */ + public function test_min_is_revision_valid_and_current_close_proximity(): void { + // A time in the near future. + $this->assertTrue(min_is_revision_valid_and_current(time() + 55)); + + // A time in the too-far future. + $this->assertFalse(min_is_revision_valid_and_current(time() + 70)); + + } } diff --git a/theme/font.php b/theme/font.php index 7a8f43a4000..be8a3af130e 100644 --- a/theme/font.php +++ b/theme/font.php @@ -49,6 +49,11 @@ if ($slashargument = min_get_slash_argument()) { $font = min_optional_param('font', '', 'RAW'); } +if (!min_is_revision_valid_and_current($rev)) { + // If the rev is invalid, normalise it to -1 to disable all caching. + $rev = -1; +} + if (!$font) { font_not_found(); } diff --git a/theme/image.php b/theme/image.php index e9cc853f81d..caa81ca9d31 100644 --- a/theme/image.php +++ b/theme/image.php @@ -58,6 +58,11 @@ if ($slashargument = min_get_slash_argument()) { $usesvg = (bool)min_optional_param('svg', '1', 'INT'); } +if (!min_is_revision_valid_and_current($rev)) { + // If the rev is invalid, normalise it to -1 to disable all caching. + $rev = -1; +} + if (empty($component) or $component === 'moodle' or $component === 'core') { $component = 'core'; } diff --git a/theme/javascript.php b/theme/javascript.php index c6bda484fb8..203ca94c08b 100644 --- a/theme/javascript.php +++ b/theme/javascript.php @@ -50,6 +50,11 @@ if ($slashargument = min_get_slash_argument()) { $type = min_optional_param('type', 'head', 'RAW'); } +if (!min_is_revision_valid_and_current($rev)) { + // If the rev is invalid, normalise it to -1 to disable all caching. + $rev = -1; +} + if ($type !== 'head' and $type !== 'footer') { header('HTTP/1.0 404 not found'); die('Theme was not found, sorry.'); diff --git a/theme/styles.php b/theme/styles.php index c64eb6546bc..4a3239aa38e 100644 --- a/theme/styles.php +++ b/theme/styles.php @@ -66,6 +66,13 @@ if (!is_null($themesubrev)) { $themesubrev = min_clean_param($themesubrev, 'INT'); } +// Note: We only check validity of the revision number here, we do not check the theme sub-revision because this is +// not solely based on time. +if (!min_is_revision_valid_and_current($rev)) { + // If the rev is invalid, normalise it to -1 to disable all caching. + $rev = -1; +} + // Check that type fits into the expected values. if (!in_array($type, ['all', 'all-rtl', 'editor', 'editor-rtl'])) { css_send_css_not_found(); diff --git a/theme/yui_combo.php b/theme/yui_combo.php index 5ffe8a07bdc..319faa4e255 100644 --- a/theme/yui_combo.php +++ b/theme/yui_combo.php @@ -231,8 +231,9 @@ while (count($parts)) { continue; } $revision = (int)array_shift($bits); - if ($revision === -1) { - // Revision -1 says please don't cache the JS + if (!min_is_revision_valid_and_current($revision)) { + // A non-current revision means please don't cache the JS + $revision = -1; $cache = false; } $frankenstyle = array_shift($bits); @@ -278,8 +279,9 @@ while (count($parts)) { continue; } $revision = (int)array_shift($bits); - if ($revision === -1) { - // Revision -1 says please don't cache the JS + if (!min_is_revision_valid_and_current($revision)) { + // A non-current revision means please don't cache the JS + $revision = -1; $cache = false; } $contentfile = "$CFG->libdir/yuilib/gallery/" . join('/', $bits);