diff --git a/core/Controller/SvgController.php b/core/Controller/SvgController.php index ba0e77602f..af71ca1733 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 76d04d1af3..f6208d6f6c 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 0000000000..81d7fca147 --- /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 0000000000..54d7d3a9b1 --- /dev/null +++ b/tests/data/svg/settings-admin-red.svg @@ -0,0 +1 @@ + diff --git a/tests/lib/App/AppManagerTest.php b/tests/lib/App/AppManagerTest.php index 1a5d6c648a..5b6fedb1cc 100644 --- a/tests/lib/App/AppManagerTest.php +++ b/tests/lib/App/AppManagerTest.php @@ -338,6 +338,35 @@ class AppManagerTest extends TestCase { $this->assertEquals(\OC::$SERVERROOT . '/apps/files', $this->manager->getAppPath('files')); } + public function testGetAppPathSymlink() { + $fakeAppDirname = sha1(uniqid('test', true)); + $fakeAppPath = sys_get_temp_dir() . '/' . $fakeAppDirname; + $fakeAppLink = \OC::$SERVERROOT . '/' . $fakeAppDirname; + + mkdir($fakeAppPath); + if (symlink($fakeAppPath, $fakeAppLink) === false) { + $this->markTestSkipped('Failed to create symlink'); + } + + // Use the symlink as the app path + \OC::$APPSROOTS[] = [ + 'path' => $fakeAppLink, + 'url' => \OC::$WEBROOT . '/' . $fakeAppDirname, + 'writable' => false, + ]; + + $fakeTestAppPath = $fakeAppPath . '/' . 'test-test-app'; + mkdir($fakeTestAppPath); + + $generatedAppPath = $this->manager->getAppPath('test-test-app'); + + rmdir($fakeTestAppPath); + unlink($fakeAppLink); + rmdir($fakeAppPath); + + $this->assertEquals($fakeAppLink . '/test-test-app', $generatedAppPath); + } + public function testGetAppPathFail() { $this->expectException(AppPathNotFoundException::class); $this->manager->getAppPath('testnotexisting');