Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX] Handle new TS-not-set limitation on v13 #1928

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading