From aa119f7e623d101a6dd1af5b0d1a7ed5ac576cb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Sat, 29 Apr 2023 18:18:04 -0300 Subject: [PATCH 1/2] Remove unused Config::$defaultServer property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- libraries/classes/Config.php | 4 ---- test/classes/ConfigTest.php | 1 - test/classes/Controllers/Table/AddFieldControllerTest.php | 2 +- test/classes/Controllers/Table/CreateControllerTest.php | 2 +- test/classes/Controllers/Table/DeleteRowsControllerTest.php | 2 +- test/classes/Controllers/Table/ExportControllerTest.php | 2 +- test/classes/Controllers/Table/ImportControllerTest.php | 2 +- test/classes/Controllers/Table/SqlControllerTest.php | 2 +- test/classes/Controllers/Table/StructureControllerTest.php | 2 +- 9 files changed, 7 insertions(+), 12 deletions(-) diff --git a/libraries/classes/Config.php b/libraries/classes/Config.php index b50633d565..a6a40a60e7 100644 --- a/libraries/classes/Config.php +++ b/libraries/classes/Config.php @@ -86,9 +86,6 @@ class Config public bool $errorConfigFile = false; - /** @var mixed[] */ - public array $defaultServer; - private bool $isHttps = false; private Settings $config; @@ -96,7 +93,6 @@ class Config public function __construct() { $this->config = new Settings([]); - $this->defaultServer = $this->config->Servers[1]->asArray(); $config = $this->config->asArray(); $this->default = $config; $this->settings = $config; diff --git a/test/classes/ConfigTest.php b/test/classes/ConfigTest.php index e46a231c80..40dda75fff 100644 --- a/test/classes/ConfigTest.php +++ b/test/classes/ConfigTest.php @@ -377,7 +377,6 @@ PHP; $config = $settings->asArray(); $this->assertIsArray($config['Servers']); $this->assertEquals($settings, $object->getSettings()); - $this->assertSame($config['Servers'][1], $object->defaultServer); $this->assertEquals($config, $object->default); $this->assertSame($config, $object->settings); $this->assertSame($config, $object->baseSettings); diff --git a/test/classes/Controllers/Table/AddFieldControllerTest.php b/test/classes/Controllers/Table/AddFieldControllerTest.php index 1b06199522..bc2b3f3253 100644 --- a/test/classes/Controllers/Table/AddFieldControllerTest.php +++ b/test/classes/Controllers/Table/AddFieldControllerTest.php @@ -21,7 +21,7 @@ class AddFieldControllerTest extends AbstractTestCase $GLOBALS['db'] = 'test_db'; $GLOBALS['table'] = 'test_table'; $GLOBALS['regenerate'] = null; - $GLOBALS['cfg']['Server'] = $GLOBALS['config']->defaultServer; + $GLOBALS['cfg']['Server'] = $GLOBALS['config']->getSettings()->Servers[1]->asArray(); $_POST = [ 'db' => 'test_db', 'table' => 'test_table', diff --git a/test/classes/Controllers/Table/CreateControllerTest.php b/test/classes/Controllers/Table/CreateControllerTest.php index 5b385c9641..7ee468807c 100644 --- a/test/classes/Controllers/Table/CreateControllerTest.php +++ b/test/classes/Controllers/Table/CreateControllerTest.php @@ -22,7 +22,7 @@ class CreateControllerTest extends AbstractTestCase { $GLOBALS['db'] = 'test_db'; $GLOBALS['table'] = 'new_test_table'; - $GLOBALS['cfg']['Server'] = $GLOBALS['config']->defaultServer; + $GLOBALS['cfg']['Server'] = $GLOBALS['config']->getSettings()->Servers[1]->asArray(); $_POST = ['db' => 'test_db', 'table' => 'new_test_table', 'num_fields' => '2']; $dummyDbi = $this->createDbiDummy(); diff --git a/test/classes/Controllers/Table/DeleteRowsControllerTest.php b/test/classes/Controllers/Table/DeleteRowsControllerTest.php index aa6819b4f9..91b15284f8 100644 --- a/test/classes/Controllers/Table/DeleteRowsControllerTest.php +++ b/test/classes/Controllers/Table/DeleteRowsControllerTest.php @@ -21,7 +21,7 @@ class DeleteRowsControllerTest extends AbstractTestCase $GLOBALS['db'] = 'test_db'; $GLOBALS['table'] = 'test_table'; $GLOBALS['urlParams'] = []; - $GLOBALS['cfg']['Server'] = $GLOBALS['config']->defaultServer; + $GLOBALS['cfg']['Server'] = $GLOBALS['config']->getSettings()->Servers[1]->asArray(); $GLOBALS['cfg']['Server']['DisableIS'] = true; $_POST = [ 'db' => 'test_db', diff --git a/test/classes/Controllers/Table/ExportControllerTest.php b/test/classes/Controllers/Table/ExportControllerTest.php index 2c1a670dd4..5fcf4e6371 100644 --- a/test/classes/Controllers/Table/ExportControllerTest.php +++ b/test/classes/Controllers/Table/ExportControllerTest.php @@ -33,7 +33,7 @@ class ExportControllerTest extends AbstractTestCase $GLOBALS['db'] = 'test_db'; $GLOBALS['table'] = 'test_table'; - $GLOBALS['cfg']['Server'] = $GLOBALS['config']->defaultServer; + $GLOBALS['cfg']['Server'] = $GLOBALS['config']->getSettings()->Servers[1]->asArray(); $GLOBALS['cfg']['Server']['DisableIS'] = true; $GLOBALS['single_table'] = '1'; diff --git a/test/classes/Controllers/Table/ImportControllerTest.php b/test/classes/Controllers/Table/ImportControllerTest.php index 97e2708672..34542efc3a 100644 --- a/test/classes/Controllers/Table/ImportControllerTest.php +++ b/test/classes/Controllers/Table/ImportControllerTest.php @@ -25,7 +25,7 @@ class ImportControllerTest extends AbstractTestCase $GLOBALS['table'] = 'test_table'; $GLOBALS['text_dir'] = 'ltr'; $GLOBALS['lang'] = 'en'; - $GLOBALS['cfg']['Server'] = $GLOBALS['config']->defaultServer; + $GLOBALS['cfg']['Server'] = $GLOBALS['config']->getSettings()->Servers[1]->asArray(); $_GET['format'] = 'xml'; $dummyDbi = $this->createDbiDummy(); diff --git a/test/classes/Controllers/Table/SqlControllerTest.php b/test/classes/Controllers/Table/SqlControllerTest.php index 52c6eea46a..86dd995e44 100644 --- a/test/classes/Controllers/Table/SqlControllerTest.php +++ b/test/classes/Controllers/Table/SqlControllerTest.php @@ -41,7 +41,7 @@ class SqlControllerTest extends AbstractTestCase $GLOBALS['table'] = 'test_table'; $GLOBALS['lang'] = 'en'; $GLOBALS['text_dir'] = 'ltr'; - $GLOBALS['cfg']['Server'] = $GLOBALS['config']->defaultServer; + $GLOBALS['cfg']['Server'] = $GLOBALS['config']->getSettings()->Servers[1]->asArray(); $this->dummyDbi->addSelectDb('test_db'); $this->dummyDbi->addResult('SHOW TABLES LIKE \'test_table\';', [['test_table']]); diff --git a/test/classes/Controllers/Table/StructureControllerTest.php b/test/classes/Controllers/Table/StructureControllerTest.php index 165f023e65..5f12d1bb96 100644 --- a/test/classes/Controllers/Table/StructureControllerTest.php +++ b/test/classes/Controllers/Table/StructureControllerTest.php @@ -41,7 +41,7 @@ class StructureControllerTest extends AbstractTestCase $GLOBALS['table'] = 'test_table'; $GLOBALS['text_dir'] = 'ltr'; $GLOBALS['lang'] = 'en'; - $GLOBALS['cfg']['Server'] = $GLOBALS['config']->defaultServer; + $GLOBALS['cfg']['Server'] = $GLOBALS['config']->getSettings()->Servers[1]->asArray(); $GLOBALS['cfg']['Server']['DisableIS'] = true; $GLOBALS['cfg']['ShowStats'] = false; $GLOBALS['cfg']['ShowPropertyComments'] = false; From 250a32b7956ace2664133afbd45980ca75ac752f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Sat, 29 Apr 2023 20:43:20 -0300 Subject: [PATCH 2/2] Add the Config::getCurrentServer() method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Config::getCurrentServer() method returns the current server configuration. It's an VO that is equivalent to $GLOBALS['cfg']['Server'] setting. Signed-off-by: Maurício Meneghini Fauth --- libraries/classes/Common.php | 30 +++++--- libraries/classes/Config.php | 73 +++++++++---------- phpstan-baseline.neon | 30 -------- psalm-baseline.xml | 23 ------ test/classes/ConfigTest.php | 27 ++++--- .../Export/ExportControllerTest.php | 2 +- .../Table/OperationsControllerTest.php | 2 +- 7 files changed, 75 insertions(+), 112 deletions(-) diff --git a/libraries/classes/Common.php b/libraries/classes/Common.php index ddee4ea67e..e4051d303f 100644 --- a/libraries/classes/Common.php +++ b/libraries/classes/Common.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace PhpMyAdmin; use PhpMyAdmin\Config\ConfigFile; +use PhpMyAdmin\Config\Settings\Server; use PhpMyAdmin\ConfigStorage\Relation; use PhpMyAdmin\Dbal\Connection; use PhpMyAdmin\Dbal\DatabaseName; @@ -185,7 +186,7 @@ final class Common return; } - self::setCurrentServerGlobal($container, $config); + self::setCurrentServerGlobal($container, $config, $request->getParam('server')); $GLOBALS['cfg'] = $config->settings; $settings = $config->getSettings(); @@ -228,7 +229,8 @@ final class Common $container->set(DatabaseInterface::class, $GLOBALS['dbi']); $container->setAlias('dbi', DatabaseInterface::class); - if (! empty($GLOBALS['cfg']['Server'])) { + $currentServer = $config->getCurrentServer(); + if ($currentServer !== null) { $config->getLoginCookieValidityFromCache($GLOBALS['server']); /** @var AuthenticationPluginFactory $authPluginFactory */ @@ -251,12 +253,12 @@ final class Common // phpcs:enable } - self::connectToDatabaseServer($GLOBALS['dbi'], $authPlugin); + self::connectToDatabaseServer($GLOBALS['dbi'], $authPlugin, $currentServer); $authPlugin->rememberCredentials(); $authPlugin->checkTwoFactor(); /* Log success */ - Logging::logUser($config, $GLOBALS['cfg']['Server']['user']); + Logging::logUser($config, $currentServer->user); if ($GLOBALS['dbi']->getVersion() < $settings->mysqlMinVersion['internal']) { echo self::getGenericError(sprintf( @@ -549,14 +551,17 @@ final class Common throw new RuntimeException(__('possible exploit')); } - private static function connectToDatabaseServer(DatabaseInterface $dbi, AuthenticationPlugin $auth): void - { + private static function connectToDatabaseServer( + DatabaseInterface $dbi, + AuthenticationPlugin $auth, + Server $currentServer, + ): void { /** * Try to connect MySQL with the control user profile (will be used to get the privileges list for the current - * user but the true user link must be open after this one so it would be default one for all the scripts). + * user but the true user link must be open after this one, so it would be default one for all the scripts). */ $controlConnection = null; - if ($GLOBALS['cfg']['Server']['controluser'] !== '') { + if ($currentServer->controluser !== '') { $controlConnection = $dbi->connect(Connection::TYPE_CONTROL); } @@ -631,9 +636,12 @@ final class Common $container->setParameter('sql_query', $sqlQuery); } - private static function setCurrentServerGlobal(ContainerInterface $container, Config $config): void - { - $server = $config->selectServer(); + private static function setCurrentServerGlobal( + ContainerInterface $container, + Config $config, + mixed $serverParamFromRequest, + ): void { + $server = $config->selectServer($serverParamFromRequest); $GLOBALS['server'] = $server; $GLOBALS['urlParams']['server'] = $server; $container->setParameter('server', $server); diff --git a/libraries/classes/Config.php b/libraries/classes/Config.php index a6a40a60e7..f048a29ba5 100644 --- a/libraries/classes/Config.php +++ b/libraries/classes/Config.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace PhpMyAdmin; use PhpMyAdmin\Config\Settings; +use PhpMyAdmin\Config\Settings\Server; use PhpMyAdmin\Dbal\Connection; use PhpMyAdmin\Exceptions\ConfigException; use PhpMyAdmin\Theme\ThemeManager; @@ -88,7 +89,9 @@ class Config private bool $isHttps = false; - private Settings $config; + public Settings $config; + /** @var int<0, max> */ + public int $server = 0; public function __construct() { @@ -1059,34 +1062,27 @@ class Config return null; } - /** - * Selects server based on request parameters. - */ - public function selectServer(): int + /** @return int<0, max> */ + public function selectServer(mixed $serverParamFromRequest): int { - $request = empty($_REQUEST['server']) ? 0 : $_REQUEST['server']; - - /** - * Lookup server by name - * (see FAQ 4.8) - */ - if (! is_numeric($request)) { - foreach ($this->settings['Servers'] as $i => $server) { - $verboseToLower = mb_strtolower($server['verbose']); - $serverToLower = mb_strtolower($request); - if ( - $server['host'] == $request - || $server['verbose'] == $request - || $verboseToLower === $serverToLower - || md5($verboseToLower) === $serverToLower - ) { - $request = $i; + $serverNumber = 0; + if (is_numeric($serverParamFromRequest)) { + $serverNumber = (int) $serverParamFromRequest; + $serverNumber = $serverNumber >= 1 ? $serverNumber : 0; + } elseif (is_string($serverParamFromRequest) && $serverParamFromRequest !== '') { + /** Lookup server by name (see FAQ 4.8) */ + foreach ($this->config->Servers as $i => $server) { + if ($server->host === $serverParamFromRequest || $server->verbose === $serverParamFromRequest) { + $serverNumber = $i; break; } - } - if (is_string($request)) { - $request = 0; + $verboseToLower = mb_strtolower($server->verbose); + $serverToLower = mb_strtolower($serverParamFromRequest); + if ($verboseToLower === $serverToLower || md5($verboseToLower) === $serverToLower) { + $serverNumber = $i; + break; + } } } @@ -1098,21 +1094,19 @@ class Config * present a choice of servers in the case that there are multiple servers * and '$this->settings['ServerDefault'] = 0' is set. */ - - if (is_numeric($request) && ! empty($request) && ! empty($this->settings['Servers'][$request])) { - $server = $request; - $this->settings['Server'] = $this->settings['Servers'][$server]; + if (isset($this->config->Servers[$serverNumber])) { + $this->settings['Server'] = $this->config->Servers[$serverNumber]->asArray(); + } elseif (isset($this->config->Servers[$this->config->ServerDefault])) { + $serverNumber = $this->config->ServerDefault; + $this->settings['Server'] = $this->config->Servers[$this->config->ServerDefault]->asArray(); } else { - if (! empty($this->settings['Servers'][$this->settings['ServerDefault']])) { - $server = $this->settings['ServerDefault']; - $this->settings['Server'] = $this->settings['Servers'][$server]; - } else { - $server = 0; - $this->settings['Server'] = []; - } + $serverNumber = 0; + $this->settings['Server'] = []; } - return (int) $server; + $this->server = $serverNumber; + + return $this->server; } /** @@ -1248,4 +1242,9 @@ class Config { return $this->config; } + + public function getCurrentServer(): Server|null + { + return $this->config->Servers[$this->server] ?? null; + } } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 29498e2935..c8999b6073 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -160,31 +160,6 @@ parameters: count: 1 path: libraries/classes/Common.php - - - message: "#^Argument of an invalid type mixed supplied for foreach, only iterables are supported\\.$#" - count: 1 - path: libraries/classes/Config.php - - - - message: "#^Cannot access offset 'host' on mixed\\.$#" - count: 1 - path: libraries/classes/Config.php - - - - message: "#^Cannot access offset 'verbose' on mixed\\.$#" - count: 2 - path: libraries/classes/Config.php - - - - message: "#^Cannot access offset float\\|int\\\\|int\\<1, max\\>\\|\\(non\\-falsy\\-string&numeric\\-string\\) on mixed\\.$#" - count: 2 - path: libraries/classes/Config.php - - - - message: "#^Cannot access offset mixed on mixed\\.$#" - count: 2 - path: libraries/classes/Config.php - - message: "#^Cannot cast mixed to string\\.$#" count: 2 @@ -200,11 +175,6 @@ parameters: count: 1 path: libraries/classes/Config.php - - - message: "#^Parameter \\#1 \\$string of function mb_strtolower expects string, mixed given\\.$#" - count: 1 - path: libraries/classes/Config.php - - message: "#^Parameter \\#1 \\$string of function rtrim expects string, mixed given\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index c11729a067..deeada0dea 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -212,17 +212,12 @@ - - $sqlDelimiter - - - @@ -260,7 +255,6 @@ $defaultValue $path - settings['ThemeDefault']]]> settings['ThemeDefault']]]> $url @@ -272,11 +266,6 @@ - - - - settings['Servers'][$server]]]> - settings['Servers'][$server]]]> @@ -289,10 +278,6 @@ $tempDir[$name] - - settings['Servers'][$server]]]> - settings['Servers'][$this->settings['ServerDefault']]]]> - $collationConnection @@ -300,15 +285,11 @@ $defaultValue $defaultValue $evalResult - $i $password $password $path $prefsType $prefsType - $request - $server - $server $server[$item] @@ -331,14 +312,10 @@ $defaultValue - $request getCookieName($cookieName)]]]> - - $request - diff --git a/test/classes/ConfigTest.php b/test/classes/ConfigTest.php index 40dda75fff..55d9ac49e2 100644 --- a/test/classes/ConfigTest.php +++ b/test/classes/ConfigTest.php @@ -719,24 +719,29 @@ PHP; /** * Test for selectServer * - * @param mixed[] $settings settings array - * @param string $request request - * @param int $expected expected result + * @param mixed[] $settings settings array + * @param string|mixed[] $request request + * @param int $expected expected result * * @dataProvider selectServerProvider */ - public function testSelectServer(array $settings, string $request, int $expected): void + public function testSelectServer(array $settings, string|array $request, int $expected): void { - $object = new Config(); - $object->settings = (new Settings(['Servers' => $settings]))->asArray(); - $_REQUEST['server'] = $request; - $this->assertEquals($expected, $object->selectServer()); + $config = new Config(); + $config->config = new Settings(['Servers' => $settings, 'ServerDefault' => 1]); + $selectedServer = $config->selectServer($request); + $this->assertSame($expected, $selectedServer); + $this->assertGreaterThanOrEqual(0, $selectedServer); + $expectedServer = $expected >= 1 ? $config->config->Servers[$expected]->asArray() : []; + $this->assertArrayHasKey('Server', $config->settings); + $this->assertSame($config->settings['Server'], $expectedServer); + $this->assertSame($expected, $config->server); } /** * Data provider for selectServer test * - * @return array + * @return array */ public static function selectServerProvider(): array { @@ -748,6 +753,10 @@ PHP; 'md5' => [[66 => ['verbose' => 'Server 66', 'host' => '']], md5('server 66'), 66], 'nonexisting_string' => [[1 => []], 'invalid', 1], 'nonexisting' => [[1 => []], '100', 1], + 'none selected' => [[2 => []], '100', 0], + 'none selected with string' => [[2 => []], 'unknown', 0], + 'negative number' => [[1 => []], '-1', 1], + 'array' => [[1 => []], ['1'], 1], ]; } diff --git a/test/classes/Controllers/Export/ExportControllerTest.php b/test/classes/Controllers/Export/ExportControllerTest.php index ff40e397a1..8d42a3e703 100644 --- a/test/classes/Controllers/Export/ExportControllerTest.php +++ b/test/classes/Controllers/Export/ExportControllerTest.php @@ -49,7 +49,7 @@ class ExportControllerTest extends AbstractTestCase $GLOBALS['lang'] = 'en'; $GLOBALS['sql_indexes'] = null; $GLOBALS['sql_auto_increments'] = null; - $GLOBALS['config']->selectServer(); + $GLOBALS['config']->selectServer('1'); $GLOBALS['cfg'] = $GLOBALS['config']->settings; $this->dummyDbi->addResult( diff --git a/test/classes/Controllers/Table/OperationsControllerTest.php b/test/classes/Controllers/Table/OperationsControllerTest.php index bf9257f438..d1c83bc0fe 100644 --- a/test/classes/Controllers/Table/OperationsControllerTest.php +++ b/test/classes/Controllers/Table/OperationsControllerTest.php @@ -38,7 +38,7 @@ class OperationsControllerTest extends AbstractTestCase $GLOBALS['db'] = 'test_db'; $GLOBALS['table'] = 'test_table'; - $GLOBALS['config']->selectServer(); + $GLOBALS['config']->selectServer('1'); $GLOBALS['cfg'] = $GLOBALS['config']->settings; $GLOBALS['cfg']['MaxDbList'] = 0;