From 3213b04aef30a66c7eebb750bfc4bbb6fe6cfa78 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 17 Feb 2015 00:47:29 +0100 Subject: [PATCH] Check if the offset exists before accessing This checks if the offset exists before accessing it and also adds unit tests to this function which would have catched this before :see_no_evil: Fixes https://github.com/owncloud/core/issues/14277 --- apps/files/controller/apicontroller.php | 52 ++-- .../tests/controller/apicontrollertest.php | 243 ++++++++++++++++++ 2 files changed, 278 insertions(+), 17 deletions(-) create mode 100644 apps/files/tests/controller/apicontrollertest.php diff --git a/apps/files/controller/apicontroller.php b/apps/files/controller/apicontroller.php index 1bb07010a2..de9fa20dde 100644 --- a/apps/files/controller/apicontroller.php +++ b/apps/files/controller/apicontroller.php @@ -1,6 +1,6 @@ + * Copyright (c) 2014-2015 Lukas Reschke * This file is licensed under the Affero General Public License version 3 or * later. * See the COPYING-README file. @@ -17,19 +17,27 @@ use OCP\AppFramework\Http\DownloadResponse; use OC\Preview; use OCA\Files\Service\TagService; +/** + * Class ApiController + * + * @package OCA\Files\Controller + */ class ApiController extends Controller { - - /** - * @var TagService $tagService - */ + /** @var TagService */ private $tagService; - public function __construct($appName, IRequest $request, TagService $tagService){ + /** + * @param string $appName + * @param IRequest $request + * @param TagService $tagService + */ + public function __construct($appName, + IRequest $request, + TagService $tagService){ parent::__construct($appName, $request); $this->tagService = $tagService; } - /** * Gets a thumbnail of the specified file * @@ -66,25 +74,31 @@ class ApiController extends Controller { * @CORS * * @param string $path path - * @param array $tags array of tags + * @param array|string $tags array of tags * @return DataResponse */ public function updateFileTags($path, $tags = null) { - $result = array(); + $result = []; // if tags specified or empty array, update tags if (!is_null($tags)) { try { $this->tagService->updateFileTags($path, $tags); } catch (\OCP\Files\NotFoundException $e) { - return new DataResponse(['message' => $e->getMessage()], Http::STATUS_NOT_FOUND); + return new DataResponse([ + 'message' => $e->getMessage() + ], Http::STATUS_NOT_FOUND); } catch (\OCP\Files\StorageNotAvailableException $e) { - return new DataResponse(['message' => $e->getMessage()], Http::STATUS_SERVICE_UNAVAILABLE); + return new DataResponse([ + 'message' => $e->getMessage() + ], Http::STATUS_SERVICE_UNAVAILABLE); } catch (\Exception $e) { - return new DataResponse(['message' => $e->getMessage()], Http::STATUS_NOT_FOUND); + return new DataResponse([ + 'message' => $e->getMessage() + ], Http::STATUS_NOT_FOUND); } $result['tags'] = $tags; } - return new DataResponse($result, Http::STATUS_OK); + return new DataResponse($result); } /** @@ -93,7 +107,7 @@ class ApiController extends Controller { * @NoAdminRequired * @CORS * - * @param array $tagName tag name to filter by + * @param array|string $tagName tag name to filter by * @return DataResponse */ public function getFilesByTag($tagName) { @@ -102,11 +116,15 @@ class ApiController extends Controller { foreach ($fileInfos as &$fileInfo) { $file = \OCA\Files\Helper::formatFileInfo($fileInfo); $parts = explode('/', dirname($fileInfo->getPath()), 4); - $file['path'] = '/' . $parts[3]; - $file['tags'] = array($tagName); + if(isset($parts[3])) { + $file['path'] = '/' . $parts[3]; + } else { + $file['path'] = '/'; + } + $file['tags'] = [$tagName]; $files[] = $file; } - return new DataResponse(array('files' => $files), Http::STATUS_OK); + return new DataResponse(['files' => $files]); } } diff --git a/apps/files/tests/controller/apicontrollertest.php b/apps/files/tests/controller/apicontrollertest.php new file mode 100644 index 0000000000..1be7a749a1 --- /dev/null +++ b/apps/files/tests/controller/apicontrollertest.php @@ -0,0 +1,243 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OCA\Files\Controller; + +use OC\Files\FileInfo; +use OCP\AppFramework\Http; +use OC\Preview; +use OCP\Files\NotFoundException; +use OCP\Files\StorageNotAvailableException; +use Test\TestCase; +use OCP\IRequest; +use OCA\Files\Service\TagService; +use OCP\AppFramework\Http\DataResponse; + +/** + * Class ApiController + * + * @package OCA\Files\Controller + */ +class ApiControllerTest extends TestCase { + /** @var string */ + private $appName = 'files'; + /** @var IRequest */ + private $request; + /** @var TagService */ + private $tagService; + /** @var ApiController */ + private $apiController; + + public function setUp() { + $this->request = $this->getMockBuilder('\OCP\IRequest') + ->disableOriginalConstructor() + ->getMock(); + $this->tagService = $this->getMockBuilder('\OCA\Files\Service\TagService') + ->disableOriginalConstructor() + ->getMock(); + + $this->apiController = new ApiController( + $this->appName, + $this->request, + $this->tagService + ); + } + + public function testGetFilesByTagEmpty() { + $tagName = 'MyTagName'; + $this->tagService->expects($this->once()) + ->method('getFilesByTag') + ->with($this->equalTo([$tagName])) + ->will($this->returnValue([])); + + $expected = new DataResponse(['files' => []]); + $this->assertEquals($expected, $this->apiController->getFilesByTag([$tagName])); + } + + public function testGetFilesByTagSingle() { + $tagName = 'MyTagName'; + $fileInfo = new FileInfo( + '/root.txt', + $this->getMockBuilder('\OC\Files\Storage\Storage') + ->disableOriginalConstructor() + ->getMock(), + '/var/www/root.txt', + [ + 'mtime' => 55, + 'mimetype' => 'application/pdf', + 'size' => 1234, + 'etag' => 'MyEtag', + ], + $this->getMockBuilder('\OCP\Files\Mount\IMountPoint') + ->disableOriginalConstructor() + ->getMock() + ); + $this->tagService->expects($this->once()) + ->method('getFilesByTag') + ->with($this->equalTo([$tagName])) + ->will($this->returnValue([$fileInfo])); + + $expected = new DataResponse([ + 'files' => [ + [ + 'id' => null, + 'parentId' => null, + 'date' => 'January 1, 1970 at 12:00:55 AM GMT+0', + 'mtime' => 55000, + 'icon' => \OCA\Files\Helper::determineIcon($fileInfo), + 'name' => 'root.txt', + 'permissions' => null, + 'mimetype' => 'application/pdf', + 'size' => 1234, + 'type' => 'file', + 'etag' => 'MyEtag', + 'path' => '/', + 'tags' => [ + [ + 'MyTagName' + ] + ], + ], + ], + ]); + $this->assertEquals($expected, $this->apiController->getFilesByTag([$tagName])); + } + + public function testGetFilesByTagMultiple() { + $tagName = 'MyTagName'; + $fileInfo1 = new FileInfo( + '/root.txt', + $this->getMockBuilder('\OC\Files\Storage\Storage') + ->disableOriginalConstructor() + ->getMock(), + '/var/www/root.txt', + [ + 'mtime' => 55, + 'mimetype' => 'application/pdf', + 'size' => 1234, + 'etag' => 'MyEtag', + ], + $this->getMockBuilder('\OCP\Files\Mount\IMountPoint') + ->disableOriginalConstructor() + ->getMock() + ); + $fileInfo2 = new FileInfo( + '/root.txt', + $this->getMockBuilder('\OC\Files\Storage\Storage') + ->disableOriginalConstructor() + ->getMock(), + '/var/www/some/sub.txt', + [ + 'mtime' => 999, + 'mimetype' => 'application/binary', + 'size' => 9876, + 'etag' => 'SubEtag', + ], + $this->getMockBuilder('\OCP\Files\Mount\IMountPoint') + ->disableOriginalConstructor() + ->getMock() + ); + $this->tagService->expects($this->once()) + ->method('getFilesByTag') + ->with($this->equalTo([$tagName])) + ->will($this->returnValue([$fileInfo1, $fileInfo2])); + + $expected = new DataResponse([ + 'files' => [ + [ + 'id' => null, + 'parentId' => null, + 'date' => 'January 1, 1970 at 12:00:55 AM GMT+0', + 'mtime' => 55000, + 'icon' => \OCA\Files\Helper::determineIcon($fileInfo1), + 'name' => 'root.txt', + 'permissions' => null, + 'mimetype' => 'application/pdf', + 'size' => 1234, + 'type' => 'file', + 'etag' => 'MyEtag', + 'path' => '/', + 'tags' => [ + [ + 'MyTagName' + ] + ], + ], + [ + 'id' => null, + 'parentId' => null, + 'date' => 'January 1, 1970 at 12:16:39 AM GMT+0', + 'mtime' => 999000, + 'icon' => \OCA\Files\Helper::determineIcon($fileInfo2), + 'name' => 'root.txt', + 'permissions' => null, + 'mimetype' => 'application/binary', + 'size' => 9876, + 'type' => 'file', + 'etag' => 'SubEtag', + 'path' => '/', + 'tags' => [ + [ + 'MyTagName' + ] + ], + ] + ], + ]); + $this->assertEquals($expected, $this->apiController->getFilesByTag([$tagName])); + } + + public function testUpdateFileTagsEmpty() { + $expected = new DataResponse([]); + $this->assertEquals($expected, $this->apiController->updateFileTags('/path.txt')); + } + + public function testUpdateFileTagsWorking() { + $this->tagService->expects($this->once()) + ->method('updateFileTags') + ->with('/path.txt', ['Tag1', 'Tag2']); + + $expected = new DataResponse([ + 'tags' => [ + 'Tag1', + 'Tag2' + ], + ]); + $this->assertEquals($expected, $this->apiController->updateFileTags('/path.txt', ['Tag1', 'Tag2'])); + } + + public function testUpdateFileTagsNotFoundException() { + $this->tagService->expects($this->once()) + ->method('updateFileTags') + ->with('/path.txt', ['Tag1', 'Tag2']) + ->will($this->throwException(new NotFoundException('My error message'))); + + $expected = new DataResponse(['message' => 'My error message'], Http::STATUS_NOT_FOUND); + $this->assertEquals($expected, $this->apiController->updateFileTags('/path.txt', ['Tag1', 'Tag2'])); + } + + public function testUpdateFileTagsStorageNotAvailableException() { + $this->tagService->expects($this->once()) + ->method('updateFileTags') + ->with('/path.txt', ['Tag1', 'Tag2']) + ->will($this->throwException(new StorageNotAvailableException('My error message'))); + + $expected = new DataResponse(['message' => 'My error message'], Http::STATUS_SERVICE_UNAVAILABLE); + $this->assertEquals($expected, $this->apiController->updateFileTags('/path.txt', ['Tag1', 'Tag2'])); + } + + public function testUpdateFileTagsStorageGenericException() { + $this->tagService->expects($this->once()) + ->method('updateFileTags') + ->with('/path.txt', ['Tag1', 'Tag2']) + ->will($this->throwException(new \Exception('My error message'))); + + $expected = new DataResponse(['message' => 'My error message'], Http::STATUS_NOT_FOUND); + $this->assertEquals($expected, $this->apiController->updateFileTags('/path.txt', ['Tag1', 'Tag2'])); + } +}