diff --git a/classes/local/store/object_file_system.php b/classes/local/store/object_file_system.php index d7df5b93..27e2c330 100644 --- a/classes/local/store/object_file_system.php +++ b/classes/local/store/object_file_system.php @@ -542,13 +542,14 @@ public function xsendfile_file(stored_file $file): bool { } $contenthash = $file->get_contenthash(); + $headers = headers_list(); try { if ( $this->presigned_url_configured() && - $this->presigned_url_should_redirect_file($file) && + $this->presigned_url_should_redirect_file($file, $headers) && $this->is_file_readable_externally_by_hash($contenthash) ) { - return $this->redirect_to_presigned_url($contenthash, headers_list()); + return $this->redirect_to_presigned_url($contenthash, $headers); } $ranges = $this->get_valid_http_ranges($file->get_filesize()); @@ -911,10 +912,11 @@ public function presigned_url_configured() { * and get file name and file size directly from the object. * * @param object $file File object + * @param array $headers Request headers. * @return bool * @throws \dml_exception */ - public function presigned_url_should_redirect_file($file) { + public function presigned_url_should_redirect_file($file, array $headers = []) { // Core will throw an exception is we try to redirect inside cli or ajax. if (CLI_SCRIPT || AJAX_SCRIPT) { @@ -925,6 +927,10 @@ public function presigned_url_should_redirect_file($file) { return false; } + if ($this->is_disallowed_presigned_cache_policy($headers)) { + return false; + } + // Redirect when the file size is bigger than presignedminfilesize setting // and file extension is whitelisted. return ($file->get_filesize() >= $this->externalclient->presignedminfilesize && @@ -975,6 +981,49 @@ public function is_disallowed_presigned_filearea(stored_file $file): bool { return isset($lookup[$needle]); } + /** + * Returns true when cache headers represent a disallowed cache policy. + * + * This can be used to avoid redirecting to pre-signed URLs when Moodle has + * already emitted response headers suitable for long-term CDN/browser caching. + * + * @param array $headers Request headers. + * @return bool + */ + public function is_disallowed_presigned_cache_policy(array $headers): bool { + if (empty(get_config('tool_objectfs', 'disallowlonglivedcache'))) { + return false; + } + + $cachecontrol = strtolower((string)manager::get_header($headers, 'Cache-Control')); + if ($cachecontrol === '') { + return false; + } + + // We don't want to presign URLs when the cache header is public and long lived. + $directives = array_map('trim', explode(',', strtolower($cachecontrol))); + if (!in_array('public', $directives)) { + return false; + } + + // Immutable assets are long-lived by definition. + if (in_array('immutable', $directives)) { + return true; + } + + // Also treat high max-age values as long-lived, even without immutable. + foreach ($directives as $directive) { + if (str_contains($directive, 'max-age')) { + $maxage = strstr($directive, '='); + if ($maxage) { + return (int)trim($maxage, ' ="') >= 30 * DAYSECS; + } + } + } + + return false; + } + /** * Returns true if the file system should redirect to pre-signed url. * @@ -984,6 +1033,10 @@ public function is_disallowed_presigned_filearea(stored_file $file): bool { * @throws \dml_exception */ public function presigned_url_should_redirect($contenthash, $headers = []) { + if ($this->is_disallowed_presigned_cache_policy($headers)) { + return false; + } + // Redirect regardless. if ( $this->externalclient->presignedminfilesize == 0 && diff --git a/lang/en/tool_objectfs.php b/lang/en/tool_objectfs.php index b1884d74..82bca5b4 100644 --- a/lang/en/tool_objectfs.php +++ b/lang/en/tool_objectfs.php @@ -257,6 +257,8 @@ $string['settings:presignedurl:deletefiles'] = 'Delete test files.'; $string['settings:presignedurl:disallowfileareas'] = 'Disallow file areas for Pre-Signed URL redirects'; $string['settings:presignedurl:disallowfileareas_help'] = 'One entry per line in the format component|filearea. Matching files will never redirect to Pre-Signed URLs.'; +$string['settings:presignedurl:disallowlonglivedcache'] = 'Disallow files with public long-lived cache headers'; +$string['settings:presignedurl:disallowlonglivedcache_help'] = 'When checked, files with public long-lived cache headers will never redirect to Pre-Signed URLs. This applies to Cache-Control directives that are public and either immutable or have max-age of 30 days or more.'; $string['settings:presignedurl:enablepresigneds3urls'] = 'S3 Pre-Signed URLs'; $string['settings:presignedurl:enablepresigneds3urls_help'] = 'Enable Pre-Signed S3 URLs to request content directly from external storage.'; $string['settings:presignedurl:enablepresignedurls'] = 'Enable Pre-Signed URLs'; diff --git a/settings.php b/settings.php index aef439fd..31e768c4 100644 --- a/settings.php +++ b/settings.php @@ -310,6 +310,13 @@ PARAM_RAW_TRIMMED )); + $settings->add(new admin_setting_configcheckbox( + 'tool_objectfs/disallowlonglivedcache', + new lang_string('settings:presignedurl:disallowlonglivedcache', 'tool_objectfs'), + new lang_string('settings:presignedurl:disallowlonglivedcache_help', 'tool_objectfs'), + '0' + )); + $settings->add( new admin_setting_filetypes( 'tool_objectfs/signingwhitelist', diff --git a/tests/object_file_system_test.php b/tests/object_file_system_test.php index 48d49db8..b72a14e1 100644 --- a/tests/object_file_system_test.php +++ b/tests/object_file_system_test.php @@ -797,6 +797,57 @@ public function test_is_disallowed_presigned_filearea_refreshes_lookup_when_conf $this->assertTrue($this->filesystem->is_disallowed_presigned_filearea($scormfile)); } + /** + * Data provider for is_disallowed_presigned_cache_policy(). + * + * @return array + */ + public static function is_disallowed_presigned_cache_policy_provider(): array { + return [ + 'public immutable' => [ + 'headers' => ['Cache-Control: public, immutable, max-age=60'], + 'expected' => true, + ], + 'public long max-age' => [ + 'headers' => ['Cache-Control: public, max-age=5184000, no-transform'], + 'expected' => true, + ], + 'public short max-age' => [ + 'headers' => ['Cache-Control: public, max-age=3600'], + 'expected' => false, + ], + 'private immutable' => [ + 'headers' => ['Cache-Control: private, immutable, max-age=5184000'], + 'expected' => false, + ], + 'quoted max-age' => [ + 'headers' => ['Cache-Control: public, max-age="5184000"'], + 'expected' => true, + ], + 'missing cache-control header' => [ + 'headers' => ['Content-Type: text/plain'], + 'expected' => false, + ], + ]; + } + + /** + * Tests detection and handling of cache control headers. + * + * @dataProvider is_disallowed_presigned_cache_policy_provider + * @covers ::is_disallowed_presigned_cache_policy + * @param array $headers response headers. + * @param bool $expected expected result. + * @return void + */ + public function test_is_disallowed_presigned_cache_policy(array $headers, bool $expected): void { + set_config('disallowlonglivedcache', 0, 'tool_objectfs'); + $this->assertFalse($this->filesystem->is_disallowed_presigned_cache_policy($headers)); + + set_config('disallowlonglivedcache', 1, 'tool_objectfs'); + $this->assertSame($expected, $this->filesystem->is_disallowed_presigned_cache_policy($headers)); + } + /** * Data provider for test_get_expiration_time_method_if_supported(). * diff --git a/version.php b/version.php index d1656ed8..598d3758 100644 --- a/version.php +++ b/version.php @@ -25,8 +25,8 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2026041008; // The current plugin version (Date: YYYYMMDDXX). -$plugin->release = 2026041008; // Same as version. +$plugin->version = 2026041009; // The current plugin version (Date: YYYYMMDDXX). +$plugin->release = 2026041009; // Same as version. $plugin->requires = 2024042200; // Requires 4.4. $plugin->component = "tool_objectfs"; $plugin->maturity = MATURITY_STABLE;