Skip to content

Commit

Permalink
Merge pull request #19151 from MauricioFauth/stmt-history-middleware
Browse files Browse the repository at this point in the history
Extract Footer::setHistory() into a middleware
  • Loading branch information
MauricioFauth committed May 9, 2024
2 parents 2a58575 + 5e429ff commit 898fb91
Show file tree
Hide file tree
Showing 6 changed files with 174 additions and 70 deletions.
17 changes: 11 additions & 6 deletions phpstan-baseline.neon
Expand Up @@ -7602,7 +7602,7 @@ parameters:

-
message: "#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#"
count: 4
count: 2
path: src/Footer.php

-
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
20 changes: 11 additions & 9 deletions psalm-baseline.xml
Expand Up @@ -5996,10 +5996,6 @@
</MixedOperand>
</file>
<file src="src/Footer.php">
<DeprecatedMethod>
<code><![CDATA[DatabaseInterface::getInstance()]]></code>
<code><![CDATA[DatabaseInterface::getInstance()]]></code>
</DeprecatedMethod>
<DeprecatedProperty>
<code><![CDATA[Routing::$route]]></code>
</DeprecatedProperty>
Expand All @@ -6014,13 +6010,8 @@
<code><![CDATA[array{revision: string, revisionUrl: string, branch: string, branchUrl: string}|[]]]></code>
<code><![CDATA[is_array($info) ? $info : []]]></code>
</MixedReturnTypeCoercion>
<RedundantCast>
<code><![CDATA[(string) $_REQUEST['no_history']]]></code>
</RedundantCast>
<RiskyTruthyFalsyComparison>
<code><![CDATA[$object]]></code>
<code><![CDATA[empty($GLOBALS['error_message'])]]></code>
<code><![CDATA[empty($GLOBALS['sql_query'])]]></code>
<code><![CDATA[empty($_REQUEST['no_debug'])]]></code>
</RiskyTruthyFalsyComparison>
<UnusedReturnValue>
Expand Down Expand Up @@ -6522,6 +6513,17 @@
<code><![CDATA[DatabaseInterface::getInstance()]]></code>
</DeprecatedMethod>
</file>
<file src="src/Http/Middleware/StatementHistory.php">
<DeprecatedMethod>
<code><![CDATA[DatabaseInterface::getInstance()]]></code>
</DeprecatedMethod>
<MixedArgument>
<code><![CDATA[$GLOBALS['sql_query']]]></code>
</MixedArgument>
<RiskyTruthyFalsyComparison>
<code><![CDATA[empty($GLOBALS['error_message'])]]></code>
</RiskyTruthyFalsyComparison>
</file>
<file src="src/Http/Middleware/TokenRequestParamChecking.php">
<MixedArgument>
<code><![CDATA[$_SESSION[' PMA_token ']]]></code>
Expand Down
2 changes: 2 additions & 0 deletions src/Application.php
Expand Up @@ -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;
Expand Down Expand Up @@ -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));

Check warning on line 120 in src/Application.php

View workflow job for this annotation

GitHub Actions / Infection (8.2, ubuntu-latest)

Escaped Mutant for Mutator "MethodCallRemoval": --- Original +++ New @@ @@ $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, new SapiEmitter(), static fn(): ServerRequestInterface => ServerRequestFactory::create()->fromGlobals()->withAttribute('isSetupPage', $isSetupPage), function (Throwable $throwable): ResponseInterface { $response = $this->responseFactory->createResponse(StatusCodeInterface::STATUS_INTERNAL_SERVER_ERROR); $response->getBody()->write(sprintf('An error occurred: %s', $throwable->getMessage()));

$runner = new RequestHandlerRunner(
$requestHandler,
Expand Down
78 changes: 23 additions & 55 deletions src/Footer.php
Expand Up @@ -7,7 +7,6 @@

namespace PhpMyAdmin;

use PhpMyAdmin\ConfigStorage\Relation;
use PhpMyAdmin\Error\ErrorHandler;
use PhpMyAdmin\Routing\Routing;
use Traversable;
Expand All @@ -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;

Expand All @@ -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());
}

/**
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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) {

Check warning on line 204 in src/Footer.php

View workflow job for this annotation

GitHub Actions / Infection (8.2, ubuntu-latest)

Escaped Mutant for Mutator "LogicalAnd": --- Original +++ New @@ @@ if (!$this->isEnabled) { return ''; } - if (!$this->isAjax && !$this->isMinimal) { + if (!$this->isAjax || !$this->isMinimal) { if (Core::getEnv('SCRIPT_NAME') !== '') { $url = $this->getSelfUrl(); }
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() . ';');

Check warning on line 209 in src/Footer.php

View workflow job for this annotation

GitHub Actions / Infection (8.2, ubuntu-latest)

Escaped Mutant for Mutator "ConcatOperandRemoval": --- Original +++ New @@ @@ if (Core::getEnv('SCRIPT_NAME') !== '') { $url = $this->getSelfUrl(); } - $this->scripts->addCode('window.Console.debugSqlInfo = ' . $this->getDebugMessage() . ';'); + $this->scripts->addCode($this->getDebugMessage() . ';'); $errorMessages = $this->getErrorMessages(); $scripts = $this->scripts->getDisplay(); if ($this->config->config->debug->demo) {

Check warning on line 209 in src/Footer.php

View workflow job for this annotation

GitHub Actions / Infection (8.2, ubuntu-latest)

Escaped Mutant for Mutator "Concat": --- Original +++ New @@ @@ if (Core::getEnv('SCRIPT_NAME') !== '') { $url = $this->getSelfUrl(); } - $this->scripts->addCode('window.Console.debugSqlInfo = ' . $this->getDebugMessage() . ';'); + $this->scripts->addCode($this->getDebugMessage() . 'window.Console.debugSqlInfo = ' . ';'); $errorMessages = $this->getErrorMessages(); $scripts = $this->scripts->getDisplay(); if ($this->config->config->debug->demo) {

Check warning on line 209 in src/Footer.php

View workflow job for this annotation

GitHub Actions / Infection (8.2, ubuntu-latest)

Escaped Mutant for Mutator "ConcatOperandRemoval": --- Original +++ New @@ @@ if (Core::getEnv('SCRIPT_NAME') !== '') { $url = $this->getSelfUrl(); } - $this->scripts->addCode('window.Console.debugSqlInfo = ' . $this->getDebugMessage() . ';'); + $this->scripts->addCode('window.Console.debugSqlInfo = ' . ';'); $errorMessages = $this->getErrorMessages(); $scripts = $this->scripts->getDisplay(); if ($this->config->config->debug->demo) {

Check warning on line 209 in src/Footer.php

View workflow job for this annotation

GitHub Actions / Infection (8.2, ubuntu-latest)

Escaped Mutant for Mutator "Concat": --- Original +++ New @@ @@ if (Core::getEnv('SCRIPT_NAME') !== '') { $url = $this->getSelfUrl(); } - $this->scripts->addCode('window.Console.debugSqlInfo = ' . $this->getDebugMessage() . ';'); + $this->scripts->addCode('window.Console.debugSqlInfo = ' . ';' . $this->getDebugMessage()); $errorMessages = $this->getErrorMessages(); $scripts = $this->scripts->getDisplay(); if ($this->config->config->debug->demo) {

Check warning on line 209 in src/Footer.php

View workflow job for this annotation

GitHub Actions / Infection (8.2, ubuntu-latest)

Escaped Mutant for Mutator "ConcatOperandRemoval": --- Original +++ New @@ @@ if (Core::getEnv('SCRIPT_NAME') !== '') { $url = $this->getSelfUrl(); } - $this->scripts->addCode('window.Console.debugSqlInfo = ' . $this->getDebugMessage() . ';'); + $this->scripts->addCode('window.Console.debugSqlInfo = ' . $this->getDebugMessage()); $errorMessages = $this->getErrorMessages(); $scripts = $this->scripts->getDisplay(); if ($this->config->config->debug->demo) {

Check warning on line 209 in src/Footer.php

View workflow job for this annotation

GitHub Actions / Infection (8.2, ubuntu-latest)

Escaped Mutant for Mutator "MethodCallRemoval": --- Original +++ New @@ @@ if (Core::getEnv('SCRIPT_NAME') !== '') { $url = $this->getSelfUrl(); } - $this->scripts->addCode('window.Console.debugSqlInfo = ' . $this->getDebugMessage() . ';'); + $errorMessages = $this->getErrorMessages(); $scripts = $this->scripts->getDisplay(); if ($this->config->config->debug->demo) {
$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 ?? '',

Check warning on line 224 in src/Footer.php

View workflow job for this annotation

GitHub Actions / Infection (8.2, ubuntu-latest)

Escaped Mutant for Mutator "Coalesce": --- Original +++ New @@ @@ } $footer = Config::renderFooter(); } - 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 ?? '']); + 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 ?? '']); } }
'scripts' => $scripts ?? '',

Check warning on line 225 in src/Footer.php

View workflow job for this annotation

GitHub Actions / Infection (8.2, ubuntu-latest)

Escaped Mutant for Mutator "Coalesce": --- Original +++ New @@ @@ } $footer = Config::renderFooter(); } - 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 ?? '']); + 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 ?? '']); } }
'is_demo' => $this->config->config->debug->demo,
'git_revision_info' => $gitRevisionInfo ?? [],
'footer' => $footer ?? '',
]);
}
}
54 changes: 54 additions & 0 deletions src/Http/Middleware/StatementHistory.php
@@ -0,0 +1,54 @@
<?php

declare(strict_types=1);

namespace PhpMyAdmin\Http\Middleware;

use PhpMyAdmin\Config;
use PhpMyAdmin\ConfigStorage\Relation;
use PhpMyAdmin\Current;
use PhpMyAdmin\DatabaseInterface;
use PhpMyAdmin\Http\ServerRequest;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;

use function assert;

final class StatementHistory implements MiddlewareInterface
{
private readonly Relation $relation;
private readonly DatabaseInterface $dbi;

public function __construct(
private readonly Config $config,
DatabaseInterface|null $dbi = null,
Relation|null $relation = null,
) {
$this->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;
}
}
73 changes: 73 additions & 0 deletions tests/unit/Http/Middleware/StatementHistoryTest.php
@@ -0,0 +1,73 @@
<?php

declare(strict_types=1);

namespace PhpMyAdmin\Tests\Http\Middleware;

use PhpMyAdmin\Config;
use PhpMyAdmin\ConfigStorage\Relation;
use PhpMyAdmin\Current;
use PhpMyAdmin\DatabaseInterface;
use PhpMyAdmin\Http\Factory\ServerRequestFactory;
use PhpMyAdmin\Http\Middleware\StatementHistory;
use PhpMyAdmin\Tests\AbstractTestCase;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\TestWith;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Server\RequestHandlerInterface;

#[CoversClass(StatementHistory::class)]
final class StatementHistoryTest extends AbstractTestCase
{
public function testStatementHistory(): void
{
Current::$database = 'test_db';
Current::$table = 'test_table';
$GLOBALS['sql_query'] = 'SELECT 1;';

$config = new Config();
$config->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));
}
}

0 comments on commit 898fb91

Please sign in to comment.