From 1f392e1e1fbec903d790f1f48e8129c8beb64bb4 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Tue, 28 Jan 2025 19:36:25 +0000 Subject: [PATCH 1/9] Add an error instead of triggering warning Signed-off-by: Kamil Tekiela --- src/Dbal/DatabaseInterface.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Dbal/DatabaseInterface.php b/src/Dbal/DatabaseInterface.php index ff2e164c5d..acf5ac617d 100644 --- a/src/Dbal/DatabaseInterface.php +++ b/src/Dbal/DatabaseInterface.php @@ -1603,7 +1603,7 @@ class DatabaseInterface try { $result = $this->extension->connect($server); } catch (ConnectionException $exception) { - trigger_error($exception->getMessage(), E_USER_WARNING); + $errorHandler->addError($exception->getMessage(), E_USER_WARNING, __FILE__, __LINE__); return null; } From bc477b68add6d5a97987cf46960e3f29e5f55245 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Tue, 28 Jan 2025 21:37:33 +0000 Subject: [PATCH 2/9] Refactor AuthenticationConfig Signed-off-by: Kamil Tekiela --- src/Dbal/DatabaseInterface.php | 5 ++++ src/Dbal/DbiExtension.php | 5 ++++ src/Dbal/DbiMysqli.php | 9 +++++++ src/Plugins/Auth/AuthenticationConfig.php | 25 ++++++------------- .../Plugins/Auth/AuthenticationConfigTest.php | 11 -------- tests/unit/Stubs/DbiDummy.php | 8 ++++++ 6 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/Dbal/DatabaseInterface.php b/src/Dbal/DatabaseInterface.php index acf5ac617d..dfd33edb1d 100644 --- a/src/Dbal/DatabaseInterface.php +++ b/src/Dbal/DatabaseInterface.php @@ -1699,6 +1699,11 @@ class DatabaseInterface return $this->extension->getError($this->connections[$connectionType->value]); } + public function getConnectionErrorNumber(): int + { + return $this->extension->getConnectionErrorNumber(); + } + /** * returns last inserted auto_increment id for given $link */ diff --git a/src/Dbal/DbiExtension.php b/src/Dbal/DbiExtension.php index 2c7e4ad33b..3257e3d931 100644 --- a/src/Dbal/DbiExtension.php +++ b/src/Dbal/DbiExtension.php @@ -74,6 +74,11 @@ interface DbiExtension */ public function getError(Connection $connection): string; + /** + * Returns the error code for the most recent connection attempt. + */ + public function getConnectionErrorNumber(): int; + /** * returns the number of rows affected by last query * diff --git a/src/Dbal/DbiMysqli.php b/src/Dbal/DbiMysqli.php index 3965544ca6..35fad40490 100644 --- a/src/Dbal/DbiMysqli.php +++ b/src/Dbal/DbiMysqli.php @@ -16,6 +16,7 @@ use PhpMyAdmin\Query\Utilities; use function __; use function defined; +use function mysqli_connect_errno; use function mysqli_get_client_info; use function mysqli_init; use function mysqli_report; @@ -254,6 +255,14 @@ class DbiMysqli implements DbiExtension return Utilities::formatError($errorNumber, $errorMessage); } + /** + * Returns the error code for the most recent connection attempt. + */ + public function getConnectionErrorNumber(): int + { + return mysqli_connect_errno(); + } + /** * returns the number of rows affected by last query * diff --git a/src/Plugins/Auth/AuthenticationConfig.php b/src/Plugins/Auth/AuthenticationConfig.php index 3a37360b75..b2c44f354e 100644 --- a/src/Plugins/Auth/AuthenticationConfig.php +++ b/src/Plugins/Auth/AuthenticationConfig.php @@ -11,7 +11,6 @@ use PhpMyAdmin\Config; use PhpMyAdmin\Dbal\DatabaseInterface; use PhpMyAdmin\Error\ErrorHandler; use PhpMyAdmin\Exceptions\AuthenticationFailure; -use PhpMyAdmin\Html\Generator; use PhpMyAdmin\Http\Response; use PhpMyAdmin\Plugins\AuthenticationPlugin; use PhpMyAdmin\ResponseRenderer; @@ -23,7 +22,6 @@ use function count; use function ob_get_clean; use function ob_start; use function sprintf; -use function trigger_error; use const E_USER_NOTICE; use const E_USER_WARNING; @@ -70,11 +68,8 @@ class AuthenticationConfig extends AuthenticationPlugin public function showFailure(AuthenticationFailure $failure): Response { $this->logFailure($failure); - - $connError = DatabaseInterface::getInstance()->getError(); - if ($connError === '' || $connError === '0') { - $connError = __('Cannot connect: invalid settings.'); - } + $dbi = DatabaseInterface::getInstance(); + $errorHandler = ErrorHandler::getInstance(); /* HTML header */ $responseRenderer = ResponseRenderer::getInstance(); @@ -97,7 +92,7 @@ class AuthenticationConfig extends AuthenticationPlugin '; $config = Config::getInstance(); if ($failure->failureType === AuthenticationFailure::ALLOW_DENIED) { - trigger_error($failure->getMessage(), E_USER_NOTICE); + $errorHandler->addError($failure->getMessage(), E_USER_NOTICE, __FILE__, __LINE__); } else { // Check whether user has configured something if ($config->sourceMtime == 0) { @@ -110,11 +105,7 @@ class AuthenticationConfig extends AuthenticationPlugin '', '', ) , '

' , "\n"; - } elseif ( - DatabaseInterface::$errorNumber === null - || DatabaseInterface::$errorNumber !== 2002 - && DatabaseInterface::$errorNumber !== 2003 - ) { + } elseif ($dbi->getConnectionErrorNumber() !== 2002 && $dbi->getConnectionErrorNumber() !== 2003) { // if we display the "Server not responding" error, do not confuse // users by telling them they have a settings problem // (note: it's true that they could have a badly typed host name, @@ -122,7 +113,7 @@ class AuthenticationConfig extends AuthenticationPlugin // rejected the connection, which is not really what happened) // 2002 is the error given by mysqli // 2003 is the error given by mysql - trigger_error( + $errorHandler->addError( __( 'phpMyAdmin tried to connect to the MySQL server, and the' . ' server rejected the connection. You should check the' @@ -131,13 +122,13 @@ class AuthenticationConfig extends AuthenticationPlugin . ' by the administrator of the MySQL server.', ), E_USER_WARNING, + __FILE__, + __LINE__, ); } - - echo Generator::mysqlDie($connError, '', true, '', false); } - ErrorHandler::getInstance()->dispUserErrors(); + $errorHandler->dispUserErrors(); echo ' diff --git a/tests/unit/Plugins/Auth/AuthenticationConfigTest.php b/tests/unit/Plugins/Auth/AuthenticationConfigTest.php index 475448d413..1e6788958d 100644 --- a/tests/unit/Plugins/Auth/AuthenticationConfigTest.php +++ b/tests/unit/Plugins/Auth/AuthenticationConfigTest.php @@ -108,17 +108,6 @@ class AuthenticationConfigTest extends AbstractTestCase $html, ); - self::assertStringContainsString( - 'MySQL said: ' . - 'Documentation', - $html, - ); - - self::assertStringContainsString('Cannot connect: invalid settings.', $html); - self::assertStringContainsString( 'Retry to connect', diff --git a/tests/unit/Stubs/DbiDummy.php b/tests/unit/Stubs/DbiDummy.php index 615f7f3f10..4987d804e0 100644 --- a/tests/unit/Stubs/DbiDummy.php +++ b/tests/unit/Stubs/DbiDummy.php @@ -253,6 +253,14 @@ class DbiDummy implements DbiExtension return array_shift($this->fifoErrorCodes) ?? ''; } + /** + * Returns the error code for the most recent connection attempt. + */ + public function getConnectionErrorNumber(): int + { + return 0; + } + /** * returns the number of rows affected by last query * From deffe7858054791e8af68ba6e3cd326c45d7196b Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 29 Jan 2025 17:50:49 +0000 Subject: [PATCH 3/9] Replace trigger_error with addError Signed-off-by: Kamil Tekiela --- src/Dbal/DatabaseInterface.php | 42 ++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/src/Dbal/DatabaseInterface.php b/src/Dbal/DatabaseInterface.php index dfd33edb1d..07021745ab 100644 --- a/src/Dbal/DatabaseInterface.php +++ b/src/Dbal/DatabaseInterface.php @@ -1046,20 +1046,24 @@ class DatabaseInterface . $this->quoteString($currentServer->sessionTimeZone); if (! $this->tryQuery($sqlQueryTz)) { - $errorMessageTz = sprintf( - __( - 'Unable to use timezone "%1$s" for server %2$d. ' - . 'Please check your configuration setting for ' - . '[em]$cfg[\'Servers\'][%3$d][\'SessionTimeZone\'][/em]. ' - . 'phpMyAdmin is currently using the default time zone ' - . 'of the database server.', + $errorHandler = ErrorHandler::getInstance(); + $errorHandler->addError( + sprintf( + __( + 'Unable to use timezone "%1$s" for server %2$d. ' + . 'Please check your configuration setting for ' + . '[em]$cfg[\'Servers\'][%3$d][\'SessionTimeZone\'][/em]. ' + . 'phpMyAdmin is currently using the default time zone ' + . 'of the database server.', + ), + $currentServer->sessionTimeZone, + Current::$server, + Current::$server, ), - $currentServer->sessionTimeZone, - Current::$server, - Current::$server, + E_USER_WARNING, + __FILE__, + __LINE__, ); - - trigger_error($errorMessageTz, E_USER_WARNING); } } @@ -1089,9 +1093,12 @@ class DatabaseInterface ); if ($result === false) { - trigger_error( + $errorHandler = ErrorHandler::getInstance(); + $errorHandler->addError( __('Failed to set configured collation connection!'), E_USER_WARNING, + __FILE__, + __LINE__, ); return; @@ -1621,11 +1628,12 @@ class DatabaseInterface } if ($connectionType === ConnectionType::ControlUser) { - trigger_error( - __( - 'Connection for controluser as defined in your configuration failed.', - ), + $errorHandler = ErrorHandler::getInstance(); + $errorHandler->addError( + __('Connection for controluser as defined in your configuration failed.'), E_USER_WARNING, + __FILE__, + __LINE__, ); } From 48ca6a188e660c97aab3e7c97fd62eee84db9b45 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 29 Jan 2025 18:03:14 +0000 Subject: [PATCH 4/9] Use new mysqli instead of mysqli_init Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 6 ------ src/Dbal/DatabaseInterface.php | 25 +++++-------------------- src/Dbal/DbiExtension.php | 2 +- src/Dbal/DbiMysqli.php | 9 ++------- tests/unit/Stubs/DbiDummy.php | 2 +- 5 files changed, 9 insertions(+), 35 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index a01801c31b..2dbc4a9d85 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -20175,12 +20175,6 @@ parameters: count: 1 path: tests/unit/SqlTest.php - - - message: '#^Method PhpMyAdmin\\Tests\\Stubs\\DbiDummy\:\:connect\(\) never returns null so it can be removed from the return type\.$#' - identifier: return.unusedType - count: 1 - path: tests/unit/Stubs/DbiDummy.php - - message: ''' #^Call to deprecated method getInstance\(\) of class PhpMyAdmin\\Config\: diff --git a/src/Dbal/DatabaseInterface.php b/src/Dbal/DatabaseInterface.php index 07021745ab..747d70c32d 100644 --- a/src/Dbal/DatabaseInterface.php +++ b/src/Dbal/DatabaseInterface.php @@ -60,7 +60,6 @@ use function strtoupper; use function strtr; use function substr; use function syslog; -use function trigger_error; use function uasort; use function uksort; use function usort; @@ -1617,27 +1616,13 @@ class DatabaseInterface $errorHandler->setHideLocation(false); - if ($result !== null) { - $this->connections[$target->value] = $result; - /* Run post connect for user connections */ - if ($target === ConnectionType::User) { - $this->postConnect($currentServer); - } - - return $result; + $this->connections[$target->value] = $result; + /* Run post connect for user connections */ + if ($target === ConnectionType::User) { + $this->postConnect($currentServer); } - if ($connectionType === ConnectionType::ControlUser) { - $errorHandler = ErrorHandler::getInstance(); - $errorHandler->addError( - __('Connection for controluser as defined in your configuration failed.'), - E_USER_WARNING, - __FILE__, - __LINE__, - ); - } - - return null; + return $result; } /** diff --git a/src/Dbal/DbiExtension.php b/src/Dbal/DbiExtension.php index 3257e3d931..dc0f9bc08c 100644 --- a/src/Dbal/DbiExtension.php +++ b/src/Dbal/DbiExtension.php @@ -20,7 +20,7 @@ interface DbiExtension * * @throws ConnectionException */ - public function connect(Server $server): Connection|null; + public function connect(Server $server): Connection; /** * selects given database diff --git a/src/Dbal/DbiMysqli.php b/src/Dbal/DbiMysqli.php index 35fad40490..17a31f6e45 100644 --- a/src/Dbal/DbiMysqli.php +++ b/src/Dbal/DbiMysqli.php @@ -18,7 +18,6 @@ use function __; use function defined; use function mysqli_connect_errno; use function mysqli_get_client_info; -use function mysqli_init; use function mysqli_report; use function sprintf; use function str_contains; @@ -40,15 +39,11 @@ use const MYSQLI_USE_RESULT; */ class DbiMysqli implements DbiExtension { - public function connect(Server $server): Connection|null + public function connect(Server $server): Connection { mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT); - $mysqli = mysqli_init(); - - if ($mysqli === false) { - return null; - } + $mysqli = new mysqli(); $clientFlags = 0; diff --git a/tests/unit/Stubs/DbiDummy.php b/tests/unit/Stubs/DbiDummy.php index 4987d804e0..35ba27a653 100644 --- a/tests/unit/Stubs/DbiDummy.php +++ b/tests/unit/Stubs/DbiDummy.php @@ -92,7 +92,7 @@ class DbiDummy implements DbiExtension $this->init(); } - public function connect(Server $server): Connection|null + public function connect(Server $server): Connection { return new Connection(new stdClass()); } From a687b114ccb48b1ccfca75fda2125bd23bb2803f Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 29 Jan 2025 18:27:20 +0000 Subject: [PATCH 5/9] Create addNotice and addUserError Signed-off-by: Kamil Tekiela --- src/Core.php | 3 +-- src/Dbal/DatabaseInterface.php | 15 +++----------- src/Error/ErrorHandler.php | 24 +++++++++++++++++++++++ src/Plugins/Auth/AuthenticationConfig.php | 10 ++-------- 4 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/Core.php b/src/Core.php index e1666e55df..4c4f0c01e6 100644 --- a/src/Core.php +++ b/src/Core.php @@ -45,7 +45,6 @@ use function unserialize; use function urldecode; use const DATE_RFC1123; -use const E_USER_WARNING; use const FILTER_VALIDATE_IP; /** @@ -115,7 +114,7 @@ class Core throw new MissingExtensionException(Sanitize::convertBBCode($message)); } - ErrorHandler::getInstance()->addError($message, E_USER_WARNING, '', 0, false); + ErrorHandler::getInstance()->addUserError($message, false); } /** diff --git a/src/Dbal/DatabaseInterface.php b/src/Dbal/DatabaseInterface.php index 747d70c32d..79505f98e1 100644 --- a/src/Dbal/DatabaseInterface.php +++ b/src/Dbal/DatabaseInterface.php @@ -64,7 +64,6 @@ use function uasort; use function uksort; use function usort; -use const E_USER_WARNING; use const LOG_INFO; use const LOG_NDELAY; use const LOG_PID; @@ -1046,7 +1045,7 @@ class DatabaseInterface if (! $this->tryQuery($sqlQueryTz)) { $errorHandler = ErrorHandler::getInstance(); - $errorHandler->addError( + $errorHandler->addUserError( sprintf( __( 'Unable to use timezone "%1$s" for server %2$d. ' @@ -1059,9 +1058,6 @@ class DatabaseInterface Current::$server, Current::$server, ), - E_USER_WARNING, - __FILE__, - __LINE__, ); } } @@ -1093,12 +1089,7 @@ class DatabaseInterface if ($result === false) { $errorHandler = ErrorHandler::getInstance(); - $errorHandler->addError( - __('Failed to set configured collation connection!'), - E_USER_WARNING, - __FILE__, - __LINE__, - ); + $errorHandler->addUserError(__('Failed to set configured collation connection!')); return; } @@ -1609,7 +1600,7 @@ class DatabaseInterface try { $result = $this->extension->connect($server); } catch (ConnectionException $exception) { - $errorHandler->addError($exception->getMessage(), E_USER_WARNING, __FILE__, __LINE__); + $errorHandler->addUserError($exception->getMessage()); return null; } diff --git a/src/Error/ErrorHandler.php b/src/Error/ErrorHandler.php index 82bcb31329..ff8666a18e 100644 --- a/src/Error/ErrorHandler.php +++ b/src/Error/ErrorHandler.php @@ -314,6 +314,30 @@ class ErrorHandler } } + public function addUserError(string $message, bool $escape = true): void + { + // The file name and line number are not relevant for user errors + $error = new Error( + E_USER_WARNING, + $escape ? htmlspecialchars($message) : $message, + __FILE__, + __LINE__, + ); + $this->errors[$error->getHash()] = $error; + } + + public function addNotice(string $message, bool $escape = true): void + { + // The file name and line number are not relevant for user errors + $error = new Error( + E_USER_NOTICE, + $escape ? htmlspecialchars($message) : $message, + __FILE__, + __LINE__, + ); + $this->errors[$error->getHash()] = $error; + } + /** * display fatal error and exit * diff --git a/src/Plugins/Auth/AuthenticationConfig.php b/src/Plugins/Auth/AuthenticationConfig.php index b2c44f354e..7f48a6ec82 100644 --- a/src/Plugins/Auth/AuthenticationConfig.php +++ b/src/Plugins/Auth/AuthenticationConfig.php @@ -23,9 +23,6 @@ use function ob_get_clean; use function ob_start; use function sprintf; -use const E_USER_NOTICE; -use const E_USER_WARNING; - /** * Handles the config authentication method */ @@ -92,7 +89,7 @@ class AuthenticationConfig extends AuthenticationPlugin '; $config = Config::getInstance(); if ($failure->failureType === AuthenticationFailure::ALLOW_DENIED) { - $errorHandler->addError($failure->getMessage(), E_USER_NOTICE, __FILE__, __LINE__); + $errorHandler->addNotice($failure->getMessage()); } else { // Check whether user has configured something if ($config->sourceMtime == 0) { @@ -113,7 +110,7 @@ class AuthenticationConfig extends AuthenticationPlugin // rejected the connection, which is not really what happened) // 2002 is the error given by mysqli // 2003 is the error given by mysql - $errorHandler->addError( + $errorHandler->addUserError( __( 'phpMyAdmin tried to connect to the MySQL server, and the' . ' server rejected the connection. You should check the' @@ -121,9 +118,6 @@ class AuthenticationConfig extends AuthenticationPlugin . ' make sure that they correspond to the information given' . ' by the administrator of the MySQL server.', ), - E_USER_WARNING, - __FILE__, - __LINE__, ); } } From ec6e1634c516d5f9bd953a9ed4158318c1469237 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 29 Jan 2025 18:47:01 +0000 Subject: [PATCH 6/9] Don't display Backtrace if there isn't one Signed-off-by: Kamil Tekiela --- resources/templates/error/get_display.twig | 2 +- src/Error/Error.php | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/resources/templates/error/get_display.twig b/resources/templates/error/get_display.twig index 81a555a0e9..1eb7fc547d 100644 --- a/resources/templates/error/get_display.twig +++ b/resources/templates/error/get_display.twig @@ -5,7 +5,7 @@ {{ message|raw }} - {%- if not is_user_error -%} + {%- if not is_user_error and formatted_backtrace is not empty -%}

Backtrace

{{- formatted_backtrace|raw -}} {%- endif -%} diff --git a/src/Error/Error.php b/src/Error/Error.php index a750701ff9..afe97ca957 100644 --- a/src/Error/Error.php +++ b/src/Error/Error.php @@ -326,6 +326,10 @@ class Error extends Message */ public static function formatBacktrace(array $backtrace): string { + if ($backtrace === []) { + return ''; + } + $retval = '
    '; foreach ($backtrace as $step) { From a087777e7480b519da6029b8bc9e7cab992b3801 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 29 Jan 2025 19:41:06 +0000 Subject: [PATCH 7/9] Convert more trigger_error Signed-off-by: Kamil Tekiela --- src/Config/FormDisplay.php | 6 ++---- src/Navigation/NavigationTree.php | 7 ++----- src/Routing/Routing.php | 6 ++---- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/Config/FormDisplay.php b/src/Config/FormDisplay.php index 326a695857..bc16adf814 100644 --- a/src/Config/FormDisplay.php +++ b/src/Config/FormDisplay.php @@ -16,6 +16,7 @@ namespace PhpMyAdmin\Config; use PhpMyAdmin\Config; use PhpMyAdmin\Config\Forms\User\UserFormList; +use PhpMyAdmin\Error\ErrorHandler; use PhpMyAdmin\Html\MySQLDocumentation; use PhpMyAdmin\Util; @@ -37,11 +38,8 @@ use function sprintf; use function str_ends_with; use function str_replace; use function str_starts_with; -use function trigger_error; use function trim; -use const E_USER_WARNING; - /** * Form management class, displays and processes forms */ @@ -391,7 +389,7 @@ class FormDisplay return $htmlOutput; case 'NULL': - trigger_error('Field ' . $systemPath . ' has no type', E_USER_WARNING); + ErrorHandler::getInstance()->addUserError('Field ' . $systemPath . ' has no type'); return null; } diff --git a/src/Navigation/NavigationTree.php b/src/Navigation/NavigationTree.php index 1235add74a..cb580fe280 100644 --- a/src/Navigation/NavigationTree.php +++ b/src/Navigation/NavigationTree.php @@ -12,6 +12,7 @@ use PhpMyAdmin\ConfigStorage\Relation; use PhpMyAdmin\ConfigStorage\RelationParameters; use PhpMyAdmin\Current; use PhpMyAdmin\Dbal\DatabaseInterface; +use PhpMyAdmin\Error\ErrorHandler; use PhpMyAdmin\Favorites\RecentFavoriteTables; use PhpMyAdmin\Favorites\TableType; use PhpMyAdmin\Html\Generator; @@ -67,12 +68,9 @@ use function strnatcasecmp; use function strrpos; use function strstr; use function substr; -use function trigger_error; use function trim; use function usort; -use const E_USER_WARNING; - /** * Displays a collapsible of database objects in the navigation frame */ @@ -739,13 +737,12 @@ class NavigationTree foreach ($prefixes as $key => $value) { // warn about large groups if ($value > 500 && ! $this->largeGroupWarning) { - trigger_error( + ErrorHandler::getInstance()->addUserError( __( 'There are large item groups in navigation panel which ' . 'may affect the performance. Consider disabling item ' . 'grouping in the navigation panel.', ), - E_USER_WARNING, ); $this->largeGroupWarning = true; } diff --git a/src/Routing/Routing.php b/src/Routing/Routing.php index f4e7a82e33..052cd5d846 100644 --- a/src/Routing/Routing.php +++ b/src/Routing/Routing.php @@ -18,6 +18,7 @@ use PhpMyAdmin\Controllers\Setup\ShowConfigController; use PhpMyAdmin\Controllers\Setup\ValidateController; use PhpMyAdmin\Core; use PhpMyAdmin\Current; +use PhpMyAdmin\Error\ErrorHandler; use PhpMyAdmin\Http\Factory\ResponseFactory; use PhpMyAdmin\Http\Response; use PhpMyAdmin\Http\ServerRequest; @@ -42,12 +43,10 @@ use function mb_strrpos; use function mb_substr; use function rawurldecode; use function sprintf; -use function trigger_error; use function urldecode; use function var_export; use const CACHE_DIR; -use const E_USER_WARNING; /** * Class used to warm up the routing cache and manage routing. @@ -119,7 +118,7 @@ class Routing && ! self::writeCache(sprintf('addUserError( sprintf( __( 'The routing cache could not be written, ' @@ -127,7 +126,6 @@ class Routing ), self::ROUTES_CACHE_FILE, ), - E_USER_WARNING, ); } } From 73ff5a43239432cec4cfb97c01c98cfe7380f1de Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 29 Jan 2025 19:57:20 +0000 Subject: [PATCH 8/9] Replace E_USER_ERROR with exception Both will stop processing the current code. Signed-off-by: Kamil Tekiela --- src/Plugins/Export/ExportSql.php | 16 +++------------- tests/unit/Plugins/Export/ExportSqlTest.php | 16 ++++++++-------- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/src/Plugins/Export/ExportSql.php b/src/Plugins/Export/ExportSql.php index 4369f93bad..8de59cdb66 100644 --- a/src/Plugins/Export/ExportSql.php +++ b/src/Plugins/Export/ExportSql.php @@ -15,6 +15,7 @@ use PhpMyAdmin\Database\Events; use PhpMyAdmin\Database\Routines; use PhpMyAdmin\Dbal\ConnectionType; use PhpMyAdmin\Dbal\DatabaseInterface; +use PhpMyAdmin\Exceptions\ExportException; use PhpMyAdmin\Export\Export; use PhpMyAdmin\Export\StructureOrData; use PhpMyAdmin\Http\ServerRequest; @@ -44,7 +45,6 @@ use PhpMyAdmin\Version; use function __; use function array_keys; use function bin2hex; -use function defined; use function explode; use function implode; use function in_array; @@ -62,9 +62,7 @@ use function str_contains; use function str_repeat; use function str_replace; use function strtoupper; -use function trigger_error; -use const E_USER_ERROR; use const PHP_VERSION; /** @@ -1419,11 +1417,8 @@ class ExportSql extends ExportPlugin if ($tmpError !== '') { $message = sprintf(__('Error reading structure for table %s:'), $db . '.' . $table); $message .= ' ' . $tmpError; - if (! defined('TESTSUITE')) { - trigger_error($message, E_USER_ERROR); - } - return $this->exportComment($message); + throw new ExportException($message); } // Old mode is stored so it can be restored once exporting is done. @@ -2050,13 +2045,8 @@ class ExportSql extends ExportPlugin if ($tmpError !== '') { $message = sprintf(__('Error reading data for table %s:'), $db . '.' . $table); $message .= ' ' . $tmpError; - if (! defined('TESTSUITE')) { - trigger_error($message, E_USER_ERROR); - } - return $this->export->outputHandler( - $this->exportComment($message), - ); + throw new ExportException($message); } if ($result === false) { diff --git a/tests/unit/Plugins/Export/ExportSqlTest.php b/tests/unit/Plugins/Export/ExportSqlTest.php index e6e7f9db60..67094ca99b 100644 --- a/tests/unit/Plugins/Export/ExportSqlTest.php +++ b/tests/unit/Plugins/Export/ExportSqlTest.php @@ -11,6 +11,7 @@ use PhpMyAdmin\ConfigStorage\RelationParameters; use PhpMyAdmin\Current; use PhpMyAdmin\Dbal\ConnectionType; use PhpMyAdmin\Dbal\DatabaseInterface; +use PhpMyAdmin\Exceptions\ExportException; use PhpMyAdmin\Export\Export; use PhpMyAdmin\Http\Factory\ServerRequestFactory; use PhpMyAdmin\Plugins\Export\ExportSql; @@ -855,11 +856,13 @@ SQL; $this->object->setExportOptions($request, []); - $result = $this->object->getTableDef('db', 'table', true, false); + $this->expectException(ExportException::class); + $this->expectExceptionMessage('Error reading structure for table db.table: error occurred'); + + $this->object->getTableDef('db', 'table', true, false); $dbiDummy->assertAllQueriesConsumed(); $dbiDummy->assertAllErrorCodesConsumed(); - self::assertStringContainsString('-- Error reading structure for table db.table: error occurred', $result); } public function testGetTableComments(): void @@ -1257,15 +1260,12 @@ SQL; $this->object->setExportOptions($request, []); - ob_start(); + $this->expectException(ExportException::class); + $this->expectExceptionMessage('Error reading data for table db.table: err'); + self::assertTrue( $this->object->exportData('db', 'table', 'SELECT'), ); - $result = ob_get_clean(); - - self::assertIsString($result); - - self::assertStringContainsString('-- Error reading data for table db.table: err', $result); } public function testMakeCreateTableMSSQLCompatible(): void From c76d1640e875577e722e518be892a5c3e76bdbcd Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 29 Jan 2025 20:18:44 +0000 Subject: [PATCH 9/9] Refactor handleError We no longer expect any E_USER_* errors to be triggered by PMA code. Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 6 ------ src/Error/ErrorHandler.php | 40 ++++++++++++++------------------------ 2 files changed, 15 insertions(+), 31 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 2dbc4a9d85..2e6b7c3e01 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -6981,12 +6981,6 @@ parameters: count: 1 path: src/Error/ErrorHandler.php - - - message: '#^Loose comparison via "\!\=" is not allowed\.$#' - identifier: notEqual.notAllowed - count: 1 - path: src/Error/ErrorHandler.php - - message: '#^Parameter \#1 \$value of function count expects array\|Countable, mixed given\.$#' identifier: argument.type diff --git a/src/Error/ErrorHandler.php b/src/Error/ErrorHandler.php index ff8666a18e..bf3c52a0e6 100644 --- a/src/Error/ErrorHandler.php +++ b/src/Error/ErrorHandler.php @@ -197,34 +197,24 @@ class ErrorHandler string $errfile, int $errline, ): bool { - if (function_exists('error_reporting')) { - /** - * Check if Error Control Operator (@) was used, but still show - * user errors even in this case. - * See: https://github.com/phpmyadmin/phpmyadmin/issues/16729 - */ - $isSilenced = (error_reporting() & $errno) === 0; - - $config = Config::getInstance(); - if ( - $config->config->environment === 'development' - && ! $isSilenced - ) { - throw new ErrorException($errstr, 0, $errno, $errfile, $errline); - } - - if ( - $isSilenced && - $this->errorReporting != 0 && - ($errno & (E_USER_WARNING | E_USER_ERROR | E_USER_NOTICE | E_USER_DEPRECATED)) === 0 - ) { - return false; - } - } elseif (($errno & (E_USER_WARNING | E_USER_ERROR | E_USER_NOTICE | E_USER_DEPRECATED)) === 0) { + if (! function_exists('error_reporting')) { return false; } - $this->addError($errstr, $errno, $errfile, $errline); + /** + * Check if Error Control Operator (@) was used. + * See: https://github.com/phpmyadmin/phpmyadmin/issues/16729 + */ + $isSilenced = (error_reporting() & $errno) === 0; + + $config = Config::getInstance(); + if ($config->config->environment === 'development' && ! $isSilenced) { + throw new ErrorException($errstr, 0, $errno, $errfile, $errline); + } + + if (! $isSilenced || $this->errorReporting === 0) { + $this->addError($errstr, $errno, $errfile, $errline); + } return false; }