fix(JSResourceLocator): Consider configured app roots for files

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
nextcloud · Mar 7, 2024 · d0d9b39 · d0d9b39
1 parent d046bf6
commit d0d9b39
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 44 deletions.
diff --git a/lib/private/Template/JSResourceLocator.php b/lib/private/Template/JSResourceLocator.php
@@ -51,6 +51,22 @@ public function doFind($script) {
 		// Extracting the appId and the script file name
 		$app = substr($script, 0, strpos($script, '/'));
 		$scriptName = basename($script);
+		// Get the app root path
+		$appRoot = $this->serverroot . 'apps/';
+		$appWebRoot = null;
+		try {
+			// We need the dir name as getAppPath appends the appid
+			$appRoot = dirname($this->appManager->getAppPath($app));
+			// Only do this if $app_path is set, because an empty argument to realpath gets turned into cwd.
+			if ($appRoot) {
+				// Handle symlinks
+				$appRoot = realpath($appRoot);
+			}
+			// Get the app webroot
+			$appWebRoot = dirname($this->appManager->getAppWebPath($app));
+		} catch (AppPathNotFoundException $e) {
+			// ignore
+		}
 
 		if (str_contains($script, '/l10n/')) {
 			// For language files we try to load them all, so themes can overwrite
@@ -60,13 +76,8 @@ public function doFind($script) {
 			$found += $this->appendScriptIfExist($this->serverroot, $theme_dir.'core/'.$script);
 			$found += $this->appendScriptIfExist($this->serverroot, $script);
 			$found += $this->appendScriptIfExist($this->serverroot, $theme_dir.$script);
-
-			foreach (\OC::$APPSROOTS as $appRoot) {
-				$dirName = basename($appRoot['path']);
-				$rootPath = dirname($appRoot['path']);
-				$found += $this->appendScriptIfExist($rootPath, $dirName.'/'.$script);
-				$found += $this->appendScriptIfExist($this->serverroot, $theme_dir.$dirName.'/'.$script);
-			}
+			$found += $this->appendScriptIfExist($appRoot, $script, $appWebRoot);
+			$found += $this->appendScriptIfExist($this->serverroot, $theme_dir.'apps/'.$script);
 
 			if ($found) {
 				return;
@@ -76,8 +87,9 @@ public function doFind($script) {
 			|| $this->appendScriptIfExist($this->serverroot, $script)
 			|| $this->appendScriptIfExist($this->serverroot, $theme_dir."dist/$app-$scriptName")
 			|| $this->appendScriptIfExist($this->serverroot, "dist/$app-$scriptName")
-			|| $this->appendScriptIfExist($this->serverroot, 'apps/'.$script)
+			|| $this->appendScriptIfExist($appRoot, $script, $appWebRoot)
 			|| $this->cacheAndAppendCombineJsonIfExist($this->serverroot, $script.'.json')
+			|| $this->cacheAndAppendCombineJsonIfExist($appRoot, $script.'.json', $appWebRoot)
 			|| $this->appendScriptIfExist($this->serverroot, $theme_dir.'core/'.$script)
 			|| $this->appendScriptIfExist($this->serverroot, 'core/'.$script)
 			|| (strpos($scriptName, '/') === -1 && ($this->appendScriptIfExist($this->serverroot, $theme_dir."dist/core-$scriptName")
@@ -87,43 +99,13 @@ public function doFind($script) {
 			return;
 		}
 
-		$script = substr($script, strpos($script, '/') + 1);
-		$app_url = null;
-
-		try {
-			$app_url = $this->appManager->getAppWebPath($app);
-		} catch (AppPathNotFoundException) {
-			// pass
-		}
-
-		try {
-			$app_path = $this->appManager->getAppPath($app);
-
-			// Account for the possibility of having symlinks in app path. Only
-			// do this if $app_path is set, because an empty argument to realpath
-			// gets turned into cwd.
-			$app_path = realpath($app_path);
-
-			// check combined files
-			if (!str_starts_with($script, 'l10n/') && $this->cacheAndAppendCombineJsonIfExist($app_path, $script.'.json', $app)) {
-				return;
-			}
-
-			// fallback to plain file location
-			if ($this->appendScriptIfExist($app_path, $script, $app_url)) {
-				return;
-			}
-		} catch (AppPathNotFoundException) {
-			// pass (general error handling happens below)
-		}
-
 		// missing translations files will be ignored
-		if (str_starts_with($script, 'l10n/')) {
+		if (str_contains($script, '/l10n/')) {
 			return;
 		}
 
 		$this->logger->error('Could not find resource {resource} to load', [
-			'resource' => $app . '/' . $script . '.js',
+			'resource' => $script . '.js',
 			'app' => 'jsresourceloader',
 		]);
 	}

diff --git a/tests/lib/Template/JSResourceLocatorTest.php b/tests/lib/Template/JSResourceLocatorTest.php
@@ -127,9 +127,9 @@ public function testFindWithAppPathSymlink() {
 		$webRoot = $resource[1];
 		$file = $resource[2];
 
-		$expectedRoot = $new_apps_path . '/test-js-app';
-		$expectedWebRoot = \OC::$WEBROOT . '/js-apps-test/test-js-app';
-		$expectedFile = 'test-file.js';
+		$expectedRoot = $new_apps_path;
+		$expectedWebRoot = \OC::$WEBROOT . '/js-apps-test';
+		$expectedFile = $appName . '/test-file.js';
 
 		$this->assertEquals($expectedRoot, $root,
 			'Ensure the app path symlink is resolved into the real path');
@@ -145,7 +145,7 @@ public function testNotExistingTranslationHandledSilent() {
 			->method('getAppPath')
 			->with('core')
 			->willThrowException(new AppPathNotFoundException());
-		$this->appManager->expects($this->once())
+		$this->appManager->expects($this->atMost(1))
 			->method('getAppWebPath')
 			->with('core')
 			->willThrowException(new AppPathNotFoundException());