From 72a16b17792e2a5353cc0ffce8fc24ed4ff1351c Mon Sep 17 00:00:00 2001 From: Daniel Kesselberg Date: Wed, 22 Jan 2020 21:13:36 +0100 Subject: [PATCH] Make it possible to resolve svg for apps_paths outside the document root Previous implementation assumes the app path is always a child \OC::$SERVERROOT. That's not always true. Signed-off-by: Daniel Kesselberg --- core/Controller/SvgController.php | 11 ++-- tests/Core/Controller/SvgControllerTest.php | 65 +++++++++++++++++++-- tests/data/svg/files-app-red.svg | 1 + tests/data/svg/settings-admin-red.svg | 1 + 4 files changed, 69 insertions(+), 9 deletions(-) create mode 100644 tests/data/svg/files-app-red.svg create mode 100644 tests/data/svg/settings-admin-red.svg diff --git a/core/Controller/SvgController.php b/core/Controller/SvgController.php index ba0e77602f2..af71ca17330 100644 --- a/core/Controller/SvgController.php +++ b/core/Controller/SvgController.php @@ -32,6 +32,7 @@ declare(strict_types=1); namespace OC\Core\Controller; use OC\Template\IconsCacher; +use OCP\App\AppPathNotFoundException; use OCP\App\IAppManager; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; @@ -97,13 +98,13 @@ class SvgController extends Controller { * @return DataDisplayResponse|NotFoundResponse */ public function getSvgFromApp(string $app, string $fileName, string $color = 'ffffff') { - $appRootPath = $this->appManager->getAppPath($app); - $appPath = substr($appRootPath, strlen($this->serverRoot)); - - if (!$appPath) { + try { + $appPath = $this->appManager->getAppPath($app); + } catch (AppPathNotFoundException $e) { return new NotFoundResponse(); } - $path = $this->serverRoot . $appPath ."/img/$fileName.svg"; + + $path = $appPath . "/img/$fileName.svg"; return $this->getSvg($path, $color, $fileName); } diff --git a/tests/Core/Controller/SvgControllerTest.php b/tests/Core/Controller/SvgControllerTest.php index 76d04d1af32..f6208d6f6c4 100644 --- a/tests/Core/Controller/SvgControllerTest.php +++ b/tests/Core/Controller/SvgControllerTest.php @@ -28,6 +28,7 @@ namespace Tests\Core\Controller; use OC\AppFramework\Http; use OC\Core\Controller\SvgController; use OC\Template\IconsCacher; +use OCP\App\AppPathNotFoundException; use OCP\App\IAppManager; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IRequest; @@ -46,6 +47,9 @@ class SvgControllerTest extends TestCase { self::TEST_IMAGE_RECT, ]; + /** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */ + private $appManager; + /** * @var SvgController */ @@ -87,17 +91,20 @@ class SvgControllerTest extends TestCase { * @return void */ public function setupSvgController() { + /** @var IRequest */ $request = $this->getMockBuilder(IRequest::class)->getMock(); + /** @var ITimeFactory $timeFactory */ $timeFactory = $this->getMockBuilder(ITimeFactory::class)->getMock(); - $appManager = $this->getMockBuilder(IAppManager::class)->getMock(); + /** @var IAppManager */ + $this->appManager = $this->getMockBuilder(IAppManager::class)->getMock(); + /** @var IconsCacher $iconsCacher */ $iconsCacher = $this->getMockBuilder(IconsCacher::class)->disableOriginalConstructor()->setMethods(['__construct'])->getMock(); - $this->svgController = new SvgController('core', $request, $timeFactory, $appManager, $iconsCacher); + $this->svgController = new SvgController('core', $request, $timeFactory, $this->appManager, $iconsCacher); } /** * Checks that requesting an unknown image results in a 404. * - * @test * @return void */ public function testGetSvgFromCoreNotFound() { @@ -120,7 +127,6 @@ class SvgControllerTest extends TestCase { /** * Tests that retrieving a colored SVG works. * - * @test * @dataProvider provideGetSvgFromCoreTestData * @param string $name The requested svg name * @param string $color The requested color @@ -138,4 +144,55 @@ class SvgControllerTest extends TestCase { self::assertEquals($expected, $response->getData()); } + + /** + * Checks that requesting an unknown image results in a 404. + */ + public function testGetSvgFromAppNotFound(): void { + $this->appManager->expects($this->once()) + ->method('getAppPath') + ->with('invalid_app') + ->willThrowException(new AppPathNotFoundException('Could not find path for invalid_app')); + + $response = $this->svgController->getSvgFromApp('invalid_app', 'some-icon', '#ff0000'); + self::assertEquals(Http::STATUS_NOT_FOUND, $response->getStatus()); + } + + /** + * Provides svg coloring test data. + * + * @return array + */ + public function provideGetSvgFromAppTestData(): array { + return [ + 'settings admin' => ['settings', 'admin', 'f00', file_get_contents(self::TEST_IMAGES_SOURCE_PATH . '/settings-admin-red.svg')], + 'files app' => ['files', 'app', 'f00', file_get_contents(self::TEST_IMAGES_SOURCE_PATH . '/files-app-red.svg')], + ]; + } + + /** + * Tests that retrieving a colored SVG works. + * + * @dataProvider provideGetSvgFromAppTestData + * @param string $appName + * @param string $name The requested svg name + * @param string $color The requested color + * @param string $expected + */ + public function testGetSvgFromApp(string $appName, string $name, string $color, string $expected): void { + $this->appManager->expects($this->once()) + ->method('getAppPath') + ->with($appName) + ->willReturn(__DIR__ . '/../../../apps/' . $appName); + + $response = $this->svgController->getSvgFromApp($appName, $name, $color); + + self::assertEquals(Http::STATUS_OK, $response->getStatus()); + + $headers = $response->getHeaders(); + self::assertArrayHasKey('Content-Type', $headers); + self::assertEquals($headers['Content-Type'], 'image/svg+xml'); + + self::assertEquals($expected, $response->getData()); + } } diff --git a/tests/data/svg/files-app-red.svg b/tests/data/svg/files-app-red.svg new file mode 100644 index 00000000000..81d7fca1472 --- /dev/null +++ b/tests/data/svg/files-app-red.svg @@ -0,0 +1 @@ + diff --git a/tests/data/svg/settings-admin-red.svg b/tests/data/svg/settings-admin-red.svg new file mode 100644 index 00000000000..54d7d3a9b15 --- /dev/null +++ b/tests/data/svg/settings-admin-red.svg @@ -0,0 +1 @@ +