Theming: Handle errors on uploaded files properly

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

2 files changed, 47 insertions, 17 deletions

diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php
index 06c2c430b7f..e73fc16b20b 100644
--- a/apps/theming/lib/Controller/ThemingController.php
+++ b/apps/theming/lib/Controller/ThemingController.php
@@ -207,11 +207,32 @@ class ThemingController extends Controller {
 		}
 		$newLogo = $this->request->getUploadedFile('uploadlogo');
 		$newBackgroundLogo = $this->request->getUploadedFile('upload-login-background');
+		$error = null;
+		$phpFileUploadErrors = array(
+			0 => $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'),
+			2 => $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'),
+			4 => $this->l10n->t('No file was uploaded'),
+			6 => $this->l10n->t('Missing a temporary folder'),
+			7 => $this->l10n->t('Failed to write file to disk.'),
+			8 => $this->l10n->t('A PHP extension stopped the file upload.'),
+		);
 		if (empty($newLogo) && empty($newBackgroundLogo)) {
+			$error = $this->l10n->t('No file uploaded');
+		}
+		if (!empty($newLogo) && array_key_exists('error', $newLogo) && $newLogo['error'] !== UPLOAD_ERR_OK) {
+			$error = $phpFileUploadErrors[$newLogo['error']];
+		}
+		if (!empty($newBackgroundLogo) && array_key_exists('error', $newBackgroundLogo) && $newBackgroundLogo['error'] !== UPLOAD_ERR_OK) {
+			$error = $phpFileUploadErrors[$newBackgroundLogo['error']];
+		}
+
+		if ($error !== null) {
 			return new DataResponse(
 				[
 					'data' => [
-						'message' => $this->l10n->t('No file uploaded')
+						'message' => $error
 					]
 				],
 				Http::STATUS_UNPROCESSABLE_ENTITY
diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php
index c03eccb6eef..96a742cfa37 100644
--- a/apps/theming/tests/Controller/ThemingControllerTest.php
+++ b/apps/theming/tests/Controller/ThemingControllerTest.php
@@ -131,8 +131,9 @@ class ThemingControllerTest extends TestCase {
 		$this->l10n
 			->expects($this->once())
 			->method('t')
-			->with($message)
-			->willReturn($message);
+			->will($this->returnCallback(function($str) {
+				return $str;
+			}));
 		$this->scssCacher
 			->expects($this->once())
 			->method('getCachedSCSS')
@@ -183,8 +184,9 @@ class ThemingControllerTest extends TestCase {
 		$this->l10n
 			->expects($this->once())
 			->method('t')
-			->with($message)
-			->willReturn($message);
+			->will($this->returnCallback(function($str) {
+				return $str;
+			}));
 
 		$expected = new DataResponse(
 			[
@@ -215,10 +217,11 @@ class ThemingControllerTest extends TestCase {
 			->with('upload-login-background')
 			->willReturn(null);
 		$this->l10n
-			->expects($this->once())
+			->expects($this->any())
 			->method('t')
-			->with('No file uploaded')
-			->willReturn('No file uploaded');
+			->will($this->returnCallback(function($str) {
+				return $str;
+			}));
 
 		$expected = new DataResponse(
 			[
@@ -282,6 +285,7 @@ class ThemingControllerTest extends TestCase {
 				'tmp_name' => $tmpLogo,
 				'type' => 'text/svg',
 				'name' => 'logo.svg',
+				'error' => 0,
 			]);
 		$this->request
 			->expects($this->at(2))
@@ -289,10 +293,11 @@ class ThemingControllerTest extends TestCase {
 			->with('upload-login-background')
 			->willReturn(null);
 		$this->l10n
-			->expects($this->once())
+			->expects($this->any())
 			->method('t')
-			->with('Saved')
-			->willReturn('Saved');
+			->will($this->returnCallback(function($str) {
+				return $str;
+			}));
 
 
 		$file = $this->createMock(ISimpleFile::class);
@@ -357,12 +362,14 @@ class ThemingControllerTest extends TestCase {
 				'tmp_name' => $tmpLogo,
 				'type' => 'text/svg',
 				'name' => 'logo.svg',
+				'error' => 0,
 			]);
 		$this->l10n
-			->expects($this->once())
+			->expects($this->any())
 			->method('t')
-			->with('Saved')
-			->willReturn('Saved');
+			->will($this->returnCallback(function($str) {
+				return $str;
+			}));
 
 		$file = $this->createMock(ISimpleFile::class);
 		$folder = $this->createMock(ISimpleFolder::class);
@@ -425,12 +432,14 @@ class ThemingControllerTest extends TestCase {
 				'tmp_name' => $tmpLogo,
 				'type' => 'text/svg',
 				'name' => 'logo.svg',
+				'error' => 0,
 			]);
 		$this->l10n
-			->expects($this->once())
+			->expects($this->any())
 			->method('t')
-			->with('Unsupported image type')
-			->willReturn('Unsupported image type');
+			->will($this->returnCallback(function($str) {
+				return $str;
+			}));
 
 		$folder = $this->createMock(ISimpleFolder::class);
 		$this->appData