Hardening ConfigUpgrade

diff --git a/bin/worker.php b/bin/worker.php
index c7174d81e1813ea0e246e2435c0132a7bdb8249a..2b5915f471d4fc76b9fdfdee1bad2ff42318cea0 100755 (executable)
--- 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 c0afd514d036068983afd1cf6c6b8a6f16b3f008..ff8cc2c12340dc81ede04070cffdef808326f9a3 100644 (file)
--- 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 f61865cee627a6e310ed8474586584c83702361b..85525b4767c3e143d9fb06d04920205a27d644db 100644 (file)
--- 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 0d7b348b42c276f4587fab076c32fa7e34896d8b..95be4022ee9cffa48d8cf358e569327fc43914f0 100644 (file)
--- 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;
-
-               if (self::updateConfigEntry($configCache, $configFileSaver,'config', 'hostname')) {
-                       $updated = true;
-               };
-
-               if (self::updateConfigEntry($configCache, $configFileSaver,'system', 'basepath', BasePath::create(dirname(__DIR__) . '/../'))) {
-                       $updated = true;
-               }
-
-               // In case there is nothing to do, skip the update
-               if (!$updated) {
-                       return true;
-               }
 
-               if (!$configFileSaver->saveToConfigFile()) {
-                       Logger::alert('Config entry update failed - maybe wrong permission?');
-                       return false;
-               }
-
-               DBA::delete('config', ['cat' => 'config', 'k' => 'hostname']);
-               DBA::delete('config', ['cat' => 'system', 'k' => 'basepath']);
-
-               return true;
-       }
+        // checks if something is to update, otherwise skip this function at all
+        $missingConfig = $configCache->keyDiff($config);
+               if (empty($missingConfig)) {
+                   return true;
+        }
+
+               // We just want one update process
+               if (Lock::acquire('config_update')) {
+            $configFileSaver = new ConfigFileSaver($basePath);
+
+            $updated = false;
+            $toDelete = [];
+
+            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;
+                    };
+                }
+            }
+
+            // In case there is nothing to do, skip the update
+            if (!$updated) {
+                Lock::release('config_update');
+                return true;
+            }
+
+            if (!$configFileSaver->saveToConfigFile()) {
+                Logger::alert('Config entry update failed - maybe wrong permission?');
+                Lock::release('config_update');
+                return false;
+            }
+
+            // 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 0ef74ea00510554d33b80c4c80804bab493cff26..e619ec89dd9bbb569c3ecb4db27c863c16fdcab2 100644 (file)
--- a/update.php
+++ b/update.php
@@ -1,6 +1,5 @@
 <?php
 
-use Friendica\BaseObject;
 use Friendica\Core\Addon;
 use Friendica\Core\Config;
 use Friendica\Core\L10n;
@@ -347,17 +346,3 @@ function update_1298()
        }
        return Update::SUCCESS;
 }
-
-/**
- * @see https://github.com/friendica/friendica/pull/6920
- * @return int Success
- */
-function update_1307()
-{
-       $app = BaseObject::getApp();
-       if (Update::saveConfigToFile($app->getBasePath(), $app->getMode())) {
-               return Update::SUCCESS;
-       } else {
-               return Update::FAILED;
-       }
-}