From 7f8f3dc21bdfa8373b79f17933aaad33ec315aed Mon Sep 17 00:00:00 2001 From: Kyle Fazzari Date: Wed, 22 Nov 2017 21:30:21 -0800 Subject: [PATCH] CSSResourceLocator: handle SCSS in apps outside root Currently static CSS files work fine in apps outside of the root. However, as soon as an app uses SCSS, Nextcloud starts being unable to find the web root. Fix this problem by backporting select snippets from master specifically targeting this issue, and add a test to ensure it doesn't regress. Fix #5289 Signed-off-by: Kyle Fazzari --- lib/private/Template/CSSResourceLocator.php | 53 +++------ lib/private/Template/ResourceLocator.php | 90 ++++++++------ tests/lib/Template/CSSResourceLocatorTest.php | 112 ++++++++++++++---- 3 files changed, 163 insertions(+), 92 deletions(-) diff --git a/lib/private/Template/CSSResourceLocator.php b/lib/private/Template/CSSResourceLocator.php index 29f3efaa8daae..d5e9ce732cc97 100644 --- a/lib/private/Template/CSSResourceLocator.php +++ b/lib/private/Template/CSSResourceLocator.php @@ -6,6 +6,7 @@ * @author Joas Schilling * @author Morris Jobke * @author Thomas Müller + * @author Kyle Fazzari * * @license AGPL-3.0 * @@ -122,45 +123,25 @@ public function append($root, $file, $webRoot = null, $throw = true, $scss = fal parent::append($root, $file, $webRoot, $throw); } else { if (!$webRoot) { - $tmpRoot = realpath($root); - /* - * traverse the potential web roots upwards in the path - * - * example: - * - root: /srv/www/apps/myapp - * - available mappings: ['/srv/www'] - * - * First we check if a mapping for /srv/www/apps/myapp is available, - * then /srv/www/apps, /srv/www/apps, /srv/www, ... until we find a - * valid web root - */ - do { - if (isset($this->mapping[$tmpRoot])) { - $webRoot = $this->mapping[$tmpRoot]; - break; + $webRoot = $this->findWebRoot($root); + + if ($webRoot === null) { + $webRoot = ''; + $this->logger->error('ResourceLocator can not find a web root (root: {root}, file: {file}, webRoot: {webRoot}, throw: {throw})', [ + 'app' => 'lib', + 'root' => $root, + 'file' => $file, + 'webRoot' => $webRoot, + 'throw' => $throw ? 'true' : 'false' + ]); + + if ($throw && $root === '/') { + throw new ResourceNotFoundException($file, $webRoot); } - - if ($tmpRoot === '/') { - $webRoot = ''; - $this->logger->error('ResourceLocator can not find a web root (root: {root}, file: {file}, webRoot: {webRoot}, throw: {throw})', [ - 'app' => 'lib', - 'root' => $root, - 'file' => $file, - 'webRoot' => $webRoot, - 'throw' => $throw ? 'true' : 'false' - ]); - break; - } - $tmpRoot = dirname($tmpRoot); - } while(true); - - } - - if ($throw && $tmpRoot === '/') { - throw new ResourceNotFoundException($file, $webRoot); + } } - $this->resources[] = array($tmpRoot, $webRoot, $file); + $this->resources[] = array($webRoot? : '/', $webRoot, $file); } } } diff --git a/lib/private/Template/ResourceLocator.php b/lib/private/Template/ResourceLocator.php index f721906e12ba3..0d7767fdd0b3e 100755 --- a/lib/private/Template/ResourceLocator.php +++ b/lib/private/Template/ResourceLocator.php @@ -7,6 +7,7 @@ * @author Jörn Friedrich Dreyer * @author Morris Jobke * @author Robin McCorkell + * @author Kyle Fazzari * * @license AGPL-3.0 * @@ -106,6 +107,50 @@ protected function appendIfExist($root, $file, $webRoot = null) { return false; } + /** + * Attempt to find the webRoot + * + * traverse the potential web roots upwards in the path + * + * example: + * - root: /srv/www/apps/myapp + * - available mappings: ['/srv/www'] + * + * First we check if a mapping for /srv/www/apps/myapp is available, + * then /srv/www/apps, /srv/www/apps, /srv/www, ... until we find a + * valid web root + * + * @param string $root + * @return string|null The web root or null on failure + */ + protected function findWebRoot($root) { + $webRoot = null; + $tmpRoot = $root; + + while ($webRoot === null) { + if (isset($this->mapping[$tmpRoot])) { + $webRoot = $this->mapping[$tmpRoot]; + break; + } + + if ($tmpRoot === '/') { + break; + } + + $tmpRoot = dirname($tmpRoot); + } + + if ($webRoot === null) { + $realpath = realpath($root); + + if ($realpath && ($realpath !== $root)) { + return $this->findWebRoot($realpath); + } + } + + return $webRoot; + } + /** * append the $file resource at $root * @@ -116,7 +161,6 @@ protected function appendIfExist($root, $file, $webRoot = null) { * @throws ResourceNotFoundException Only thrown when $throw is true and the resource is missing */ protected function append($root, $file, $webRoot = null, $throw = true) { - if (!is_string($root)) { if ($throw) { throw new ResourceNotFoundException($file, $webRoot); @@ -125,38 +169,18 @@ protected function append($root, $file, $webRoot = null, $throw = true) { } if (!$webRoot) { - $tmpRoot = realpath($root); - /* - * traverse the potential web roots upwards in the path - * - * example: - * - root: /srv/www/apps/myapp - * - available mappings: ['/srv/www'] - * - * First we check if a mapping for /srv/www/apps/myapp is available, - * then /srv/www/apps, /srv/www/apps, /srv/www, ... until we find a - * valid web root - */ - do { - if (isset($this->mapping[$tmpRoot])) { - $webRoot = $this->mapping[$tmpRoot]; - break; - } - - if ($tmpRoot === '/') { - $webRoot = ''; - $this->logger->error('ResourceLocator can not find a web root (root: {root}, file: {file}, webRoot: {webRoot}, throw: {throw})', [ - 'app' => 'lib', - 'root' => $root, - 'file' => $file, - 'webRoot' => $webRoot, - 'throw' => $throw ? 'true' : 'false' - ]); - break; - } - $tmpRoot = dirname($tmpRoot); - } while(true); - + $webRoot = $this->findWebRoot($root); + + if ($webRoot === null) { + $webRoot = ''; + $this->logger->error('ResourceLocator can not find a web root (root: {root}, file: {file}, webRoot: {webRoot}, throw: {throw})', [ + 'app' => 'lib', + 'root' => $root, + 'file' => $file, + 'webRoot' => $webRoot, + 'throw' => $throw ? 'true' : 'false' + ]); + } } $this->resources[] = array($root, $webRoot, $file); diff --git a/tests/lib/Template/CSSResourceLocatorTest.php b/tests/lib/Template/CSSResourceLocatorTest.php index a16cc18cb0a66..ee209a599d64a 100644 --- a/tests/lib/Template/CSSResourceLocatorTest.php +++ b/tests/lib/Template/CSSResourceLocatorTest.php @@ -24,6 +24,9 @@ namespace Test\Template; use OC\Files\AppData\Factory; +use OCP\Files\IAppData; +use OCP\Files\SimpleFS\ISimpleFolder; +use OCP\Files\SimpleFS\ISimpleFile; use OCP\ILogger; use OCP\IURLGenerator; use OCP\IConfig; @@ -45,6 +48,10 @@ class CSSResourceLocatorTest extends \Test\TestCase { protected $depsCache; /** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */ protected $logger; + protected $appname; + protected $appdir; + protected $appdirLink; + protected $appurl; protected function setUp() { parent::setUp(); @@ -55,6 +62,20 @@ protected function setUp() { $this->config = $this->createMock(IConfig::class); $this->depsCache = $this->createMock(ICache::class); $this->themingDefaults = $this->createMock(ThemingDefaults::class); + + $this->appdir = null; + $this->themingDefaults + ->expects($this->any()) + ->method('getScssVariables') + ->willReturn([]); + } + + protected function tearDown() { + if (!is_null($this->appdir)) { + array_pop(\OC::$APPSROOTS); + unlink($this->appdirLink); + $this->rrmdir($this->appdir); + } } private function cssResourceLocator() { @@ -95,36 +116,58 @@ private function randomString() { return sha1(uniqid(mt_rand(), true)); } - public function testConstructor() { - $locator = $this->cssResourceLocator(); - $this->assertAttributeEquals('theme', 'theme', $locator); - $this->assertAttributeEquals('core', 'serverroot', $locator); - $this->assertAttributeEquals(array('core'=>'map','3rd'=>'party'), 'mapping', $locator); - $this->assertAttributeEquals('3rd', 'thirdpartyroot', $locator); - $this->assertAttributeEquals('map', 'webroot', $locator); - $this->assertAttributeEquals(array(), 'resources', $locator); - } + private function setupAppDir() { + $this->appname = 'test-app-'.$this->randomString(); + $folder = $this->createMock(ISimpleFolder::class); + $this->appData->method('getFolder') + ->with($this->appname) + ->willReturn($folder); + + $file = $this->createMock(ISimpleFile::class); + $folder->method('getFile') + ->will($this->returnCallback(function($path) use ($file) { + return $file; + })); + + $this->urlGenerator + ->method('linkToRoute') + ->willReturn(\OC::$WEBROOT . '/test-file'); - public function testFindWithAppPathSymlink() { // First create new apps path, and a symlink to it $apps_dirname = $this->randomString(); - $new_apps_path = sys_get_temp_dir() . '/' . $apps_dirname; - $new_apps_path_symlink = $new_apps_path . '_link'; - mkdir($new_apps_path); - symlink($apps_dirname, $new_apps_path_symlink); + $this->appdir = sys_get_temp_dir() . '/' . $apps_dirname; + $this->appdirLink = $this->appdir . '_link'; + mkdir($this->appdir); + symlink($apps_dirname, $this->appdirLink); // Create an app within that path - mkdir($new_apps_path . '/' . 'test-css-app'); + mkdir($this->appdir . '/' . $this->appname); + + $this->appurl = 'css-apps-test'; // Use the symlink as the app path \OC::$APPSROOTS[] = [ - 'path' => $new_apps_path_symlink, - 'url' => '/css-apps-test', + 'path' => $this->appdirLink, + 'url' => '/' . $this->appurl, 'writable' => false, ]; + } + + public function testConstructor() { + $locator = $this->cssResourceLocator(); + $this->assertAttributeEquals('theme', 'theme', $locator); + $this->assertAttributeEquals('core', 'serverroot', $locator); + $this->assertAttributeEquals(array('core'=>'map','3rd'=>'party'), 'mapping', $locator); + $this->assertAttributeEquals('3rd', 'thirdpartyroot', $locator); + $this->assertAttributeEquals('map', 'webroot', $locator); + $this->assertAttributeEquals(array(), 'resources', $locator); + } + + public function testFindCSSWithAppPathSymlink() { + $this->setupAppDir(); $locator = $this->cssResourceLocator(); - $locator->find(array('test-css-app/test-file')); + $locator->find(array($this->appname . '/test-file')); $resources = $locator->getResources(); $this->assertCount(1, $resources); @@ -134,17 +177,40 @@ public function testFindWithAppPathSymlink() { $webRoot = $resource[1]; $file = $resource[2]; - $expectedRoot = $new_apps_path . '/test-css-app'; - $expectedWebRoot = \OC::$WEBROOT . '/css-apps-test/test-css-app'; + $expectedRoot = $this->appdir . '/' . $this->appname; + $expectedWebRoot = \OC::$WEBROOT . '/' . $this->appurl . '/' . $this->appname; $expectedFile = 'test-file.css'; $this->assertEquals($expectedRoot, $root, 'Ensure the app path symlink is resolved into the real path'); $this->assertEquals($expectedWebRoot, $webRoot); $this->assertEquals($expectedFile, $file); + } - array_pop(\OC::$APPSROOTS); - unlink($new_apps_path_symlink); - $this->rrmdir($new_apps_path); + public function testFindSCSSWithAppPathSymlink() { + $this->setupAppDir(); + + // Create an SCSS file there + touch($this->appdir . '/' . $this->appname . '/test-file.scss'); + + $locator = $this->cssResourceLocator(); + $locator->find(array($this->appname . '/test-file')); + + $resources = $locator->getResources(); + $this->assertCount(1, $resources); + $resource = $resources[0]; + $this->assertCount(3, $resource); + $root = $resource[0]; + $webRoot = $resource[1]; + $file = $resource[2]; + + $expectedRoot = '/'; + $expectedWebRoot = ''; + $expectedFile = 'test-file'; + + $this->assertEquals($expectedRoot, $root, + 'Ensure the app path symlink is resolved into the real path'); + $this->assertEquals($expectedWebRoot, $webRoot); + $this->assertEquals($expectedFile, $file); } }