Merge pull request #61302 from nextcloud/backport/61253/stable34

[stable34] fix(theming): preserve uploaded favicon and touch icon

Author: AndyScherzinger

apps/theming/lib/Controller/IconController.php

@@ -99,8 +99,8 @@ public function getFavicon(string $app = 'core'): Response {
 			$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']);
 		} catch (NotFoundException $e) {
 		}
-		// retrieve or generate app specific favicon
-		if (($this->imageManager->canConvert('PNG') || $this->imageManager->canConvert('SVG')) && $this->imageManager->canConvert('ICO')) {
+		// retrieve or generate app specific favicon, but only if no custom favicon was uploaded
+		if ($iconFile === null && ($this->imageManager->canConvert('PNG') || $this->imageManager->canConvert('SVG')) && $this->imageManager->canConvert('ICO')) {
 			$color = $this->themingDefaults->getColorPrimary();
 			try {
 				$iconFile = $this->imageManager->getCachedImage('favIcon-' . $app . $color);
@@ -141,14 +141,15 @@ public function getTouchIcon(string $app = 'core'): Response {
 		}
 
 		$response = null;
+		$iconFile = null;
 		// retrieve instance favicon
 		try {
 			$iconFile = $this->imageManager->getImage('favicon');
 			$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => $iconFile->getMimeType()]);
 		} catch (NotFoundException $e) {
 		}
-		// retrieve or generate app specific touch icon
-		if ($this->imageManager->canConvert('PNG')) {
+		// retrieve or generate app specific touch icon, but only if no custom favicon was uploaded
+		if ($iconFile === null && $this->imageManager->canConvert('PNG')) {
 			$color = $this->themingDefaults->getColorPrimary();
 			try {
 				$iconFile = $this->imageManager->getCachedImage('touchIcon-' . $app . $color);

apps/theming/tests/Controller/IconControllerTest.php

@@ -124,6 +124,24 @@ public function testGetFaviconThemed(): void {
 		$this->assertEquals($expected, $this->iconController->getFavicon());
 	}
 
+	public function testGetFaviconUploaded(): void {
+		// a custom favicon was uploaded, so it must be served as-is and the
+		// app-specific generation path must not overwrite it
+		$file = $this->iconFileMock('favicon.ico', 'filecontent');
+		$this->imageManager->expects($this->once())
+			->method('getImage')
+			->with('favicon', false)
+			->willReturn($file);
+		$this->imageManager->expects($this->never())
+			->method('getCachedImage');
+		$this->iconBuilder->expects($this->never())
+			->method('getFavicon');
+
+		$expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']);
+		$expected->cacheFor(86400);
+		$this->assertEquals($expected, $this->iconController->getFavicon());
+	}
+
 	public function testGetFaviconDefault(): void {
 		$this->imageManager->expects($this->once())
 			->method('getImage')
@@ -179,6 +197,24 @@ public function testGetTouchIconDefault(): void {
 		$this->assertEquals($expected, $this->iconController->getTouchIcon());
 	}
 
+	public function testGetTouchIconUploaded(): void {
+		// a custom favicon was uploaded, so it must be served as-is and the
+		// app-specific generation path must not overwrite it
+		$file = $this->iconFileMock('favicon.png', 'filecontent');
+		$this->imageManager->expects($this->once())
+			->method('getImage')
+			->with('favicon')
+			->willReturn($file);
+		$this->imageManager->expects($this->never())
+			->method('getCachedImage');
+		$this->iconBuilder->expects($this->never())
+			->method('getTouchIcon');
+
+		$expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image type']);
+		$expected->cacheFor(86400);
+		$this->assertEquals($expected, $this->iconController->getTouchIcon());
+	}
+
 	public function testGetTouchIconFail(): void {
 		$this->imageManager->expects($this->once())
 			->method('getImage')