From 5e429ffc7d6bafbe56a570fd8cbf0bcd9860d879 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maur=C3=ADcio=20Meneghini=20Fauth?= Date: Wed, 8 May 2024 17:11:57 -0300 Subject: [PATCH] Extract Footer::setHistory() into a middleware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is always done after a response, so is better to extract it into a middleware instead. Signed-off-by: Maurício Meneghini Fauth --- phpstan-baseline.neon | 17 ++-- psalm-baseline.xml | 20 ++--- src/Application.php | 2 + src/Footer.php | 78 ++++++------------- src/Http/Middleware/StatementHistory.php | 54 +++++++++++++ .../Http/Middleware/StatementHistoryTest.php | 73 +++++++++++++++++ 6 files changed, 174 insertions(+), 70 deletions(-) create mode 100644 src/Http/Middleware/StatementHistory.php create mode 100644 tests/unit/Http/Middleware/StatementHistoryTest.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index f78492c0ac50..a16ea8d6ed4c 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -7602,7 +7602,7 @@ parameters: - message: "#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#" - count: 4 + count: 2 path: src/Footer.php - @@ -7620,11 +7620,6 @@ parameters: count: 1 path: src/Footer.php - - - message: "#^Parameter \\#4 \\$sqlquery of method PhpMyAdmin\\\\ConfigStorage\\\\Relation\\:\\:setHistory\\(\\) expects string, mixed given\\.$#" - count: 1 - path: src/Footer.php - - message: "#^Only booleans are allowed in an elseif condition, int\\|false given\\.$#" count: 1 @@ -8230,6 +8225,16 @@ parameters: count: 1 path: src/Http/Middleware/SetupPageRedirection.php + - + message: "#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#" + count: 1 + path: src/Http/Middleware/StatementHistory.php + + - + message: "#^Parameter \\#4 \\$sqlquery of method PhpMyAdmin\\\\ConfigStorage\\\\Relation\\:\\:setHistory\\(\\) expects string, mixed given\\.$#" + count: 1 + path: src/Http/Middleware/StatementHistory.php + - message: "#^Parameter \\#1 \\$known_string of function hash_equals expects string, mixed given\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 0ca131742275..b625624dd351 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -5996,10 +5996,6 @@ - - - - @@ -6014,13 +6010,8 @@ - - - - - @@ -6522,6 +6513,17 @@ + + + + + + + + + + + diff --git a/src/Application.php b/src/Application.php index 720563d6379e..736cc1a158d0 100644 --- a/src/Application.php +++ b/src/Application.php @@ -38,6 +38,7 @@ use PhpMyAdmin\Http\Middleware\SetupPageRedirection; use PhpMyAdmin\Http\Middleware\SqlDelimiterSetting; use PhpMyAdmin\Http\Middleware\SqlQueryGlobalSetting; +use PhpMyAdmin\Http\Middleware\StatementHistory; use PhpMyAdmin\Http\Middleware\ThemeInitialization; use PhpMyAdmin\Http\Middleware\TokenMismatchChecking; use PhpMyAdmin\Http\Middleware\TokenRequestParamChecking; @@ -116,6 +117,7 @@ public function run(bool $isSetupPage = false): void $requestHandler->add(new ProfilingChecking()); $requestHandler->add(new UserPreferencesLoading($this->config)); $requestHandler->add(new RecentTableHandling($this->config)); + $requestHandler->add(new StatementHistory($this->config)); $runner = new RequestHandlerRunner( $requestHandler, diff --git a/src/Footer.php b/src/Footer.php index 46f3a240a4db..8044d81a04f4 100644 --- a/src/Footer.php +++ b/src/Footer.php @@ -7,7 +7,6 @@ namespace PhpMyAdmin; -use PhpMyAdmin\ConfigStorage\Relation; use PhpMyAdmin\Error\ErrorHandler; use PhpMyAdmin\Routing\Routing; use Traversable; @@ -17,7 +16,6 @@ use function in_array; use function is_array; use function is_object; -use function is_scalar; use function json_encode; use function json_last_error; @@ -44,12 +42,9 @@ class Footer */ private bool $isEnabled = true; - private Relation $relation; - public function __construct(private readonly Template $template, private readonly Config $config) { $this->scripts = new Scripts($this->template); - $this->relation = new Relation(DatabaseInterface::getInstance()); } /** @@ -160,32 +155,6 @@ public function getErrorMessages(): string return $retval; } - /** - * Saves query in history - */ - private function setHistory(): void - { - if ( - ( - isset($_REQUEST['no_history']) - && is_scalar($_REQUEST['no_history']) - && (string) $_REQUEST['no_history'] !== '' - ) - || ! empty($GLOBALS['error_message']) - || empty($GLOBALS['sql_query']) - || ! DatabaseInterface::getInstance()->isConnected() - ) { - return; - } - - $this->relation->setHistory( - Current::$database, - Current::$table, - $this->config->selectedServer['user'], - $GLOBALS['sql_query'], - ); - } - /** * Disables the rendering of the footer */ @@ -228,36 +197,35 @@ public function getScripts(): Scripts */ public function getDisplay(): string { - $this->setHistory(); - if ($this->isEnabled) { - if (! $this->isAjax && ! $this->isMinimal) { - if (Core::getEnv('SCRIPT_NAME') !== '') { - $url = $this->getSelfUrl(); - } + if (! $this->isEnabled) { + return ''; + } - $this->scripts->addCode('window.Console.debugSqlInfo = ' . $this->getDebugMessage() . ';'); - $errorMessages = $this->getErrorMessages(); - $scripts = $this->scripts->getDisplay(); + if (! $this->isAjax && ! $this->isMinimal) { + if (Core::getEnv('SCRIPT_NAME') !== '') { + $url = $this->getSelfUrl(); + } - if ($this->config->config->debug->demo) { - $gitRevisionInfo = $this->getGitRevisionInfo(); - } + $this->scripts->addCode('window.Console.debugSqlInfo = ' . $this->getDebugMessage() . ';'); + $errorMessages = $this->getErrorMessages(); + $scripts = $this->scripts->getDisplay(); - $footer = Config::renderFooter(); + if ($this->config->config->debug->demo) { + $gitRevisionInfo = $this->getGitRevisionInfo(); } - return $this->template->render('footer', [ - 'is_ajax' => $this->isAjax, - 'is_minimal' => $this->isMinimal, - 'self_url' => $url ?? null, - 'error_messages' => $errorMessages ?? '', - 'scripts' => $scripts ?? '', - 'is_demo' => $this->config->config->debug->demo, - 'git_revision_info' => $gitRevisionInfo ?? [], - 'footer' => $footer ?? '', - ]); + $footer = Config::renderFooter(); } - return ''; + return $this->template->render('footer', [ + 'is_ajax' => $this->isAjax, + 'is_minimal' => $this->isMinimal, + 'self_url' => $url ?? null, + 'error_messages' => $errorMessages ?? '', + 'scripts' => $scripts ?? '', + 'is_demo' => $this->config->config->debug->demo, + 'git_revision_info' => $gitRevisionInfo ?? [], + 'footer' => $footer ?? '', + ]); } } diff --git a/src/Http/Middleware/StatementHistory.php b/src/Http/Middleware/StatementHistory.php new file mode 100644 index 000000000000..d6aab96e71bc --- /dev/null +++ b/src/Http/Middleware/StatementHistory.php @@ -0,0 +1,54 @@ +dbi = $dbi ?? DatabaseInterface::getInstance(); + $this->relation = $relation ?? new Relation($this->dbi, $this->config); + } + + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + assert($request instanceof ServerRequest); + $response = $handler->handle($request); + + if ( + ! $request->has('no_history') + && empty($GLOBALS['error_message']) + && $GLOBALS['sql_query'] !== '' + && $this->dbi->isConnected() + ) { + $this->relation->setHistory( + Current::$database, + Current::$table, + $this->config->selectedServer['user'], + $GLOBALS['sql_query'], + ); + } + + return $response; + } +} diff --git a/tests/unit/Http/Middleware/StatementHistoryTest.php b/tests/unit/Http/Middleware/StatementHistoryTest.php new file mode 100644 index 000000000000..079e0494143e --- /dev/null +++ b/tests/unit/Http/Middleware/StatementHistoryTest.php @@ -0,0 +1,73 @@ +selectedServer['user'] = 'test_user'; + $dbi = self::createStub(DatabaseInterface::class); + $dbi->method('isConnected')->willReturn(true); + $relation = self::createMock(Relation::class); + $relation->expects(self::once())->method('setHistory')->with( + self::identicalTo('test_db'), + self::identicalTo('test_table'), + self::identicalTo('test_user'), + self::identicalTo('SELECT 1;'), + ); + $statementHistory = new StatementHistory($config, $dbi, $relation); + + $response = self::createStub(ResponseInterface::class); + $handler = self::createStub(RequestHandlerInterface::class); + $handler->method('handle')->willReturn($response); + + $request = ServerRequestFactory::create()->createServerRequest('POST', 'https://example.com/'); + + self::assertSame($response, $statementHistory->process($request, $handler)); + } + + #[TestWith(['true', 'SELECT 1;', true])] + #[TestWith([null, '', true])] + #[TestWith([null, 'SELECT 1;', false])] + public function testSkipHistory(string|null $noHistoryParam, string $sqlQuery, bool $isConnected): void + { + $GLOBALS['sql_query'] = $sqlQuery; + + $dbi = self::createStub(DatabaseInterface::class); + $dbi->method('isConnected')->willReturn($isConnected); + $relation = self::createMock(Relation::class); + $relation->expects(self::never())->method('setHistory'); + $statementHistory = new StatementHistory(new Config(), $dbi, $relation); + + $response = self::createStub(ResponseInterface::class); + $handler = self::createStub(RequestHandlerInterface::class); + $handler->method('handle')->willReturn($response); + + $parsedBody = $noHistoryParam !== null ? ['no_history' => $noHistoryParam] : []; + $request = ServerRequestFactory::create()->createServerRequest('POST', 'https://example.com/') + ->withParsedBody($parsedBody); + + self::assertSame($response, $statementHistory->process($request, $handler)); + } +}