Theming: Add tests for mimetype and upload error checks

Signed-off-by: Julius Härtl <jus@bitgrid.net>

apps/theming/lib/Controller/ThemingController.php

@@ -208,16 +208,16 @@ class ThemingController extends Controller {
 		$newLogo = $this->request->getUploadedFile('uploadlogo');
 		$newBackgroundLogo = $this->request->getUploadedFile('upload-login-background');
 		$error = null;
-		$phpFileUploadErrors = array(
+		$phpFileUploadErrors = [
-			0 => $this->l10n->t('There is no error, the file uploaded with success'),
+			UPLOAD_ERR_OK => $this->l10n->t('There is no error, the file uploaded with success'),
-			1 => $this->l10n->t('The uploaded file exceeds the upload_max_filesize directive in php.ini'),
+			UPLOAD_ERR_INI_SIZE => $this->l10n->t('The uploaded file exceeds the upload_max_filesize directive in php.ini'),
-			2 => $this->l10n->t('The uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the HTML form'),
+			UPLOAD_ERR_FORM_SIZE => $this->l10n->t('The uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the HTML form'),
-			3 => $this->l10n->t('The uploaded file was only partially uploaded'),
+			UPLOAD_ERR_PARTIAL => $this->l10n->t('The uploaded file was only partially uploaded'),
-			4 => $this->l10n->t('No file was uploaded'),
+			UPLOAD_ERR_NO_FILE => $this->l10n->t('No file was uploaded'),
-			6 => $this->l10n->t('Missing a temporary folder'),
+			UPLOAD_ERR_NO_TMP_DIR => $this->l10n->t('Missing a temporary folder'),
-			7 => $this->l10n->t('Failed to write file to disk.'),
+			UPLOAD_ERR_CANT_WRITE => $this->l10n->t('Failed to write file to disk.'),
-			8 => $this->l10n->t('A PHP extension stopped the file upload.'),
+			UPLOAD_ERR_EXTENSION => $this->l10n->t('A PHP extension stopped the file upload.'),
-		);
+		];
 		if (empty($newLogo) && empty($newBackgroundLogo)) {
 			$error = $this->l10n->t('No file uploaded');
 		}
@@ -233,7 +233,8 @@ class ThemingController extends Controller {
 				[
 					'data' => [
 						'message' => $error
-					]
+					],
+					'status' => 'failure',
 				],
 				Http::STATUS_UNPROCESSABLE_ENTITY
 			);

apps/theming/tests/Controller/ThemingControllerTest.php

@@ -229,6 +229,56 @@ class ThemingControllerTest extends TestCase {
 					[
 						'message' => 'No file uploaded',
 					],
+				'status' => 'failure',
+			],
+			Http::STATUS_UNPROCESSABLE_ENTITY
+		);
+
+		$this->assertEquals($expected, $this->themingController->updateLogo());
+	}
+
+	public function testUpdateLogoInvalidMimeType() {
+		$this->request
+			->expects($this->at(0))
+			->method('getParam')
+			->with('backgroundColor')
+			->willReturn(false);
+		$this->request
+			->expects($this->at(1))
+			->method('getUploadedFile')
+			->with('uploadlogo')
+			->willReturn([
+				'tmp_name' => 'logo.pdf',
+				'type' => 'application/pdf',
+				'name' => 'logo.pdf',
+				'error' => 0,
+			]);
+		$this->request
+			->expects($this->at(2))
+			->method('getUploadedFile')
+			->with('upload-login-background')
+			->willReturn(null);
+		$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
 		);
@@ -261,13 +311,17 @@ class ThemingControllerTest extends TestCase {
 
 	public function dataUpdateImages() {
 		return [
-			[false],
+			['image/jpeg', false],
-			[true]
+			['image/jpeg', true],
+			['image/gif'],
+			['image/png'],
+			['image/svg+xml'],
+			['text/svg'],
 		];
 	}
 
 	/** @dataProvider dataUpdateImages */
-	public function testUpdateLogoNormalLogoUpload($folderExists) {
+	public function testUpdateLogoNormalLogoUpload($mimeType, $folderExists=true) {
 		$tmpLogo = \OC::$server->getTempManager()->getTemporaryFolder() . '/logo.svg';
 		$destination = \OC::$server->getTempManager()->getTemporaryFolder();
 
@@ -283,7 +337,7 @@ class ThemingControllerTest extends TestCase {
 			->with('uploadlogo')
 			->willReturn([
 				'tmp_name' => $tmpLogo,
-				'type' => 'text/svg',
+				'type' => $mimeType,
 				'name' => 'logo.svg',
 				'error' => 0,
 			]);
@@ -461,6 +515,106 @@ class ThemingControllerTest extends TestCase {
 		$this->assertEquals($expected, $this->themingController->updateLogo());
 	}
 
+	public function dataPhpUploadErrors() {
+		return [
+			[UPLOAD_ERR_INI_SIZE, 'The uploaded file exceeds the upload_max_filesize directive in php.ini'],
+			[UPLOAD_ERR_FORM_SIZE, 'The uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the HTML form'],
+			[UPLOAD_ERR_PARTIAL, 'The uploaded file was only partially uploaded'],
+			[UPLOAD_ERR_NO_FILE, 'No file was uploaded'],
+			[UPLOAD_ERR_NO_TMP_DIR, 'Missing a temporary folder'],
+			[UPLOAD_ERR_CANT_WRITE, 'Failed to write file to disk.'],
+			[UPLOAD_ERR_EXTENSION, 'A PHP extension stopped the file upload.'],
+		];
+	}
+
+	/**
+	 * @dataProvider dataPhpUploadErrors
+	 */
+	public function testUpdateLogoLoginScreenUploadWithInvalidImageUpload($error, $expectedErrorMessage) {
+		$this->request
+			->expects($this->at(0))
+			->method('getParam')
+			->with('backgroundColor')
+			->willReturn(false);
+		$this->request
+			->expects($this->at(1))
+			->method('getUploadedFile')
+			->with('uploadlogo')
+			->willReturn(null);
+		$this->request
+			->expects($this->at(2))
+			->method('getUploadedFile')
+			->with('upload-login-background')
+			->willReturn([
+				'tmp_name' => '',
+				'type' => 'text/svg',
+				'name' => 'logo.svg',
+				'error' => $error,
+			]);
+		$this->l10n
+			->expects($this->any())
+			->method('t')
+			->will($this->returnCallback(function($str) {
+				return $str;
+			}));
+
+		$expected = new DataResponse(
+			[
+				'data' =>
+					[
+						'message' => $expectedErrorMessage,
+					],
+				'status' => 'failure'
+			],
+			Http::STATUS_UNPROCESSABLE_ENTITY
+		);
+		$this->assertEquals($expected, $this->themingController->updateLogo());
+	}
+
+	/**
+	 * @dataProvider dataPhpUploadErrors
+	 */
+	public function testUpdateLogoUploadWithInvalidImageUpload($error, $expectedErrorMessage) {
+		$this->request
+			->expects($this->at(0))
+			->method('getParam')
+			->with('backgroundColor')
+			->willReturn(false);
+		$this->request
+			->expects($this->at(1))
+			->method('getUploadedFile')
+			->with('uploadlogo')
+			->willReturn([
+				'tmp_name' => '',
+				'type' => 'text/svg',
+				'name' => 'logo.svg',
+				'error' => $error,
+			]);
+		$this->request
+			->expects($this->at(2))
+			->method('getUploadedFile')
+			->with('upload-login-background')
+			->willReturn(null);
+		$this->l10n
+			->expects($this->any())
+			->method('t')
+			->will($this->returnCallback(function($str) {
+				return $str;
+			}));
+
+		$expected = new DataResponse(
+			[
+				'data' =>
+					[
+						'message' => $expectedErrorMessage
+					],
+				'status' => 'failure'
+			],
+			Http::STATUS_UNPROCESSABLE_ENTITY
+		);
+		$this->assertEquals($expected, $this->themingController->updateLogo());
+	}
+
 	public function testUndo() {
 		$this->l10n
 			->expects($this->once())