diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php
index 3b83e755b65da..0692528987b96 100644
--- a/apps/theming/lib/Controller/ThemingController.php
+++ b/apps/theming/lib/Controller/ThemingController.php
@@ -356,7 +356,13 @@ public function getImage(string $key, bool $useSvg = true) {
$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;
}
@@ -435,7 +441,7 @@ public function getThemeStylesheet(string $themeId, bool $plain = false, bool $w
#[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 1979656dd1e82..ad3903f148e9b 100644
--- a/apps/theming/lib/ImageManager.php
+++ b/apps/theming/lib/ImageManager.php
@@ -8,6 +8,7 @@
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 @@ public function __construct(
private LoggerInterface $logger,
private ITempManager $tempManager,
private BackgroundService $backgroundService,
+ private IAppConfig $appConfig,
) {
}
@@ -40,7 +42,7 @@ public function __construct(
* @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 @@ public function getImageUrlAbsolute(string $key): string {
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 @@ public function getImage(string $key, bool $useSvg = true): ISimpleFile {
$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 @@ public function getCustomImages(): array {
* @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 @@ public function delete(string $key): void {
} 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/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php
index 24b53b53d5190..2b684eb8c3dc8 100644
--- a/apps/theming/tests/Controller/ThemingControllerTest.php
+++ b/apps/theming/tests/Controller/ThemingControllerTest.php
@@ -666,6 +666,29 @@ public function testGetLogo(): void {
}
+ public function testGetLogoOriginalFile(): void {
+ $file = $this->createMock(ISimpleFile::class);
+ $file->method('getName')->willReturn('logo');
+ $file->method('getMTime')->willReturn(42);
+ $this->imageManager->expects($this->once())
+ ->method('getImage')
+ ->willReturn($file);
+ $this->appConfig
+ ->expects($this->once())
+ ->method('getAppValueString')
+ ->with('logoMime', '')
+ ->willReturn('image/png');
+
+ @$expected = new FileDisplayResponse($file);
+ $expected->cacheFor(3600);
+ $expected->addHeader('Content-Type', 'image/png');
+ $expected->addHeader('Content-Disposition', 'attachment; filename="logo"');
+ $csp = new ContentSecurityPolicy();
+ $csp->allowInlineStyle();
+ $expected->setContentSecurityPolicy($csp);
+ @$this->assertEquals($expected, $this->themingController->getImage('logo', false));
+ }
+
public function testGetLoginBackgroundNotExistent(): void {
$this->imageManager->method('getImage')
->with($this->equalTo('background'))
@@ -708,10 +731,10 @@ public static function dataGetManifest(): array {
#[\PHPUnit\Framework\Attributes\DataProvider('dataGetManifest')]
public function testGetManifest(bool $standalone): void {
- $this->config
+ $this->appConfig
->expects($this->once())
- ->method('getAppValue')
- ->with('theming', 'cachebuster', '0')
+ ->method('getAppValueString')
+ ->with('cachebuster', '0')
->willReturn('0');
$this->themingDefaults
->expects($this->any())
diff --git a/apps/theming/tests/ImageManagerTest.php b/apps/theming/tests/ImageManagerTest.php
index 0c4d555cc00ef..4e9da5986784f 100644
--- a/apps/theming/tests/ImageManagerTest.php
+++ b/apps/theming/tests/ImageManagerTest.php
@@ -9,6 +9,7 @@
use OCA\Theming\ImageManager;
use OCA\Theming\Service\BackgroundService;
+use OCP\AppFramework\Services\IAppConfig;
use OCP\Files\IAppData;
use OCP\Files\NotFoundException;
use OCP\Files\SimpleFS\ISimpleFile;
@@ -29,6 +30,7 @@ class ImageManagerTest extends TestCase {
private LoggerInterface&MockObject $logger;
private ITempManager&MockObject $tempManager;
private ISimpleFolder&MockObject $rootFolder;
+ private IAppConfig&MockObject $appConfig;
protected ImageManager $imageManager;
protected function setUp(): void {
@@ -41,6 +43,7 @@ protected function setUp(): void {
$this->tempManager = $this->createMock(ITempManager::class);
$this->rootFolder = $this->createMock(ISimpleFolder::class);
$backgroundService = $this->createMock(BackgroundService::class);
+ $this->appConfig = $this->createMock(IAppConfig::class);
$this->imageManager = new ImageManager(
$this->config,
$this->appData,
@@ -49,6 +52,7 @@ protected function setUp(): void {
$this->logger,
$this->tempManager,
$backgroundService,
+ $this->appConfig,
);
$this->appData
->expects($this->any())
@@ -79,26 +83,14 @@ public function mockGetImage($key, $file) {
->with('logo')
->willThrowException(new NotFoundException());
} else {
- $file->expects($this->once())
- ->method('getContent')
- ->willReturn(file_get_contents(__DIR__ . '/../../../tests/data/testimage.png'));
- $folder->expects($this->exactly(2))
+ $folder->expects($this->once())
->method('fileExists')
- ->willReturnMap([
- ['logo', true],
- ['logo.png', false],
- ]);
+ ->with('logo')
+ ->willReturn(true);
$folder->expects($this->once())
->method('getFile')
->with('logo')
->willReturn($file);
- $newFile = $this->createMock(ISimpleFile::class);
- $folder->expects($this->once())
- ->method('newFile')
- ->with('logo.png')
- ->willReturn($newFile);
- $newFile->expects($this->once())
- ->method('putContent');
$this->rootFolder->expects($this->once())
->method('getFolder')
->with('images')
@@ -108,12 +100,14 @@ public function mockGetImage($key, $file) {
public function testGetImageUrl(): void {
$this->checkImagick();
- $this->config->expects($this->exactly(2))
+ $this->appConfig->expects($this->once())
+ ->method('getAppValueInt')
+ ->with('cachebuster')
+ ->willReturn(0);
+ $this->config->expects($this->once())
->method('getAppValue')
- ->willReturnMap([
- ['theming', 'cachebuster', '0', '0'],
- ['theming', 'logoMime', '', '0'],
- ]);
+ ->with('theming', 'logoMime', '')
+ ->willReturn('image/png');
$this->urlGenerator->expects($this->once())
->method('linkToRoute')
->willReturn('url-to-image');
@@ -121,12 +115,14 @@ public function testGetImageUrl(): void {
}
public function testGetImageUrlDefault(): void {
- $this->config->expects($this->exactly(2))
+ $this->appConfig->expects($this->once())
+ ->method('getAppValueInt')
+ ->with('cachebuster')
+ ->willReturn(0);
+ $this->config->expects($this->once())
->method('getAppValue')
- ->willReturnMap([
- ['theming', 'cachebuster', '0', '0'],
- ['theming', 'logoMime', '', ''],
- ]);
+ ->with('theming', 'logoMime', '')
+ ->willReturn('');
$this->urlGenerator->expects($this->once())
->method('imagePath')
->with('core', 'logo/logo.png')
@@ -136,12 +132,14 @@ public function testGetImageUrlDefault(): void {
public function testGetImageUrlAbsolute(): void {
$this->checkImagick();
- $this->config->expects($this->exactly(2))
+ $this->appConfig->expects($this->once())
+ ->method('getAppValueInt')
+ ->with('cachebuster')
+ ->willReturn(0);
+ $this->config->expects($this->once())
->method('getAppValue')
- ->willReturnMap([
- ['theming', 'cachebuster', '0', '0'],
- ['theming', 'logoMime', '', ''],
- ]);
+ ->with('theming', 'logoMime', '')
+ ->willReturn('');
$this->urlGenerator->expects($this->any())
->method('getAbsoluteUrl')
->willReturn('url-to-image-absolute?v=0');
@@ -149,15 +147,69 @@ public function testGetImageUrlAbsolute(): void {
}
public function testGetImage(): void {
- $this->checkImagick();
$this->config->expects($this->once())
- ->method('getAppValue')->with('theming', 'logoMime', false)
- ->willReturn('png');
+ ->method('getAppValue')->with('theming', 'logoMime', '')
+ ->willReturn('image/png');
$file = $this->createMock(ISimpleFile::class);
$this->mockGetImage('logo', $file);
$this->assertEquals($file, $this->imageManager->getImage('logo', false));
}
+ public function testGetImageSvgToSvg(): void {
+ $this->config->expects($this->once())
+ ->method('getAppValue')->with('theming', 'logoMime', '')
+ ->willReturn('image/svg+xml');
+ $folder = $this->createMock(ISimpleFolder::class);
+ $file = $this->createMock(ISimpleFile::class);
+ $folder->expects($this->once())
+ ->method('fileExists')
+ ->with('logo')
+ ->willReturn(true);
+ $folder->expects($this->once())
+ ->method('getFile')
+ ->with('logo')
+ ->willReturn($file);
+ $this->rootFolder->expects($this->once())
+ ->method('getFolder')
+ ->with('images')
+ ->willReturn($folder);
+ $this->assertEquals($file, $this->imageManager->getImage('logo', true));
+ }
+
+ public function testGetImageSvgToPng(): void {
+ $this->checkImagick();
+ $this->config->expects($this->once())
+ ->method('getAppValue')->with('theming', 'logoMime', '')
+ ->willReturn('image/svg+xml');
+ $folder = $this->createMock(ISimpleFolder::class);
+ $svgFile = $this->createMock(ISimpleFile::class);
+ $pngFile = $this->createMock(ISimpleFile::class);
+ $svgFile->expects($this->once())
+ ->method('getContent')
+ ->willReturn(file_get_contents(__DIR__ . '/../../../core/img/logo/logo.svg'));
+ $folder->expects($this->exactly(2))
+ ->method('fileExists')
+ ->willReturnMap([
+ ['logo', true],
+ ['logo.png', false],
+ ]);
+ $folder->expects($this->once())
+ ->method('getFile')
+ ->with('logo')
+ ->willReturn($svgFile);
+ $folder->expects($this->once())
+ ->method('newFile')
+ ->with('logo.png')
+ ->willReturn($pngFile);
+ $pngFile->expects($this->once())
+ ->method('putContent');
+ $this->rootFolder->expects($this->once())
+ ->method('getFolder')
+ ->with('images')
+ ->willReturn($folder);
+ $this->assertEquals($pngFile, $this->imageManager->getImage('logo', false));
+ }
+
public function testGetImageUnset(): void {
$this->expectException(NotFoundException::class);
@@ -170,10 +222,10 @@ public function testGetImageUnset(): void {
public function testGetCacheFolder(): void {
$folder = $this->createMock(ISimpleFolder::class);
- $this->config->expects($this->once())
- ->method('getAppValue')
- ->with('theming', 'cachebuster', '0')
- ->willReturn('0');
+ $this->appConfig->expects($this->once())
+ ->method('getAppValueInt')
+ ->with('cachebuster')
+ ->willReturn(0);
$this->rootFolder->expects($this->once())
->method('getFolder')
->with('0')
@@ -182,10 +234,10 @@ public function testGetCacheFolder(): void {
}
public function testGetCacheFolderCreate(): void {
$folder = $this->createMock(ISimpleFolder::class);
- $this->config->expects($this->exactly(2))
- ->method('getAppValue')
- ->with('theming', 'cachebuster', '0')
- ->willReturn('0');
+ $this->appConfig->expects($this->exactly(2))
+ ->method('getAppValueInt')
+ ->with('cachebuster')
+ ->willReturn(0);
$this->rootFolder->expects($this->exactly(2))
->method('getFolder')
->with('0')
@@ -261,10 +313,10 @@ public function testSetCachedImageCreate(): void {
private function setupCacheFolder() {
$folder = $this->createMock(ISimpleFolder::class);
- $this->config->expects($this->once())
- ->method('getAppValue')
- ->with('theming', 'cachebuster', '0')
- ->willReturn('0');
+ $this->appConfig->expects($this->once())
+ ->method('getAppValueInt')
+ ->with('cachebuster')
+ ->willReturn(0);
$this->rootFolder->expects($this->once())
->method('getFolder')
->with('0')
@@ -286,10 +338,10 @@ public function testCleanup(): void {
$folders[0]->expects($this->once())->method('delete');
$folders[1]->expects($this->once())->method('delete');
$folders[2]->expects($this->never())->method('delete');
- $this->config->expects($this->once())
- ->method('getAppValue')
- ->with('theming', 'cachebuster', '0')
- ->willReturn('2');
+ $this->appConfig->expects($this->once())
+ ->method('getAppValueInt')
+ ->with('cachebuster')
+ ->willReturn(2);
$this->rootFolder->expects($this->once())
->method('getDirectoryListing')
->willReturn($folders);
diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml
index b92298da7d122..410c062fd51a1 100644
--- a/build/psalm-baseline.xml
+++ b/build/psalm-baseline.xml
@@ -2324,13 +2324,6 @@