From 440d3eb9c462b163c6ba04a25f69d54bcdab92f1 Mon Sep 17 00:00:00 2001
From: Hypolite Petovan <hypolite@mrpetovan.com>
Date: Thu, 28 Nov 2019 12:27:16 -0500
Subject: [PATCH] Simplify Theme::getPathForfile to expand its uses

---
 src/App/Page.php   | 30 +++++++++++++++---------------
 src/Core/Theme.php | 46 +++++++++++++++++-----------------------------
 2 files changed, 32 insertions(+), 44 deletions(-)

diff --git a/src/App/Page.php b/src/App/Page.php
index 7af0bc8995..7b3bc286f3 100644
--- a/src/App/Page.php
+++ b/src/App/Page.php
@@ -15,6 +15,7 @@ use Friendica\Core\Renderer;
 use Friendica\Core\Theme;
 use Friendica\Module\Special\HTTPException as ModuleHTTPException;
 use Friendica\Network\HTTPException;
+use Friendica\Util\Strings;
 
 /**
  * Contains the page specific environment variables for the current Page
@@ -224,15 +225,15 @@ class Page implements ArrayAccess
 		 * being first
 		 */
 		$this->page['htmlhead'] = Renderer::replaceMacros($tpl, [
-				'$local_user'      => local_user(),
-				'$generator'       => 'Friendica' . ' ' . FRIENDICA_VERSION,
-				'$delitem'         => $l10n->t('Delete this item?'),
-				'$update_interval' => $interval,
-				'$shortcut_icon'   => $shortcut_icon,
-				'$touch_icon'      => $touch_icon,
-				'$block_public'    => intval($config->get('system', 'block_public')),
-				'$stylesheets'     => $this->stylesheets,
-			]) . $this->page['htmlhead'];
+			'$local_user'      => local_user(),
+			'$generator'       => 'Friendica' . ' ' . FRIENDICA_VERSION,
+			'$delitem'         => $l10n->t('Delete this item?'),
+			'$update_interval' => $interval,
+			'$shortcut_icon'   => $shortcut_icon,
+			'$touch_icon'      => $touch_icon,
+			'$block_public'    => intval($config->get('system', 'block_public')),
+			'$stylesheets'     => array_unique($this->stylesheets),
+		]) . $this->page['htmlhead'];
 	}
 
 	/**
@@ -282,8 +283,8 @@ class Page implements ArrayAccess
 
 		$tpl                  = Renderer::getMarkupTemplate('footer.tpl');
 		$this->page['footer'] = Renderer::replaceMacros($tpl, [
-				'$footerScripts' => $this->footerScripts,
-			]) . $this->page['footer'];
+			'$footerScripts' => array_unique($this->footerScripts),
+		]) . $this->page['footer'];
 	}
 
 	/**
@@ -455,13 +456,13 @@ class Page implements ArrayAccess
 		 * to load another page template than the default one.
 		 * The page templates are located in /view/php/ or in the theme directory.
 		 */
-		if (isset($_GET["mode"])) {
-			$template = Theme::getPathForFile($_GET["mode"] . '.php');
+		if (isset($_GET['mode'])) {
+			$template = Theme::getPathForFile('php/' . Strings::sanitizeFilePathItem($_GET['mode']) . '.php');
 		}
 
 		// If there is no page template use the default page template
 		if (empty($template)) {
-			$template = Theme::getPathForFile("default.php");
+			$template = Theme::getPathForFile('php/default.php');
 		}
 
 		// Theme templates expect $a as an App instance
@@ -470,7 +471,6 @@ class Page implements ArrayAccess
 		// Used as is in view/php/default.php
 		$lang = $l10n->getCurrentLang();
 
-		/// @TODO Looks unsafe (remote-inclusion), is maybe not but Core\Theme::getPathForFile() uses file_exists() but does not escape anything
 		require_once $template;
 	}
 }
diff --git a/src/Core/Theme.php b/src/Core/Theme.php
index 61798a3969..7a59f11325 100644
--- a/src/Core/Theme.php
+++ b/src/Core/Theme.php
@@ -185,45 +185,33 @@ class Theme
 	/**
 	 * @brief Get the full path to relevant theme files by filename
 	 *
-	 * This function search in the theme directory (and if not present in global theme directory)
-	 * if there is a directory with the file extension and  for a file with the given
-	 * filename.
+	 * This function searches in order in the current theme directory, in the current theme parent directory, and lastly
+	 * in the base view/ folder.
 	 *
 	 * @param string $file Filename
-	 * @param string $root Full root path
 	 * @return string Path to the file or empty string if the file isn't found
-	 * @throws \Friendica\Network\HTTPException\InternalServerErrorException
+	 * @throws \Exception
 	 */
-	public static function getPathForFile($file, $root = '')
+	public static function getPathForFile($file)
 	{
-		$file = basename($file);
+		$a = BaseObject::getApp();
+
+		$theme = $a->getCurrentTheme();
+
+		$parent = Strings::sanitizeFilePathItem($a->theme_info['extends'] ?? $theme);
 
-		// Make sure $root ends with a slash / if it's not blank
-		if ($root !== '' && $root[strlen($root) - 1] !== '/') {
-			$root = $root . '/';
-		}
-		$theme_info = \get_app()->theme_info;
-		if (is_array($theme_info) && array_key_exists('extends', $theme_info)) {
-			$parent = $theme_info['extends'];
-		} else {
-			$parent = 'NOPATH';
-		}
-		$theme = \get_app()->getCurrentTheme();
-		$parent = Strings::sanitizeFilePathItem($parent);
-		$ext = substr($file, strrpos($file, '.') + 1);
 		$paths = [
-			"{$root}view/theme/$theme/$ext/$file",
-			"{$root}view/theme/$parent/$ext/$file",
-			"{$root}view/$ext/$file",
+			"view/theme/$theme/$file",
+			"view/theme/$parent/$file",
+			"view/$file",
 		];
-		foreach ($paths as $p) {
-			// strpos() is faster than strstr when checking if one string is in another (http://php.net/manual/en/function.strstr.php)
-			if (strpos($p, 'NOPATH') !== false) {
-				continue;
-			} elseif (file_exists($p)) {
-				return $p;
+
+		foreach ($paths as $path) {
+			if (file_exists($path)) {
+				return $path;
 			}
 		}
+
 		return '';
 	}
 
-- 
2.39.5