From 1f4bbe5636d8e15c1835497046f647532eebea2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Mon, 12 Jan 2026 19:52:01 -0300 Subject: [PATCH] Replace request globals with ServerRequest object in Sql class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maurício Meneghini Fauth --- phpstan-baseline.neon | 30 ++----- psalm-baseline.xml | 16 +--- .../MultiTableQuery/QueryController.php | 1 + src/Controllers/Import/ImportController.php | 3 + .../Sql/RelationalValuesController.php | 4 +- src/Controllers/Sql/SqlController.php | 1 + .../Table/DeleteRowsController.php | 1 + src/Controllers/Table/SearchController.php | 5 +- .../Table/Structure/BrowseController.php | 5 +- src/Sql.php | 81 +++++++++++++------ .../Import/ImportControllerTest.php | 21 ++--- tests/unit/SqlTest.php | 5 ++ 12 files changed, 90 insertions(+), 83 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 14b1e9ccdf..d02e3a3ad3 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -12297,13 +12297,13 @@ parameters: - message: '#^Cannot cast mixed to int\.$#' identifier: cast.int - count: 1 + count: 3 path: src/Sql.php - message: '#^Construct empty\(\) is not allowed\. Use more strict comparison\.$#' identifier: empty.notAllowed - count: 10 + count: 4 path: src/Sql.php - @@ -12330,12 +12330,6 @@ parameters: count: 1 path: src/Sql.php - - - message: '#^Only booleans are allowed in &&, mixed given on the right side\.$#' - identifier: booleanAnd.rightNotBoolean - count: 3 - path: src/Sql.php - - message: '#^Only booleans are allowed in &&, string given on the left side\.$#' identifier: booleanAnd.leftNotBoolean @@ -12361,14 +12355,14 @@ parameters: path: src/Sql.php - - message: '#^Only booleans are allowed in a negated boolean, mixed given\.$#' - identifier: booleanNot.exprNotBoolean + message: '#^Only booleans are allowed in an if condition, PhpMyAdmin\\Dbal\\ResultInterface\|false given\.$#' + identifier: if.condNotBoolean count: 1 path: src/Sql.php - - message: '#^Only booleans are allowed in an if condition, PhpMyAdmin\\Dbal\\ResultInterface\|false given\.$#' - identifier: if.condNotBoolean + message: '#^Parameter \#1 \$label of method PhpMyAdmin\\Sql\:\:getBookmarkCreatedMessage\(\) expects string\|null, mixed given\.$#' + identifier: argument.type count: 1 path: src/Sql.php @@ -12414,24 +12408,12 @@ parameters: count: 2 path: src/Sql.php - - - message: '#^Parameter \#3 \$column of method PhpMyAdmin\\Sql\:\:cleanupRelations\(\) expects string, mixed given\.$#' - identifier: argument.type - count: 1 - path: src/Sql.php - - message: '#^Parameter \#3 \$subject of function str_replace expects array\\|string, mixed given\.$#' identifier: argument.type count: 1 path: src/Sql.php - - - message: '#^Parameter \#4 \$bookmarkLabel of method PhpMyAdmin\\Sql\:\:storeTheQueryAsBookmark\(\) expects string, mixed given\.$#' - identifier: argument.type - count: 1 - path: src/Sql.php - - message: ''' #^Call to deprecated method getInstance\(\) of class PhpMyAdmin\\Config\: diff --git a/psalm-baseline.xml b/psalm-baseline.xml index e22b742292..791b0db176 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -8008,6 +8008,7 @@ + getQueryParam('label')]]> @@ -8050,17 +8051,11 @@ - - - - - - @@ -8085,16 +8080,7 @@ isUnique()]]> - - - - - - - - - altered[0]->field->column)]]> diff --git a/src/Controllers/Database/MultiTableQuery/QueryController.php b/src/Controllers/Database/MultiTableQuery/QueryController.php index f532d27919..e75231315f 100644 --- a/src/Controllers/Database/MultiTableQuery/QueryController.php +++ b/src/Controllers/Database/MultiTableQuery/QueryController.php @@ -32,6 +32,7 @@ final class QueryController implements InvocableController $goto = Url::getFromRoute('/database/multi-table-query'); $this->response->addHTML($this->sql->executeQueryAndSendQueryResponse( + $request, null, false, // is_gotofile $db, // db diff --git a/src/Controllers/Import/ImportController.php b/src/Controllers/Import/ImportController.php index a8308ae652..3903740bbc 100644 --- a/src/Controllers/Import/ImportController.php +++ b/src/Controllers/Import/ImportController.php @@ -632,6 +632,7 @@ final readonly class ImportController implements InvocableController } $htmlOutput .= $this->sql->executeQueryAndGetQueryResponse( + $request, $statementInfo, false, // is_gotofile Current::$database, // db @@ -656,6 +657,7 @@ final readonly class ImportController implements InvocableController $request->getParsedBodyParamAsString('sql_query'), $request->getParsedBodyParamAsString('bkm_label'), $request->hasBodyParam('bkm_replace'), + $request->hasBodyParam('bkm_all_users'), ); } @@ -683,6 +685,7 @@ final readonly class ImportController implements InvocableController $request->getParsedBodyParamAsString('sql_query'), $request->getParsedBodyParamAsString('bkm_label'), $request->hasBodyParam('bkm_replace'), + $request->hasBodyParam('bkm_all_users'), ); } diff --git a/src/Controllers/Sql/RelationalValuesController.php b/src/Controllers/Sql/RelationalValuesController.php index a717f58f9f..2fb33408a9 100644 --- a/src/Controllers/Sql/RelationalValuesController.php +++ b/src/Controllers/Sql/RelationalValuesController.php @@ -29,10 +29,11 @@ final class RelationalValuesController implements InvocableController $column = $request->getParsedBodyParamAsString('column', ''); $relationKeyOrDisplayColumn = $request->getParsedBodyParamAsStringOrNull('relation_key_or_display_column'); + $currentValueParam = $request->getParsedBodyParamAsString('curr_value', ''); if ($_SESSION['tmpval']['relational_display'] === 'D' && $relationKeyOrDisplayColumn !== null) { $currValue = $relationKeyOrDisplayColumn; } else { - $currValue = $request->getParsedBodyParamAsString('curr_value', ''); + $currValue = $currentValueParam; } $dropdown = $this->sql->getHtmlForRelationalColumnDropdown( @@ -40,6 +41,7 @@ final class RelationalValuesController implements InvocableController Current::$table, $column, $currValue, + $currentValueParam, ); $this->response->addJSON('dropdown', $dropdown); diff --git a/src/Controllers/Sql/SqlController.php b/src/Controllers/Sql/SqlController.php index a06b292ecc..a54c9ea076 100644 --- a/src/Controllers/Sql/SqlController.php +++ b/src/Controllers/Sql/SqlController.php @@ -164,6 +164,7 @@ readonly class SqlController implements InvocableController Current::$messageToShow = $request->getParsedBodyParamAsString('message_to_show', ''); $this->response->addHTML($this->sql->executeQueryAndSendQueryResponse( + $request, $statementInfo, $isGotofile, Current::$database, diff --git a/src/Controllers/Table/DeleteRowsController.php b/src/Controllers/Table/DeleteRowsController.php index 9ba77543f2..ca1ef2b5ab 100644 --- a/src/Controllers/Table/DeleteRowsController.php +++ b/src/Controllers/Table/DeleteRowsController.php @@ -66,6 +66,7 @@ final readonly class DeleteRowsController implements InvocableController } $this->response->addHTML($this->sql->executeQueryAndSendQueryResponse( + $request, null, false, Current::$database, diff --git a/src/Controllers/Table/SearchController.php b/src/Controllers/Table/SearchController.php index 1761932021..a27e228743 100644 --- a/src/Controllers/Table/SearchController.php +++ b/src/Controllers/Table/SearchController.php @@ -211,7 +211,7 @@ final class SearchController implements InvocableController if (! isset($_POST['columnsToDisplay']) && ! isset($_POST['displayAllColumns'])) { $this->displaySelectionFormAction(); } else { - $this->doSelectionAction(); + $this->doSelectionAction($request); } return $this->response->response(); @@ -220,7 +220,7 @@ final class SearchController implements InvocableController /** * Do selection action */ - private function doSelectionAction(): void + private function doSelectionAction(ServerRequest $request): void { /** * Selection criteria have been submitted -> do the work @@ -231,6 +231,7 @@ final class SearchController implements InvocableController * Add this to ensure following procedures included running correctly. */ $this->response->addHTML($this->sql->executeQueryAndSendQueryResponse( + $request, null, false, // is_gotofile Current::$database, // db diff --git a/src/Controllers/Table/Structure/BrowseController.php b/src/Controllers/Table/Structure/BrowseController.php index 9d31dbec25..d24c7bc945 100644 --- a/src/Controllers/Table/Structure/BrowseController.php +++ b/src/Controllers/Table/Structure/BrowseController.php @@ -35,7 +35,7 @@ final class BrowseController implements InvocableController return $this->response->response(); } - $this->displayTableBrowseForSelectedColumns(UrlParams::$goto); + $this->displayTableBrowseForSelectedColumns($request, UrlParams::$goto); return $this->response->response(); } @@ -45,7 +45,7 @@ final class BrowseController implements InvocableController * * @param string $goto goto page url */ - private function displayTableBrowseForSelectedColumns(string $goto): void + private function displayTableBrowseForSelectedColumns(ServerRequest $request, string $goto): void { $fields = []; foreach ($_POST['selected_fld'] as $sval) { @@ -64,6 +64,7 @@ final class BrowseController implements InvocableController $this->response->addHTML( $this->sql->executeQueryAndGetQueryResponse( + $request, $statementInfo, false, // is_gotofile Current::$database, // db diff --git a/src/Sql.php b/src/Sql.php index b91e22d21e..c76906eade 100644 --- a/src/Sql.php +++ b/src/Sql.php @@ -14,6 +14,7 @@ use PhpMyAdmin\Display\DisplayParts; use PhpMyAdmin\Display\Results as DisplayResults; use PhpMyAdmin\Html\Generator; use PhpMyAdmin\Html\MySQLDocumentation; +use PhpMyAdmin\Http\ServerRequest; use PhpMyAdmin\Identifiers\DatabaseName; use PhpMyAdmin\Indexes\Index; use PhpMyAdmin\Query\Utilities; @@ -235,6 +236,7 @@ class Sql string $table, string $column, string $currentValue, + string $currentValueParam, ): string { $foreignData = $this->relation->getForeignData( $this->relation->getForeigners($db, $table, $column), @@ -250,7 +252,7 @@ class Sql $urlParams = ['db' => $db, 'table' => $table, 'field' => $column]; return $this->template->render('sql/relational_column_dropdown', [ - 'current_value' => $_POST['curr_value'], + 'current_value' => $currentValueParam, 'params' => $urlParams, ]); } @@ -511,6 +513,7 @@ class Sql string $sqlQueryForBookmark, string $bookmarkLabel, bool $bookmarkReplace, + bool $hasBookmarkAllUsers, ): void { if ($bookmarkReplace) { $bookmarks = $this->bookmarkRepository->getList($this->config->selectedServer['user'], $db); @@ -528,7 +531,7 @@ class Sql $bookmarkLabel, $bookmarkUser, $db, - isset($_POST['bkm_all_users']), + $hasBookmarkAllUsers, ); if ($bookmark === false) { @@ -737,6 +740,7 @@ class Sql * } */ private function executeTheQuery( + ServerRequest $request, StatementInfo $statementInfo, string $fullSqlQuery, bool $isGotoFile, @@ -774,13 +778,15 @@ class Sql // If there are no errors and bookmarklabel was given, // store the query as a bookmark - if (! empty($_POST['bkm_label']) && $sqlQueryForBookmark) { + $bookmarkLabel = $request->getParsedBodyParamAsString('bkm_label', ''); + if ($bookmarkLabel !== '' && $sqlQueryForBookmark) { $this->storeTheQueryAsBookmark( $db, $this->config->selectedServer['user'], $sqlQueryForBookmark, - $_POST['bkm_label'], - isset($_POST['bkm_replace']), + $bookmarkLabel, + $request->hasBodyParam('bkm_replace'), + $request->hasBodyParam('bkm_all_users'), ); } @@ -795,7 +801,12 @@ class Sql $unlimNumRows = $this->countQueryResults($numRows, $justBrowsing, $db, $table, $statementInfo); - $this->cleanupRelations($db, $table, $_POST['dropped_column'] ?? '', ! empty($_POST['purge'])); + $this->cleanupRelations( + $db, + $table, + $request->getParsedBodyParamAsString('dropped_column', ''), + $request->hasBodyParam('purge'), + ); return [$result, $numRows, $unlimNumRows, $profilingResults, $errorMessage]; } @@ -836,6 +847,7 @@ class Sql string $messageToShow, StatementInfo $statementInfo, int|string $numRows, + bool $hasRollbackQuery, ): Message { if ($statementInfo->flags->queryType === StatementType::Delete) { $message = Message::getMessageForDeletedRows($numRows); @@ -892,7 +904,7 @@ class Sql } // In case of ROLLBACK, notify the user. - if (isset($_POST['rollback_query'])) { + if ($hasRollbackQuery) { $message->addText(__('[ROLLBACK occurred.]')); } @@ -916,6 +928,7 @@ class Sql * @return string html */ private function getQueryResponseForNoResultsReturned( + ServerRequest $request, StatementInfo $statementInfo, string $db, string|null $table, @@ -935,7 +948,12 @@ class Sql if ($errorMessage !== '') { $message = Message::rawError($errorMessage); } else { - $message = $this->getMessageForNoRowsReturned($messageToShow, $statementInfo, $numRows); + $message = $this->getMessageForNoRowsReturned( + $messageToShow, + $statementInfo, + $numRows, + $request->hasBodyParam('rollback_query'), + ); } $queryMessage = Generator::getMessage($message, $sqlQuery, MessageType::Success); @@ -951,7 +969,7 @@ class Sql } // For ajax requests add message and sql_query as JSON - if (empty($_REQUEST['ajax_page_request'])) { + if (! $request->has('ajax_page_request')) { $extraData['message'] = $message; if ($this->config->settings['ShowSQL']) { $extraData['sql_query'] = $queryMessage; @@ -959,7 +977,7 @@ class Sql } if ( - isset($_POST['dropped_column']) + $request->hasBodyParam('dropped_column') && $db !== '' && $table !== null && $table !== '' ) { // to refresh the list of indexes (Ajax mode) @@ -988,6 +1006,7 @@ class Sql ]); $sqlQueryResultsTable = $this->getHtmlForSqlQueryResultsTable( + $request, $displayResultsObject, $displayParts, false, @@ -1004,7 +1023,7 @@ class Sql $bookmarkFeature = $this->relation->getRelationParameters()->bookmarkFeature; if ( $bookmarkFeature !== null - && empty($_GET['id_bookmark']) + && (int) $request->getQueryParam('id_bookmark') <= 0 && $sqlQuery ) { $bookmark = $this->template->render('sql/bookmark', [ @@ -1055,14 +1074,14 @@ class Sql * Returns a message for successful creation of a bookmark or null if a bookmark * was not created */ - private function getBookmarkCreatedMessage(): string + private function getBookmarkCreatedMessage(string|null $label): string { $output = ''; - if (isset($_GET['label'])) { + if ($label !== null && $label !== '') { $message = Message::success( __('Bookmark %s has been created.'), ); - $message->addParam($_GET['label']); + $message->addParam($label); $output = $message->getDisplay(); } @@ -1082,6 +1101,7 @@ class Sql * @psalm-param int|numeric-string $numRows */ private function getHtmlForSqlQueryResultsTable( + ServerRequest $request, DisplayResults $displayResultsObject, DisplayParts $displayParts, bool $editable, @@ -1091,8 +1111,8 @@ class Sql StatementInfo $statementInfo, bool $isLimitedDisplay = false, ): string { - $printView = isset($_POST['printview']) && $_POST['printview']; - $isBrowseDistinct = ! empty($_POST['is_browse_distinct']); + $printView = $request->hasBodyParam('printview'); + $isBrowseDistinct = (bool) $request->getParsedBodyParamAsStringOrNull('is_browse_distinct'); if ($statementInfo->flags->isProcedure) { return $this->getHtmlForStoredProcedureResults( @@ -1257,6 +1277,7 @@ class Sql * @return string html */ private function getQueryResponseForResultsReturned( + ServerRequest $request, ResultInterface $result, StatementInfo $statementInfo, string $db, @@ -1272,7 +1293,7 @@ class Sql ): string { // If we are retrieving the full value of a truncated field or the original // value of a transformed field, show it here - if (isset($_POST['grid_edit']) && $_POST['grid_edit']) { + if ($request->hasBodyParam('grid_edit')) { $this->getResponseForGridEdit($result); ResponseRenderer::getInstance()->callExit(); } @@ -1342,7 +1363,7 @@ class Sql ]); } - if (isset($_POST['printview']) && $_POST['printview']) { + if ($request->hasBodyParam('printview')) { $displayParts = DisplayParts::fromArray([ 'hasEditLink' => false, 'deleteLink' => DeleteLinkEnum::NO_DELETE, @@ -1352,9 +1373,7 @@ class Sql 'hasTextButton' => false, 'hasPrintLink' => false, ]); - } - - if (! isset($_POST['printview']) || ! $_POST['printview']) { + } else { $scripts->addFile('makegrid.js'); $scripts->addFile('sql.js'); Current::$message = null; @@ -1372,9 +1391,10 @@ class Sql ? $this->getMessageIfMissingColumnIndex($db, $editable, $hasUnique) : ''; - $bookmarkCreatedMessage = $this->getBookmarkCreatedMessage(); + $bookmarkCreatedMessage = $this->getBookmarkCreatedMessage($request->getQueryParam('label')); $tableHtml = $this->getHtmlForSqlQueryResultsTable( + $request, $displayResultsObject, $displayParts, $editable, @@ -1389,7 +1409,7 @@ class Sql if ( $bookmarkFeature !== null && $displayParts->hasBookmarkForm - && empty($_GET['id_bookmark']) + && (int) $request->getQueryParam('id_bookmark') <= 0 && $sqlQuery ) { $bookmarkSupportHtml = $this->template->render('sql/bookmark', [ @@ -1430,6 +1450,7 @@ class Sql * @param string $completeQuery complete query */ public function executeQueryAndSendQueryResponse( + ServerRequest $request, StatementInfo|null $statementInfo, bool $isGotoFile, string $db, @@ -1450,6 +1471,7 @@ class Sql } return $this->executeQueryAndGetQueryResponse( + $request, $statementInfo, $isGotoFile, // is_gotofile $db, // db @@ -1481,6 +1503,7 @@ class Sql * @return string html */ public function executeQueryAndGetQueryResponse( + ServerRequest $request, StatementInfo $statementInfo, bool $isGotoFile, string $db, @@ -1501,7 +1524,7 @@ class Sql if ( $this->isRememberSortingOrder($statementInfo) && ! $statementInfo->flags->union - && ! isset($_POST['sort_by_key']) + && ! $request->hasBodyParam('sort_by_key') ) { if (! isset($_SESSION['sql_from_query_box'])) { $statementInfo = $this->handleSortOrder($db, $table ?? '', $statementInfo, $sqlQuery); @@ -1540,7 +1563,12 @@ class Sql $this->deleteTransformationInfo($db, $table ?? '', $statementInfo); } - $message = $this->getMessageForNoRowsReturned($messageToShow, $statementInfo, 0); + $message = $this->getMessageForNoRowsReturned( + $messageToShow, + $statementInfo, + 0, + $request->hasBodyParam('rollback_query'), + ); return Generator::getMessage($message, Current::$sqlQuery, MessageType::Success); } @@ -1549,6 +1577,7 @@ class Sql $defaultFkCheck = ForeignKey::handleDisableCheckInit(); [$result, $numRows, $unlimNumRows, $profilingResults, $errorMessage] = $this->executeTheQuery( + $request, $statementInfo, $fullSqlQuery, $isGotoFile, @@ -1562,6 +1591,7 @@ class Sql // No rows returned -> move back to the calling page if (($numRows === 0 && $unlimNumRows === 0) || $statementInfo->flags->isAffected || $result === false) { $htmlOutput = $this->getQueryResponseForNoResultsReturned( + $request, $statementInfo, $db, $table, @@ -1577,6 +1607,7 @@ class Sql } else { // At least one row is returned -> displays a table with results $htmlOutput = $this->getQueryResponseForResultsReturned( + $request, $result, $statementInfo, $db, diff --git a/tests/unit/Controllers/Import/ImportControllerTest.php b/tests/unit/Controllers/Import/ImportControllerTest.php index 0c87a52701..2c4e9a103d 100644 --- a/tests/unit/Controllers/Import/ImportControllerTest.php +++ b/tests/unit/Controllers/Import/ImportControllerTest.php @@ -11,7 +11,7 @@ use PhpMyAdmin\ConfigStorage\RelationCleanup; use PhpMyAdmin\Controllers\Import\ImportController; use PhpMyAdmin\Current; use PhpMyAdmin\Dbal\DatabaseInterface; -use PhpMyAdmin\Http\ServerRequest; +use PhpMyAdmin\Http\Factory\ServerRequestFactory; use PhpMyAdmin\Import\Import; use PhpMyAdmin\Sql; use PhpMyAdmin\Template; @@ -52,19 +52,12 @@ class ImportControllerTest extends AbstractTestCase . 'FROM table1 A' . "\n" . 'WHERE A.nomEtablissement = :nomEta AND foo = :1 AND `:a` IS NULL'; - $request = self::createStub(ServerRequest::class); - $request->method('getParsedBodyParam')->willReturnMap([ - ['db', null, Current::$database], - ['table', null, Current::$table], - ['parameters', null, [':nomEta' => 'Saint-Louis - Châteaulin', ':1' => '4']], - ['sql_query', null, Current::$sqlQuery], - ]); - $request->method('hasBodyParam')->willReturnMap([ - ['parameterized', true], - ['rollback_query', false], - ['allow_interrupt', false], - ['skip', false], - ['show_as_php', false], + $request = ServerRequestFactory::create()->createServerRequest('POST', 'https://example.com')->withParsedBody([ + 'db' => Current::$database, + 'table' => Current::$table, + 'parameters' => [':nomEta' => 'Saint-Louis - Châteaulin', ':1' => '4'], + 'sql_query' => Current::$sqlQuery, + 'parameterized' => '1', ]); $this->dummyDbi->addResult( diff --git a/tests/unit/SqlTest.php b/tests/unit/SqlTest.php index e1def0a638..4dc0676d00 100644 --- a/tests/unit/SqlTest.php +++ b/tests/unit/SqlTest.php @@ -10,6 +10,7 @@ use PhpMyAdmin\ConfigStorage\Relation; use PhpMyAdmin\ConfigStorage\RelationCleanup; use PhpMyAdmin\Current; use PhpMyAdmin\Dbal\DatabaseInterface; +use PhpMyAdmin\Http\Factory\ServerRequestFactory; use PhpMyAdmin\ParseAnalyze; use PhpMyAdmin\Sql; use PhpMyAdmin\Template; @@ -620,7 +621,11 @@ class SqlTest extends AbstractTestCase $config = Config::getInstance(); $config->selectedServer['DisableIS'] = true; $config->selectedServer['user'] = 'user'; + + $request = ServerRequestFactory::create()->createServerRequest('POST', 'https://example.com'); + $actual = $this->sql->executeQueryAndSendQueryResponse( + $request, null, false, 'sakila',