diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php index ed63025aca499..26233dff920d8 100644 --- a/core/Controller/AvatarController.php +++ b/core/Controller/AvatarController.php @@ -30,6 +30,7 @@ use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\DataDisplayResponse; use OCP\Files\NotFoundException; use OCP\IAvatarManager; @@ -147,7 +148,7 @@ public function getAvatar($userId, $size) { * @NoAdminRequired * * @param string $path - * @return DataResponse + * @return DataResponse|JSONResponse */ public function postAvatar($path) { $userId = $this->userSession->getUser()->getUID(); @@ -172,7 +173,22 @@ public function postAvatar($path) { $headers ); } - $content = $node->getContent(); + + if ($node->getMimeType() !== 'image/jpeg' && $node->getMimeType() !== 'image/png') { + return new JSONResponse( + ['data' => ['message' => $this->l->t('The selected file is not an image.')]], + Http::STATUS_BAD_REQUEST + ); + } + + try { + $content = $node->getContent(); + } catch (\OCP\Files\NotPermittedException $e) { + return new JSONResponse( + ['data' => ['message' => $this->l->t('The selected file cannot be read.')]], + Http::STATUS_BAD_REQUEST + ); + } } elseif (!is_null($files)) { if ( $files['error'][0] === 0 && diff --git a/settings/js/personal.js b/settings/js/personal.js index 16a8d184da6f1..0f13d0b8b5eb8 100644 --- a/settings/js/personal.js +++ b/settings/js/personal.js @@ -286,8 +286,8 @@ $(document).ready(function () { msg = data.jqXHR.responseJSON.data.message; } avatarResponseHandler({ - data: { - message: t('settings', 'An error occurred: {message}', { message: msg }) + data: { + message: msg } }); } @@ -304,7 +304,7 @@ $(document).ready(function () { url: OC.generateUrl('/avatar/'), data: { path: path } }).done(avatarResponseHandler) - .fail(function(jqXHR, status){ + .fail(function(jqXHR) { var msg = jqXHR.statusText + ' (' + jqXHR.status + ')'; if (!_.isUndefined(jqXHR.responseJSON) && !_.isUndefined(jqXHR.responseJSON.data) && @@ -314,7 +314,7 @@ $(document).ready(function () { } avatarResponseHandler({ data: { - message: t('settings', 'An error occurred: {message}', { message: msg }) + message: msg } }); }); diff --git a/tests/Core/Controller/AvatarControllerTest.php b/tests/Core/Controller/AvatarControllerTest.php index d45d0618230d2..272666deaf9d8 100644 --- a/tests/Core/Controller/AvatarControllerTest.php +++ b/tests/Core/Controller/AvatarControllerTest.php @@ -36,9 +36,9 @@ function is_uploaded_file($filename) { use OCP\AppFramework\Http; use OCP\Files\File; use OCP\Files\NotFoundException; -use OCP\IUser; +use OCP\Files\NotPermittedException; use OCP\IAvatar; -use Punic\Exception; +use OCP\IUser; use Test\Traits\UserTrait; /** @@ -314,7 +314,13 @@ public function testPostAvatarFromFile() { //Mock node API call $file = $this->getMockBuilder('OCP\Files\File') ->disableOriginalConstructor()->getMock(); - $file->method('getContent')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); + + $file->expects($this->once()) + ->method('getContent') + ->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); + $file->expects($this->once()) + ->method('getMimeType') + ->willReturn('image/jpeg'); $this->container['UserFolder']->method('get')->willReturn($file); //Create request return @@ -341,6 +347,36 @@ public function testPostAvatarFromNoFile() { $this->assertEquals(['data' => ['message' => 'Please select a file.']], $response->getData()); } + public function testPostAvatarInvalidType() { + $file = $this->getMockBuilder('OCP\Files\File') + ->disableOriginalConstructor()->getMock(); + $file->expects($this->never()) + ->method('getContent'); + $file->expects($this->exactly(2)) + ->method('getMimeType') + ->willReturn('text/plain'); + $this->container['UserFolder']->method('get')->willReturn($file); + + $expectedResponse = new Http\JSONResponse(['data' => ['message' => 'The selected file is not an image.']], Http::STATUS_BAD_REQUEST); + $this->assertEquals($expectedResponse, $this->avatarController->postAvatar('avatar.jpg')); + } + + public function testPostAvatarNotPermittedException() { + $file = $this->getMockBuilder('OCP\Files\File') + ->disableOriginalConstructor()->getMock(); + $file->expects($this->once()) + ->method('getContent') + ->willThrowException(new NotPermittedException()); + $file->expects($this->once()) + ->method('getMimeType') + ->willReturn('image/jpeg'); + $userFolder = $this->getMockBuilder('OCP\Files\Folder')->getMock(); + $this->container['UserFolder']->method('get')->willReturn($file); + + $expectedResponse = new Http\JSONResponse(['data' => ['message' => 'The selected file cannot be read.']], Http::STATUS_BAD_REQUEST); + $this->assertEquals($expectedResponse, $this->avatarController->postAvatar('avatar.jpg')); + } + /** * Test what happens if the upload of the avatar fails */ @@ -350,7 +386,13 @@ public function testPostAvatarException() { ->will($this->throwException(new \Exception("foo"))); $file = $this->getMockBuilder('OCP\Files\File') ->disableOriginalConstructor()->getMock(); - $file->method('getContent')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); + + $file->expects($this->once()) + ->method('getContent') + ->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); + $file->expects($this->once()) + ->method('getMimeType') + ->willReturn('image/jpeg'); $this->container['UserFolder']->method('get')->willReturn($file); $this->container['Logger']->expects($this->once())