From 8a0080cbbbeb33006f70aafca2f8cc4fdb0b2b29 Mon Sep 17 00:00:00 2001 From: Anna Larch Date: Thu, 7 May 2026 12:09:28 +0200 Subject: [PATCH] fix(theming): fix broken custom images introduced in 32.0.9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #58224 introduced a raster→SVG conversion path in ImageManager::getImage() that breaks display of custom theming images. The root cause is a three-part bug chain: 1. getImage() attempted to convert raster images (PNG/JPEG) to SVG format, which Imagick cannot do meaningfully and produces broken output. 2. getMimeType() returns 'application/octet-stream' for extensionless stored files, so the Content-Type response header was wrong. 3. Stale .svg cache files persisted after image replacement, causing subsequent requests to serve the wrong format. Fix by: - Restricting the Imagick conversion to SVG→PNG only (not raster→SVG) - Reading the stored MIME type from IAppConfig for extensionless files in ThemingController::getImage() - Deleting .svg cache files in ImageManager::delete() - Injecting IAppConfig into ImageManager and reading the cachebuster via IAppConfig::getAppValueInt() so the URL returned after upload always carries the freshly-incremented value (IConfig::getAppValue() can return a stale cached value within the same request) - Updating the FileInputField Vue component to use a reactive cacheKey ref that increments on every upload, so the thumbnail refreshes even when the MIME type of the new image is the same as the old one AI-Assisted-By: Claude Sonnet 4.6 Signed-off-by: Anna Larch --- .../lib/Controller/ThemingController.php | 10 ++++- apps/theming/lib/ImageManager.php | 40 +++++++------------ .../src/components/admin/FileInputField.vue | 4 +- lib/private/Server.php | 5 +++ 4 files changed, 31 insertions(+), 28 deletions(-) diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index 467caef40a0..bbd598a2c28 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -372,7 +372,13 @@ class ThemingController extends Controller { $csp->allowInlineStyle(); $response->setContentSecurityPolicy($csp); $response->cacheFor(3600); - $response->addHeader('Content-Type', $file->getMimeType()); + // The original stored file has no extension (e.g. "logo"), so getMimeType() returns + // application/octet-stream for it. Use the config-stored MIME type for the original + // file, and getMimeType() only for converted files which have a proper extension. + $mimeType = $file->getName() === $key + ? $this->appConfig->getAppValueString($key . 'Mime', '') + : $file->getMimeType(); + $response->addHeader('Content-Type', $mimeType); $response->addHeader('Content-Disposition', 'attachment; filename="' . $key . '"'); return $response; } @@ -450,7 +456,7 @@ class ThemingController extends Controller { #[BruteForceProtection(action: 'manifest')] #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] public function getManifest(string $app): JSONResponse { - $cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0'); + $cacheBusterValue = $this->appConfig->getAppValueString('cachebuster', '0'); if ($app === 'core' || $app === 'settings') { $name = $this->themingDefaults->getName(); $shortName = $this->themingDefaults->getName(); diff --git a/apps/theming/lib/ImageManager.php b/apps/theming/lib/ImageManager.php index 1979656dd1e..ad3903f148e 100644 --- a/apps/theming/lib/ImageManager.php +++ b/apps/theming/lib/ImageManager.php @@ -8,6 +8,7 @@ namespace OCA\Theming; use OCA\Theming\AppInfo\Application; use OCA\Theming\Service\BackgroundService; +use OCP\AppFramework\Services\IAppConfig; use OCP\Files\IAppData; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; @@ -30,6 +31,7 @@ class ImageManager { private LoggerInterface $logger, private ITempManager $tempManager, private BackgroundService $backgroundService, + private IAppConfig $appConfig, ) { } @@ -40,7 +42,7 @@ class ImageManager { * @return string the image url */ public function getImageUrl(string $key): string { - $cacheBusterCounter = $this->config->getAppValue(Application::APP_ID, 'cachebuster', '0'); + $cacheBusterCounter = (string)$this->appConfig->getAppValueInt(ConfigLexicon::CACHE_BUSTER); if ($this->hasImage($key)) { return $this->urlGenerator->linkToRoute('theming.Theming.getImage', [ 'key' => $key ]) . '?v=' . $cacheBusterCounter; } elseif ($key === 'backgroundDark' && $this->hasImage('background')) { @@ -85,31 +87,14 @@ class ImageManager { public function getImage(string $key, bool $useSvg = true): ISimpleFile { $mime = $this->config->getAppValue('theming', $key . 'Mime', ''); $folder = $this->getRootFolder()->getFolder('images'); - $useSvg = $useSvg && $this->canConvert('SVG'); if ($mime === '' || !$folder->fileExists($key)) { throw new NotFoundException(); } - // if SVG was requested and is supported - if ($useSvg) { - if (!$folder->fileExists($key . '.svg')) { - try { - $finalIconFile = new \Imagick(); - $finalIconFile->setBackgroundColor('none'); - $finalIconFile->readImageBlob($folder->getFile($key)->getContent()); - $finalIconFile->setImageFormat('SVG'); - $svgFile = $folder->newFile($key . '.svg'); - $svgFile->putContent($finalIconFile->getImageBlob()); - return $svgFile; - } catch (\ImagickException $e) { - $this->logger->info('The image was requested to be no SVG file, but converting it to SVG failed: ' . $e->getMessage()); - } - } else { - return $folder->getFile($key . '.svg'); - } - } - // if SVG was not requested, but PNG is supported - if (!$useSvg && $this->canConvert('PNG')) { + // only convert SVG originals to PNG when SVG output is not desired; + // converting raster images to SVG produces broken output and is not useful + $isOriginalSvg = ($mime === 'image/svg+xml' || $mime === 'image/svg'); + if ($isOriginalSvg && !$useSvg && $this->canConvert('SVG') && $this->canConvert('PNG')) { if (!$folder->fileExists($key . '.png')) { try { $finalIconFile = new \Imagick(); @@ -120,13 +105,12 @@ class ImageManager { $pngFile->putContent($finalIconFile->getImageBlob()); return $pngFile; } catch (\ImagickException $e) { - $this->logger->info('The image was requested to be no SVG file, but converting it to PNG failed: ' . $e->getMessage()); + $this->logger->info('Converting SVG to PNG failed: ' . $e->getMessage()); } } else { return $folder->getFile($key . '.png'); } } - // fallback to the original file return $folder->getFile($key); } @@ -157,7 +141,7 @@ class ImageManager { * @throws NotPermittedException */ public function getCacheFolder(): ISimpleFolder { - $cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0'); + $cacheBusterValue = (string)$this->appConfig->getAppValueInt(ConfigLexicon::CACHE_BUSTER); try { $folder = $this->getRootFolder()->getFolder($cacheBusterValue); } catch (NotFoundException $e) { @@ -214,6 +198,12 @@ class ImageManager { } catch (NotFoundException $e) { } catch (NotPermittedException $e) { } + try { + $file = $this->getRootFolder()->getFolder('images')->getFile($key . '.svg'); + $file->delete(); + } catch (NotFoundException $e) { + } catch (NotPermittedException $e) { + } if ($key === 'logo') { $this->config->deleteAppValue('theming', 'logoDimensions'); diff --git a/apps/theming/src/components/admin/FileInputField.vue b/apps/theming/src/components/admin/FileInputField.vue index 0e60e89d100..2b7b45a3702 100644 --- a/apps/theming/src/components/admin/FileInputField.vue +++ b/apps/theming/src/components/admin/FileInputField.vue @@ -29,12 +29,13 @@ const emit = defineEmits<{ const isSaving = ref(false) const mime = ref(loadState('theming', 'adminThemingParameters')[props.name + 'Mime'] as string) +const cacheKey = ref(Date.now()) const inputElement = useTemplateRef('input') const background = computed(() => { const baseUrl = generateUrl('/apps/theming/image/{key}', { key: props.name }) - return `url(${baseUrl}?v=${Date.now()}&m=${encodeURIComponent(mime.value)})` + return `url(${baseUrl}?v=${cacheKey.value}&m=${encodeURIComponent(mime.value)})` }) /** @@ -75,6 +76,7 @@ async function onChange() { }, }) mime.value = file.type + cacheKey.value = Date.now() emit('updated') } catch (error) { if (isAxiosError(error) && error.response?.status === 422) { diff --git a/lib/private/Server.php b/lib/private/Server.php index 2a33c18eb05..c379932e936 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -1078,6 +1078,11 @@ class Server extends ServerContainer implements IServerContainer { $c->get(LoggerInterface::class), $c->get(ITempManager::class), $backgroundService, + new AppConfig( + $c->get(IConfig::class), + $c->get(IAppConfig::class), + 'theming', + ), ); return new ThemingDefaults( new AppConfig(