From f15e85c4f50b6c9e77742e15f8291a8628a28ef7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 21 Sep 2017 11:36:27 +0200 Subject: [PATCH] Theming: Add tests for mimetype and upload error checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- .../lib/Controller/ThemingController.php | 23 +-- .../Controller/ThemingControllerTest.php | 162 +++++++++++++++++- 2 files changed, 170 insertions(+), 15 deletions(-) diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index e6aa3a380b..ccc2634ec1 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/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( - 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.'), - ); + $phpFileUploadErrors = [ + UPLOAD_ERR_OK => $this->l10n->t('There is no error, the file uploaded with success'), + UPLOAD_ERR_INI_SIZE => $this->l10n->t('The uploaded file exceeds the upload_max_filesize directive in php.ini'), + UPLOAD_ERR_FORM_SIZE => $this->l10n->t('The uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the HTML form'), + UPLOAD_ERR_PARTIAL => $this->l10n->t('The uploaded file was only partially uploaded'), + UPLOAD_ERR_NO_FILE => $this->l10n->t('No file was uploaded'), + UPLOAD_ERR_NO_TMP_DIR => $this->l10n->t('Missing a temporary folder'), + UPLOAD_ERR_CANT_WRITE => $this->l10n->t('Failed to write file to disk.'), + 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 ); diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index 96a742cfa3..e964e886e5 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/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], - [true] + ['image/jpeg', false], + ['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())