From 92b4a4e26fd62e21202373921e63c59d167e7486 Mon Sep 17 00:00:00 2001 From: Claus Due Date: Sun, 26 Jan 2025 22:21:48 +0100 Subject: [PATCH] [BUGFIX] Handle new TS-not-set limitation on v13 Uncached rendering context no longer provides TS through ConfigurationManager. An exception is raised instead. To avoid this, we store a representation of the plugin.tx_vhs TS scope in a separate cache when the page is rendered before it is cached - and retrieve this cached TS when needed. --- Classes/Service/AssetService.php | 98 ++++++++++++++++++++----- Tests/Unit/Service/AssetServiceTest.php | 26 +++++-- ext_localconf.php | 1 + 3 files changed, 103 insertions(+), 22 deletions(-) diff --git a/Classes/Service/AssetService.php b/Classes/Service/AssetService.php index b91d172a7..6da096373 100644 --- a/Classes/Service/AssetService.php +++ b/Classes/Service/AssetService.php @@ -13,7 +13,9 @@ use FluidTYPO3\Vhs\ViewHelpers\Asset\AssetInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Log\LoggerInterface; +use TYPO3\CMS\Core\Cache\CacheManager; use TYPO3\CMS\Core\Log\LogManager; +use TYPO3\CMS\Core\Routing\PageArguments; use TYPO3\CMS\Core\SingletonInterface; use TYPO3\CMS\Core\Utility\ArrayUtility; use TYPO3\CMS\Core\Utility\GeneralUtility; @@ -40,6 +42,11 @@ class AssetService implements SingletonInterface */ protected $configurationManager; + /** + * @var CacheManager + */ + protected $cacheManager; + protected static bool $typoScriptAssetsBuilt = false; protected static ?array $settingsCache = null; protected static array $cachedDependencies = []; @@ -50,6 +57,11 @@ public function injectConfigurationManager(ConfigurationManagerInterface $config $this->configurationManager = $configurationManager; } + public function injectCacheManager(CacheManager $cacheManager): void + { + $this->cacheManager = $cacheManager; + } + public function usePageCache(object $caller, bool $shouldUsePageCache): bool { $this->buildAll([], $caller); @@ -63,7 +75,10 @@ public function buildAll(array $parameters, object $caller, bool $cached = true, } $settings = $this->getSettings(); - $buildTypoScriptAssets = (!static::$typoScriptAssetsBuilt && ($cached || $GLOBALS['TSFE']->no_cache)); + $buildTypoScriptAssets = ( + !static::$typoScriptAssetsBuilt + && ($cached || $this->readCacheDisabledInstructionFromContext()) + ); if ($buildTypoScriptAssets && isset($settings['asset']) && is_array($settings['asset'])) { foreach ($settings['asset'] as $name => $typoScriptAsset) { if (!isset($GLOBALS['VhsAssets'][$name]) && is_array($typoScriptAsset)) { @@ -130,15 +145,53 @@ public function isAlreadyDefined(string $assetName): bool public function getSettings(): array { if (null === static::$settingsCache) { + static::$settingsCache = $this->getTypoScript()['settings'] ?? []; + } + $settings = (array) static::$settingsCache; + return $settings; + } + + protected function getTypoScript(): array + { + $cache = $this->cacheManager->getCache('vhs_main'); + $pageUid = $this->readPageUidFromContext(); + $cacheId = 'vhs_asset_ts_' . $pageUid; + $cacheTag = 'pageId_' . $pageUid; + + try { $allTypoScript = $this->configurationManager->getConfiguration( ConfigurationManagerInterface::CONFIGURATION_TYPE_FULL_TYPOSCRIPT ); - static::$settingsCache = GeneralUtility::removeDotsFromTS( - $allTypoScript['plugin.']['tx_vhs.']['settings.'] ?? [] - ); + $typoScript = GeneralUtility::removeDotsFromTS($allTypoScript['plugin.']['tx_vhs.'] ?? []); + $cache->set($cacheId, $typoScript, [$cacheTag]); + } catch (\RuntimeException $exception) { + if ($exception->getCode() !== 1666513645) { + // Re-throw, but only if the exception is not the specific "Setup array has not been initialized" one. + throw $exception; + } + + // Note: this case will only ever be entered on TYPO3v13 and above. Earlier versions will consistently + // produce the necessary TS array from ConfigurationManager - and will not raise the specified exception. + + // We will only look in VHS's cache for a TS array if it wasn't already retrieved by ConfigurationManager. + // This is for performance reasons: the TS array may be relatively large and the cache may be DB-based. + // Whereas if the TS is already in ConfigurationManager, it costs nearly nothing to read. The TS is returned + // even if it is empty. + /** @var array|false $fromCache */ + $fromCache = $cache->get($cacheId); + if (is_array($fromCache)) { + return $fromCache; + } + + // Graceful: it's better to return empty settings than either adding massive code chunks dealing with + // custom TS reading or allowing an exception to be raised. Note that reaching this case means that the + // PAGE was cached, but VHS's cache for the page is empty. This can be caused by TTL skew. The solution is + // to flush all caches tagged with the page's ID, so the next request will correctly regenerate the entry. + $typoScript = []; + $this->cacheManager->flushCachesByTag($cacheTag); } - $settings = (array) static::$settingsCache; - return $settings; + + return $typoScript; } /** @@ -274,8 +327,9 @@ protected function writeCachedMergedFileAndReturnTag(array $assets, string $type sort($keys); $assetName = implode('-', $keys); unset($keys); - if (isset($GLOBALS['TSFE']->tmpl->setup['plugin.']['tx_vhs.']['assets.']['mergedAssetsUseHashedFilename'])) { - if ($GLOBALS['TSFE']->tmpl->setup['plugin.']['tx_vhs.']['assets.']['mergedAssetsUseHashedFilename']) { + $typoScript = $this->getTypoScript(); + if (isset($typoScript['assets']['mergedAssetsUseHashedFilename'])) { + if ($typoScript['assets']['mergedAssetsUseHashedFilename']) { $assetName = md5($assetName); } } @@ -284,8 +338,7 @@ protected function writeCachedMergedFileAndReturnTag(array $assets, string $type if (!file_exists($fileAbsolutePathAndFilename) || 0 === filemtime($fileAbsolutePathAndFilename) || isset($GLOBALS['BE_USER']) - || ($GLOBALS['TSFE']->no_cache ?? false) - || ($GLOBALS['TSFE']->page['no_cache'] ?? false) + || $this->readCacheDisabledInstructionFromContext() ) { foreach ($assets as $name => $asset) { $assetSettings = $this->extractAssetSettings($asset); @@ -348,6 +401,7 @@ protected function generateTagForAssetType( $file = PathUtility::getAbsoluteWebPath($file); $file = $this->prefixPath($file); } + $settings = $this->getTypoScript(); switch ($type) { case 'js': $tagBuilder->setTagName('script'); @@ -359,7 +413,7 @@ protected function generateTagForAssetType( $tagBuilder->addAttribute('src', (string) $file); } if (!empty($integrity)) { - if (!empty($GLOBALS['TSFE']->tmpl->setup['plugin.']['tx_vhs.']['settings.']['prependPath'])) { + if (!empty($settings['prependPath'])) { $tagBuilder->addAttribute('crossorigin', 'anonymous'); } $tagBuilder->addAttribute('integrity', $integrity); @@ -387,7 +441,7 @@ protected function generateTagForAssetType( $tagBuilder->addAttribute('href', $file); } if (!empty($integrity)) { - if (!empty($GLOBALS['TSFE']->tmpl->setup['plugin.']['tx_vhs.']['settings.']['prependPath'])) { + if (!empty($settings['prependPath'])) { $tagBuilder->addAttribute('crossorigin', 'anonymous'); } $tagBuilder->addAttribute('integrity', $integrity); @@ -742,11 +796,11 @@ protected function mergeArrays(array $array1, array $array2): array protected function getFileIntegrity(string $file): ?string { - $typoScript = $GLOBALS['TSFE']->tmpl->setup['plugin.']['tx_vhs.'] ?? null; - if (isset($typoScript['assets.']['tagsAddSubresourceIntegrity'])) { + $typoScript = $this->getTypoScript(); + if (isset($typoScript['assets']['tagsAddSubresourceIntegrity'])) { // Note: 3 predefined hashing strategies (the ones suggestes in the rfc sheet) - if (0 < $typoScript['assets.']['tagsAddSubresourceIntegrity'] - && $typoScript['assets.']['tagsAddSubresourceIntegrity'] < 4 + if (0 < $typoScript['assets']['tagsAddSubresourceIntegrity'] + && $typoScript['assets']['tagsAddSubresourceIntegrity'] < 4 ) { if (!file_exists($file)) { return null; @@ -754,7 +808,7 @@ protected function getFileIntegrity(string $file): ?string $integrity = null; $integrityMethod = ['sha256','sha384','sha512'][ - $typoScript['assets.']['tagsAddSubresourceIntegrity'] - 1 + $typoScript['assets']['tagsAddSubresourceIntegrity'] - 1 ]; $integrityFile = sprintf( $this->getTempPath() . 'vhs-assets-%s.%s', @@ -799,6 +853,16 @@ protected function resolveAbsolutePathForFile(string $filename): string return GeneralUtility::getFileAbsFileName($filename); } + protected function readPageUidFromContext(): int + { + /** @var ServerRequestInterface $serverRequest */ + $serverRequest = $GLOBALS['TYPO3_REQUEST']; + + /** @var PageArguments $pageArguments */ + $pageArguments = $serverRequest->getAttribute('routing'); + return $pageArguments->getPageId(); + } + protected function readCacheDisabledInstructionFromContext(): bool { $hasDisabledInstructionInRequest = false; diff --git a/Tests/Unit/Service/AssetServiceTest.php b/Tests/Unit/Service/AssetServiceTest.php index 98672fb88..fbbb948fa 100644 --- a/Tests/Unit/Service/AssetServiceTest.php +++ b/Tests/Unit/Service/AssetServiceTest.php @@ -47,12 +47,21 @@ public function testBuildAll(array $assets, $cached, $expectedFiles) ->getMock(); $GLOBALS['TSFE']->content = 'content'; $instance = $this->getMockBuilder(AssetService::class) - ->setMethods(['writeFile', 'getSettings', 'resolveAbsolutePathForFile']) + ->onlyMethods( + [ + 'writeFile', + 'getSettings', + 'resolveAbsolutePathForFile', + 'getTypoScript', + 'readCacheDisabledInstructionFromContext' + ] + ) ->getMock(); $instance->expects($this->exactly($expectedFiles)) ->method('writeFile') ->with($this->anything(), $this->anything()); $instance->method('getSettings')->willReturn([]); + $instance->method('getTypoScript')->willReturn([]); $instance->method('resolveAbsolutePathForFile')->willReturnArgument(0); if (true === $cached) { $instance->buildAll([], $this, $cached); @@ -118,13 +127,20 @@ public function testIntegrityCalculation() 'sha384-aieE32yQSOy7uEhUkUvR9bVgfJgMsP+B9TthbxbjDDZ2hd4tjV5jMUoj9P8aeSHI', 'sha512-0bz2YVKEoytikWIUFpo6lK/k2cVVngypgaItFoRvNfux/temtdCVxsu+HxmdRT8aNOeJxxREUphbkcAK8KpkWg==', ]; + $file = 'Tests/Fixtures/Files/dummy.js'; - $method = (new \ReflectionClass(AssetService::class))->getMethod('getFileIntegrity'); - $instance = $this->getMockBuilder(AssetService::class)->setMethods(['writeFile'])->getMock(); - $method->setAccessible(true); foreach ($expectedIntegrities as $settingLevel => $expectedIntegrity) { - $GLOBALS['TSFE']->tmpl->setup['plugin.']['tx_vhs.']['assets.']['tagsAddSubresourceIntegrity'] = $settingLevel; + $method = (new \ReflectionClass(AssetService::class))->getMethod('getFileIntegrity'); + $instance = $this->getMockBuilder(AssetService::class)->onlyMethods(['writeFile', 'getTypoScript'])->getMock(); + $instance->method('getTypoScript')->willReturn( + [ + 'assets' => [ + 'tagsAddSubresourceIntegrity' => $settingLevel, + ], + ] + ); + $method->setAccessible(true); $this->assertEquals($expectedIntegrity, $method->invokeArgs($instance, [$file])); } } diff --git a/ext_localconf.php b/ext_localconf.php index 7bd745fc3..6ed024d1a 100644 --- a/ext_localconf.php +++ b/ext_localconf.php @@ -27,6 +27,7 @@ $GLOBALS['TYPO3_CONF_VARS']['SYS']['caching']['cacheConfigurations']['vhs_markdown'] = [ 'frontend' => \TYPO3\CMS\Core\Cache\Frontend\VariableFrontend::class, 'options' => [ + // You should keep this value HIGHER than the lifetime of TYPO3's page caches at all times. 'defaultLifetime' => 804600 ], 'groups' => ['pages', 'all']