From 950098444615ee9c3027493d00a9ad157ea55009 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 10 Oct 2024 18:26:52 +0200 Subject: [PATCH] fix: Shipped apps should include the Nextcloud version in the cache buster Co-authored-by: Ferdinand Thiessen Co-authored-by: Kate <26026535+provokateurin@users.noreply.github.com> Signed-off-by: Ferdinand Thiessen --- build/psalm-baseline.xml | 13 ---- lib/private/TemplateLayout.php | 126 +++++++++++++++++---------------- 2 files changed, 66 insertions(+), 73 deletions(-) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index 31fac17ef4944..f3f53d02215f3 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -2910,19 +2910,6 @@ $script - - - string - string - - - $appName - $appName - - - getInitialStates - - $path diff --git a/lib/private/TemplateLayout.php b/lib/private/TemplateLayout.php index d17189a3bd017..3f6521a8e3788 100644 --- a/lib/private/TemplateLayout.php +++ b/lib/private/TemplateLayout.php @@ -56,11 +56,14 @@ use OCP\INavigationManager; use OCP\IURLGenerator; use OCP\IUserSession; +use OCP\L10N\IFactory; use OCP\Support\Subscription\IRegistry; use OCP\Util; class TemplateLayout extends \OC_Template { private static $versionHash = ''; + /** @var string[] */ + private static $cacheBusterCache = []; /** @var CSSResourceLocator|null */ public static $cssLocator = null; @@ -68,38 +71,29 @@ class TemplateLayout extends \OC_Template { /** @var JSResourceLocator|null */ public static $jsLocator = null; - /** @var IConfig */ - private $config; - - /** @var IInitialStateService */ - private $initialState; - - /** @var INavigationManager */ - private $navigationManager; + private IConfig $config; + private IAppManager $appManager; + private InitialStateService $initialState; + private INavigationManager $navigationManager; /** * @param string $renderAs * @param string $appId application id */ public function __construct($renderAs, $appId = '') { - /** @var IConfig */ - $this->config = \OC::$server->get(IConfig::class); - - /** @var IInitialStateService */ - $this->initialState = \OC::$server->get(IInitialStateService::class); + $this->config = \OCP\Server::get(IConfig::class); + $this->appManager = \OCP\Server::get(IAppManager::class); + $this->initialState = \OCP\Server::get(InitialStateService::class); + $this->navigationManager = \OCP\Server::get(INavigationManager::class); - // Add fallback theming variables if theming is disabled - if ($renderAs !== TemplateResponse::RENDER_AS_USER - || !\OC::$server->getAppManager()->isEnabledForUser('theming')) { + // Add fallback theming variables if not rendered as user + if ($renderAs !== TemplateResponse::RENDER_AS_USER) { // TODO cache generated default theme if enabled for fallback if server is erroring ? Util::addStyle('theming', 'default'); } // Decide which page we show if ($renderAs === TemplateResponse::RENDER_AS_USER) { - /** @var INavigationManager */ - $this->navigationManager = \OC::$server->get(INavigationManager::class); - parent::__construct('core', 'layout.user'); if (in_array(\OC_App::getCurrentApp(), ['settings','admin', 'help']) !== false) { $this->assign('bodyid', 'body-settings'); @@ -120,7 +114,7 @@ public function __construct($renderAs, $appId = '') { } // Set body data-theme $this->assign('enabledThemes', []); - if (\OC::$server->getAppManager()->isEnabledForUser('theming') && class_exists('\OCA\Theming\Service\ThemesService')) { + if ($this->appManager->isEnabledForUser('theming') && class_exists('\OCA\Theming\Service\ThemesService')) { /** @var \OCA\Theming\Service\ThemesService */ $themesService = \OC::$server->get(\OCA\Theming\Service\ThemesService::class); $this->assign('enabledThemes', $themesService->getEnabledThemes()); @@ -131,9 +125,9 @@ public function __construct($renderAs, $appId = '') { $this->assign('logoUrl', $logoUrl); // Set default app name - $defaultApp = \OC::$server->getAppManager()->getDefaultAppForUser(); - $defaultAppInfo = \OC::$server->getAppManager()->getAppInfo($defaultApp); - $l10n = \OC::$server->getL10NFactory()->get($defaultApp); + $defaultApp = $this->appManager->getDefaultAppForUser(); + $defaultAppInfo = $this->appManager->getAppInfo($defaultApp); + $l10n = \OC::$server->get(IFactory::class)->get($defaultApp); $this->assign('defaultAppName', $l10n->t($defaultAppInfo['name'])); // Add navigation entry @@ -213,8 +207,7 @@ public function __construct($renderAs, $appId = '') { $showSimpleSignup = true; } - $appManager = \OCP\Server::get(IAppManager::class); - if ($appManager->isEnabledForUser('registration')) { + if ($this->appManager->isEnabledForUser('registration')) { $urlGenerator = \OCP\Server::get(IURLGenerator::class); $signUpLink = $urlGenerator->getAbsoluteURL('/index.php/apps/registration/'); } @@ -232,7 +225,7 @@ public function __construct($renderAs, $appId = '') { $this->assign('language', $lang); $this->assign('locale', $locale); - if (\OC::$server->getSystemConfig()->getValue('installed', false)) { + if ($this->config->getSystemValueBool('installed', false)) { if (empty(self::$versionHash)) { $v = \OC_App::getAppVersions(); $v['core'] = implode('.', \OCP\Util::getVersion()); @@ -252,7 +245,7 @@ public function __construct($renderAs, $appId = '') { $jsConfigHelper = new JSConfigHelper( \OCP\Util::getL10N('lib'), \OCP\Server::get(Defaults::class), - \OC::$server->getAppManager(), + $this->appManager, \OC::$server->getSession(), \OC::$server->getUserSession()->getUser(), $this->config, @@ -287,6 +280,7 @@ public function __construct($renderAs, $appId = '') { if (\OC::$server->getSystemConfig()->getValue('installed', false) && !\OCP\Util::needUpgrade() && $pathInfo !== '' + && $pathInfo !== false && !preg_match('/^\/login/', $pathInfo) && $renderAs !== TemplateResponse::RENDER_AS_ERROR ) { @@ -295,7 +289,7 @@ public function __construct($renderAs, $appId = '') { // If we ignore the scss compiler, // we need to load the guest css fallback \OC_Util::addStyle('guest'); - $cssFiles = self::findStylesheetFiles(\OC_Util::$styles, false); + $cssFiles = self::findStylesheetFiles(\OC_Util::$styles); } $this->assign('cssfiles', []); @@ -324,51 +318,64 @@ public function __construct($renderAs, $appId = '') { $this->assign('id-app-navigation', $renderAs === TemplateResponse::RENDER_AS_USER ? '#app-navigation' : null); } - /** - * @param string $path - * @param string $file - * @return string - */ - protected function getVersionHashSuffix($path = false, $file = false) { + protected function getVersionHashSuffix(string $path = '', string $file = ''): string { if ($this->config->getSystemValueBool('debug', false)) { // allows chrome workspace mapping in debug mode return ""; } - $themingSuffix = ''; - $v = []; - if ($this->config->getSystemValueBool('installed', false)) { - if (\OC::$server->getAppManager()->isInstalled('theming')) { - $themingSuffix = '-' . $this->config->getAppValue('theming', 'cachebuster', '0'); - } - $v = \OC_App::getAppVersions(); + if ($this->config->getSystemValueBool('installed', false) === false) { + // if not installed just return the version hash + return '?v=' . self::$versionHash; } - // Try the webroot path for a match - if ($path !== false && $path !== '') { - $appName = $this->getAppNamefromPath($path); - if (array_key_exists($appName, $v)) { - $appVersion = $v[$appName]; - return '?v=' . substr(md5($appVersion), 0, 8) . $themingSuffix; - } + $hash = false; + // Try the web-root first + if ($path !== '') { + $hash = $this->getVersionHashByPath($path); + } + // If not found try the file + if ($hash === false && $file !== '') { + $hash = $this->getVersionHashByPath($file); + } + // As a last resort we use the server version hash + if ($hash === false) { + $hash = self::$versionHash; } - // fallback to the file path instead - if ($file !== false && $file !== '') { - $appName = $this->getAppNamefromPath($file); - if (array_key_exists($appName, $v)) { - $appVersion = $v[$appName]; - return '?v=' . substr(md5($appVersion), 0, 8) . $themingSuffix; + + // The theming app is force-enabled thus the cache buster is always available + $themingSuffix = '-' . $this->config->getAppValue('theming', 'cachebuster', '0'); + + return '?v=' . $hash . $themingSuffix; + } + + private function getVersionHashByPath(string $path): string|false { + if (array_key_exists($path, self::$cacheBusterCache) === false) { + // Not yet cached, so lets find the cache buster string + $appId = $this->getAppNamefromPath($path); + if ($appId === false) { + // No app Id could be guessed + return false; } + + $appVersion = $this->appManager->getAppVersion($appId); + // For shipped apps the app version is not a single source of truth, we rather also need to consider the Nextcloud version + if ($this->appManager->isShipped($appId)) { + $appVersion .= '-' . self::$versionHash; + } + + $hash = substr(md5($appVersion), 0, 8); + self::$cacheBusterCache[$path] = $hash; } - return '?v=' . self::$versionHash . $themingSuffix; + return self::$cacheBusterCache[$path]; } /** * @param array $styles * @return array */ - public static function findStylesheetFiles($styles, $compileScss = true) { + public static function findStylesheetFiles($styles) { if (!self::$cssLocator) { self::$cssLocator = \OCP\Server::get(CSSResourceLocator::class); } @@ -377,11 +384,10 @@ public static function findStylesheetFiles($styles, $compileScss = true) { } /** - * @param string $path - * @return string|boolean + * @return string|false */ - public function getAppNamefromPath($path) { - if ($path !== '' && is_string($path)) { + public function getAppNamefromPath(string $path) { + if ($path !== '') { $pathParts = explode('/', $path); if ($pathParts[0] === 'css') { // This is a scss request