Skip to content

Commit

Permalink
[BUGFIX] Handle new TS-not-set limitation on v13 (#1928)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
NamelessCoder authored Jan 26, 2025
1 parent 2411ce6 commit e84af96
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 22 deletions.
98 changes: 81 additions & 17 deletions Classes/Service/AssetService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 = [];
Expand All @@ -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);
Expand All @@ -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)) {
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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);
Expand Down Expand Up @@ -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');
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -742,19 +796,19 @@ 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;
}

$integrity = null;
$integrityMethod = ['sha256','sha384','sha512'][
$typoScript['assets.']['tagsAddSubresourceIntegrity'] - 1
$typoScript['assets']['tagsAddSubresourceIntegrity'] - 1
];
$integrityFile = sprintf(
$this->getTempPath() . 'vhs-assets-%s.%s',
Expand Down Expand Up @@ -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;
Expand Down
26 changes: 21 additions & 5 deletions Tests/Unit/Service/AssetServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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]));
}
}
Expand Down
1 change: 1 addition & 0 deletions ext_localconf.php
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down

0 comments on commit e84af96

Please sign in to comment.