diff --git a/lib/private/Installer.php b/lib/private/Installer.php index a0c7daae293a..164c8bdf041d 100644 --- a/lib/private/Installer.php +++ b/lib/private/Installer.php @@ -222,16 +222,18 @@ public static function updateApp($info= [], $isShipped=false) { $info = self::checkAppsIntegrity($info, $extractDir, $path, $isShipped); $currentDir = OC_App::getAppPath($info['id']); + if (\is_dir("$currentDir/.git")) { + throw new AppAlreadyInstalledException("App <{$info['id']}> is a git clone - it will not be updated."); + } + $basedir = OC_App::getInstallPath(); $basedir .= '/'; $basedir .= $info['id']; - if ($currentDir !== false && \is_writable($currentDir)) { + if ($currentDir !== false && OC_App::isAppDirWritable($info['id'])) { $basedir = $currentDir; } - if (\is_dir("$basedir/.git")) { - throw new AppAlreadyInstalledException("App <{$info['id']}> is a git clone - it will not be updated."); - } + if (\is_dir($basedir)) { OC_Helper::rmdirr($basedir); } diff --git a/lib/private/legacy/app.php b/lib/private/legacy/app.php index 5cd34e9e2228..1a12c117622c 100644 --- a/lib/private/legacy/app.php +++ b/lib/private/legacy/app.php @@ -568,6 +568,20 @@ public static function getAppWebPath($appId) { */ public static function isAppDirWritable($appId) { $path = self::getAppPath($appId); + // Check if the parent directory is marked as writable in config.php + if ($path !== false) { + $appDir = \substr($path, 0, -\strlen("/$appId")); + foreach (OC::$APPSROOTS as $dir) { + if ($dir['path'] !== $appDir) { + continue; + } + if (!isset($dir['writable']) + || $dir['writable'] !== true + ) { + return false; + } + } + } return ($path !== false) ? \is_writable($path) : false; } @@ -942,6 +956,7 @@ public static function getAppVersions() { * @return bool */ public static function updateApp($appId) { + \OC::$server->getAppManager()->clearAppsCache(); $appPath = self::getAppPath($appId); if ($appPath === false) { return false; @@ -958,7 +973,6 @@ public static function updateApp($appId) { } self::executeRepairSteps($appId, $appData['repair-steps']['post-migration']); self::setupLiveMigrations($appId, $appData['repair-steps']['live-migration']); - \OC::$server->getAppManager()->clearAppsCache(); // run upgrade code if (\file_exists($appPath . '/appinfo/update.php')) { self::loadApp($appId, false); diff --git a/tests/lib/InstallerTest.php b/tests/lib/InstallerTest.php index 4c8eac34afa7..ebaa4ec9e1fe 100644 --- a/tests/lib/InstallerTest.php +++ b/tests/lib/InstallerTest.php @@ -17,11 +17,12 @@ protected function setUp() { parent::setUp(); Installer::removeApp(self::$appid); + \OC::$server->getConfig()->deleteAppValues(self::$appid); } protected function tearDown() { Installer::removeApp(self::$appid); - + \OC::$server->getConfig()->deleteAppValues(self::$appid); parent::tearDown(); } @@ -89,4 +90,69 @@ public function testUpdateApp() { $this->assertNotEquals($oldVersionNumber, $newVersionNumber); } + + /** + * Tests that update is installed into writable app dir if the original app dir is not writable + */ + public function testUpdateIntoWritableAppDir() { + $oldAppRoots = \OC::$APPSROOTS; + $relativePath = "/anotherdir"; + + // Install old version + $pathOfOldTestApp = __DIR__ . '/../data/testapp.zip'; + $oldTmp = \OC::$server->getTempManager()->getTemporaryFile('.zip'); + \OC_Helper::copyr($pathOfOldTestApp, $oldTmp); + $oldData = [ + 'path' => $oldTmp, + 'source' => 'path', + 'appdata' => [ + 'id' => 'testapp', + 'level' => 100, + ] + ]; + $installResult = Installer::installApp($oldData); + $this->assertEquals('testapp', $installResult); + $oldAppPath = \OC_App::getAppPath(self::$appid); + + // Mark the first app as dir non-writable and create the second as writable + $firstAppDir = \array_shift(\OC::$APPSROOTS); + $firstAppDir['writable'] = false; + + $path = \dirname($firstAppDir['path']) . $relativePath; + \mkdir($path); + \clearstatcache(); + \OC::$APPSROOTS = [ + $firstAppDir, + [ + 'path' => $path, + 'url' => $relativePath, + 'writable' => true + ] + ]; + \OC::$server->getAppManager()->clearAppsCache(); + + // Update app + $pathOfNewTestApp = __DIR__ . '/../data/testapp2.zip'; + $newTmp = \OC::$server->getTempManager()->getTemporaryFile('.zip'); + \OC_Helper::copyr($pathOfNewTestApp, $newTmp); + + $newData = [ + 'path' => $newTmp, + 'source' => 'path', + 'appdata' => [ + 'id' => 'testapp', + 'level' => 100, + ] + ]; + $updateResult = Installer::updateApp($newData); + $this->assertTrue($updateResult); + $newAppPath = \OC_App::getAppPath(self::$appid); + + $this->assertNotEquals($oldAppPath, $newAppPath); + $this->assertStringStartsWith($path, $newAppPath); + + \OC_Helper::rmdirr($path); + \OC::$APPSROOTS = $oldAppRoots; + \OC::$server->getAppManager()->clearAppsCache(); + } }