Remove Config::checkServers() method

Removes the Config::checkServers() method as it's duplicated in Config\Settings class. Also uses the Settings class to filter invalid config keys when loading a config file. Signed-off-by: Maurício Meneghini Fauth <mauricio@fauth.dev>
phpmyadmin · Apr 29, 2023 · 5ffe284 · 5ffe284
1 parent 42d01cc
commit 5ffe284
Show file tree

Hide file tree

Showing 8 changed files with 45 additions and 241 deletions.
diff --git a/libraries/classes/Common.php b/libraries/classes/Common.php
@@ -633,7 +633,6 @@ private static function setSQLQueryGlobalFromRequest(ContainerInterface $contain
 
     private static function setCurrentServerGlobal(ContainerInterface $container, Config $config): void
     {
-        $config->checkServers();
         $server = $config->selectServer();
         $GLOBALS['server'] = $server;
         $GLOBALS['urlParams']['server'] = $server;

diff --git a/libraries/classes/Config.php b/libraries/classes/Config.php
@@ -11,9 +11,7 @@
 use Throwable;
 
 use function __;
-use function array_filter;
 use function array_key_last;
-use function array_merge;
 use function array_replace_recursive;
 use function array_slice;
 use function count;
@@ -34,7 +32,6 @@
 use function is_array;
 use function is_bool;
 use function is_dir;
-use function is_int;
 use function is_numeric;
 use function is_readable;
 use function is_string;
@@ -53,15 +50,13 @@
 use function rtrim;
 use function setcookie;
 use function sprintf;
-use function str_contains;
 use function stripos;
 use function strtolower;
 use function substr;
 use function sys_get_temp_dir;
 use function time;
 use function trim;
 
-use const ARRAY_FILTER_USE_KEY;
 use const DIRECTORY_SEPARATOR;
 use const PHP_OS;
 use const PHP_URL_PATH;
@@ -103,7 +98,6 @@ public function __construct()
         $this->config = new Settings([]);
         $this->defaultServer = $this->config->Servers[1]->asArray();
         $config = $this->config->asArray();
-        unset($config['Servers']);
         $this->default = $config;
         $this->settings = $config;
         $this->baseSettings = $config;
@@ -346,6 +340,7 @@ public function load(string|null $source = null): bool
             return false;
         }
 
+        /** @var mixed $cfg */
         $cfg = [];
 
         /**
@@ -379,25 +374,11 @@ public function load(string|null $source = null): bool
             $this->sourceMtime = (int) filemtime($this->getSource());
         }
 
-        /**
-         * Ignore keys with / as we do not use these
-         *
-         * These can be confusing for user configuration layer as it
-         * flatten array using / and thus don't see difference between
-         * $cfg['Export/method'] and $cfg['Export']['method'], while rest
-         * of the code uses the setting only in latter form.
-         *
-         * This could be removed once we consistently handle both values
-         * in the functional code as well.
-         */
-        $cfg = array_filter(
-            $cfg,
-            static fn (string $key): bool => ! str_contains($key, '/'),
-            ARRAY_FILTER_USE_KEY,
-        );
+        if (is_array($cfg)) {
+            $this->config = new Settings($cfg);
+        }
 
-        $this->settings = array_replace_recursive($this->settings, $cfg);
-        $this->config = new Settings($cfg);
+        $this->settings = array_replace_recursive($this->settings, $this->config->asArray());
 
         return true;
     }
@@ -1138,52 +1119,6 @@ public function selectServer(): int
         return (int) $server;
     }
 
-    /**
-     * Checks whether Servers configuration is valid and possibly apply fixups.
-     */
-    public function checkServers(): void
-    {
-        // Do we have some server?
-        if (! isset($this->settings['Servers']) || count($this->settings['Servers']) === 0) {
-            // No server => create one with defaults
-            $this->settings['Servers'] = [1 => $this->defaultServer];
-            $this->config = new Settings($this->settings);
-
-            return;
-        }
-
-        // We have server(s) => apply default configuration
-        $newServers = [];
-
-        foreach ($this->settings['Servers'] as $serverIndex => $server) {
-            // Detect wrong configuration
-            if (! is_int($serverIndex) || $serverIndex < 1 || ! is_array($server)) {
-                continue;
-            }
-
-            $server = array_merge($this->defaultServer, $server);
-
-            // Final solution to bug #582890
-            // If we are using a socket connection
-            // and there is nothing in the verbose server name
-            // or the host field, then generate a name for the server
-            // in the form of "Server 2", localized of course!
-            if (empty($server['host']) && empty($server['verbose'])) {
-                $server['verbose'] = sprintf(__('Server %d'), $serverIndex);
-            }
-
-            $newServers[$serverIndex] = $server;
-        }
-
-        if ($newServers === []) {
-            // Ensures it has at least one valid server config.
-            $newServers = [1 => $this->defaultServer];
-        }
-
-        $this->settings['Servers'] = $newServers;
-        $this->config = new Settings($this->settings);
-    }
-
     /**
      * Return connection parameters for the database server
      *

diff --git a/libraries/classes/Config/Settings.php b/libraries/classes/Config/Settings.php
@@ -13,6 +13,7 @@
 use PhpMyAdmin\Config\Settings\SqlQueryBox;
 use PhpMyAdmin\Config\Settings\Transformations;
 
+use function __;
 use function array_map;
 use function count;
 use function defined;
@@ -21,6 +22,7 @@
 use function is_int;
 use function is_string;
 use function min;
+use function sprintf;
 use function strlen;
 
 use const DIRECTORY_SEPARATOR;
@@ -2929,19 +2931,28 @@ private function setServers(array $settings): array
         }
 
         $servers = [];
-        /**
-         * @var int|string $key
-         * @var mixed $server
-         */
         foreach ($settings['Servers'] as $key => $server) {
             if (! is_int($key) || $key < 1 || ! is_array($server)) {
                 continue;
             }
 
             $servers[$key] = new Server($server);
+            if ($servers[$key]->host !== '' || $servers[$key]->verbose !== '') {
+                continue;
+            }
+
+            /**
+             * Ensures that the database server has a name.
+             *
+             * @link https://github.com/phpmyadmin/phpmyadmin/issues/6878
+             *
+             * @psalm-suppress ImpureFunctionCall
+             */
+            $server['verbose'] = sprintf(__('Server %d'), $key);
+            $servers[$key] = new Server($server);
         }
 
-        if (count($servers) === 0) {
+        if ($servers === []) {
             return [1 => new Server()];
         }
 

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -162,7 +162,7 @@ parameters:
 
 		-
 			message: "#^Argument of an invalid type mixed supplied for foreach, only iterables are supported\\.$#"
-			count: 2
+			count: 1
 			path: libraries/classes/Config.php
 
 		-
@@ -215,11 +215,6 @@ parameters:
 			count: 2
 			path: libraries/classes/Config.php
 
-		-
-			message: "#^Parameter \\#1 \\$value of function count expects array\\|Countable, mixed given\\.$#"
-			count: 1
-			path: libraries/classes/Config.php
-
 		-
 			message: "#^Parameter \\#3 \\$default of method PhpMyAdmin\\\\Config\\:\\:setCookie\\(\\) expects string\\|null, mixed given\\.$#"
 			count: 1
@@ -9185,26 +9180,11 @@ parameters:
 			count: 1
 			path: test/classes/Config/SettingsTest.php
 
-		-
-			message: "#^Cannot access offset 1 on mixed\\.$#"
-			count: 1
-			path: test/classes/ConfigTest.php
-
 		-
 			message: "#^Parameter \\#1 \\$haystack of function mb_strstr expects string, array\\<string\\> given\\.$#"
 			count: 1
 			path: test/classes/ConfigTest.php
 
-		-
-			message: "#^Parameter \\#2 \\$array of method PHPUnit\\\\Framework\\\\Assert\\:\\:assertArrayHasKey\\(\\) expects array\\|ArrayAccess, mixed given\\.$#"
-			count: 3
-			path: test/classes/ConfigTest.php
-
-		-
-			message: "#^Parameter \\#2 \\$array of method PHPUnit\\\\Framework\\\\Assert\\:\\:assertArrayNotHasKey\\(\\) expects array\\|ArrayAccess, mixed given\\.$#"
-			count: 7
-			path: test/classes/ConfigTest.php
-
 		-
 			message: "#^Cannot access offset 'favoriteTables' on mixed\\.$#"
 			count: 1

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -261,7 +261,6 @@
       <code><![CDATA[$gdInfo['GD Version']]]></code>
       <code>$path</code>
       <code><![CDATA[$server['verbose']]]></code>
-      <code><![CDATA[$this->settings['Servers']]]></code>
       <code><![CDATA[$this->settings['ThemeDefault']]]></code>
       <code><![CDATA[$this->settings['ThemeDefault']]]></code>
       <code>$url</code>
@@ -14157,8 +14156,6 @@
       <code>mixed[]</code>
       <code>mixed[]</code>
       <code>mixed[]</code>
-      <code>mixed[]</code>
-      <code>mixed[]</code>
     </MixedInferredReturnType>
   </file>
   <file src="test/classes/Controllers/Database/Structure/FavoriteTableControllerTest.php">

diff --git a/test/classes/AbstractTestCase.php b/test/classes/AbstractTestCase.php
@@ -167,7 +167,6 @@ protected function createConfig(): Config
     protected function setGlobalConfig(): void
     {
         $GLOBALS['config'] = $this->createConfig();
-        $GLOBALS['config']->checkServers();
         $GLOBALS['config']->set('environment', 'development');
         $GLOBALS['cfg'] = $GLOBALS['config']->settings;
     }

diff --git a/test/classes/Config/SettingsTest.php b/test/classes/Config/SettingsTest.php
@@ -1172,6 +1172,11 @@ public static function valuesForServersProvider(): iterable
         yield 'null value' => [null, [1 => $server]];
         yield 'valid value' => [[1 => [], 2 => []], [1 => $server, 2 => $server]];
         yield 'valid value 2' => [[2 => ['host' => 'test']], [2 => new Server(['host' => 'test'])]];
+        yield 'valid value 3' => [
+            [4 => ['host' => '', 'verbose' => '']],
+            [4 => new Server(['host' => '', 'verbose' => 'Server 4'])],
+        ];
+
         yield 'invalid value' => ['invalid', [1 => $server]];
         yield 'invalid value 2' => [[0 => [], 2 => 'invalid', 'invalid' => [], 4 => []], [4 => $server]];
         yield 'invalid value 3' => [[0 => []], [1 => $server]];