From 4530585ad3dedd342df3cf0c1562347f384bec94 Mon Sep 17 00:00:00 2001 From: Evgeny Skorlov Date: Wed, 29 Mar 2023 12:02:58 +1100 Subject: [PATCH] Replace superglobals with serverrequest in controllers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Evgeny Skorlov Signed-off-by: Maurício Meneghini Fauth --- .../Controllers/Database/ImportController.php | 4 +- .../Controllers/Server/ImportController.php | 4 +- .../Server/UserGroupsFormController.php | 8 +- .../classes/Controllers/Sql/SqlController.php | 7 +- .../Controllers/Table/ChangeController.php | 7 +- .../Controllers/Table/ExportController.php | 6 +- .../Controllers/Table/ImportController.php | 4 +- .../Controllers/Table/SqlController.php | 7 +- .../Controllers/View/CreateController.php | 12 +-- phpstan-baseline.neon | 60 +++++++++++++ psalm-baseline.xml | 84 ++++--------------- .../Controllers/Table/SqlControllerTest.php | 6 +- 12 files changed, 114 insertions(+), 95 deletions(-) diff --git a/libraries/classes/Controllers/Database/ImportController.php b/libraries/classes/Controllers/Database/ImportController.php index d21e9ec502..20e37ba1f6 100644 --- a/libraries/classes/Controllers/Database/ImportController.php +++ b/libraries/classes/Controllers/Database/ImportController.php @@ -77,7 +77,9 @@ final class ImportController extends AbstractController $idKey = $_SESSION[$GLOBALS['SESSION_KEY']]['handler']::getIdKey(); $hiddenInputs = [$idKey => $uploadId, 'import_type' => 'database', 'db' => $GLOBALS['db']]; - $default = isset($_GET['format']) ? (string) $_GET['format'] : Plugins::getDefault('Import', 'format'); + $default = $request->hasQueryParam('format') + ? (string) $request->getQueryParam('format') + : Plugins::getDefault('Import', 'format'); $choice = Plugins::getChoice($importList, $default); $options = Plugins::getOptions('Import', $importList); $skipQueriesDefault = Plugins::getDefault('Import', 'skip_queries'); diff --git a/libraries/classes/Controllers/Server/ImportController.php b/libraries/classes/Controllers/Server/ImportController.php index f669b3196c..04c2aa93ca 100644 --- a/libraries/classes/Controllers/Server/ImportController.php +++ b/libraries/classes/Controllers/Server/ImportController.php @@ -73,7 +73,9 @@ final class ImportController extends AbstractController $idKey = $_SESSION[$GLOBALS['SESSION_KEY']]['handler']::getIdKey(); $hiddenInputs = [$idKey => $uploadId, 'import_type' => 'server']; - $default = isset($_GET['format']) ? (string) $_GET['format'] : Plugins::getDefault('Import', 'format'); + $default = $request->hasQueryParam('format') + ? (string) $request->getQueryParam('format') + : Plugins::getDefault('Import', 'format'); $choice = Plugins::getChoice($importList, $default); $options = Plugins::getOptions('Import', $importList); $skipQueriesDefault = Plugins::getDefault('Import', 'skip_queries'); diff --git a/libraries/classes/Controllers/Server/UserGroupsFormController.php b/libraries/classes/Controllers/Server/UserGroupsFormController.php index 2d66a42f38..4d98579799 100644 --- a/libraries/classes/Controllers/Server/UserGroupsFormController.php +++ b/libraries/classes/Controllers/Server/UserGroupsFormController.php @@ -17,7 +17,6 @@ use PhpMyAdmin\Util; use function __; use function sprintf; -use function strlen; final class UserGroupsFormController extends AbstractController { @@ -34,7 +33,10 @@ final class UserGroupsFormController extends AbstractController { $this->response->setAjax(true); - if (! isset($_GET['username']) || strlen((string) $_GET['username']) === 0) { + /** @var string $username */ + $username = $request->getQueryParam('username', ''); + + if ($username === '') { $this->response->setRequestStatus(false); $this->response->setHttpResponseCode(400); $this->response->addJSON('message', __('Missing parameter:') . ' username'); @@ -42,8 +44,6 @@ final class UserGroupsFormController extends AbstractController return; } - $username = $_GET['username']; - $checkUserPrivileges = new CheckUserPrivileges($this->dbi); $checkUserPrivileges->getPrivileges(); diff --git a/libraries/classes/Controllers/Sql/SqlController.php b/libraries/classes/Controllers/Sql/SqlController.php index 65c6238f70..6fb6d9d600 100644 --- a/libraries/classes/Controllers/Sql/SqlController.php +++ b/libraries/classes/Controllers/Sql/SqlController.php @@ -112,9 +112,10 @@ class SqlController extends AbstractController $GLOBALS['sql_query'] = $bkmFields['bkm_sql_query']; } elseif ($sqlQuery !== null) { $GLOBALS['sql_query'] = $sqlQuery; - } elseif (isset($_GET['sql_query'], $_GET['sql_signature'])) { - if (Core::checkSqlQuerySignature($_GET['sql_query'], $_GET['sql_signature'])) { - $GLOBALS['sql_query'] = $_GET['sql_query']; + } elseif ($request->hasQueryParam('sql_query') && $request->hasQueryParam('sql_signature')) { + $sqlQuery = $request->getQueryParam('sql_query'); + if (Core::checkSqlQuerySignature($sqlQuery, $request->getQueryParam('sql_signature'))) { + $GLOBALS['sql_query'] = $sqlQuery; } } diff --git a/libraries/classes/Controllers/Table/ChangeController.php b/libraries/classes/Controllers/Table/ChangeController.php index aa722b1991..14cb266685 100644 --- a/libraries/classes/Controllers/Table/ChangeController.php +++ b/libraries/classes/Controllers/Table/ChangeController.php @@ -75,9 +75,10 @@ class ChangeController extends AbstractController DbTableExists::check($GLOBALS['db'], $GLOBALS['table']); - if (isset($_GET['where_clause'], $_GET['where_clause_signature'])) { - if (Core::checkSqlQuerySignature($_GET['where_clause'], $_GET['where_clause_signature'])) { - $GLOBALS['where_clause'] = $_GET['where_clause']; + if ($request->hasQueryParam('where_clause') && $request->hasQueryParam('where_clause_signature')) { + $whereClause = $request->getQueryParam('where_clause'); + if (Core::checkSqlQuerySignature($whereClause, $request->getQueryParam('where_clause_signature'))) { + $GLOBALS['where_clause'] = $whereClause; } } diff --git a/libraries/classes/Controllers/Table/ExportController.php b/libraries/classes/Controllers/Table/ExportController.php index 4ef854f2d1..4024f1ce6e 100644 --- a/libraries/classes/Controllers/Table/ExportController.php +++ b/libraries/classes/Controllers/Table/ExportController.php @@ -92,7 +92,7 @@ class ExportController extends AbstractController $GLOBALS['unlim_num_rows'] = 0; } - $GLOBALS['single_table'] = $_POST['single_table'] ?? $_GET['single_table'] ?? $GLOBALS['single_table'] ?? null; + $GLOBALS['single_table'] = $request->getParam('single_table') ?? $GLOBALS['single_table'] ?? null; $exportList = Plugins::getExport('table', isset($GLOBALS['single_table'])); @@ -105,8 +105,8 @@ class ExportController extends AbstractController } $exportType = 'table'; - $isReturnBackFromRawExport = isset($_POST['export_type']) && $_POST['export_type'] === 'raw'; - if (isset($_POST['raw_query']) || $isReturnBackFromRawExport) { + $isReturnBackFromRawExport = $request->getParsedBodyParam('export_type') === 'raw'; + if ($request->hasBodyParam('raw_query') || $isReturnBackFromRawExport) { $exportType = 'raw'; } diff --git a/libraries/classes/Controllers/Table/ImportController.php b/libraries/classes/Controllers/Table/ImportController.php index 39a573c1f7..9d8e04d73e 100644 --- a/libraries/classes/Controllers/Table/ImportController.php +++ b/libraries/classes/Controllers/Table/ImportController.php @@ -89,7 +89,9 @@ final class ImportController extends AbstractController 'table' => $GLOBALS['table'], ]; - $default = isset($_GET['format']) ? (string) $_GET['format'] : Plugins::getDefault('Import', 'format'); + $default = $request->hasQueryParam('format') + ? (string) $request->getQueryParam('format') + : Plugins::getDefault('Import', 'format'); $choice = Plugins::getChoice($importList, $default); $options = Plugins::getOptions('Import', $importList); $skipQueriesDefault = Plugins::getDefault('Import', 'skip_queries'); diff --git a/libraries/classes/Controllers/Table/SqlController.php b/libraries/classes/Controllers/Table/SqlController.php index 6140f3aa85..ecc15ed180 100644 --- a/libraries/classes/Controllers/Table/SqlController.php +++ b/libraries/classes/Controllers/Table/SqlController.php @@ -55,15 +55,14 @@ final class SqlController extends AbstractController */ $GLOBALS['goto'] = Url::getFromRoute('/table/sql'); $GLOBALS['back'] = Url::getFromRoute('/table/sql'); + $delimiter = $request->getParsedBodyParam('delimiter', ';'); $this->response->addHTML($this->sqlQueryForm->getHtml( $GLOBALS['db'], $GLOBALS['table'], - $_GET['sql_query'] ?? true, + $request->getQueryParam('sql_query', true), false, - isset($_POST['delimiter']) - ? htmlspecialchars($_POST['delimiter']) - : ';', + htmlspecialchars($delimiter), )); } } diff --git a/libraries/classes/Controllers/View/CreateController.php b/libraries/classes/Controllers/View/CreateController.php index ac2f004de8..ba0af4381b 100644 --- a/libraries/classes/Controllers/View/CreateController.php +++ b/libraries/classes/Controllers/View/CreateController.php @@ -121,18 +121,20 @@ class CreateController extends AbstractController ]; // Used to prefill the fields when editing a view - if (isset($_GET['db'], $_GET['table'])) { + if ($request->hasQueryParam('db') && $request->hasQueryParam('table')) { + $db = $request->getQueryParam('db'); + $table = $request->getQueryParam('table'); $item = $this->dbi->fetchSingleRow( sprintf( 'SELECT `VIEW_DEFINITION`, `CHECK_OPTION`, `DEFINER`, `SECURITY_TYPE` FROM `INFORMATION_SCHEMA`.`VIEWS` WHERE TABLE_SCHEMA=%s AND TABLE_NAME=%s;', - $this->dbi->quoteString($_GET['db']), - $this->dbi->quoteString($_GET['table']), + $this->dbi->quoteString($db), + $this->dbi->quoteString($table), ), ); - $createView = $this->dbi->getTable($_GET['db'], $_GET['table']) + $createView = $this->dbi->getTable($db, $table) ->showCreate(); // CREATE ALGORITHM= DE... @@ -141,7 +143,7 @@ class CreateController extends AbstractController $viewData['operation'] = 'alter'; $viewData['definer'] = $item['DEFINER']; $viewData['sql_security'] = $item['SECURITY_TYPE']; - $viewData['name'] = $_GET['table']; + $viewData['name'] = $table; $viewData['as'] = $item['VIEW_DEFINITION']; $viewData['with'] = $item['CHECK_OPTION']; $viewData['algorithm'] = $item['ALGORITHM']; diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index c972d22bef..c804a41160 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1035,6 +1035,11 @@ parameters: count: 1 path: libraries/classes/Controllers/Database/DesignerController.php + - + message: "#^Cannot cast mixed to string\\.$#" + count: 1 + path: libraries/classes/Controllers/Database/ImportController.php + - message: "#^Parameter \\#1 \\$sqlQuery of static method PhpMyAdmin\\\\Database\\\\MultiTableQuery\\:\\:displayResults\\(\\) expects string, mixed given\\.$#" count: 1 @@ -1810,6 +1815,11 @@ parameters: count: 1 path: libraries/classes/Controllers/Server/DatabasesController.php + - + message: "#^Cannot cast mixed to string\\.$#" + count: 1 + path: libraries/classes/Controllers/Server/ImportController.php + - message: "#^Method PhpMyAdmin\\\\Controllers\\\\Server\\\\PrivilegesController\\:\\:getExportPageTitle\\(\\) has parameter \\$selectedUsers with no value type specified in iterable type array\\.$#" count: 1 @@ -2075,11 +2085,31 @@ parameters: count: 1 path: libraries/classes/Controllers/Sql/SetValuesController.php + - + message: "#^Parameter \\#1 \\$sqlQuery of static method PhpMyAdmin\\\\Core\\:\\:checkSqlQuerySignature\\(\\) expects string, mixed given\\.$#" + count: 1 + path: libraries/classes/Controllers/Sql/SqlController.php + + - + message: "#^Parameter \\#2 \\$signature of static method PhpMyAdmin\\\\Core\\:\\:checkSqlQuerySignature\\(\\) expects string, mixed given\\.$#" + count: 1 + path: libraries/classes/Controllers/Sql/SqlController.php + - message: "#^Parameter \\#1 \\$target of static method PhpMyAdmin\\\\Util\\:\\:getScriptNameForOption\\(\\) expects string, mixed given\\.$#" count: 1 path: libraries/classes/Controllers/Table/AddFieldController.php + - + message: "#^Parameter \\#1 \\$sqlQuery of static method PhpMyAdmin\\\\Core\\:\\:checkSqlQuerySignature\\(\\) expects string, mixed given\\.$#" + count: 1 + path: libraries/classes/Controllers/Table/ChangeController.php + + - + message: "#^Parameter \\#2 \\$signature of static method PhpMyAdmin\\\\Core\\:\\:checkSqlQuerySignature\\(\\) expects string, mixed given\\.$#" + count: 1 + path: libraries/classes/Controllers/Table/ChangeController.php + - message: "#^Parameter \\#2 \\$offset of class PhpMyAdmin\\\\SqlParser\\\\Components\\\\Limit constructor expects int, \\(float\\|int\\) given\\.$#" count: 1 @@ -2130,6 +2160,11 @@ parameters: count: 1 path: libraries/classes/Controllers/Table/GetFieldController.php + - + message: "#^Cannot cast mixed to string\\.$#" + count: 1 + path: libraries/classes/Controllers/Table/ImportController.php + - message: "#^Parameter \\#1 \\$value of function count expects array\\|Countable, mixed given\\.$#" count: 1 @@ -2275,6 +2310,16 @@ parameters: count: 1 path: libraries/classes/Controllers/Table/SearchController.php + - + message: "#^Parameter \\#1 \\$string of function htmlspecialchars expects string, mixed given\\.$#" + count: 1 + path: libraries/classes/Controllers/Table/SqlController.php + + - + message: "#^Parameter \\#3 \\$query of method PhpMyAdmin\\\\SqlQueryForm\\:\\:getHtml\\(\\) expects bool\\|string, mixed given\\.$#" + count: 1 + path: libraries/classes/Controllers/Table/SqlController.php + - message: "#^Parameter \\#1 \\$selected of method PhpMyAdmin\\\\Controllers\\\\Table\\\\Structure\\\\ChangeController\\:\\:displayHtmlForColumnChange\\(\\) expects array\\, array\\ given\\.$#" count: 1 @@ -2610,11 +2655,26 @@ parameters: count: 1 path: libraries/classes/Controllers/View/CreateController.php + - + message: "#^Parameter \\#1 \\$dbName of method PhpMyAdmin\\\\DatabaseInterface\\:\\:getTable\\(\\) expects string, mixed given\\.$#" + count: 1 + path: libraries/classes/Controllers/View/CreateController.php + + - + message: "#^Parameter \\#1 \\$str of method PhpMyAdmin\\\\DatabaseInterface\\:\\:quoteString\\(\\) expects string, mixed given\\.$#" + count: 2 + path: libraries/classes/Controllers/View/CreateController.php + - message: "#^Parameter \\#2 \\$string of function explode expects string, mixed given\\.$#" count: 1 path: libraries/classes/Controllers/View/CreateController.php + - + message: "#^Parameter \\#2 \\$tableName of method PhpMyAdmin\\\\DatabaseInterface\\:\\:getTable\\(\\) expects string, mixed given\\.$#" + count: 1 + path: libraries/classes/Controllers/View/CreateController.php + - message: "#^Property PhpMyAdmin\\\\SqlParser\\\\Statements\\\\CreateStatement\\:\\:\\$body \\(array\\\\|string\\) in isset\\(\\) is not nullable\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 751b629acd..4246b0c4aa 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1191,15 +1191,9 @@ - - - __construct - - $request - @@ -2802,15 +2796,9 @@ - - - __construct - - $request - @@ -3085,22 +3073,12 @@ - - $username - - - - $username - $allUserGroups __construct - - $request - @@ -3235,6 +3213,8 @@ + getQueryParam('sql_signature')]]> + $sqlQuery @@ -3255,22 +3235,11 @@ $bkmAllUsers $sqlQuery + $sqlQuery - - - - - - - - - - - - @@ -3346,6 +3315,8 @@ $isUpload + getQueryParam('where_clause_signature')]]> + $whereClause $rowId @@ -3379,17 +3350,11 @@ $isUpload + $whereClause - - - - - - - $isUpload @@ -3557,9 +3522,6 @@ list]]> - - $request - @@ -3691,12 +3653,6 @@ - - - - - $request - @@ -4048,17 +4004,15 @@ + + $delimiter + getQueryParam('sql_query', true)]]> + + $delimiter - - - - - - $request - @@ -4588,6 +4542,8 @@ + $db + $table @@ -4598,6 +4554,8 @@ + $db + $table @@ -4611,18 +4569,6 @@ - - - - - - - - - - - - __construct diff --git a/test/classes/Controllers/Table/SqlControllerTest.php b/test/classes/Controllers/Table/SqlControllerTest.php index 00e1c933d3..52c6eea46a 100644 --- a/test/classes/Controllers/Table/SqlControllerTest.php +++ b/test/classes/Controllers/Table/SqlControllerTest.php @@ -78,10 +78,14 @@ class SqlControllerTest extends AbstractTestCase 'is_foreign_key_check' => true, ]); + $request = $this->createStub(ServerRequest::class); + $request->method('getParsedBodyParam')->willReturnMap([['delimiter', ';', ';']]); + $request->method('getQueryParam')->willReturnMap([['sql_query', true, true]]); + $response = new ResponseRenderer(); ( new SqlController($response, $template, new SqlQueryForm($template, $this->dbi)) - )($this->createStub(ServerRequest::class)); + )($request); $this->assertSame($expected, $response->getHTMLResult()); } }