16276 - Reduce the usage of SuperGlobals (#17795)

Signed-off-by: Luca Perna <luca@perna.rocks>
This commit is contained in:
Luca Perna 2022-10-20 06:52:38 +02:00 committed by GitHub
parent d569fe2f05
commit d1b5cb6987
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 61 additions and 62 deletions

View File

@ -218,7 +218,7 @@ final class Common
Tracker::enable();
if ($route === '/url') {
UrlRedirector::redirect();
UrlRedirector::redirect($_GET['url'] ?? '');
}
return;

View File

@ -75,7 +75,15 @@ class ReplicationController extends AbstractController
isset($_POST['replica_changeprimary']),
isset($_POST['sr_replica_server_control']),
$_POST['sr_replica_action'] ?? null,
isset($_POST['sr_replica_skip_error'])
isset($_POST['sr_replica_skip_error']),
isset($_POST['sr_skip_errors_count']) ? (int) $_POST['sr_skip_errors_count'] : 1,
$_POST['sr_replica_control_param'] ?? null,
[
'username' => $GLOBALS['dbi']->escapeString($_POST['username']),
'pma_pw' => $GLOBALS['dbi']->escapeString($_POST['pma_pw']),
'hostname' => $GLOBALS['dbi']->escapeString($_POST['hostname']),
'port' => (int) $GLOBALS['dbi']->escapeString($_POST['text_port']),
]
);
}
@ -85,7 +93,9 @@ class ReplicationController extends AbstractController
$primaryReplicationHtml = $this->replicationGui->getHtmlForPrimaryReplication(
$_POST['primary_connection'] ?? null,
$params['repl_clear_scr'],
$_POST['primary_add_user'] ?? null
$_POST['primary_add_user'] ?? null,
$_POST['username'] ?? null,
$_POST['hostname'] ?? null
);
}
@ -96,7 +106,8 @@ class ReplicationController extends AbstractController
$replicaConfigurationHtml = $this->replicationGui->getHtmlForReplicaConfiguration(
$_POST['primary_connection'] ?? null,
$replicaInfo['status'],
$replicationInfo->getReplicaStatus()
$replicationInfo->getReplicaStatus(),
isset($_POST['replica_configure'])
);
}

View File

@ -139,7 +139,7 @@ final class TrackingController extends AbstractController
// Export as file download
if (isset($_POST['report_export']) && $_POST['export_type'] === 'sqldumpfile') {
$this->tracking->exportAsFileDownload($GLOBALS['entries']);
$this->tracking->exportAsFileDownload($_POST['table'], $GLOBALS['entries']);
}
$actionMessage = '';

View File

@ -17,7 +17,6 @@ use function mb_strtolower;
use function mb_substr;
use function sprintf;
use function str_replace;
use function strlen;
use function strtok;
use function time;
@ -73,7 +72,9 @@ class ReplicationGui
public function getHtmlForPrimaryReplication(
?string $connection,
?bool $replClearScr,
?string $primaryAddUser
?string $primaryAddUser,
?string $username,
?string $hostname
): string {
if ($replClearScr === null) {
$primaryStatusTable = $this->getHtmlForReplicationStatusTable($connection, 'primary', true, false);
@ -85,7 +86,7 @@ class ReplicationGui
}
if ($primaryAddUser !== null) {
$primaryAddReplicaUser = $this->getHtmlForReplicationPrimaryAddReplicaUser();
$primaryAddReplicaUser = $this->getHtmlForReplicationPrimaryAddReplicaUser($username, $hostname);
}
return $this->template->render('server/replication/primary_replication', [
@ -116,16 +117,18 @@ class ReplicationGui
/**
* returns HTML for replica replication configuration
*
* @param ?string $connection Primary connection
* @param bool $serverReplicaStatus Whether it is Primary or Replica
* @param array $serverReplicaReplication Replica replication
* @param string|null $connection Primary connection
* @param bool $serverReplicaStatus Whether it is Primary or Replica
* @param array $serverReplicaReplication Replica replication
* @param bool $replicaConfigure Replica configure
*
* @return string HTML code
*/
public function getHtmlForReplicaConfiguration(
?string $connection,
$serverReplicaStatus,
array $serverReplicaReplication
array $serverReplicaReplication,
bool $replicaConfigure
): string {
$serverReplicaMultiReplication = $GLOBALS['dbi']->fetchResult('SHOW ALL SLAVES STATUS');
if ($serverReplicaStatus) {
@ -197,7 +200,7 @@ class ReplicationGui
'replica_control_io_link' => $replicaControlIoLink ?? '',
'replica_skip_error_link' => $replicaSkipErrorLink ?? '',
'reconfigure_primary_link' => $reconfigurePrimaryLink ?? '',
'has_replica_configure' => isset($_POST['replica_configure']),
'has_replica_configure' => $replicaConfigure,
]);
}
@ -245,10 +248,10 @@ class ReplicationGui
/**
* This function returns html code for table with replication status.
*
* @param ?string $connection primary connection
* @param string $type either primary or replica
* @param bool $isHidden if true, then default style is set to hidden, default value false
* @param bool $hasTitle if true, then title is displayed, default true
* @param string|null $connection primary connection
* @param string $type either primary or replica
* @param bool $isHidden if true, then default style is set to hidden, default value false
* @param bool $hasTitle if true, then title is displayed, default true
*
* @return string HTML code
*/
@ -356,20 +359,18 @@ class ReplicationGui
*
* @return string HTML code
*/
public function getHtmlForReplicationPrimaryAddReplicaUser(): string
public function getHtmlForReplicationPrimaryAddReplicaUser(?string $postUsername, ?string $hostname): string
{
[
$usernameLength,
$hostnameLength,
] = $this->getUsernameHostnameLength();
if (isset($_POST['username']) && strlen($_POST['username']) === 0) {
$GLOBALS['pred_username'] = 'any';
}
$username = '';
if (! empty($_POST['username'])) {
$username = $GLOBALS['new_username'] ?? $_POST['username'];
if ($postUsername === '') {
$GLOBALS['pred_username'] = 'any';
} elseif ($postUsername !== null && $postUsername !== '0') {
$username = $GLOBALS['new_username'] ?? $postUsername;
}
$currentUser = $GLOBALS['dbi']->fetchValue('SELECT USER();');
@ -388,8 +389,8 @@ class ReplicationGui
}
// when we start editing a user, $GLOBALS['pred_hostname'] is not defined
if (! isset($GLOBALS['pred_hostname']) && isset($_POST['hostname'])) {
switch (mb_strtolower($_POST['hostname'])) {
if (! isset($GLOBALS['pred_hostname']) && $hostname !== null) {
switch (mb_strtolower($hostname)) {
case 'localhost':
case '127.0.0.1':
$GLOBALS['pred_hostname'] = 'localhost';
@ -406,9 +407,9 @@ class ReplicationGui
return $this->template->render('server/replication/primary_add_replica_user', [
'username_length' => $usernameLength,
'hostname_length' => $hostnameLength,
'has_username' => isset($_POST['username']),
'has_username' => $postUsername !== null,
'username' => $username,
'hostname' => $_POST['hostname'] ?? '',
'hostname' => $hostname ?? '',
'predefined_username' => $GLOBALS['pred_username'] ?? '',
'predefined_hostname' => $GLOBALS['pred_hostname'] ?? '',
'this_host' => $thisHost ?? null,
@ -423,7 +424,10 @@ class ReplicationGui
bool $replicaChangePrimary,
bool $srReplicaServerControl,
?string $srReplicaAction,
bool $srReplicaSkipError
bool $srReplicaSkipError,
int $srSkipErrorsCount,
?string $srReplicaControlParam,
array $sr
): void {
if (! $srTakeAction) {
return;
@ -441,9 +445,9 @@ class ReplicationGui
. ' $cfg[\'AllowArbitraryServer\'] in phpMyAdmin configuration.'
);
} elseif ($replicaChangePrimary) {
$result = $this->handleRequestForReplicaChangePrimary();
$result = $this->handleRequestForReplicaChangePrimary($sr);
} elseif ($srReplicaServerControl) {
$result = $this->handleRequestForReplicaServerControl($srReplicaAction);
$result = $this->handleRequestForReplicaServerControl($srReplicaAction, $srReplicaControlParam);
$refresh = true;
switch ($srReplicaAction) {
@ -465,7 +469,7 @@ class ReplicationGui
break;
}
} elseif ($srReplicaSkipError) {
$result = $this->handleRequestForReplicaSkipError();
$result = $this->handleRequestForReplicaSkipError($srSkipErrorsCount);
}
if ($refresh) {
@ -489,15 +493,8 @@ class ReplicationGui
unset($refresh);
}
public function handleRequestForReplicaChangePrimary(): bool
public function handleRequestForReplicaChangePrimary(array $sr): bool
{
$sr = [
'username' => $GLOBALS['dbi']->escapeString($_POST['username']),
'pma_pw' => $GLOBALS['dbi']->escapeString($_POST['pma_pw']),
'hostname' => $GLOBALS['dbi']->escapeString($_POST['hostname']),
'port' => (int) $GLOBALS['dbi']->escapeString($_POST['text_port']),
];
$_SESSION['replication']['m_username'] = $sr['username'];
$_SESSION['replication']['m_password'] = $sr['pma_pw'];
$_SESSION['replication']['m_hostname'] = $sr['hostname'];
@ -559,11 +556,8 @@ class ReplicationGui
return $_SESSION['replication']['sr_action_status'] === 'success';
}
public function handleRequestForReplicaServerControl(?string $srReplicaAction): bool
public function handleRequestForReplicaServerControl(?string $srReplicaAction, ?string $control): bool
{
/** @var string|null $control */
$control = $_POST['sr_replica_control_param'] ?? null;
if ($srReplicaAction === 'reset') {
$qStop = $this->replication->replicaControl('STOP', null, DatabaseInterface::CONNECT_USER);
$qReset = $GLOBALS['dbi']->tryQuery('RESET SLAVE;');
@ -583,15 +577,10 @@ class ReplicationGui
return $result;
}
public function handleRequestForReplicaSkipError(): bool
public function handleRequestForReplicaSkipError(int $srSkipErrorsCount): bool
{
$count = 1;
if (isset($_POST['sr_skip_errors_count'])) {
$count = $_POST['sr_skip_errors_count'] * 1;
}
$qStop = $this->replication->replicaControl('STOP', null, DatabaseInterface::CONNECT_USER);
$qSkip = $GLOBALS['dbi']->tryQuery('SET GLOBAL SQL_SLAVE_SKIP_COUNTER = ' . $count . ';');
$qSkip = $GLOBALS['dbi']->tryQuery('SET GLOBAL SQL_SLAVE_SKIP_COUNTER = ' . $srSkipErrorsCount . ';');
$qStart = $this->replication->replicaControl('START', null, DatabaseInterface::CONNECT_USER);
return $qStop !== false && $qStop !== -1 && $qSkip !== false && $qStart !== false && $qStart !== -1;

View File

@ -837,12 +837,12 @@ class Tracking
*
* @param array $entries entries
*/
public function exportAsFileDownload(array $entries): void
public function exportAsFileDownload(string $table, array $entries): void
{
ini_set('url_rewriter.tags', '');
// Replace all multiple whitespaces by a single space
$table = htmlspecialchars(preg_replace('/\s+/', ' ', $_POST['table']));
$table = htmlspecialchars(preg_replace('/\s+/', ' ', $table));
$dump = '# ' . sprintf(
__('Tracking report for table `%s`'),
$table

View File

@ -5,9 +5,7 @@ declare(strict_types=1);
namespace PhpMyAdmin;
use function __;
use function is_scalar;
use function preg_match;
use function strlen;
/**
* URL redirector to avoid leaking Referer with some sensitive information.
@ -17,7 +15,7 @@ final class UrlRedirector
/**
* @psalm-return never
*/
public static function redirect(): void
public static function redirect(string $url): void
{
// Load database service because services.php is not available here
$GLOBALS['dbi'] = DatabaseInterface::load();
@ -30,9 +28,9 @@ final class UrlRedirector
$response->disable();
if (
! isset($_GET['url']) || ! is_scalar($_GET['url']) || strlen((string) $_GET['url']) === 0
|| ! preg_match('/^https:\/\/[^\n\r]*$/', (string) $_GET['url'])
|| ! Core::isAllowedDomain((string) $_GET['url'])
$url === ''
|| ! preg_match('/^https:\/\/[^\n\r]*$/', $url)
|| ! Core::isAllowedDomain($url)
) {
Core::sendHeaderLocation('./');
@ -47,7 +45,7 @@ final class UrlRedirector
*/
$template = $container->get('template');
echo $template->render('javascript/redirect', [
'url' => (string) $_GET['url'],
'url' => Sanitize::escapeJsString($url),
]);
// Display redirecting msg on screen.
// Do not display the value of $_GET['url'] to avoid showing injected content

View File

@ -55,7 +55,7 @@ class ReplicationGuiTest extends AbstractTestCase
*/
public function testGetHtmlForPrimaryReplication(): void
{
$html = $this->replicationGui->getHtmlForPrimaryReplication(null, null, 'primary_add_user');
$html = $this->replicationGui->getHtmlForPrimaryReplication(null, null, 'primary_add_user', null, null);
//validate 1: Primary replication
$this->assertStringContainsString('<div class="card-header">Primary replication</div>', $html);
@ -98,7 +98,8 @@ class ReplicationGuiTest extends AbstractTestCase
$html = $this->replicationGui->getHtmlForReplicaConfiguration(
null,
true,
$replicationInfo->getReplicaStatus()
$replicationInfo->getReplicaStatus(),
isset($_POST['replica_configure'])
);
//legend