Merge pull request #52911 from nextcloud/chore/deprecate-ochelper-copyr

chore: deprecate OC_Helper::copyr

build/psalm-baseline.xml

@@ -4619,9 +4619,6 @@
       <code><![CDATA[$matches[0][$last_match]]]></code>
       <code><![CDATA[$matches[1][$last_match]]]></code>
     </InvalidArrayOffset>
-    <InvalidScalarArgument>
-      <code><![CDATA[$path]]></code>
-    </InvalidScalarArgument>
     <UndefinedInterfaceMethod>
       <code><![CDATA[getQuota]]></code>
     </UndefinedInterfaceMethod>

lib/private/Installer.php

@@ -16,8 +16,8 @@ use OC\AppFramework\Bootstrap\Coordinator;
 use OC\Archive\TAR;
 use OC\DB\Connection;
 use OC\DB\MigrationService;
+use OC\Files\FilenameValidator;
 use OC_App;
-use OC_Helper;
 use OCP\App\IAppManager;
 use OCP\Files;
 use OCP\HintException;
@@ -25,6 +25,7 @@ use OCP\Http\Client\IClientService;
 use OCP\IConfig;
 use OCP\ITempManager;
 use OCP\Migration\IOutput;
+use OCP\Server;
 use phpseclib\File\X509;
 use Psr\Log\LoggerInterface;
 
@@ -242,6 +243,10 @@ class Installer {
 
 				// Download the release
 				$tempFile = $this->tempManager->getTemporaryFile('.tar.gz');
+				if ($tempFile === false) {
+					throw new \RuntimeException('Could not create temporary file for downloading app archive.');
+				}
+
 				$timeout = $this->isCLI ? 0 : 120;
 				$client = $this->clientService->newClient();
 				$client->get($app['releases'][0]['download'], ['sink' => $tempFile, 'timeout' => $timeout]);
@@ -253,8 +258,11 @@ class Installer {
 				if ($verified === true) {
 					// Seems to match, let's proceed
 					$extractDir = $this->tempManager->getTemporaryFolder();
-					$archive = new TAR($tempFile);
+					if ($extractDir === false) {
+						throw new \RuntimeException('Could not create temporary directory for unpacking app.');
+					}
 
+					$archive = new TAR($tempFile);
 					if (!$archive->extract($extractDir)) {
 						$errorMessage = 'Could not extract app ' . $appId;
 
@@ -329,9 +337,9 @@ class Installer {
 					// Move to app folder
 					if (@mkdir($baseDir)) {
 						$extractDir .= '/' . $folders[0];
-						OC_Helper::copyr($extractDir, $baseDir);
 					}
-					OC_Helper::copyr($extractDir, $baseDir);
+					// otherwise we just copy the outer directory
+					$this->copyRecursive($extractDir, $baseDir);
 					Files::rmdirr($extractDir);
 					return;
 				}
@@ -588,4 +596,33 @@ class Installer {
 			include $script;
 		}
 	}
+
+	/**
+	 * Recursive copying of local folders.
+	 *
+	 * @param string $src source folder
+	 * @param string $dest target folder
+	 */
+	private function copyRecursive(string $src, string $dest): void {
+		if (!file_exists($src)) {
+			return;
+		}
+
+		if (is_dir($src)) {
+			if (!is_dir($dest)) {
+				mkdir($dest);
+			}
+			$files = scandir($src);
+			foreach ($files as $file) {
+				if ($file != '.' && $file != '..') {
+					$this->copyRecursive("$src/$file", "$dest/$file");
+				}
+			}
+		} else {
+			$validator = Server::get(FilenameValidator::class);
+			if (!$validator->isForbidden($src)) {
+				copy($src, $dest);
+			}
+		}
+	}
 }

lib/private/legacy/OC_Helper.php

@@ -66,6 +66,7 @@ class OC_Helper {
 	 * @param string $src source folder
 	 * @param string $dest target folder
 	 * @return void
+	 * @deprecated 32.0.0 - use \OCP\Files\Folder::copy
 	 */
 	public static function copyr($src, $dest) {
 		if (!file_exists($src)) {
@@ -131,7 +132,7 @@ class OC_Helper {
 		$exts = [''];
 		$check_fn = 'is_executable';
 		// Default check will be done with $path directories :
-		$dirs = explode(PATH_SEPARATOR, $path);
+		$dirs = explode(PATH_SEPARATOR, (string)$path);
 		// WARNING : We have to check if open_basedir is enabled :
 		$obd = OC::$server->get(IniGetWrapper::class)->getString('open_basedir');
 		if ($obd != 'none') {