Merge pull request #18372 from MauricioFauth/config-check-servers-removal
Remove `Config::checkServers()` method
This commit is contained in:
Maurício Meneghini Fauth
GitHub
commit
14084fdae5
@ -633,7 +633,6 @@ final class Common
|
||||
|
||||
private static function setCurrentServerGlobal(ContainerInterface $container, Config $config): void
|
||||
{
|
||||
$config->checkServers();
|
||||
$server = $config->selectServer();
|
||||
$GLOBALS['server'] = $server;
|
||||
$GLOBALS['urlParams']['server'] = $server;
|
||||
|
||||
@ -11,9 +11,7 @@ use PhpMyAdmin\Theme\ThemeManager;
|
||||
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 intval;
|
||||
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,7 +50,6 @@ use function realpath;
|
||||
use function rtrim;
|
||||
use function setcookie;
|
||||
use function sprintf;
|
||||
use function str_contains;
|
||||
use function stripos;
|
||||
use function strtolower;
|
||||
use function substr;
|
||||
@ -61,7 +57,6 @@ 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 @@ class Config
|
||||
$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 @@ class Config
|
||||
return false;
|
||||
}
|
||||
|
||||
/** @var mixed $cfg */
|
||||
$cfg = [];
|
||||
|
||||
/**
|
||||
@ -379,25 +374,11 @@ class Config
|
||||
$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 @@ class Config
|
||||
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
|
||||
*
|
||||
|
||||
@ -13,6 +13,7 @@ use PhpMyAdmin\Config\Settings\Server;
|
||||
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_array;
|
||||
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 @@ final class Settings
|
||||
}
|
||||
|
||||
$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()];
|
||||
}
|
||||
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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">
|
||||
|
||||
@ -167,7 +167,6 @@ abstract class AbstractTestCase extends TestCase
|
||||
protected function setGlobalConfig(): void
|
||||
{
|
||||
$GLOBALS['config'] = $this->createConfig();
|
||||
$GLOBALS['config']->checkServers();
|
||||
$GLOBALS['config']->set('environment', 'development');
|
||||
$GLOBALS['cfg'] = $GLOBALS['config']->settings;
|
||||
}
|
||||
|
||||
@ -1172,6 +1172,11 @@ class SettingsTest extends TestCase
|
||||
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]];
|
||||
|
||||
@ -8,7 +8,6 @@ use PhpMyAdmin\Config;
|
||||
use PhpMyAdmin\Config\Settings;
|
||||
use PhpMyAdmin\Dbal\Connection;
|
||||
|
||||
use function array_merge;
|
||||
use function define;
|
||||
use function defined;
|
||||
use function file_exists;
|
||||
@ -17,6 +16,7 @@ use function fileperms;
|
||||
use function function_exists;
|
||||
use function gd_info;
|
||||
use function mb_strstr;
|
||||
use function md5;
|
||||
use function ob_end_clean;
|
||||
use function ob_get_contents;
|
||||
use function ob_start;
|
||||
@ -83,9 +83,6 @@ class ConfigTest extends AbstractTestCase
|
||||
unset($this->permTestObj);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test for load
|
||||
*/
|
||||
public function testLoadConfigs(): void
|
||||
{
|
||||
$defaultConfig = $this->createConfig();
|
||||
@ -102,70 +99,24 @@ class ConfigTest extends AbstractTestCase
|
||||
$config = new Config();
|
||||
$config->loadAndCheck($tmpConfig);
|
||||
$this->assertSame($defaultConfig->settings, $config->settings);
|
||||
$this->assertEquals($defaultConfig->getSettings(), $config->getSettings());
|
||||
|
||||
$contents = '<?php' . "\n"
|
||||
. '$cfg[\'ProtectBinary\'] = true;';
|
||||
$contents = <<<'PHP'
|
||||
<?php
|
||||
$cfg['environment'] = 'development';
|
||||
$cfg['UnknownKey'] = true;
|
||||
PHP;
|
||||
file_put_contents($tmpConfig, $contents);
|
||||
|
||||
// Test loading a config changes the setup
|
||||
$config = new Config();
|
||||
$config->loadAndCheck($tmpConfig);
|
||||
$defaultConfig->settings['ProtectBinary'] = true;
|
||||
$defaultConfig->set('environment', 'development');
|
||||
$this->assertSame($defaultConfig->settings, $config->settings);
|
||||
$defaultConfig->settings['ProtectBinary'] = 'blob';
|
||||
|
||||
// Teardown
|
||||
unlink($tmpConfig);
|
||||
$this->assertFalse(file_exists($tmpConfig));
|
||||
}
|
||||
|
||||
/**
|
||||
* Test for load
|
||||
*/
|
||||
public function testLoadInvalidConfigs(): void
|
||||
{
|
||||
$defaultConfig = $this->createConfig();
|
||||
$tmpConfig = tempnam('./', 'config.test.inc.php');
|
||||
if ($tmpConfig === false) {
|
||||
$this->markTestSkipped('Creating a temporary file does not work');
|
||||
}
|
||||
|
||||
$this->assertFileExists($tmpConfig);
|
||||
|
||||
// end of setup
|
||||
|
||||
// Test loading an empty file does not change the default config
|
||||
$config = new Config();
|
||||
$config->loadAndCheck($tmpConfig);
|
||||
$this->assertSame($defaultConfig->settings, $config->settings);
|
||||
|
||||
$contents = '<?php' . "\n"
|
||||
. '$cfg[\'fooBar\'] = true;';
|
||||
file_put_contents($tmpConfig, $contents);
|
||||
|
||||
// Test loading a custom key config changes the setup
|
||||
$config = new Config();
|
||||
$config->loadAndCheck($tmpConfig);
|
||||
$defaultConfig->settings['fooBar'] = true;
|
||||
// Equals because of the key sorting
|
||||
$this->assertEquals($defaultConfig->settings, $config->settings);
|
||||
unset($defaultConfig->settings['fooBar']);
|
||||
|
||||
$contents = '<?php' . "\n"
|
||||
. '$cfg[\'/InValidKey\'] = true;' . "\n"
|
||||
. '$cfg[\'In/ValidKey\'] = true;' . "\n"
|
||||
. '$cfg[\'/InValid/Key\'] = true;' . "\n"
|
||||
. '$cfg[\'In/Valid/Key\'] = true;' . "\n"
|
||||
. '$cfg[\'ValidKey\'] = true;';
|
||||
file_put_contents($tmpConfig, $contents);
|
||||
|
||||
// Test loading a custom key config changes the setup
|
||||
$config = new Config();
|
||||
$config->loadAndCheck($tmpConfig);
|
||||
$defaultConfig->settings['ValidKey'] = true;
|
||||
// Equals because of the key sorting
|
||||
$this->assertEquals($defaultConfig->settings, $config->settings);
|
||||
unset($defaultConfig->settings['ValidKey']);
|
||||
$this->assertArrayHasKey('environment', $config->settings);
|
||||
$this->assertSame($config->settings['environment'], 'development');
|
||||
$this->assertArrayNotHasKey('UnknownKey', $config->settings);
|
||||
$this->assertEquals($defaultConfig->getSettings(), $config->getSettings());
|
||||
|
||||
// Teardown
|
||||
unlink($tmpConfig);
|
||||
@ -427,8 +378,7 @@ class ConfigTest extends AbstractTestCase
|
||||
$this->assertIsArray($config['Servers']);
|
||||
$this->assertEquals($settings, $object->getSettings());
|
||||
$this->assertSame($config['Servers'][1], $object->defaultServer);
|
||||
unset($config['Servers']);
|
||||
$this->assertSame($config, $object->default);
|
||||
$this->assertEquals($config, $object->default);
|
||||
$this->assertSame($config, $object->settings);
|
||||
$this->assertSame($config, $object->baseSettings);
|
||||
}
|
||||
@ -767,77 +717,6 @@ class ConfigTest extends AbstractTestCase
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test for checkServers
|
||||
*
|
||||
* @param mixed[] $settings settings array
|
||||
* @param mixed[] $expected expected result
|
||||
*
|
||||
* @dataProvider serverSettingsProvider
|
||||
*/
|
||||
public function testCheckServers(array $settings, array $expected): void
|
||||
{
|
||||
$this->object->settings['Servers'] = $settings;
|
||||
$this->object->checkServers();
|
||||
$expected = array_merge($this->object->defaultServer, $expected);
|
||||
|
||||
$this->assertEquals($expected, $this->object->settings['Servers'][1]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Data provider for checkServers test
|
||||
*
|
||||
* @return mixed[]
|
||||
*/
|
||||
public static function serverSettingsProvider(): array
|
||||
{
|
||||
return [
|
||||
'empty' => [[], []],
|
||||
'only_host' => [[1 => ['host' => '127.0.0.1']], ['host' => '127.0.0.1']],
|
||||
'empty_host' => [[1 => ['host' => '']], ['verbose' => 'Server 1', 'host' => '']],
|
||||
];
|
||||
}
|
||||
|
||||
public function testCheckServersWithInvalidServer(): void
|
||||
{
|
||||
$server = ['host' => '127.0.0.1'];
|
||||
$this->object->settings['Servers'] = ['invalid' => $server, 1 => $server, 0 => $server, 2 => 'invalid'];
|
||||
$this->object->checkServers();
|
||||
$expected = array_merge($this->object->defaultServer, $server);
|
||||
|
||||
$this->assertArrayNotHasKey('invalid', $this->object->settings['Servers']);
|
||||
$this->assertArrayNotHasKey(0, $this->object->settings['Servers']);
|
||||
$this->assertArrayNotHasKey(2, $this->object->settings['Servers']);
|
||||
$this->assertArrayHasKey(1, $this->object->settings['Servers']);
|
||||
$this->assertEquals($expected, $this->object->settings['Servers'][1]);
|
||||
}
|
||||
|
||||
public function testCheckServersWithOnlyInvalidServers(): void
|
||||
{
|
||||
$server = ['host' => '127.0.0.1'];
|
||||
$this->object->settings['Servers'] = ['invalid' => $server, -1 => $server, 0 => $server];
|
||||
$this->object->checkServers();
|
||||
|
||||
$this->assertArrayNotHasKey('invalid', $this->object->settings['Servers']);
|
||||
$this->assertArrayNotHasKey(0, $this->object->settings['Servers']);
|
||||
$this->assertArrayNotHasKey(-1, $this->object->settings['Servers']);
|
||||
$this->assertArrayHasKey(1, $this->object->settings['Servers']);
|
||||
/** @psalm-suppress InvalidArrayOffset */
|
||||
$this->assertEquals($this->object->defaultServer, $this->object->settings['Servers'][1]);
|
||||
}
|
||||
|
||||
public function testCheckServersWithServerKeysGreaterThanOne(): void
|
||||
{
|
||||
$server = ['host' => '127.0.0.1'];
|
||||
$this->object->settings['Servers'] = [2 => $server];
|
||||
$this->object->checkServers();
|
||||
$expected = array_merge($this->object->defaultServer, $server);
|
||||
|
||||
$this->assertArrayNotHasKey(1, $this->object->settings['Servers']);
|
||||
$this->assertArrayHasKey(2, $this->object->settings['Servers']);
|
||||
$this->assertEquals($expected, $this->object->settings['Servers'][2]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test for selectServer
|
||||
*
|
||||
@ -846,20 +725,19 @@ class ConfigTest extends AbstractTestCase
|
||||
* @param int $expected expected result
|
||||
*
|
||||
* @dataProvider selectServerProvider
|
||||
* @depends testCheckServers
|
||||
*/
|
||||
public function testSelectServer(array $settings, string $request, int $expected): void
|
||||
{
|
||||
$this->object->settings['Servers'] = $settings;
|
||||
$this->object->checkServers();
|
||||
$object = new Config();
|
||||
$object->settings = (new Settings(['Servers' => $settings]))->asArray();
|
||||
$_REQUEST['server'] = $request;
|
||||
$this->assertEquals($expected, $this->object->selectServer());
|
||||
$this->assertEquals($expected, $object->selectServer());
|
||||
}
|
||||
|
||||
/**
|
||||
* Data provider for selectServer test
|
||||
*
|
||||
* @return mixed[]
|
||||
* @return array<string, array{mixed[], string, int}>
|
||||
*/
|
||||
public static function selectServerProvider(): array
|
||||
{
|
||||
@ -868,7 +746,7 @@ class ConfigTest extends AbstractTestCase
|
||||
'number' => [[1 => []], '1', 1],
|
||||
'host' => [[2 => ['host' => '127.0.0.1']], '127.0.0.1', 2],
|
||||
'verbose' => [[1 => ['verbose' => 'Server 1', 'host' => '']], 'Server 1', 1],
|
||||
'md5' => [[66 => ['verbose' => 'Server 1', 'host' => '']], '753f173bd4ac8a45eae0fe9a4fbe0fc0', 66],
|
||||
'md5' => [[66 => ['verbose' => 'Server 66', 'host' => '']], md5('server 66'), 66],
|
||||
'nonexisting_string' => [[1 => []], 'invalid', 1],
|
||||
'nonexisting' => [[1 => []], '100', 1],
|
||||
];
|
||||
|
||||
Loading…
Reference in New Issue
Block a user
