Merge pull request #9024 from nextcloud/fix/app-fetcher-major-minor-versions

Fix version comparison with minor and patch level requirements

4 files changed, 122 insertions, 11 deletions

diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index 76c1c0f7eef..167e3033bca 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -381,6 +381,7 @@ return array(
     'OC\\App\\CodeChecker\\NodeVisitor' => $baseDir . '/lib/private/App/CodeChecker/NodeVisitor.php',
     'OC\\App\\CodeChecker\\PrivateCheck' => $baseDir . '/lib/private/App/CodeChecker/PrivateCheck.php',
     'OC\\App\\CodeChecker\\StrongComparisonCheck' => $baseDir . '/lib/private/App/CodeChecker/StrongComparisonCheck.php',
+    'OC\\App\\CompareVersion' => $baseDir . '/lib/private/App/CompareVersion.php',
     'OC\\App\\DependencyAnalyzer' => $baseDir . '/lib/private/App/DependencyAnalyzer.php',
     'OC\\App\\InfoParser' => $baseDir . '/lib/private/App/InfoParser.php',
     'OC\\App\\Platform' => $baseDir . '/lib/private/App/Platform.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index 73c3bbf4d05..4bf613d1256 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -411,6 +411,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\App\\CodeChecker\\NodeVisitor' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/NodeVisitor.php',
         'OC\\App\\CodeChecker\\PrivateCheck' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/PrivateCheck.php',
         'OC\\App\\CodeChecker\\StrongComparisonCheck' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/StrongComparisonCheck.php',
+        'OC\\App\\CompareVersion' => __DIR__ . '/../../..' . '/lib/private/App/CompareVersion.php',
         'OC\\App\\DependencyAnalyzer' => __DIR__ . '/../../..' . '/lib/private/App/DependencyAnalyzer.php',
         'OC\\App\\InfoParser' => __DIR__ . '/../../..' . '/lib/private/App/InfoParser.php',
         'OC\\App\\Platform' => __DIR__ . '/../../..' . '/lib/private/App/Platform.php',
diff --git a/lib/private/App/AppStore/Fetcher/AppFetcher.php b/lib/private/App/AppStore/Fetcher/AppFetcher.php
index 6c6fa68429f..5203515e09b 100644
--- a/lib/private/App/AppStore/Fetcher/AppFetcher.php
+++ b/lib/private/App/AppStore/Fetcher/AppFetcher.php
@@ -27,24 +27,32 @@
 namespace OC\App\AppStore\Fetcher;
 
 use OC\App\AppStore\Version\VersionParser;
+use OC\App\CompareVersion;
 use OC\Files\AppData\Factory;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Http\Client\IClientService;
 use OCP\IConfig;
 use OCP\ILogger;
+use OCP\Util;
 
 class AppFetcher extends Fetcher {
+
+	/** @var CompareVersion */
+	private $compareVersion;
+
 	/**
 	 * @param Factory $appDataFactory
 	 * @param IClientService $clientService
 	 * @param ITimeFactory $timeFactory
 	 * @param IConfig $config
+	 * @param CompareVersion $compareVersion
 	 * @param ILogger $logger
 	 */
 	public function __construct(Factory $appDataFactory,
 								IClientService $clientService,
 								ITimeFactory $timeFactory,
 								IConfig $config,
+								CompareVersion $compareVersion,
 								ILogger $logger) {
 		parent::__construct(
 			$appDataFactory,
@@ -56,6 +64,7 @@ class AppFetcher extends Fetcher {
 
 		$this->fileName = 'apps.json';
 		$this->setEndpoint();
+		$this->compareVersion = $compareVersion;
 	}
 
 	/**
@@ -70,8 +79,6 @@ class AppFetcher extends Fetcher {
 		/** @var mixed[] $response */
 		$response = parent::fetch($ETag, $content);
 
-		$ncVersion = $this->getVersion();
-		$ncMajorVersion = explode('.', $ncVersion)[0];
 		foreach($response['data'] as $dataKey => $app) {
 			$releases = [];
 
@@ -81,15 +88,20 @@ class AppFetcher extends Fetcher {
 				if($release['isNightly'] === false
 					&& strpos($release['version'], '-') === false) {
 					// Exclude all versions not compatible with the current version
-					$versionParser = new VersionParser();
-					$version = $versionParser->getVersion($release['rawPlatformVersionSpec']);
-					if (
-						// Major version is bigger or equals to the minimum version of the app
-						version_compare($ncMajorVersion, $version->getMinimumVersion(), '>=')
-						// Major version is smaller or equals to the maximum version of the app
-						&& version_compare($ncMajorVersion, $version->getMaximumVersion(), '<=')
-					) {
-						$releases[] = $release;
+					try {
+						$versionParser = new VersionParser();
+						$version = $versionParser->getVersion($release['rawPlatformVersionSpec']);
+						$ncVersion = $this->getVersion();
+						$min = $version->getMinimumVersion();
+						$max = $version->getMaximumVersion();
+						$minFulfilled = $this->compareVersion->isCompatible($ncVersion, $min, '>=');
+						$maxFulfilled = $max !== '' &&
+							$this->compareVersion->isCompatible($ncVersion, $max, '<=');
+						if ($minFulfilled && $maxFulfilled) {
+							$releases[] = $release;
+						}
+					} catch (\InvalidArgumentException $e) {
+						$this->logger->logException($e, ['app' => 'appstoreFetcher', 'level' => Util::WARN]);
 					}
 				}
 			}
diff --git a/lib/private/App/CompareVersion.php b/lib/private/App/CompareVersion.php
new file mode 100644
index 00000000000..ee25a8b9460
--- /dev/null
+++ b/lib/private/App/CompareVersion.php
@@ -0,0 +1,97 @@
+<?php
+
+/**
+ * @copyright 2018 Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @author 2018 Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC\App;
+
+use InvalidArgumentException;
+
+class CompareVersion {
+
+	const REGEX_MAJOR = '/^\d+$/';
+	const REGEX_MAJOR_MINOR = '/^\d+.\d+$/';
+	const REGEX_MAJOR_MINOR_PATCH = '/^\d+.\d+.\d+$/';
+	const REGEX_SERVER = '/^\d+.\d+.\d+(.\d+)?$/';
+
+	/**
+	 * Checks if the given server version fulfills the given (app) version requirements.
+	 *
+	 * Version requirements can be 'major.minor.patch', 'major.minor' or just 'major',
+	 * so '13.0.1', '13.0' and '13' are valid.
+	 *
+	 * @param string $actual version as major.minor.patch notation
+	 * @param string $required version where major is requried and minor and patch are optional
+	 * @param string $comparator passed to `version_compare`
+	 * @return bool whether the requirement is fulfilled
+	 * @throws InvalidArgumentException if versions specified in an invalid format
+	 */
+	public function isCompatible(string $actual, string $required,
+		string $comparator = '>='): bool {
+
+		if (!preg_match(self::REGEX_SERVER, $actual)) {
+			throw new InvalidArgumentException('server version is invalid');
+		}
+
+		if (preg_match(self::REGEX_MAJOR, $required) === 1) {
+			return $this->compareMajor($actual, $required, $comparator);
+		} else if (preg_match(self::REGEX_MAJOR_MINOR, $required) === 1) {
+			return $this->compareMajorMinor($actual, $required, $comparator);
+		} else if (preg_match(self::REGEX_MAJOR_MINOR_PATCH, $required) === 1) {
+			return $this->compareMajorMinorPatch($actual, $required, $comparator);
+		} else {
+			throw new InvalidArgumentException('required version is invalid');
+		}
+	}
+
+	private function compareMajor(string $actual, string $required,
+		string $comparator) {
+		$actualMajor = explode('.', $actual)[0];
+		$requiredMajor = explode('.', $required)[0];
+
+		return version_compare($actualMajor, $requiredMajor, $comparator);
+	}
+
+	private function compareMajorMinor(string $actual, string $required,
+		string $comparator) {
+		$actualMajor = explode('.', $actual)[0];
+		$actualMinor = explode('.', $actual)[1];
+		$requiredMajor = explode('.', $required)[0];
+		$requiredMinor = explode('.', $required)[1];
+
+		return version_compare("$actualMajor.$actualMinor",
+			"$requiredMajor.$requiredMinor", $comparator);
+	}
+
+	private function compareMajorMinorPatch($actual, $required, $comparator) {
+		$actualMajor = explode('.', $actual)[0];
+		$actualMinor = explode('.', $actual)[1];
+		$actualPatch = explode('.', $actual)[2];
+		$requiredMajor = explode('.', $required)[0];
+		$requiredMinor = explode('.', $required)[1];
+		$requiredPatch = explode('.', $required)[2];
+
+		return version_compare("$actualMajor.$actualMinor.$actualPatch",
+			"$requiredMajor.$requiredMinor.$requiredPatch", $comparator);
+	}
+
+}