From e447622a3d334832265bec884ea6b759343bdada Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Sat, 6 Apr 2024 17:12:35 +0200 Subject: [PATCH 1/5] Refactor handleRollbackRequest() Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 5 ----- psalm-baseline.xml | 1 - src/Controllers/Import/ImportController.php | 6 ++--- src/Import/Import.php | 25 +++++---------------- 4 files changed, 8 insertions(+), 29 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 86fd4d008a..b1c38b9e72 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -8545,11 +8545,6 @@ parameters: count: 1 path: src/Import/Import.php - - - message: "#^Only booleans are allowed in an if condition, string\\|false given\\.$#" - count: 1 - path: src/Import/Import.php - - message: "#^Parameter \\#1 \\$precision of static method PhpMyAdmin\\\\Import\\\\DecimalSize\\:\\:fromPrecisionAndScale\\(\\) expects int, int\\|PhpMyAdmin\\\\Import\\\\DecimalSize given\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 3fb7fe2d7f..7861285346 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -6680,7 +6680,6 @@ - diff --git a/src/Controllers/Import/ImportController.php b/src/Controllers/Import/ImportController.php index 1c405f73fa..aad79613f2 100644 --- a/src/Controllers/Import/ImportController.php +++ b/src/Controllers/Import/ImportController.php @@ -712,12 +712,10 @@ final class ImportController extends AbstractController } // If there is request for ROLLBACK in the end. - if (! $request->hasBodyParam('rollback_query')) { - return null; + if ($request->hasBodyParam('rollback_query')) { + $this->dbi->query('ROLLBACK'); } - $this->dbi->query('ROLLBACK'); - return null; } } diff --git a/src/Import/Import.php b/src/Import/Import.php index 19a20a8d2b..fa6bcd7b6f 100644 --- a/src/Import/Import.php +++ b/src/Import/Import.php @@ -928,20 +928,10 @@ class Import ImportSettings::$importNotice = $message; } - /** - * Handles request for ROLLBACK. - * - * @param string $sqlQuery SQL query(s) - */ public function handleRollbackRequest(string $sqlQuery): void { $sqlDelimiter = $_POST['sql_delimiter']; $queries = explode($sqlDelimiter, $sqlQuery); - $error = false; - $errorMsg = __( - 'Only INSERT, UPDATE, DELETE and REPLACE ' - . 'SQL queries containing transactional engine tables can be rolled back.', - ); $dbi = DatabaseInterface::getInstance(); foreach ($queries as $sqlQuery) { if ($sqlQuery === '') { @@ -953,17 +943,14 @@ class Import continue; } - $globalError = $dbi->getError(); - $error = $globalError !== '' ? $globalError : $errorMsg; + $sqlError = $dbi->getError(); + $error = $sqlError !== '' ? $sqlError : __( + 'Only INSERT, UPDATE, DELETE and REPLACE ' + . 'SQL queries containing transactional engine tables can be rolled back.', + ); - break; - } - - if ($error) { - unset($_POST['rollback_query']); $response = ResponseRenderer::getInstance(); - $message = Message::rawError($error); - $response->addJSON('message', $message); + $response->addJSON('message', Message::rawError($error)); $response->callExit(); } From b8dd3a4f0980d7625b974cb2c1b9d93ebbcf47e8 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Sat, 6 Apr 2024 17:42:27 +0200 Subject: [PATCH 2/5] Move clean up earlier Signed-off-by: Kamil Tekiela --- src/Controllers/Import/ImportController.php | 12 +++++++----- src/Import/Import.php | 7 ------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/Controllers/Import/ImportController.php b/src/Controllers/Import/ImportController.php index aad79613f2..2855036ad1 100644 --- a/src/Controllers/Import/ImportController.php +++ b/src/Controllers/Import/ImportController.php @@ -682,6 +682,13 @@ final class ImportController extends AbstractController return null; } + if ($request->hasBodyParam('rollback_query')) { + // We rollback because there might be other queries that need to be executed after this, + // e.g. creation of a bookmark. + $this->dbi->query('ROLLBACK'); + ImportSettings::$message .= __('[ROLLBACK occurred.]'); + } + if ($GLOBALS['result']) { // Save a Bookmark with more than one queries (if Bookmark label given). if (! empty($request->getParsedBodyParam('bkm_label')) && ! empty($GLOBALS['import_text'])) { @@ -711,11 +718,6 @@ final class ImportController extends AbstractController include ROOT_PATH . $GLOBALS['goto']; } - // If there is request for ROLLBACK in the end. - if ($request->hasBodyParam('rollback_query')) { - $this->dbi->query('ROLLBACK'); - } - return null; } } diff --git a/src/Import/Import.php b/src/Import/Import.php index fa6bcd7b6f..dfef41c653 100644 --- a/src/Import/Import.php +++ b/src/Import/Import.php @@ -240,13 +240,6 @@ class Import // Do we have something to push into buffer? $this->importRunBuffer = $sql !== '' ? $sql . ';' : null; - - // In case of ROLLBACK, notify the user. - if (! isset($_POST['rollback_query'])) { - return; - } - - ImportSettings::$message .= __('[ROLLBACK occurred.]'); } /** From 48a8dae59d716ee7c18922496b8d8b58fcd5a3e4 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Sat, 6 Apr 2024 18:19:47 +0200 Subject: [PATCH 3/5] Fix truncated SQL in "Create PHP code" Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 2 +- src/Html/Generator.php | 22 +++------------------- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index b1c38b9e72..7870359059 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -8342,7 +8342,7 @@ parameters: - message: "#^Parameter \\#1 \\$string of function mb_substr expects string, mixed given\\.$#" - count: 2 + count: 1 path: src/Html/Generator.php - diff --git a/src/Html/Generator.php b/src/Html/Generator.php index 75b81a8fda..dff1d8aece 100644 --- a/src/Html/Generator.php +++ b/src/Html/Generator.php @@ -455,34 +455,19 @@ class Generator $retval .= ' ' . $message->getMessage() . "\n"; $retval .= '' . "\n"; - $queryTooBig = false; - - $queryLength = mb_strlen($sqlQuery); - if ($queryLength > $config->settings['MaxCharactersInDisplayedSQL']) { - // when the query is large (for example an INSERT of binary - // data), the parser chokes; so avoid parsing the query - $queryTooBig = true; - $queryBase = mb_substr($sqlQuery, 0, $config->settings['MaxCharactersInDisplayedSQL']) . '[...]'; - } else { - $queryBase = $sqlQuery; - } - // Html format the query to be displayed // If we want to show some sql code it is easiest to create it here /* SQL-Parser-Analyzer */ if (! empty($GLOBALS['show_as_php'])) { $newLine = '\\n"
' . "\n" . '    . "'; - $queryBase = htmlspecialchars(addslashes($queryBase)); + $queryBase = htmlspecialchars(addslashes($sqlQuery)); $queryBase = preg_replace('/((\015\012)|(\015)|(\012))/', $newLine, $queryBase); $queryBase = '
' . "\n"
                 . '$sql = "' . $queryBase . '";' . "\n"
                 . '
'; - } elseif ($queryTooBig) { - $queryBase = '
' . "\n"
-                . htmlspecialchars($queryBase, ENT_COMPAT) . '
'; } else { - $queryBase = self::formatSql($queryBase); + $queryBase = self::formatSql($sqlQuery, true); } // Prepares links that may be displayed to edit/explain the query @@ -507,6 +492,7 @@ class Generator // but only explain a SELECT (that has not been explained) /* SQL-Parser-Analyzer */ $explainLink = ''; + $queryTooBig = mb_strlen($sqlQuery) > $config->settings['MaxCharactersInDisplayedSQL']; $isSelect = preg_match('@^SELECT[[:space:]]+@i', $sqlQuery); if (! empty($config->settings['SQLQuery']['Explain']) && ! $queryTooBig) { $explainParams = $urlParams; @@ -1085,8 +1071,6 @@ class Generator * @param bool $truncate truncate the query if it is too long * * @return string the formatted sql - * - * @global array $cfg the configuration array */ public static function formatSql(string $sqlQuery, bool $truncate = false): string { From a0c6b569f02c5664afc4c18157d4ea2246eed69b Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Sat, 6 Apr 2024 18:30:23 +0200 Subject: [PATCH 4/5] Cast globals to string to make PHPStan happy Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 37 ++++++------------------------------- psalm-baseline.xml | 5 +++++ src/Html/Generator.php | 8 ++++---- 3 files changed, 15 insertions(+), 35 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 7870359059..c23f71b71e 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -8280,9 +8280,14 @@ parameters: count: 5 path: src/Html/Generator.php + - + message: "#^Cannot cast mixed to string\\.$#" + count: 3 + path: src/Html/Generator.php + - message: "#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#" - count: 16 + count: 15 path: src/Html/Generator.php - @@ -8315,46 +8320,16 @@ parameters: count: 2 path: src/Html/Generator.php - - - message: "#^Parameter \\#1 \\$sqlQuery of static method PhpMyAdmin\\\\Html\\\\Generator\\:\\:formatSql\\(\\) expects string, mixed given\\.$#" - count: 1 - path: src/Html/Generator.php - - - - message: "#^Parameter \\#1 \\$string of function addslashes expects string, mixed given\\.$#" - count: 1 - path: src/Html/Generator.php - - message: "#^Parameter \\#1 \\$string of function htmlspecialchars expects string, \\(int\\|string\\) given\\.$#" count: 1 path: src/Html/Generator.php - - - message: "#^Parameter \\#1 \\$string of function htmlspecialchars expects string, mixed given\\.$#" - count: 1 - path: src/Html/Generator.php - - - - message: "#^Parameter \\#1 \\$string of function mb_strlen expects string, mixed given\\.$#" - count: 1 - path: src/Html/Generator.php - - - - message: "#^Parameter \\#1 \\$string of function mb_substr expects string, mixed given\\.$#" - count: 1 - path: src/Html/Generator.php - - message: "#^Parameter \\#1 \\$string of function trim expects string, mixed given\\.$#" count: 1 path: src/Html/Generator.php - - - message: "#^Parameter \\#2 \\$subject of function preg_match expects string, mixed given\\.$#" - count: 3 - path: src/Html/Generator.php - - message: "#^Parameter \\#2 \\.\\.\\.\\$values of function sprintf expects bool\\|float\\|int\\|string\\|null, mixed given\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 7861285346..694d7748db 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -6551,6 +6551,11 @@ + + + + + diff --git a/src/Html/Generator.php b/src/Html/Generator.php index dff1d8aece..6edb9896e8 100644 --- a/src/Html/Generator.php +++ b/src/Html/Generator.php @@ -407,18 +407,18 @@ class Generator if ($sqlQuery === null) { if (! empty($GLOBALS['display_query'])) { - $sqlQuery = $GLOBALS['display_query']; + $sqlQuery = (string) $GLOBALS['display_query']; } elseif (! empty($GLOBALS['unparsed_sql'])) { - $sqlQuery = $GLOBALS['unparsed_sql']; + $sqlQuery = (string) $GLOBALS['unparsed_sql']; } elseif (! empty($GLOBALS['sql_query'])) { - $sqlQuery = $GLOBALS['sql_query']; + $sqlQuery = (string) $GLOBALS['sql_query']; } else { $sqlQuery = ''; } } $config = Config::getInstance(); - $renderSql = $config->settings['ShowSQL'] == true && ! empty($sqlQuery) && $sqlQuery !== ';'; + $renderSql = $config->settings['ShowSQL'] == true && $sqlQuery !== '' && $sqlQuery !== ';'; if (isset($GLOBALS['using_bookmark_message'])) { $retval .= $GLOBALS['using_bookmark_message']->getDisplay(); From cfa9beaa7fb1292401a6c5dd1fb22e57a70d3b64 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Sat, 6 Apr 2024 18:37:05 +0200 Subject: [PATCH 5/5] Drop $GLOBALS['unparsed_sql'] Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 4 ++-- psalm-baseline.xml | 2 -- psalm.xml | 1 - src/Html/Generator.php | 2 -- src/ParseAnalyze.php | 3 --- tests/unit/Html/GeneratorTest.php | 2 -- tests/unit/ParseAnalyzeTest.php | 8 -------- 7 files changed, 2 insertions(+), 20 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index c23f71b71e..7840df0d8c 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -8282,12 +8282,12 @@ parameters: - message: "#^Cannot cast mixed to string\\.$#" - count: 3 + count: 2 path: src/Html/Generator.php - message: "#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#" - count: 15 + count: 14 path: src/Html/Generator.php - diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 694d7748db..a74993b366 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -6554,7 +6554,6 @@ - @@ -6566,7 +6565,6 @@ - diff --git a/psalm.xml b/psalm.xml index 992652ebab..a1b5ac3433 100644 --- a/psalm.xml +++ b/psalm.xml @@ -111,7 +111,6 @@ tbl_storage_engine: string, token_mismatch: bool, token_provided: bool, - unparsed_sql?: string, urlParams: array, username: string, xml_export_triggers: bool, diff --git a/src/Html/Generator.php b/src/Html/Generator.php index 6edb9896e8..01f8f8997c 100644 --- a/src/Html/Generator.php +++ b/src/Html/Generator.php @@ -408,8 +408,6 @@ class Generator if ($sqlQuery === null) { if (! empty($GLOBALS['display_query'])) { $sqlQuery = (string) $GLOBALS['display_query']; - } elseif (! empty($GLOBALS['unparsed_sql'])) { - $sqlQuery = (string) $GLOBALS['unparsed_sql']; } elseif (! empty($GLOBALS['sql_query'])) { $sqlQuery = (string) $GLOBALS['sql_query']; } else { diff --git a/src/ParseAnalyze.php b/src/ParseAnalyze.php index 948d4a8d35..61e88e3b65 100644 --- a/src/ParseAnalyze.php +++ b/src/ParseAnalyze.php @@ -26,9 +26,6 @@ class ParseAnalyze */ public static function sqlQuery(string $sqlQuery, string $db): array { - // @todo: move to returned results (also in all the calling chain) - $GLOBALS['unparsed_sql'] = $sqlQuery; - $info = Query::getAll($sqlQuery); $table = ''; diff --git a/tests/unit/Html/GeneratorTest.php b/tests/unit/Html/GeneratorTest.php index 97badffb91..390deba9b4 100644 --- a/tests/unit/Html/GeneratorTest.php +++ b/tests/unit/Html/GeneratorTest.php @@ -476,7 +476,6 @@ class GeneratorTest extends AbstractTestCase { Config::getInstance()->settings['ShowSQL'] = true; $GLOBALS['display_query'] = null; - $GLOBALS['unparsed_sql'] = null; $GLOBALS['sql_query'] = 'SELECT 1;'; $usingBookmarkMessage = Message::notice('Bookmark message'); $GLOBALS['using_bookmark_message'] = $usingBookmarkMessage; @@ -533,7 +532,6 @@ HTML; $config->settings['SQLQuery']['Edit'] = false; $config->settings['SQLQuery']['Refresh'] = true; $GLOBALS['display_query'] = 'EXPLAIN SELECT 1;'; - $GLOBALS['unparsed_sql'] = null; $GLOBALS['sql_query'] = null; DatabaseInterface::$instance = $this->createDatabaseInterface(); Current::$database = 'test_db'; diff --git a/tests/unit/ParseAnalyzeTest.php b/tests/unit/ParseAnalyzeTest.php index 0de673be2d..8019facfdb 100644 --- a/tests/unit/ParseAnalyzeTest.php +++ b/tests/unit/ParseAnalyzeTest.php @@ -24,12 +24,8 @@ class ParseAnalyzeTest extends AbstractTestCase $GLOBALS['lang'] = 'en'; ResponseRenderer::getInstance()->setAjax(false); - $GLOBALS['unparsed_sql'] = ''; - $actual = ParseAnalyze::sqlQuery('SELECT * FROM `sakila`.`actor`', 'sakila_test'); - /** @psalm-suppress TypeDoesNotContainType */ - self::assertSame('SELECT * FROM `sakila`.`actor`', $GLOBALS['unparsed_sql']); self::assertSame('sakila', $actual[1]); self::assertSame('actor', $actual[2]); self::assertTrue($actual[0]->flags->reload); @@ -44,12 +40,8 @@ class ParseAnalyzeTest extends AbstractTestCase $GLOBALS['lang'] = 'en'; ResponseRenderer::getInstance()->setAjax(false); - $GLOBALS['unparsed_sql'] = ''; - $actual = ParseAnalyze::sqlQuery('SELECT `first_name`, `title` FROM `actor`, `film`', 'sakila'); - /** @psalm-suppress TypeDoesNotContainType */ - self::assertSame('SELECT `first_name`, `title` FROM `actor`, `film`', $GLOBALS['unparsed_sql']); self::assertSame('sakila', $actual[1]); self::assertSame('', $actual[2]); self::assertFalse($actual[0]->flags->reload);