From 31bb65fa360d1f6bb5fb5e4569160e8f006b841a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 9 May 2017 16:49:56 +0200 Subject: [PATCH] Check the name length of database items Signed-off-by: Joas Schilling --- core/Command/App/CheckCode.php | 13 +++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + .../App/CodeChecker/DatabaseSchemaChecker.php | 107 ++++++++++++++++++ 4 files changed, 122 insertions(+) create mode 100644 lib/private/App/CodeChecker/DatabaseSchemaChecker.php diff --git a/core/Command/App/CheckCode.php b/core/Command/App/CheckCode.php index 1effed99566..f787529bef9 100644 --- a/core/Command/App/CheckCode.php +++ b/core/Command/App/CheckCode.php @@ -26,6 +26,7 @@ namespace OC\Core\Command\App; use OC\App\CodeChecker\CodeChecker; +use OC\App\CodeChecker\DatabaseSchemaChecker; use OC\App\CodeChecker\EmptyCheck; use OC\App\CodeChecker\InfoChecker; use OC\App\CodeChecker\LanguageParseChecker; @@ -181,6 +182,18 @@ class CheckCode extends Command implements CompletionAwareInterface { } $errors = array_merge($errors, $languageErrors); + + $databaseSchema = new DatabaseSchemaChecker(); + $schemaErrors = $databaseSchema->analyse($appId); + + foreach ($schemaErrors['errors'] as $schemaError) { + $output->writeln("$schemaError"); + } + foreach ($schemaErrors['warnings'] as $schemaWarning) { + $output->writeln("$schemaWarning"); + } + + $errors = array_merge($errors, $schemaErrors['errors']); } $this->analyseUpdateFile($appId, $output); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 19380f6571b..10233f07a2a 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -332,6 +332,7 @@ return array( 'OC\\App\\AppStore\\Version\\VersionParser' => $baseDir . '/lib/private/App/AppStore/Version/VersionParser.php', 'OC\\App\\CodeChecker\\AbstractCheck' => $baseDir . '/lib/private/App/CodeChecker/AbstractCheck.php', 'OC\\App\\CodeChecker\\CodeChecker' => $baseDir . '/lib/private/App/CodeChecker/CodeChecker.php', + 'OC\\App\\CodeChecker\\DatabaseSchemaChecker' => $baseDir . '/lib/private/App/CodeChecker/DatabaseSchemaChecker.php', 'OC\\App\\CodeChecker\\DeprecationCheck' => $baseDir . '/lib/private/App/CodeChecker/DeprecationCheck.php', 'OC\\App\\CodeChecker\\EmptyCheck' => $baseDir . '/lib/private/App/CodeChecker/EmptyCheck.php', 'OC\\App\\CodeChecker\\ICheck' => $baseDir . '/lib/private/App/CodeChecker/ICheck.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 46285356489..9c0f0871133 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -362,6 +362,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\App\\AppStore\\Version\\VersionParser' => __DIR__ . '/../../..' . '/lib/private/App/AppStore/Version/VersionParser.php', 'OC\\App\\CodeChecker\\AbstractCheck' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/AbstractCheck.php', 'OC\\App\\CodeChecker\\CodeChecker' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/CodeChecker.php', + 'OC\\App\\CodeChecker\\DatabaseSchemaChecker' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/DatabaseSchemaChecker.php', 'OC\\App\\CodeChecker\\DeprecationCheck' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/DeprecationCheck.php', 'OC\\App\\CodeChecker\\EmptyCheck' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/EmptyCheck.php', 'OC\\App\\CodeChecker\\ICheck' => __DIR__ . '/../../..' . '/lib/private/App/CodeChecker/ICheck.php', diff --git a/lib/private/App/CodeChecker/DatabaseSchemaChecker.php b/lib/private/App/CodeChecker/DatabaseSchemaChecker.php new file mode 100644 index 00000000000..30226139b19 --- /dev/null +++ b/lib/private/App/CodeChecker/DatabaseSchemaChecker.php @@ -0,0 +1,107 @@ + + * + * @author Joas Schilling + * + * @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 . + * + */ + +namespace OC\App\CodeChecker; + +class DatabaseSchemaChecker { + + /** + * @param string $appId + * @return array + */ + public function analyse($appId) { + $appPath = \OC_App::getAppPath($appId); + if ($appPath === false) { + throw new \RuntimeException("No app with given id <$appId> known."); + } + + if (!file_exists($appPath . '/appinfo/database.xml')) { + return ['errors' => [], 'warnings' => []]; + } + + libxml_use_internal_errors(true); + $loadEntities = libxml_disable_entity_loader(false); + $xml = simplexml_load_file($appPath . '/appinfo/database.xml'); + libxml_disable_entity_loader($loadEntities); + + + $errors = $warnings = []; + + foreach ($xml->table as $table) { + // Table names + if (strpos($table->name, '*dbprefix*') !== 0) { + $errors[] = 'Database schema error: name of table ' . $table->name . ' does not start with *dbprefix*'; + } + $tableName = substr($table->name, strlen('*dbprefix*')); + if (strpos($tableName, '*dbprefix*') !== false) { + $warnings[] = 'Database schema warning: *dbprefix* should only appear once in name of table ' . $table->name; + } + + if (strlen($tableName) > 27) { + $errors[] = 'Database schema error: Name of table ' . $table->name . ' is too long (' . strlen($tableName) . '), max. 27 characters (21 characters for tables with autoincrement) + *dbprefix* allowed'; + } + + $hasAutoIncrement = false; + + // Column names + foreach ($table->declaration->field as $column) { + if (strpos($column->name, '*dbprefix*') !== false) { + $warnings[] = 'Database schema warning: *dbprefix* should not appear in name of column ' . $column->name . ' on table ' . $table->name; + } + + if (strlen($column->name) > 30) { + $errors[] = 'Database schema error: Name of column ' . $column->name . ' on table ' . $table->name . ' is too long (' . strlen($tableName) . '), max. 30 characters allowed'; + } + + if ($column->autoincrement) { + if ($hasAutoIncrement) { + $errors[] = 'Database schema error: Table ' . $table->name . ' has multiple autoincrement columns'; + } + + if (strlen($tableName) > 21) { + $errors[] = 'Database schema error: Name of table ' . $table->name . ' is too long (' . strlen($tableName) . '), max. 27 characters (21 characters for tables with autoincrement) + *dbprefix* allowed'; + } + + $hasAutoIncrement = true; + } + } + + // Index names + foreach ($table->declaration->index as $index) { + if (strpos($index->name, '*dbprefix*') !== 0) { + $warnings[] = 'Database schema warning: name of index ' . $index->name . ' on table ' . $table->name . ' does not start with *dbprefix*'; + } + $indexName = substr($index->name, strlen('*dbprefix*')); + if (strpos($indexName, '*dbprefix*') !== false) { + $warnings[] = 'Database schema warning: *dbprefix* should only appear once in name of index ' . $index->name . ' on table ' . $table->name; + } + + if (strlen($indexName) > 27) { + $errors[] = 'Database schema error: Name of index ' . $index->name . ' on table ' . $table->name . ' is too long (' . strlen($tableName) . '), max. 27 characters + *dbprefix* allowed'; + } + } + } + + return ['errors' => $errors, 'warnings' => $warnings]; + } +} -- 2.39.5