Merge pull request #7257 from kyrofa/bugfix/5289/apps_outside_webroot

[stable12] CSSResourceLocator: handle SCSS in apps outside root

3 files changed, 163 insertions, 92 deletions

diff --git a/lib/private/Template/CSSResourceLocator.php b/lib/private/Template/CSSResourceLocator.php
index 29f3efaa8da..d5e9ce732cc 100644
--- a/lib/private/Template/CSSResourceLocator.php
+++ b/lib/private/Template/CSSResourceLocator.php
@@ -6,6 +6,7 @@
  * @author Joas Schilling <coding@schilljs.com>
  * @author Morris Jobke <hey@morrisjobke.de>
  * @author Thomas Müller <thomas.mueller@tmit.eu>
+ * @author Kyle Fazzari <kyrofa@ubuntu.com>
  *
  * @license AGPL-3.0
  *
@@ -122,45 +123,25 @@ class CSSResourceLocator extends ResourceLocator {
 			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 f721906e12b..0d7767fdd0b 100755
--- a/lib/private/Template/ResourceLocator.php
+++ b/lib/private/Template/ResourceLocator.php
@@ -7,6 +7,7 @@
  * @author Jörn Friedrich Dreyer <jfd@butonic.de>
  * @author Morris Jobke <hey@morrisjobke.de>
  * @author Robin McCorkell <robin@mccorkell.me.uk>
+ * @author Kyle Fazzari <kyrofa@ubuntu.com>
  *
  * @license AGPL-3.0
  *
@@ -107,6 +108,50 @@ abstract class ResourceLocator {
 	}
 
 	/**
+	 * 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
 	 *
 	 * @param string $root path to check
@@ -116,7 +161,6 @@ abstract class ResourceLocator {
 	 * @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 @@ abstract class ResourceLocator {
 		}
 
 		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 a16cc18cb0a..ee209a599d6 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 @@ class CSSResourceLocatorTest extends \Test\TestCase {
 		$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 @@ class CSSResourceLocatorTest extends \Test\TestCase {
 		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 @@ class CSSResourceLocatorTest extends \Test\TestCase {
 		$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);
 	}
 }