From 2b08659f7d18d458339bcd89687469d8926f4941 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Sun, 30 Aug 2015 16:21:55 +0200 Subject: [PATCH 1/4] Restrict upgrades to explicit allowed version version.php now contains the previous ownCloud version from which upgrades are allowed. Any other upgrades will show a message that the upgrade/downgrade is not supported. --- lib/private/updater.php | 40 ++++++++++++++++++---- tests/lib/updater.php | 74 +++++++++++++++++++++++++++++++++++------ version.php | 11 +++--- 3 files changed, 105 insertions(+), 20 deletions(-) diff --git a/lib/private/updater.php b/lib/private/updater.php index f73fa8ff65..0f9ecfe93d 100644 --- a/lib/private/updater.php +++ b/lib/private/updater.php @@ -211,20 +211,47 @@ class Updater extends BasicEmitter { return $success; } + /** + * Return version from which this version is allowed to upgrade from + * + * @return string allowed previous version + */ + private function getAllowedPreviousVersion() { + // this should really be a JSON file + require \OC::$SERVERROOT . '/version.php'; + return implode('.', $OC_VersionCanBeUpgradedFrom); + } + /** * Whether an upgrade to a specified version is possible * @param string $oldVersion * @param string $newVersion + * @param string $allowedPreviousVersion * @return bool */ - public function isUpgradePossible($oldVersion, $newVersion) { + public function isUpgradePossible($oldVersion, $newVersion, $allowedPreviousVersion) { + // downgrade is never allowed + if (version_compare($oldVersion, $newVersion, '>')) { + return false; + } + $oldVersion = explode('.', $oldVersion); $newVersion = explode('.', $newVersion); - if($newVersion[0] > ($oldVersion[0] + 1) || $oldVersion[0] > $newVersion[0]) { - return false; + while (count($oldVersion) > 2) { + array_pop($oldVersion); } - return true; + + while (count($newVersion) > 2) { + array_pop($newVersion); + } + + $oldVersion = implode('.', $oldVersion); + $newVersion = implode('.', $newVersion); + + // either we're updating from an allowed version or the current version + return (version_compare($allowedPreviousVersion, $oldVersion) === 0 + || version_compare($newVersion, $oldVersion) === 0); } /** @@ -259,8 +286,9 @@ class Updater extends BasicEmitter { */ private function doUpgrade($currentVersion, $installedVersion) { // Stop update if the update is over several major versions - if (!self::isUpgradePossible($installedVersion, $currentVersion)) { - throw new \Exception('Updates between multiple major versions are unsupported.'); + $allowedPreviousVersion = $this->getAllowedPreviousVersion(); + if (!self::isUpgradePossible($installedVersion, $currentVersion, $allowedPreviousVersion)) { + throw new \Exception('Updates between multiple major versions and downgrades are unsupported.'); } // Update .htaccess files diff --git a/tests/lib/updater.php b/tests/lib/updater.php index 28577071b4..763858acf5 100644 --- a/tests/lib/updater.php +++ b/tests/lib/updater.php @@ -67,14 +67,68 @@ class UpdaterTest extends \Test\TestCase { */ public function versionCompatibilityTestData() { return [ - ['1.0.0.0', '2.2.0', true], - ['1.1.1.1', '2.0.0', true], - ['5.0.3', '4.0.3', false], - ['12.0.3', '13.4.5', true], - ['1', '2', true], - ['2', '2', true], - ['6.0.5', '6.0.6', true], - ['5.0.6', '7.0.4', false], + ['1', '2', '1', true], + ['2', '2', '2', true], + ['6.0.5.0', '6.0.6.0', '5.0', true], + ['5.0.6.0', '7.0.4.0', '6.0', false], + // allow upgrading within the same major release + ['8.0.0.0', '8.0.0.0', '8.0', true], + ['8.0.0.0', '8.0.0.4', '8.0', true], + ['8.0.0.0', '8.0.1.0', '8.0', true], + ['8.0.0.0', '8.0.2.0', '8.0', true], + // does not allow downgrading within the same major release + ['8.0.1.0', '8.0.0.0', '8.0', false], + ['8.0.2.0', '8.0.1.0', '8.0', false], + ['8.0.0.4', '8.0.0.0', '8.0', false], + // allows upgrading within the patch version + ['8.0.0.0', '8.0.0.1', '8.0', true], + ['8.0.0.0', '8.0.0.2', '8.0', true], + // does not allow downgrading within the same major release + ['8.0.0.1', '8.0.0.0', '8.0', false], + ['8.0.0.2', '8.0.0.0', '8.0', false], + // allow upgrading to the next major release + ['8.0.0.0', '8.1.0.0', '8.0', true], + ['8.0.0.0', '8.1.1.0', '8.0', true], + ['8.0.0.0', '8.1.1.5', '8.0', true], + ['8.0.0.2', '8.1.1.5', '8.0', true], + ['8.1.0.0', '8.2.0.0', '8.1', true], + ['8.1.0.2', '8.2.0.4', '8.1', true], + ['8.1.0.5', '8.2.0.1', '8.1', true], + ['8.1.0.0', '8.2.1.0', '8.1', true], + ['8.1.0.2', '8.2.1.5', '8.1', true], + ['8.1.0.5', '8.2.1.1', '8.1', true], + // does not allow downgrading to the previous major release + ['8.1.0.0', '8.0.0.0', '7.0', false], + ['8.1.1.0', '8.0.0.0', '7.0', false], + // does not allow skipping major releases + ['8.0.0.0', '8.2.0.0', '8.1', false], + ['8.0.0.0', '8.2.1.0', '8.1', false], + ['8.0.0.0', '9.0.1.0', '8.2', false], + ['8.0.0.0', '10.0.0.0', '9.3', false], + // allows updating to the next major release + ['8.2.0.0', '9.0.0.0', '8.2', true], + ['8.2.0.0', '9.0.0.0', '8.2', true], + ['8.2.0.0', '9.0.1.0', '8.2', true], + ['8.2.0.0', '9.0.1.1', '8.2', true], + ['8.2.0.2', '9.0.1.1', '8.2', true], + ['8.2.2.0', '9.0.1.0', '8.2', true], + ['8.2.2.2', '9.0.1.1', '8.2', true], + ['9.0.0.0', '9.1.0.0', '9.0', true], + ['9.0.0.0', '9.1.0.2', '9.0', true], + ['9.0.0.2', '9.1.0.1', '9.0', true], + ['9.1.0.0', '9.2.0.0', '9.1', true], + ['9.2.0.0', '9.3.0.0', '9.2', true], + ['9.3.0.0', '10.0.0.0', '9.3', true], + // does not allow updating to the next major release (first number) + ['9.0.0.0', '8.2.0.0', '8.1', false], + // other cases + ['8.0.0.0', '8.1.5.0', '8.0', true], + ['8.2.0.0', '9.0.0.0', '8.2', true], + ['8.2.0.0', '9.1.0.0', '9.0', false], + ['9.0.0.0', '8.1.0.0', '8.0', false], + ['9.0.0.0', '8.0.0.0', '7.0', false], + ['9.1.0.0', '8.0.0.0', '7.0', false], + ['8.2.0.0', '8.1.0.0', '8.0', false], ]; } @@ -106,9 +160,9 @@ class UpdaterTest extends \Test\TestCase { * @param string $newVersion * @param bool $result */ - public function testIsUpgradePossible($oldVersion, $newVersion, $result) { + public function testIsUpgradePossible($oldVersion, $newVersion, $allowedVersion, $result) { $updater = new Updater($this->httpHelper, $this->config); - $this->assertSame($result, $updater->isUpgradePossible($oldVersion, $newVersion)); + $this->assertSame($result, $updater->isUpgradePossible($oldVersion, $newVersion, $allowedVersion)); } public function testCheckInCache() { diff --git a/version.php b/version.php index a115f4b26b..a6b49d9dc7 100644 --- a/version.php +++ b/version.php @@ -2,6 +2,7 @@ /** * @author Frank Karlitschek * @author Lukas Reschke + * @author Vincent Petry * * @copyright Copyright (c) 2015, ownCloud, Inc. * @license AGPL-3.0 @@ -22,14 +23,16 @@ // We only can count up. The 4. digit is only for the internal patchlevel to trigger DB upgrades // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version=array(8, 2, 0, 4); +$OC_Version = [8, 2, 0, 4]; // The human readable string -$OC_VersionString='8.2 pre alpha'; +$OC_VersionString = '8.2 pre alpha'; + +$OC_VersionCanBeUpgradedFrom = [8, 1]; // The ownCloud channel -$OC_Channel='git'; +$OC_Channel = 'git'; // The build number -$OC_Build=''; +$OC_Build = ''; From d5b0b55eefe6c397df2eafabc67514b4a38175fc Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Sun, 30 Aug 2015 18:03:33 +0200 Subject: [PATCH 2/4] Throw exception on downgrade attempt --- lib/private/util.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/private/util.php b/lib/private/util.php index edd375b5c3..47e42d8b7d 100644 --- a/lib/private/util.php +++ b/lib/private/util.php @@ -1448,8 +1448,12 @@ class OC_Util { if ($config->getSystemValue('installed', false)) { $installedVersion = $config->getSystemValue('version', '0.0.0'); $currentVersion = implode('.', OC_Util::getVersion()); - if (version_compare($currentVersion, $installedVersion, '>')) { + $versionDiff = version_compare($currentVersion, $installedVersion); + if ($versionDiff > 0) { return true; + } else if ($versionDiff < 0) { + // downgrade attempt, throw exception + throw new \OC\HintException('Downgrading is not supported and is likely to cause unpredictable issues (from ' . $installedVersion . ' to ' . $currentVersion . ')'); } // also check for upgrades for apps (independently from the user) From 3b37c203193b91253f814fa76257352279f5b6f3 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 31 Aug 2015 14:31:17 +0200 Subject: [PATCH 3/4] Simplify comparison algo --- lib/private/updater.php | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/lib/private/updater.php b/lib/private/updater.php index 0f9ecfe93d..639571e8f7 100644 --- a/lib/private/updater.php +++ b/lib/private/updater.php @@ -235,22 +235,8 @@ class Updater extends BasicEmitter { return false; } - $oldVersion = explode('.', $oldVersion); - $newVersion = explode('.', $newVersion); - - while (count($oldVersion) > 2) { - array_pop($oldVersion); - } - - while (count($newVersion) > 2) { - array_pop($newVersion); - } - - $oldVersion = implode('.', $oldVersion); - $newVersion = implode('.', $newVersion); - // either we're updating from an allowed version or the current version - return (version_compare($allowedPreviousVersion, $oldVersion) === 0 + return (version_compare($allowedPreviousVersion, $oldVersion, '<=') || version_compare($newVersion, $oldVersion) === 0); } From 6ccbf4bce64bc7016d65f270c22f37fdbca8e21f Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 31 Aug 2015 18:34:44 +0200 Subject: [PATCH 4/4] Improved update version detection logic --- lib/private/updater.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/private/updater.php b/lib/private/updater.php index 639571e8f7..71e9732c30 100644 --- a/lib/private/updater.php +++ b/lib/private/updater.php @@ -230,14 +230,8 @@ class Updater extends BasicEmitter { * @return bool */ public function isUpgradePossible($oldVersion, $newVersion, $allowedPreviousVersion) { - // downgrade is never allowed - if (version_compare($oldVersion, $newVersion, '>')) { - return false; - } - - // either we're updating from an allowed version or the current version return (version_compare($allowedPreviousVersion, $oldVersion, '<=') - || version_compare($newVersion, $oldVersion) === 0); + && version_compare($oldVersion, $newVersion, '<=')); } /**