Check the mimetype before reading the content and catch exception

Signed-off-by: Joas Schilling <coding@schilljs.com>

diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php
index ed63025aca499d39f7983ddaedeabff014e14cd4..26233dff920d8f52ede7daebe9ddaa023dff35b4 100644 (file)
--- a/core/Controller/AvatarController.php
+++ b/core/Controller/AvatarController.php
@@ -30,6 +30,7 @@ namespace OC\Core\Controller;
 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 @@ class AvatarController extends Controller {
         * @NoAdminRequired
         *
         * @param string $path
-        * @return DataResponse
+        * @return DataResponse|JSONResponse
         */
        public function postAvatar($path) {
                $userId = $this->userSession->getUser()->getUID();
@@ -172,7 +173,22 @@ class AvatarController extends Controller {
                                        $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/tests/Core/Controller/AvatarControllerTest.php b/tests/Core/Controller/AvatarControllerTest.php
index d45d0618230d2db0318f231ba7317eaebbaf3736..272666deaf9d8348f0803018e3abf01fb6a994c2 100644 (file)
--- a/tests/Core/Controller/AvatarControllerTest.php
+++ b/tests/Core/Controller/AvatarControllerTest.php
@@ -36,9 +36,9 @@ use OCP\AppFramework\IAppContainer;
 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 @@ class AvatarControllerTest extends \Test\TestCase {
                //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 @@ class AvatarControllerTest extends \Test\TestCase {
                $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 @@ class AvatarControllerTest extends \Test\TestCase {
                        ->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())