From e0b1f4f2519c6a5e02f98423f34648f21aa5c879 Mon Sep 17 00:00:00 2001 From: Philipp Holzer Date: Sat, 30 Mar 2019 18:54:22 +0100 Subject: [PATCH] Hardening ConfigUpgrade --- bin/worker.php | 2 +- src/App.php | 2 +- src/Core/Config/Cache/ConfigCache.php | 28 ++++++ src/Core/Update.php | 136 ++++++++++++++++++-------- update.php | 15 --- 5 files changed, 126 insertions(+), 57 deletions(-) diff --git a/bin/worker.php b/bin/worker.php index c7174d81e1..2b5915f471 100755 --- a/bin/worker.php +++ b/bin/worker.php @@ -33,7 +33,7 @@ require dirname(__DIR__) . '/vendor/autoload.php'; $a = Factory\DependencyFactory::setUp('worker', dirname(__DIR__)); // Check the database structure and possibly fixes it -Update::check($a->getBasePath(), true); +Update::check($a->getBasePath(), true, $a->getMode()); // Quit when in maintenance if (!$a->getMode()->has(App\Mode::MAINTENANCEDISABLED)) { diff --git a/src/App.php b/src/App.php index c0afd514d0..ff8cc2c123 100644 --- a/src/App.php +++ b/src/App.php @@ -1187,7 +1187,7 @@ class App $this->module = 'maintenance'; } else { $this->checkURL(); - Core\Update::check($this->getBasePath(), false); + Core\Update::check($this->getBasePath(), false, $this->getMode()); Core\Addon::loadAddons(); Core\Hook::loadHooks(); } diff --git a/src/Core/Config/Cache/ConfigCache.php b/src/Core/Config/Cache/ConfigCache.php index f61865cee6..85525b4767 100644 --- a/src/Core/Config/Cache/ConfigCache.php +++ b/src/Core/Config/Cache/ConfigCache.php @@ -188,4 +188,32 @@ class ConfigCache implements IConfigCache, IPConfigCache { return $this->config; } + + /** + * Returns an array with missing categories/Keys + * + * @param array $config The array to check + * + * @return array + */ + public function keyDiff(array $config) + { + $return = []; + + $categories = array_keys($config); + + foreach ($categories as $category) { + if (is_array($config[$category])) { + $keys = array_keys($config[$category]); + + foreach ($keys as $key) { + if (!isset($this->config[$category][$key])) { + $return[$category][$key] = $config[$category][$key]; + } + } + } + } + + return $return; + } } diff --git a/src/Core/Update.php b/src/Core/Update.php index 0d7b348b42..95be4022ee 100644 --- a/src/Core/Update.php +++ b/src/Core/Update.php @@ -19,16 +19,21 @@ class Update /** * @brief Function to check if the Database structure needs an update. * - * @param string $basePath The base path of this application - * @param boolean $via_worker boolean Is the check run via the worker? + * @param string $basePath The base path of this application + * @param boolean $via_worker Is the check run via the worker? + * @param App\Mode $mode The current app mode + * * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function check($basePath, $via_worker) + public static function check($basePath, $via_worker, App\Mode $mode) { if (!DBA::connected()) { return; } + // Check if the config files are set correctly + self::checkConfigFile($basePath, $mode); + // Don't check the status if the last update was failed if (Config::get('system', 'update', Update::SUCCESS, true) == Update::FAILED) { return; @@ -227,43 +232,80 @@ class Update /** * Checks the config settings and saves given config values into the config file * - * @param string $basePath The basepath of Friendica - * @param App\Mode $mode The Application mode + * @param string $basePath The basepath of Friendica + * @param App\Mode $$mode The current App mode * * @return bool True, if something has been saved */ - public static function saveConfigToFile($basePath, App\Mode $mode) + public static function checkConfigFile($basePath, App\Mode $mode) { + if (empty($basePath)) { + $basePath = BasePath::create(dirname(__DIR__, 2)); + } + + $config = [ + 'config' => [ + 'hostname' => [ + 'allowEmpty' => false, + 'default' => '', + ], + ], + 'system' => [ + 'basepath' => [ + 'allowEmpty' => false, + 'default' => $basePath, + ], + ] + ]; + $configFileLoader = new ConfigFileLoader($basePath, $mode); $configCache = new Config\Cache\ConfigCache(); $configFileLoader->setupCache($configCache, true); - $configFileSaver = new ConfigFileSaver($basePath); - $updated = false; + // checks if something is to update, otherwise skip this function at all + $missingConfig = $configCache->keyDiff($config); + if (empty($missingConfig)) { + return true; + } - if (self::updateConfigEntry($configCache, $configFileSaver,'config', 'hostname')) { - $updated = true; - }; + // We just want one update process + if (Lock::acquire('config_update')) { + $configFileSaver = new ConfigFileSaver($basePath); - if (self::updateConfigEntry($configCache, $configFileSaver,'system', 'basepath', BasePath::create(dirname(__DIR__) . '/../'))) { - $updated = true; - } + $updated = false; + $toDelete = []; - // In case there is nothing to do, skip the update - if (!$updated) { - return true; - } + foreach ($missingConfig as $category => $keys) { + foreach ($keys as $key => $value) { + if (self::updateConfigEntry($configCache, $configFileSaver, $category, $key, $value['allowEmpty'], $value['default'])) { + $toDelete[] = ['cat' => $category, 'key' => $key]; + $updated = true; + }; + } + } - if (!$configFileSaver->saveToConfigFile()) { - Logger::alert('Config entry update failed - maybe wrong permission?'); - return false; - } + // In case there is nothing to do, skip the update + if (!$updated) { + Lock::release('config_update'); + return true; + } - DBA::delete('config', ['cat' => 'config', 'k' => 'hostname']); - DBA::delete('config', ['cat' => 'system', 'k' => 'basepath']); + if (!$configFileSaver->saveToConfigFile()) { + Logger::alert('Config entry update failed - maybe wrong permission?'); + Lock::release('config_update'); + return false; + } - return true; - } + // After the successful save, remove the db values + foreach ($toDelete as $delete) { + DBA::delete('config', ['cat' => $delete['cat'], 'k' => $delete['key']]); + } + + Lock::release('config_update'); + } + + return true; + } /** * Adds a value to the ConfigFileSave in case it isn't already updated @@ -272,33 +314,47 @@ class Update * @param ConfigFileSaver $configFileSaver The config file saver * @param string $cat The config category * @param string $key The config key + * @param bool $allowEmpty If true, empty values are valid (Default there has to be a variable) * @param string $default A default value, if none of the settings are valid * * @return boolean True, if a value was updated * * @throws \Exception if DBA or Logger doesn't work */ - private static function updateConfigEntry(IConfigCache $configCache, ConfigFileSaver $configFileSaver, $cat, $key, $default = '') + private static function updateConfigEntry( + IConfigCache $configCache, + ConfigFileSaver $configFileSaver, + $cat, + $key, + $allowEmpty = false, + $default = '') { - // check if the config file differs from the whole configuration (= The db contains other values) - $fileConfig = $configCache->get($cat, $key); - $savedConfig = DBA::selectFirst('config', ['v'], ['cat' => $cat, 'k' => $key]); + // check if the config file differs from the whole configuration (= The db contains other values) + $fileValue = $configCache->get($cat, $key); + $dbConfig = DBA::selectFirst('config', ['v'], ['cat' => $cat, 'k' => $key]); - if (DBA::isResult($savedConfig)) { - $savedValue = $savedConfig['v']; - } else { - $savedValue = null; - } + if (DBA::isResult($dbConfig)) { + $dbValue = $dbConfig['v']; + } else { + $dbValue = null; + } // If the db contains a config value, check it - if (isset($savedValue) && $fileConfig !== $savedValue) { - Logger::info('Difference in config found', ['cat' => $cat, 'key' => $key, 'file' => $fileConfig, 'saved' => $savedValue]); - $configFileSaver->addConfigValue($cat, $key, $savedValue); + if (( + ($allowEmpty && isset($dbValue)) || + (!$allowEmpty && !empty($dbValue)) + ) && + $fileValue !== $dbValue) { + Logger::info('Difference in config found', ['cat' => $cat, 'key' => $key, 'file' => $fileValue, 'db' => $dbValue]); + $configFileSaver->addConfigValue($cat, $key, $dbValue); return true; // If both config values are not set, use the default value - } elseif (!isset($fileConfig) && !isset($savedValue)) { + } elseif ( + ($allowEmpty && !isset($fileValue) && !isset($dbValue)) || + (!$allowEmpty && empty($fileValue) && empty($dbValue) && !empty($default))) { + Logger::info('Using default for config', ['cat' => $cat, 'key' => $key, 'value' => $default]); $configFileSaver->addConfigValue($cat, $key, $default); return true; @@ -306,7 +362,7 @@ class Update // If either the file config value isn't empty or the db value is the same as the // file config value, skip it } else { - Logger::info('No Difference in config found', ['cat' => $cat, 'key' => $key, 'value' => $fileConfig, 'saved' => $savedValue]); + Logger::debug('No Difference in config found', ['cat' => $cat, 'key' => $key, 'value' => $fileValue, 'db' => $dbValue]); return false; } } diff --git a/update.php b/update.php index 0ef74ea005..e619ec89dd 100644 --- a/update.php +++ b/update.php @@ -1,6 +1,5 @@ getBasePath(), $app->getMode())) { - return Update::SUCCESS; - } else { - return Update::FAILED; - } -}