Skip to content

Commit 3551e61

Browse files
miaulalalabackportbot[bot]
authored andcommitted
fix(theming): fix broken custom images introduced in 32.0.9
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 <noreply@anthropic.com> Signed-off-by: Anna Larch <anna@nextcloud.com>
1 parent ab6a1e6 commit 3551e61

3 files changed

Lines changed: 28 additions & 27 deletions

File tree

apps/theming/lib/Controller/ThemingController.php

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,13 @@ public function getImage(string $key, bool $useSvg = true) {
356356
$csp->allowInlineStyle();
357357
$response->setContentSecurityPolicy($csp);
358358
$response->cacheFor(3600);
359-
$response->addHeader('Content-Type', $file->getMimeType());
359+
// The original stored file has no extension (e.g. "logo"), so getMimeType() returns
360+
// application/octet-stream for it. Use the config-stored MIME type for the original
361+
// file, and getMimeType() only for converted files which have a proper extension.
362+
$mimeType = $file->getName() === $key
363+
? $this->appConfig->getAppValueString($key . 'Mime', '')
364+
: $file->getMimeType();
365+
$response->addHeader('Content-Type', $mimeType);
360366
$response->addHeader('Content-Disposition', 'attachment; filename="' . $key . '"');
361367
return $response;
362368
}
@@ -435,7 +441,7 @@ public function getThemeStylesheet(string $themeId, bool $plain = false, bool $w
435441
#[BruteForceProtection(action: 'manifest')]
436442
#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)]
437443
public function getManifest(string $app): JSONResponse {
438-
$cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0');
444+
$cacheBusterValue = $this->appConfig->getAppValueString('cachebuster', '0');
439445
if ($app === 'core' || $app === 'settings') {
440446
$name = $this->themingDefaults->getName();
441447
$shortName = $this->themingDefaults->getName();

apps/theming/lib/ImageManager.php

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
use OCA\Theming\AppInfo\Application;
1010
use OCA\Theming\Service\BackgroundService;
11+
use OCP\AppFramework\Services\IAppConfig;
1112
use OCP\Files\IAppData;
1213
use OCP\Files\NotFoundException;
1314
use OCP\Files\NotPermittedException;
@@ -30,6 +31,7 @@ public function __construct(
3031
private LoggerInterface $logger,
3132
private ITempManager $tempManager,
3233
private BackgroundService $backgroundService,
34+
private IAppConfig $appConfig,
3335
) {
3436
}
3537

@@ -40,7 +42,7 @@ public function __construct(
4042
* @return string the image url
4143
*/
4244
public function getImageUrl(string $key): string {
43-
$cacheBusterCounter = $this->config->getAppValue(Application::APP_ID, 'cachebuster', '0');
45+
$cacheBusterCounter = (string)$this->appConfig->getAppValueInt(ConfigLexicon::CACHE_BUSTER);
4446
if ($this->hasImage($key)) {
4547
return $this->urlGenerator->linkToRoute('theming.Theming.getImage', [ 'key' => $key ]) . '?v=' . $cacheBusterCounter;
4648
} elseif ($key === 'backgroundDark' && $this->hasImage('background')) {
@@ -85,31 +87,14 @@ public function getImageUrlAbsolute(string $key): string {
8587
public function getImage(string $key, bool $useSvg = true): ISimpleFile {
8688
$mime = $this->config->getAppValue('theming', $key . 'Mime', '');
8789
$folder = $this->getRootFolder()->getFolder('images');
88-
$useSvg = $useSvg && $this->canConvert('SVG');
8990

9091
if ($mime === '' || !$folder->fileExists($key)) {
9192
throw new NotFoundException();
9293
}
93-
// if SVG was requested and is supported
94-
if ($useSvg) {
95-
if (!$folder->fileExists($key . '.svg')) {
96-
try {
97-
$finalIconFile = new \Imagick();
98-
$finalIconFile->setBackgroundColor('none');
99-
$finalIconFile->readImageBlob($folder->getFile($key)->getContent());
100-
$finalIconFile->setImageFormat('SVG');
101-
$svgFile = $folder->newFile($key . '.svg');
102-
$svgFile->putContent($finalIconFile->getImageBlob());
103-
return $svgFile;
104-
} catch (\ImagickException $e) {
105-
$this->logger->info('The image was requested to be no SVG file, but converting it to SVG failed: ' . $e->getMessage());
106-
}
107-
} else {
108-
return $folder->getFile($key . '.svg');
109-
}
110-
}
111-
// if SVG was not requested, but PNG is supported
112-
if (!$useSvg && $this->canConvert('PNG')) {
94+
// only convert SVG originals to PNG when SVG output is not desired;
95+
// converting raster images to SVG produces broken output and is not useful
96+
$isOriginalSvg = ($mime === 'image/svg+xml' || $mime === 'image/svg');
97+
if ($isOriginalSvg && !$useSvg && $this->canConvert('SVG') && $this->canConvert('PNG')) {
11398
if (!$folder->fileExists($key . '.png')) {
11499
try {
115100
$finalIconFile = new \Imagick();
@@ -120,13 +105,12 @@ public function getImage(string $key, bool $useSvg = true): ISimpleFile {
120105
$pngFile->putContent($finalIconFile->getImageBlob());
121106
return $pngFile;
122107
} catch (\ImagickException $e) {
123-
$this->logger->info('The image was requested to be no SVG file, but converting it to PNG failed: ' . $e->getMessage());
108+
$this->logger->info('Converting SVG to PNG failed: ' . $e->getMessage());
124109
}
125110
} else {
126111
return $folder->getFile($key . '.png');
127112
}
128113
}
129-
// fallback to the original file
130114
return $folder->getFile($key);
131115
}
132116

@@ -157,7 +141,7 @@ public function getCustomImages(): array {
157141
* @throws NotPermittedException
158142
*/
159143
public function getCacheFolder(): ISimpleFolder {
160-
$cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0');
144+
$cacheBusterValue = (string)$this->appConfig->getAppValueInt(ConfigLexicon::CACHE_BUSTER);
161145
try {
162146
$folder = $this->getRootFolder()->getFolder($cacheBusterValue);
163147
} catch (NotFoundException $e) {
@@ -214,6 +198,12 @@ public function delete(string $key): void {
214198
} catch (NotFoundException $e) {
215199
} catch (NotPermittedException $e) {
216200
}
201+
try {
202+
$file = $this->getRootFolder()->getFolder('images')->getFile($key . '.svg');
203+
$file->delete();
204+
} catch (NotFoundException $e) {
205+
} catch (NotPermittedException $e) {
206+
}
217207

218208
if ($key === 'logo') {
219209
$this->config->deleteAppValue('theming', 'logoDimensions');

lib/private/Server.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,6 +1030,11 @@ public function __construct($webRoot, \OC\Config $config) {
10301030
$c->get(LoggerInterface::class),
10311031
$c->get(ITempManager::class),
10321032
$backgroundService,
1033+
new AppConfig(
1034+
$c->get(IConfig::class),
1035+
$c->get(IAppConfig::class),
1036+
'theming',
1037+
),
10331038
);
10341039
return new ThemingDefaults(
10351040
$c->get(\OCP\IConfig::class),

0 commit comments

Comments
 (0)