mirror of
https://github.com/nextcloud/server.git
synced 2026-05-12 16:39:39 -04:00
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>
This commit is contained in:
parent
92d196d0bc
commit
8a0080cbbb
4 changed files with 31 additions and 28 deletions
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
|
|
|
|||
|
|
@ -29,12 +29,13 @@ const emit = defineEmits<{
|
|||
|
||||
const isSaving = ref(false)
|
||||
const mime = ref(loadState<AdminThemingParameters>('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) {
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
Loading…
Reference in a new issue