Merge pull request #1665 from nextcloud/speed-up-is-subdirectory-test

Simplify isSubDirectory check

3 files changed, 26 insertions, 43 deletions

diff --git a/lib/private/L10N/Factory.php b/lib/private/L10N/Factory.php
index 91233a0c4a7..8aad395065c 100644
--- a/lib/private/L10N/Factory.php
+++ b/lib/private/L10N/Factory.php
@@ -2,6 +2,7 @@
 /**
  * @copyright Copyright (c) 2016, ownCloud, Inc.
  * @copyright 2016 Roeland Jago Douma <roeland@famdouma.nl>
+ * @copyright 2016 Lukas Reschke <lukas@statuscode.ch>
  *
  * @author Bart Visscher <bartv@thisnet.nl>
  * @author Joas Schilling <coding@schilljs.com>
@@ -288,6 +289,27 @@ class Factory implements IFactory {
 	}
 
 	/**
+	 * Checks if $sub is a subdirectory of $parent
+	 *
+	 * @param string $sub
+	 * @param string $parent
+	 * @return bool
+	 */
+	private function isSubDirectory($sub, $parent) {
+		// Check whether $sub contains no ".."
+		if(strpos($sub, '..') !== false) {
+			return false;
+		}
+
+		// Check whether $sub is a subdirectory of $parent
+		if (strpos($sub, $parent) === 0) {
+			return true;
+		}
+
+		return false;
+	}
+
+	/**
 	 * Get a list of language files that should be loaded
 	 *
 	 * @param string $app
@@ -302,10 +324,10 @@ class Factory implements IFactory {
 		$i18nDir = $this->findL10nDir($app);
 		$transFile = strip_tags($i18nDir) . strip_tags($lang) . '.json';
 
-		if ((\OC_Helper::isSubDirectory($transFile, $this->serverRoot . '/core/l10n/')
-				|| \OC_Helper::isSubDirectory($transFile, $this->serverRoot . '/lib/l10n/')
-				|| \OC_Helper::isSubDirectory($transFile, $this->serverRoot . '/settings/l10n/')
-				|| \OC_Helper::isSubDirectory($transFile, \OC_App::getAppPath($app) . '/l10n/')
+		if (($this->isSubDirectory($transFile, $this->serverRoot . '/core/l10n/')
+				|| $this->isSubDirectory($transFile, $this->serverRoot . '/lib/l10n/')
+				|| $this->isSubDirectory($transFile, $this->serverRoot . '/settings/l10n/')
+				|| $this->isSubDirectory($transFile, \OC_App::getAppPath($app) . '/l10n/')
 			)
 			&& file_exists($transFile)) {
 			// load the translations file
diff --git a/lib/private/legacy/helper.php b/lib/private/legacy/helper.php
index b19e58a9e6c..0b9477dacd4 100644
--- a/lib/private/legacy/helper.php
+++ b/lib/private/legacy/helper.php
@@ -378,32 +378,6 @@ class OC_Helper {
 	}
 
 	/**
-	 * Checks if $sub is a subdirectory of $parent
-	 *
-	 * @param string $sub
-	 * @param string $parent
-	 * @return bool
-	 */
-	public static function isSubDirectory($sub, $parent) {
-		$realpathSub = realpath($sub);
-		$realpathParent = realpath($parent);
-
-		// realpath() may return false in case the directory does not exist
-		// since we can not be sure how different PHP versions may behave here
-		// we do an additional check whether realpath returned false
-		if($realpathSub === false ||  $realpathParent === false) {
-			return false;
-		}
-
-		// Check whether $sub is a subdirectory of $parent
-		if (strpos($realpathSub, $realpathParent) === 0) {
-			return true;
-		}
-
-		return false;
-	}
-
-	/**
 	 * Returns an array with all keys from input lowercased or uppercased. Numbered indices are left as is.
 	 *
 	 * @param array $input The array to work on
diff --git a/tests/lib/LegacyHelperTest.php b/tests/lib/LegacyHelperTest.php
index faea95190ec..f1e22ea600e 100644
--- a/tests/lib/LegacyHelperTest.php
+++ b/tests/lib/LegacyHelperTest.php
@@ -76,19 +76,6 @@ class LegacyHelperTest extends \Test\TestCase {
 		];
 	}
 
-	function testIsSubDirectory() {
-		$result = OC_Helper::isSubDirectory("./data/", "/anotherDirectory/");
-		$this->assertFalse($result);
-
-		$result = OC_Helper::isSubDirectory("./data/", "./data/");
-		$this->assertTrue($result);
-
-		mkdir("data/TestSubdirectory", 0777);
-		$result = OC_Helper::isSubDirectory("data/TestSubdirectory/", "data");
-		rmdir("data/TestSubdirectory");
-		$this->assertTrue($result);
-	}
-
 	function testMb_array_change_key_case() {
 		$arrayStart = array(
 			"Foo" => "bar",