Merge pull request #10963 from nextcloud/fix/10957/logo-scss

Updates logo scss to regard default values and updates favicon upload check

4 files changed, 77 insertions, 4 deletions

diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php
index a2727546e09..bffabf43dd7 100644
--- a/apps/theming/lib/Controller/IconController.php
+++ b/apps/theming/lib/Controller/IconController.php
@@ -115,7 +115,7 @@ class IconController extends Controller {
 		$response = null;
 		$iconFile = null;
 		try {
-			$iconFile = $this->imageManager->getImage('favicon');
+			$iconFile = $this->imageManager->getImage('favicon', false);
 			$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']);
 		} catch (NotFoundException $e) {
 		}
diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php
index 44f1ea51c06..a1fa5e57836 100644
--- a/apps/theming/lib/Controller/ThemingController.php
+++ b/apps/theming/lib/Controller/ThemingController.php
@@ -265,7 +265,7 @@ class ThemingController extends Controller {
 		$this->imageManager->delete($key);
 
 		$target = $folder->newFile($key);
-		$supportedFormats = ['image/jpeg', 'image/png', 'image/gif', 'image/svg+xml', 'image/svg'];
+		$supportedFormats = $this->getSupportedUploadImageFormats($key);
 		$detectedMimeType = mime_content_type($image['tmp_name']);
 		if (!in_array($image['type'], $supportedFormats) || !in_array($detectedMimeType, $supportedFormats)) {
 			return new DataResponse(
@@ -319,6 +319,24 @@ class ThemingController extends Controller {
 	}
 
 	/**
+	 * Returns a list of supported mime types for image uploads.
+	 * "favicon" images are only allowed to be SVG when imagemagick with SVG support is available.
+	 *
+	 * @param string $key The image key, e.g. "favicon"
+	 * @return array
+	 */
+	private function getSupportedUploadImageFormats(string $key): array {
+		$supportedFormats = ['image/jpeg', 'image/png', 'image/gif',];
+
+		if ($key !== 'favicon' || $this->imageManager->shouldReplaceIcons() === true) {
+			$supportedFormats[] = 'image/svg+xml';
+			$supportedFormats[] = 'image/svg';
+		}
+
+		return $supportedFormats;
+	}
+
+	/**
 	 * Revert setting to default value
 	 *
 	 * @param string $setting setting which should be reverted
diff --git a/apps/theming/tests/Controller/IconControllerTest.php b/apps/theming/tests/Controller/IconControllerTest.php
index b4b45a065b0..e749a1dbd44 100644
--- a/apps/theming/tests/Controller/IconControllerTest.php
+++ b/apps/theming/tests/Controller/IconControllerTest.php
@@ -117,7 +117,7 @@ class IconControllerTest extends TestCase {
 		}
 		$file = $this->iconFileMock('filename', 'filecontent');
 		$this->imageManager->expects($this->once())
-			->method('getImage')
+			->method('getImage', false)
 			->with('favicon')
 			->will($this->throwException(new NotFoundException()));
 		$this->imageManager->expects($this->any())
@@ -142,7 +142,7 @@ class IconControllerTest extends TestCase {
 	public function testGetFaviconFail() {
 		$this->imageManager->expects($this->once())
 			->method('getImage')
-			->with('favicon')
+			->with('favicon', false)
 			->will($this->throwException(new NotFoundException()));
 		$this->imageManager->expects($this->any())
 			->method('shouldReplaceIcons')
diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php
index a2105264f10..457e9900b5e 100644
--- a/apps/theming/tests/Controller/ThemingControllerTest.php
+++ b/apps/theming/tests/Controller/ThemingControllerTest.php
@@ -246,6 +246,61 @@ class ThemingControllerTest extends TestCase {
 		$this->assertEquals($expected, $this->themingController->uploadImage());
 	}
 
+	/**
+	 * Checks that trying to upload an SVG favicon without imagemagick
+	 * results in an unsupported media type response.
+	 *
+	 * @test
+	 * @return void
+	 */
+	public function testUploadSVGFaviconWithoutImagemagick() {
+		$this->imageManager
+			->method('shouldReplaceIcons')
+			->willReturn(false);
+
+		$this->request
+			->expects($this->at(0))
+			->method('getParam')
+			->with('key')
+			->willReturn('favicon');
+		$this->request
+			->expects($this->at(1))
+			->method('getUploadedFile')
+			->with('image')
+			->willReturn([
+				'tmp_name' => __DIR__  . '/../../../../tests/data/testimagelarge.svg',
+				'type' => 'image/svg',
+				'name' => 'testimagelarge.svg',
+				'error' => 0,
+			]);
+		$this->l10n
+			->expects($this->any())
+			->method('t')
+			->will($this->returnCallback(function($str) {
+				return $str;
+			}));
+
+		$folder = $this->createMock(ISimpleFolder::class);
+		$this->appData
+			->expects($this->once())
+			->method('getFolder')
+			->with('images')
+			->willReturn($folder);
+
+		$expected = new DataResponse(
+			[
+				'data' =>
+					[
+						'message' => 'Unsupported image type',
+					],
+				'status' => 'failure'
+			],
+			Http::STATUS_UNPROCESSABLE_ENTITY
+		);
+
+		$this->assertEquals($expected, $this->themingController->uploadImage());
+	}
+
 	public function testUpdateLogoInvalidMimeType() {
 		$this->request
 			->expects($this->at(0))