diff --git a/core/Command/App/CheckCode.php b/core/Command/App/CheckCode.php
index 82a137e58e..a129bcf1e1 100644
--- a/core/Command/App/CheckCode.php
+++ b/core/Command/App/CheckCode.php
@@ -44,20 +44,12 @@ use OC\App\CodeChecker\PrivateCheck;
class CheckCode extends Command implements CompletionAwareInterface {
- /** @var InfoParser */
- private $infoParser;
-
protected $checkers = [
'private' => PrivateCheck::class,
'deprecation' => DeprecationCheck::class,
'strong-comparison' => StrongComparisonCheck::class,
];
- public function __construct(InfoParser $infoParser) {
- parent::__construct();
- $this->infoParser = $infoParser;
- }
-
protected function configure() {
$this
->setName('app:check-code')
@@ -134,50 +126,12 @@ class CheckCode extends Command implements CompletionAwareInterface {
}
if(!$input->getOption('skip-validate-info')) {
- $infoChecker = new InfoChecker($this->infoParser);
-
- $infoChecker->listen('InfoChecker', 'mandatoryFieldMissing', function($key) use ($output) {
- $output->writeln("Mandatory field missing: $key");
+ // Can not inject because of circular dependency
+ $infoChecker = new InfoChecker(\OC::$server->getAppManager());
+ $infoChecker->listen('InfoChecker', 'parseError', function($error) use ($output) {
+ $output->writeln("Invalid appinfo.xml file found: $error");
});
- $infoChecker->listen('InfoChecker', 'deprecatedFieldFound', function($key, $value) use ($output) {
- if($value === [] || is_null($value) || $value === '') {
- $output->writeln("Deprecated field available: $key");
- } else {
- $output->writeln("Deprecated field available: $key => $value");
- }
- });
-
- $infoChecker->listen('InfoChecker', 'missingRequirement', function($minMax) use ($output) {
- $output->writeln("Nextcloud $minMax version requirement missing");
- });
-
- $infoChecker->listen('InfoChecker', 'differentVersions', function($versionFile, $infoXML) use ($output) {
- $output->writeln("Different versions provided (appinfo/version: $versionFile - appinfo/info.xml: $infoXML)");
- });
-
- $infoChecker->listen('InfoChecker', 'sameVersions', function($path) use ($output) {
- $output->writeln("Version file isn't needed anymore and can be safely removed ($path)");
- });
-
- $infoChecker->listen('InfoChecker', 'migrateVersion', function($version) use ($output) {
- $output->writeln("Migrate the app version to appinfo/info.xml (add $version to appinfo/info.xml and remove appinfo/version)");
- });
-
- if(OutputInterface::VERBOSITY_VERBOSE <= $output->getVerbosity()) {
- $infoChecker->listen('InfoChecker', 'mandatoryFieldFound', function($key, $value) use ($output) {
- $output->writeln("Mandatory field available: $key => $value");
- });
-
- $infoChecker->listen('InfoChecker', 'optionalFieldFound', function($key, $value) use ($output) {
- $output->writeln("Optional field available: $key => $value");
- });
-
- $infoChecker->listen('InfoChecker', 'unusedFieldFound', function($key, $value) use ($output) {
- $output->writeln("Unused field available: $key => $value");
- });
- }
-
$infoErrors = $infoChecker->analyse($appId);
$errors = array_merge($errors, $infoErrors);
diff --git a/core/register_command.php b/core/register_command.php
index 372d775dc1..578b4f799b 100644
--- a/core/register_command.php
+++ b/core/register_command.php
@@ -40,8 +40,7 @@
$application->add(new \Stecman\Component\Symfony\Console\BashCompletion\CompletionCommand());
$application->add(new OC\Core\Command\Status);
$application->add(new OC\Core\Command\Check(\OC::$server->getSystemConfig()));
-$infoParser = new \OC\App\InfoParser();
-$application->add(new OC\Core\Command\App\CheckCode($infoParser));
+$application->add(new OC\Core\Command\App\CheckCode());
$application->add(new OC\Core\Command\L10n\CreateJs());
$application->add(new \OC\Core\Command\Integrity\SignApp(
\OC::$server->getIntegrityCodeChecker(),
diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php
index 0e921ba1b7..81037b4261 100644
--- a/lib/private/App/AppManager.php
+++ b/lib/private/App/AppManager.php
@@ -411,6 +411,7 @@ class AppManager implements IAppManager {
/**
* @inheritdoc
+ * In case you change this method, also change \OC\App\CodeChecker\InfoChecker::isShipped()
*/
public function isShipped($appId) {
$this->loadShippedJson();
@@ -422,6 +423,10 @@ class AppManager implements IAppManager {
return in_array($appId, $alwaysEnabled, true);
}
+ /**
+ * In case you change this method, also change \OC\App\CodeChecker\InfoChecker::loadShippedJson()
+ * @throws \Exception
+ */
private function loadShippedJson() {
if ($this->shippedApps === null) {
$shippedJson = \OC::$SERVERROOT . '/core/shipped.json';
diff --git a/lib/private/App/CodeChecker/InfoChecker.php b/lib/private/App/CodeChecker/InfoChecker.php
index e8791bd703..f5de6d376d 100644
--- a/lib/private/App/CodeChecker/InfoChecker.php
+++ b/lib/private/App/CodeChecker/InfoChecker.php
@@ -23,121 +23,88 @@
namespace OC\App\CodeChecker;
-use OC\App\InfoParser;
use OC\Hooks\BasicEmitter;
+use OCP\App\AppPathNotFoundException;
+use OCP\App\IAppManager;
class InfoChecker extends BasicEmitter {
- /** @var InfoParser */
- private $infoParser;
+ /** @var string[] */
+ private $shippedApps;
- private $mandatoryFields = [
- 'author',
- 'description',
- 'dependencies',
- 'id',
- 'licence',
- 'name',
- 'version',
- ];
- private $optionalFields = [
- 'bugs',
- 'category',
- 'default_enable',
- 'documentation',
- 'namespace',
- 'ocsid',
- 'public',
- 'remote',
- 'repository',
- 'types',
- 'website',
- ];
- private $deprecatedFields = [
- 'info',
- 'require',
- 'requiremax',
- 'requiremin',
- 'shipped',
- 'standalone',
- ];
-
- public function __construct(InfoParser $infoParser) {
- $this->infoParser = $infoParser;
- }
+ /** @var string[] */
+ private $alwaysEnabled;
/**
* @param string $appId
* @return array
+ * @throws \RuntimeException
*/
- public function analyse($appId) {
+ public function analyse($appId): array {
$appPath = \OC_App::getAppPath($appId);
if ($appPath === false) {
throw new \RuntimeException("No app with given id <$appId> known.");
}
+ $xml = new \DOMDocument();
+ $xml->load($appPath . '/appinfo/info.xml');
+
+ $schema = \OC::$SERVERROOT . '/resources/app-info.xsd';
+ try {
+ if ($this->isShipped($appId)) {
+ // Shipped apps are allowed to have the public and default_enabled tags
+ $schema = \OC::$SERVERROOT . '/resources/app-info-shipped.xsd';
+ }
+ } catch (\Exception $e) {
+ // Assume it is not shipped
+ }
+
$errors = [];
-
- $info = $this->infoParser->parse($appPath . '/appinfo/info.xml');
-
- if (!isset($info['dependencies']['nextcloud']['@attributes']['min-version'])) {
- $errors[] = [
- 'type' => 'missingRequirement',
- 'field' => 'min',
- ];
- $this->emit('InfoChecker', 'missingRequirement', ['min']);
- }
-
- if (!isset($info['dependencies']['nextcloud']['@attributes']['max-version'])) {
- $errors[] = [
- 'type' => 'missingRequirement',
- 'field' => 'max',
- ];
- $this->emit('InfoChecker', 'missingRequirement', ['max']);
- }
-
- foreach ($info as $key => $value) {
- if(is_array($value)) {
- $value = json_encode($value);
- }
- if (in_array($key, $this->mandatoryFields)) {
- $this->emit('InfoChecker', 'mandatoryFieldFound', [$key, $value]);
- continue;
- }
-
- if (in_array($key, $this->optionalFields)) {
- $this->emit('InfoChecker', 'optionalFieldFound', [$key, $value]);
- continue;
- }
-
- if (in_array($key, $this->deprecatedFields)) {
- // skip empty arrays - empty arrays for remote and public are always added
- if($value === '[]' && in_array($key, ['public', 'remote', 'info'])) {
- continue;
- }
- $this->emit('InfoChecker', 'deprecatedFieldFound', [$key, $value]);
- continue;
- }
-
- $this->emit('InfoChecker', 'unusedFieldFound', [$key, $value]);
- }
-
- foreach ($this->mandatoryFields as $key) {
- if(!isset($info[$key])) {
- $this->emit('InfoChecker', 'mandatoryFieldMissing', [$key]);
+ if (!$xml->schemaValidate($schema)) {
+ foreach (libxml_get_errors() as $error) {
$errors[] = [
- 'type' => 'mandatoryFieldMissing',
- 'field' => $key,
+ 'type' => 'parseError',
+ 'field' => $error->message,
];
+ $this->emit('InfoChecker', 'parseError', [$error->message]);
}
}
- $versionFile = $appPath . '/appinfo/version';
- if (is_file($versionFile)) {
- $version = trim(file_get_contents($versionFile));
- $this->emit('InfoChecker', 'migrateVersion', [$version]);
- }
-
return $errors;
}
+
+ /**
+ * This is a copy of \OC\App\AppManager::isShipped(), keep both in sync.
+ * This method is copied, so the code checker works even when Nextcloud is
+ * not installed yet. The AppManager requires a database connection, which
+ * fails in that case.
+ *
+ * @param string $appId
+ * @return bool
+ * @throws \Exception
+ */
+ protected function isShipped(string $appId): bool {
+ $this->loadShippedJson();
+ return \in_array($appId, $this->shippedApps, true);
+ }
+
+ /**
+ * This is a copy of \OC\App\AppManager::loadShippedJson(), keep both in sync
+ * This method is copied, so the code checker works even when Nextcloud is
+ * not installed yet. The AppManager requires a database connection, which
+ * fails in that case.
+ *
+ * @throws \Exception
+ */
+ protected function loadShippedJson() {
+ if ($this->shippedApps === null) {
+ $shippedJson = \OC::$SERVERROOT . '/core/shipped.json';
+ if (!file_exists($shippedJson)) {
+ throw new \Exception("File not found: $shippedJson");
+ }
+ $content = json_decode(file_get_contents($shippedJson), true);
+ $this->shippedApps = $content['shippedApps'];
+ $this->alwaysEnabled = $content['alwaysEnabled'];
+ }
+ }
}
diff --git a/resources/app-info-shipped.xsd b/resources/app-info-shipped.xsd
new file mode 100644
index 0000000000..97221d1fb9
--- /dev/null
+++ b/resources/app-info-shipped.xsd
@@ -0,0 +1,671 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/resources/app-info.xsd b/resources/app-info.xsd
new file mode 100644
index 0000000000..4460b0f63b
--- /dev/null
+++ b/resources/app-info.xsd
@@ -0,0 +1,652 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/apps/testapp-dependency-missing/appinfo/info.xml b/tests/apps/testapp-dependency-missing/appinfo/info.xml
deleted file mode 100644
index c765400a76..0000000000
--- a/tests/apps/testapp-dependency-missing/appinfo/info.xml
+++ /dev/null
@@ -1,9 +0,0 @@
-
-
- testapp-infoxml-version
- 1.2.3
- Jane
- A b c
- Abc
- Test app
-
diff --git a/tests/apps/testapp-infoxml/appinfo/info.xml b/tests/apps/testapp-infoxml/appinfo/info.xml
deleted file mode 100644
index d4df1c3cd3..0000000000
--- a/tests/apps/testapp-infoxml/appinfo/info.xml
+++ /dev/null
@@ -1,12 +0,0 @@
-
-
- testapp-infoxml
- 1.2.3
- Jane
- A b c
- Abc
- Test app
-
-
-
-
diff --git a/tests/apps/testapp-name-missing/appinfo/info.xml b/tests/apps/testapp-name-missing/appinfo/info.xml
deleted file mode 100644
index 591c436189..0000000000
--- a/tests/apps/testapp-name-missing/appinfo/info.xml
+++ /dev/null
@@ -1,11 +0,0 @@
-
-
- testapp-version
- 1.1.1
- Jane
- A b c
- Abc
-
-
-
-
diff --git a/tests/apps/testapp-version/appinfo/info.xml b/tests/apps/testapp-version/appinfo/info.xml
deleted file mode 100644
index 28e2475800..0000000000
--- a/tests/apps/testapp-version/appinfo/info.xml
+++ /dev/null
@@ -1,11 +0,0 @@
-
-
- testapp-version
- Jane
- A b c
- Abc
- Test app
-
-
-
-
diff --git a/tests/apps/testapp-version/appinfo/version b/tests/apps/testapp-version/appinfo/version
deleted file mode 100644
index 0495c4a88c..0000000000
--- a/tests/apps/testapp-version/appinfo/version
+++ /dev/null
@@ -1 +0,0 @@
-1.2.3
diff --git a/tests/apps/testapp_dependency_missing/appinfo/info.xml b/tests/apps/testapp_dependency_missing/appinfo/info.xml
new file mode 100644
index 0000000000..b0ca188aef
--- /dev/null
+++ b/tests/apps/testapp_dependency_missing/appinfo/info.xml
@@ -0,0 +1,13 @@
+
+
+ testapp_infoxml
+ Test app
+ A b c
+ A b c
+ 1.2.3
+ agpl
+ Jane
+ games
+ https://example.org
+
diff --git a/tests/apps/testapp_infoxml/appinfo/info.xml b/tests/apps/testapp_infoxml/appinfo/info.xml
new file mode 100644
index 0000000000..b7575687fe
--- /dev/null
+++ b/tests/apps/testapp_infoxml/appinfo/info.xml
@@ -0,0 +1,16 @@
+
+
+ testapp_infoxml
+ Test app
+ A b c
+ A b c
+ 1.2.3
+ agpl
+ Jane
+ games
+ https://example.org
+
+
+
+
diff --git a/tests/apps/testapp_name_missing/appinfo/info.xml b/tests/apps/testapp_name_missing/appinfo/info.xml
new file mode 100644
index 0000000000..872c8752c7
--- /dev/null
+++ b/tests/apps/testapp_name_missing/appinfo/info.xml
@@ -0,0 +1,15 @@
+
+
+ testapp_name_missing
+ A b c
+ A b c
+ 1.2.3
+ agpl
+ Jane
+ games
+ https://example.org
+
+
+
+
diff --git a/tests/apps/testapp_version/appinfo/info.xml b/tests/apps/testapp_version/appinfo/info.xml
new file mode 100644
index 0000000000..3a48fccd45
--- /dev/null
+++ b/tests/apps/testapp_version/appinfo/info.xml
@@ -0,0 +1,15 @@
+
+
+ testapp_version
+ Test app
+ A b c
+ A b c
+ agpl
+ Jane
+ games
+ https://example.org
+
+
+
+
diff --git a/tests/lib/App/CodeChecker/InfoCheckerTest.php b/tests/lib/App/CodeChecker/InfoCheckerTest.php
index 760d988073..9f354a4611 100644
--- a/tests/lib/App/CodeChecker/InfoCheckerTest.php
+++ b/tests/lib/App/CodeChecker/InfoCheckerTest.php
@@ -44,19 +44,21 @@ class InfoCheckerTest extends TestCase {
protected function setUp() {
parent::setUp();
- $this->infoChecker = new InfoChecker(new InfoParser());
+ $this->infoChecker = new InfoChecker(\OC::$server->getAppManager());
}
public function appInfoData() {
return [
- ['testapp-infoxml', []],
- ['testapp-version', [['type' => 'mandatoryFieldMissing', 'field' => 'version']]],
- ['testapp-dependency-missing', [
- ['type' => 'missingRequirement', 'field' => 'min'],
- ['type' => 'missingRequirement', 'field' => 'max'],
- ['type' => 'mandatoryFieldMissing', 'field' => 'dependencies'],
+ ['testapp_infoxml', []],
+ ['testapp_version', [
+ ['type' => 'parseError', 'field' => 'Element \'licence\': This element is not expected. Expected is one of ( description, version ).' . "\n"],
+ ]],
+ ['testapp_dependency_missing', [
+ ['type' => 'parseError', 'field' => 'Element \'info\': Missing child element(s). Expected is one of ( repository, screenshot, dependencies ).' . "\n"],
+ ]],
+ ['testapp_name_missing', [
+ ['type' => 'parseError', 'field' => 'Element \'summary\': This element is not expected. Expected is ( name ).' . "\n"],
]],
- ['testapp-name-missing', [['type' => 'mandatoryFieldMissing', 'field' => 'name']]],
];
}
@@ -68,6 +70,7 @@ class InfoCheckerTest extends TestCase {
*/
public function testApps($appId, $expectedErrors) {
$errors = $this->infoChecker->analyse($appId);
+ libxml_clear_errors();
$this->assertEquals($expectedErrors, $errors);
}