From a2a35c19de86cbd15a0088fc58f5cc7533e78bae Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 19 Apr 2023 22:50:12 +0100 Subject: [PATCH 01/15] Split up loadFirstRow Signed-off-by: Kamil Tekiela --- libraries/classes/InsertEdit.php | 22 +++++++-------- psalm-baseline.xml | 1 + test/classes/InsertEditTest.php | 47 +++++++++++++++++++------------- 3 files changed, 39 insertions(+), 31 deletions(-) diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index 8dfdf059c1..8572a97fbe 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -244,23 +244,20 @@ class InsertEdit /** * No primary key given, just load first row - * - * @param string $table name of the table - * @param string $db name of the database - * - * @return array - * @phpstan-return array{ResultInterface, false[]} */ - private function loadFirstRow(string $table, string $db): array + private function loadFirstRow(string $table, string $db): ResultInterface { - $result = $this->dbi->query( + return $this->dbi->query( 'SELECT * FROM ' . Util::backquote($db) . '.' . Util::backquote($table) . ' LIMIT 1;', ); - // Can be a string on some old configuration storage settings - $rows = array_fill(0, (int) $GLOBALS['cfg']['InsertRows'], false); + } - return [$result, $rows]; + /** @return false[] */ + private function getInsertRows(): array + { + // Can be a string on some old configuration storage settings + return array_fill(0, (int) $GLOBALS['cfg']['InsertRows'], false); } /** @@ -1770,7 +1767,8 @@ class InsertEdit // we are inserting $insertMode = true; $whereClause = null; - [$result, $rows] = $this->loadFirstRow($table, $db); + $result = $this->loadFirstRow($table, $db); + $rows = $this->getInsertRows(); $whereClauses = null; $whereClauseArray = []; $foundUniqueKey = false; diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 8d73a2f27e..1371fb7330 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -14897,6 +14897,7 @@ $result $result $result + $result diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index 59f750fafd..d167bae171 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -336,23 +336,8 @@ class InsertEditTest extends AbstractTestCase $this->assertFalse($result); } - /** @return list}> */ - public static function dataProviderConfigValueInsertRows(): array + public function testLoadFirstRow(): void { - return [[2, [false, false]], ['2', [false, false]], [3, [false, false, false]], ['3', [false, false, false]]]; - } - - /** - * Test for loadFirstRow - * - * @param array $rowsValue - * - * @dataProvider dataProviderConfigValueInsertRows - */ - public function testLoadFirstRow(string|int $configValue, array $rowsValue): void - { - $GLOBALS['cfg']['InsertRows'] = $configValue; - $resultStub = $this->createMock(DummyResult::class); $dbi = $this->getMockBuilder(DatabaseInterface::class) @@ -380,10 +365,34 @@ class InsertEditTest extends AbstractTestCase ['table', 'db'], ); - $this->assertEquals( - [$resultStub, $rowsValue], - $result, + $this->assertEquals($resultStub, $result); + } + + /** @return list}> */ + public static function dataProviderConfigValueInsertRows(): array + { + return [[2, [false, false]], ['2', [false, false]], [3, [false, false, false]], ['3', [false, false, false]]]; + } + + /** + * Test for loadFirstRow + * + * @param array $rowsValue + * + * @dataProvider dataProviderConfigValueInsertRows + */ + public function testGetInsertRows(string|int $configValue, array $rowsValue): void + { + $GLOBALS['cfg']['InsertRows'] = $configValue; + + $result = $this->callFunction( + $this->insertEdit, + InsertEdit::class, + 'getInsertRows', + [], ); + + $this->assertEquals($rowsValue, $result); } /** From 0bb63c997f7ec90fba6cd70c04ba64f4307c6d55 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 19 Apr 2023 22:57:47 +0100 Subject: [PATCH 02/15] Remove getWhereClauseArray() This weird array cast is not adding anything useful. The psalm type hint tells us that elements are strings, but that's not guaranteed in any way. Signed-off-by: Kamil Tekiela --- libraries/classes/InsertEdit.php | 22 +------------------ psalm-baseline.xml | 3 +-- test/classes/InsertEditTest.php | 36 -------------------------------- 3 files changed, 2 insertions(+), 59 deletions(-) diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index 8572a97fbe..5db59c5166 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -132,26 +132,6 @@ class InsertEdit return $formParams; } - /** - * Creates array of where clauses - * - * @param string[]|string|null $whereClause where clause - * - * @return string[] whereClauseArray array of where clauses - */ - private function getWhereClauseArray(array|string|null $whereClause): array - { - if ($whereClause === null) { - return []; - } - - if (is_array($whereClause)) { - return $whereClause; - } - - return [$whereClause]; - } - /** * Analysing where clauses array * @@ -1757,7 +1737,7 @@ class InsertEdit if (isset($whereClause)) { // we are editing $insertMode = false; - $whereClauseArray = $this->getWhereClauseArray($whereClause); + $whereClauseArray = (array) $whereClause; [$whereClauses, $result, $rows, $foundUniqueKey] = $this->analyzeWhereClauses( $whereClauseArray, $table, diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 1371fb7330..d7995ad5a5 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -7981,7 +7981,6 @@ $whereClause $whereClause $whereClause - $whereClause @@ -7991,6 +7990,7 @@ $thisUrlParams $urlParams $valueSets + $whereClauseArray @@ -8093,7 +8093,6 @@ $whereClause - $whereClause diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index d167bae171..f1440aaa7e 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -181,42 +181,6 @@ class InsertEditTest extends AbstractTestCase ); } - /** - * Test for getWhereClauseArray - */ - public function testGetWhereClauseArray(): void - { - $this->assertEquals( - [], - $this->callFunction( - $this->insertEdit, - InsertEdit::class, - 'getWhereClauseArray', - [null], - ), - ); - - $this->assertEquals( - [1, 2, 3], - $this->callFunction( - $this->insertEdit, - InsertEdit::class, - 'getWhereClauseArray', - [[1, 2, 3]], - ), - ); - - $this->assertEquals( - ['clause'], - $this->callFunction( - $this->insertEdit, - InsertEdit::class, - 'getWhereClauseArray', - ['clause'], - ), - ); - } - /** * Test for analyzeWhereClauses */ From 9800c3574fc4bd8be0aaa5cca021cc5b354fc269 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 19 Apr 2023 23:59:04 +0100 Subject: [PATCH 03/15] Move urlParamsInEditMode to ChangeController Signed-off-by: Kamil Tekiela --- .../Controllers/Table/ChangeController.php | 29 ++++++++++++++++--- libraries/classes/InsertEdit.php | 23 --------------- phpstan-baseline.neon | 7 ++++- psalm-baseline.xml | 4 +-- .../Table/ChangeControllerTest.php | 23 +++++++++++++++ test/classes/InsertEditTest.php | 16 ---------- 6 files changed, 56 insertions(+), 46 deletions(-) diff --git a/libraries/classes/Controllers/Table/ChangeController.php b/libraries/classes/Controllers/Table/ChangeController.php index 14cb266685..17f89440ed 100644 --- a/libraries/classes/Controllers/Table/ChangeController.php +++ b/libraries/classes/Controllers/Table/ChangeController.php @@ -24,6 +24,7 @@ use function is_string; use function str_contains; use function strlen; use function strpos; +use function trim; /** * Displays form for editing and inserting new table rows. @@ -186,10 +187,7 @@ class ChangeController extends AbstractController $GLOBALS['urlParams']['db'] = $GLOBALS['db']; $GLOBALS['urlParams']['table'] = $GLOBALS['table']; - $GLOBALS['urlParams'] = $this->insertEdit->urlParamsInEditMode( - $GLOBALS['urlParams'], - $GLOBALS['where_clause_array'], - ); + $GLOBALS['urlParams'] = $this->urlParamsInEditMode($GLOBALS['urlParams'], $GLOBALS['where_clause_array']); $GLOBALS['has_blob_field'] = false; foreach ($GLOBALS['table_columns'] as $column) { @@ -308,4 +306,27 @@ class ChangeController extends AbstractController $this->response->addHTML($htmlOutput); } + + /** + * Add some url parameters + * + * @param mixed[] $urlParams containing $db and $table as url parameters + * @param mixed[] $whereClauseArray where clauses array + * + * @return mixed[] Add some url parameters to $url_params array and return it + */ + public function urlParamsInEditMode( + array $urlParams, + array $whereClauseArray, + ): array { + foreach ($whereClauseArray as $whereClause) { + $urlParams['where_clause'] = trim($whereClause); + } + + if (! empty($_POST['sql_query'])) { + $urlParams['sql_query'] = $_POST['sql_query']; + } + + return $urlParams; + } } diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index 5db59c5166..c1af786bb0 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -240,29 +240,6 @@ class InsertEdit return array_fill(0, (int) $GLOBALS['cfg']['InsertRows'], false); } - /** - * Add some url parameters - * - * @param mixed[] $urlParams containing $db and $table as url parameters - * @param mixed[] $whereClauseArray where clauses array - * - * @return mixed[] Add some url parameters to $url_params array and return it - */ - public function urlParamsInEditMode( - array $urlParams, - array $whereClauseArray, - ): array { - foreach ($whereClauseArray as $whereClause) { - $urlParams['where_clause'] = trim($whereClause); - } - - if (! empty($_POST['sql_query'])) { - $urlParams['sql_query'] = $_POST['sql_query']; - } - - return $urlParams; - } - /** * Show type information or function selectors in Insert/Edit * diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 4801b729d4..47bac5f852 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -2085,6 +2085,11 @@ parameters: count: 1 path: libraries/classes/Controllers/Table/ChangeController.php + - + message: "#^Parameter \\#1 \\$string of function trim expects string, mixed given\\.$#" + count: 1 + path: libraries/classes/Controllers/Table/ChangeController.php + - message: "#^Parameter \\#2 \\$signature of static method PhpMyAdmin\\\\Core\\:\\:checkSqlQuerySignature\\(\\) expects string, mixed given\\.$#" count: 1 @@ -4822,7 +4827,7 @@ parameters: - message: "#^Parameter \\#1 \\$string of function trim expects string, mixed given\\.$#" - count: 2 + count: 1 path: libraries/classes/InsertEdit.php - diff --git a/psalm-baseline.xml b/psalm-baseline.xml index d7995ad5a5..735ebbccc1 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -3305,6 +3305,7 @@ $isUpload getQueryParam('where_clause_signature')]]> $whereClause + $whereClause $rowId @@ -3339,6 +3340,7 @@ $isUpload $whereClause + $whereClause @@ -7980,7 +7982,6 @@ $whereClause $whereClause - $whereClause @@ -8054,7 +8055,6 @@ $whereClause $whereClause $whereClause - $whereClause int diff --git a/test/classes/Controllers/Table/ChangeControllerTest.php b/test/classes/Controllers/Table/ChangeControllerTest.php index bb90395d48..8d74f722f7 100644 --- a/test/classes/Controllers/Table/ChangeControllerTest.php +++ b/test/classes/Controllers/Table/ChangeControllerTest.php @@ -79,4 +79,27 @@ class ChangeControllerTest extends AbstractTestCase $actual, ); } + + /** + * Test for urlParamsInEditMode + */ + public function testUrlParamsInEditMode(): void + { + $changeController = new ChangeController( + $this->createStub(ResponseRenderer::class), + $this->createStub(Template::class), + $this->createStub(InsertEdit::class), + $this->createStub(Relation::class), + ); + + $whereClauseArray = ['foo=1', 'bar=2']; + $_POST['sql_query'] = 'SELECT 1'; + + $result = $changeController->urlParamsInEditMode([1], $whereClauseArray); + + $this->assertEquals( + ['0' => 1, 'where_clause' => 'bar=2', 'sql_query' => 'SELECT 1'], + $result, + ); + } } diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index f1440aaa7e..166cf2069e 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -359,22 +359,6 @@ class InsertEditTest extends AbstractTestCase $this->assertEquals($rowsValue, $result); } - /** - * Test for urlParamsInEditMode - */ - public function testUrlParamsInEditMode(): void - { - $whereClauseArray = ['foo=1', 'bar=2']; - $_POST['sql_query'] = 'SELECT 1'; - - $result = $this->insertEdit->urlParamsInEditMode([1], $whereClauseArray); - - $this->assertEquals( - ['0' => 1, 'where_clause' => 'bar=2', 'sql_query' => 'SELECT 1'], - $result, - ); - } - /** * Test for showTypeOrFunction */ From aa689f04f33d3549f3087e84ce74a47247ad62e9 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Thu, 20 Apr 2023 00:12:20 +0100 Subject: [PATCH 04/15] Drop getEnumSetAndTimestampColumns() Signed-off-by: Kamil Tekiela --- libraries/classes/InsertEdit.php | 40 +++++------------ test/classes/InsertEditTest.php | 77 -------------------------------- 2 files changed, 12 insertions(+), 105 deletions(-) diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index c1af786bb0..78aeb8145c 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -329,11 +329,18 @@ class InsertEdit ['char', 'varchar'], ); - [ - $column['pma_type'], - $column['wrap'], - $column['first_timestamp'], - ] = $this->getEnumSetAndTimestampColumns($column, $timestampSeen); + $column['pma_type'] = match ($column['True_Type']) { + 'set', 'enum' => $column['True_Type'], + default => $column['Type'] + }; + + $column['wrap'] = match ($column['True_Type']) { + 'set', 'enum' => '', + default => ' text-nowrap' + }; + + // can only occur once per table + $column['first_timestamp'] = $column['True_Type'] === 'timestamp' ? ! $timestampSeen : false; return $column; } @@ -376,29 +383,6 @@ class InsertEdit return false; } - /** - * Retrieve set, enum, timestamp table columns - * - * @param mixed[] $column description of column in given table - * @param bool $timestampSeen whether a timestamp has been seen - * - * @return mixed[] $column['pma_type'], $column['wrap'], $column['first_timestamp'] - * @psalm-return array{0: mixed, 1: string, 2: bool} - */ - private function getEnumSetAndTimestampColumns(array $column, bool $timestampSeen): array - { - return match ($column['True_Type']) { - 'set' => ['set', '', false], - 'enum' => ['enum', '', false], - 'timestamp' => [ - $column['Type'], - ' text-nowrap', - ! $timestampSeen, // can only occur once per table - ], - default => [$column['Type'], ' text-nowrap', false], - }; - } - /** * Retrieve the nullify code for the null column * diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index 166cf2069e..66bf95c010 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -530,83 +530,6 @@ class InsertEditTest extends AbstractTestCase $this->assertFalse($this->insertEdit->isColumn($column, $types)); } - /** - * Test for getEnumSetAndTimestampColumns - */ - public function testGetEnumAndTimestampColumns(): void - { - $column = []; - $column['True_Type'] = 'set'; - $this->assertEquals( - ['set', '', false], - $this->callFunction( - $this->insertEdit, - InsertEdit::class, - 'getEnumSetAndTimestampColumns', - [$column, false], - ), - ); - - $column['True_Type'] = 'enum'; - $this->assertEquals( - ['enum', '', false], - $this->callFunction( - $this->insertEdit, - InsertEdit::class, - 'getEnumSetAndTimestampColumns', - [$column, false], - ), - ); - - $column['True_Type'] = 'timestamp'; - $column['Type'] = 'date'; - $this->assertEquals( - ['date', ' text-nowrap', true], - $this->callFunction( - $this->insertEdit, - InsertEdit::class, - 'getEnumSetAndTimestampColumns', - [$column, false], - ), - ); - - $column['True_Type'] = 'timestamp'; - $column['Type'] = 'date'; - $this->assertEquals( - ['date', ' text-nowrap', false], - $this->callFunction( - $this->insertEdit, - InsertEdit::class, - 'getEnumSetAndTimestampColumns', - [$column, true], - ), - ); - - $column['True_Type'] = 'SET'; - $column['Type'] = 'num'; - $this->assertEquals( - ['num', ' text-nowrap', false], - $this->callFunction( - $this->insertEdit, - InsertEdit::class, - 'getEnumSetAndTimestampColumns', - [$column, false], - ), - ); - - $column['True_Type'] = ''; - $column['Type'] = 'num'; - $this->assertEquals( - ['num', ' text-nowrap', false], - $this->callFunction( - $this->insertEdit, - InsertEdit::class, - 'getEnumSetAndTimestampColumns', - [$column, false], - ), - ); - } - /** * Test for getNullifyCodeForNullColumn */ From 2d7617fe475633805187c472f02dd66d34444590 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Thu, 20 Apr 2023 00:27:02 +0100 Subject: [PATCH 05/15] Drop getColumnEnumValues() Signed-off-by: Kamil Tekiela --- libraries/classes/InsertEdit.php | 28 ++++---------------------- phpstan-baseline.neon | 5 ----- psalm-baseline.xml | 6 +----- templates/table/insert/column_row.twig | 6 +++--- test/classes/InsertEditTest.php | 19 ----------------- 5 files changed, 8 insertions(+), 56 deletions(-) diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index 78aeb8145c..a40bc1a11c 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -480,24 +480,6 @@ class InsertEdit . ''; } - /** - * Get column values - * - * @param string[] $enumSetValues - * - * @return mixed[] column values as an associative array - * @psalm-return list - */ - private function getColumnEnumValues(array $enumSetValues): array - { - $values = []; - foreach ($enumSetValues as $val) { - $values[] = ['plain' => $val, 'html' => htmlspecialchars($val)]; - } - - return $values; - } - /** * Retrieve column 'set' value and select size * @@ -2037,17 +2019,15 @@ class InsertEdit } if ($column['pma_type'] === 'enum') { - if (! isset($column['values'])) { - $column['values'] = $this->getColumnEnumValues($extractedColumnspec['enum_set_values']); - } + $column['values'] ??= $extractedColumnspec['enum_set_values']; foreach ($column['values'] as $enumValue) { if ( - $data == $enumValue['plain'] || ($data == '' + $data == $enumValue || ($data == '' && (! isset($_POST['where_clause']) || $column['Null'] !== 'YES') - && isset($column['Default']) && $enumValue['plain'] == $column['Default']) + && isset($column['Default']) && $enumValue == $column['Default']) ) { - $enumSelectedValue = $enumValue['plain']; + $enumSelectedValue = $enumValue; break; } } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 47bac5f852..44bbd7ac80 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -4765,11 +4765,6 @@ parameters: count: 1 path: libraries/classes/InsertEdit.php - - - message: "#^Parameter \\#1 \\$enumSetValues of method PhpMyAdmin\\\\InsertEdit\\:\\:getColumnEnumValues\\(\\) expects array\\, mixed given\\.$#" - count: 1 - path: libraries/classes/InsertEdit.php - - message: "#^Parameter \\#1 \\$haystack of function mb_stripos expects string, mixed given\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 735ebbccc1..903d0a3c2a 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -7959,7 +7959,6 @@ $data $data - @@ -7996,9 +7995,6 @@ $currCellEditedValues[$columnName] - - - $editedValues[$cellIndex][$columnName] @@ -8034,6 +8030,7 @@ + $currCellEditedValues @@ -14896,7 +14893,6 @@ $result $result $result - $result diff --git a/templates/table/insert/column_row.twig b/templates/table/insert/column_row.twig index c5e4d761b5..73dd1963b9 100644 --- a/templates/table/insert/column_row.twig +++ b/templates/table/insert/column_row.twig @@ -71,13 +71,13 @@ {% else %} {% for enum_value in column.values %} - - + + {% endfor %} {% endif %} {% elseif column.pma_type == 'set' %} diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index 66bf95c010..af5cdc31b5 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -619,25 +619,6 @@ class InsertEditTest extends AbstractTestCase ); } - /** - * Test for getColumnEnumValues - */ - public function testGetColumnEnumValues(): void - { - $enumSetValues = ['', '"foo"']; - - $result = $this->callFunction( - $this->insertEdit, - InsertEdit::class, - 'getColumnEnumValues', - [$enumSetValues], - ); - $this->assertEquals( - [['plain' => '', 'html' => '<abc>'], ['plain' => '"foo"', 'html' => '"foo"']], - $result, - ); - } - /** * Test for getColumnSetValueAndSelectSize */ From fa7fe0da9716413992a90ea08e2dad1ea48f83e5 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Thu, 20 Apr 2023 00:39:31 +0100 Subject: [PATCH 06/15] Drop getColumnSetValueAndSelectSize() Signed-off-by: Kamil Tekiela --- libraries/classes/InsertEdit.php | 32 +++--------------------- phpstan-baseline.neon | 10 ++++---- psalm-baseline.xml | 5 ++-- templates/table/insert/column_row.twig | 2 +- test/classes/InsertEditTest.php | 34 -------------------------- 5 files changed, 12 insertions(+), 71 deletions(-) diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index a40bc1a11c..75085c24a6 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -480,30 +480,6 @@ class InsertEdit . ''; } - /** - * Retrieve column 'set' value and select size - * - * @param mixed[] $column description of column in given table - * @param string[] $enumSetValues - * - * @return mixed[] $column['values'], $column['select_size'] - */ - private function getColumnSetValueAndSelectSize( - array $column, - array $enumSetValues, - ): array { - if (! isset($column['values'])) { - $column['values'] = []; - foreach ($enumSetValues as $val) { - $column['values'][] = ['plain' => $val, 'html' => htmlspecialchars($val)]; - } - - $column['select_size'] = min(4, count($column['values'])); - } - - return [$column['values'], $column['select_size']]; - } - /** * Get HTML input type * @@ -2032,10 +2008,10 @@ class InsertEdit } } } elseif ($column['pma_type'] === 'set') { - [$columnSetValues, $setSelectSize] = $this->getColumnSetValueAndSelectSize( - $column, - $extractedColumnspec['enum_set_values'], - ); + $columnSetValues = $column['values'] ?? $extractedColumnspec['enum_set_values']; + $setSelectSize = ! isset($column['values']) + ? min(4, count($extractedColumnspec['enum_set_values'])) + : $column['select_size']; } elseif ($column['is_binary'] || $column['is_blob']) { $isColumnProtectedBlob = ($GLOBALS['cfg']['ProtectBinary'] === 'blob' && $column['is_blob']) || ($GLOBALS['cfg']['ProtectBinary'] === 'all') diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 44bbd7ac80..cf64841660 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -4835,6 +4835,11 @@ parameters: count: 1 path: libraries/classes/InsertEdit.php + - + message: "#^Parameter \\#1 \\$value of function count expects array\\|Countable, mixed given\\.$#" + count: 1 + path: libraries/classes/InsertEdit.php + - message: "#^Parameter \\#1 \\$value of static method PhpMyAdmin\\\\Util\\:\\:addMicroseconds\\(\\) expects string, mixed given\\.$#" count: 1 @@ -4855,11 +4860,6 @@ parameters: count: 1 path: libraries/classes/InsertEdit.php - - - message: "#^Parameter \\#2 \\$enumSetValues of method PhpMyAdmin\\\\InsertEdit\\:\\:getColumnSetValueAndSelectSize\\(\\) expects array\\, mixed given\\.$#" - count: 1 - path: libraries/classes/InsertEdit.php - - message: "#^Parameter \\#2 \\$foreignField of method PhpMyAdmin\\\\ConfigStorage\\\\Relation\\:\\:foreignDropdown\\(\\) expects string, mixed given\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 903d0a3c2a..9f54d5d123 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -8029,6 +8029,7 @@ + $columnSetValues $currCellEditedValues @@ -8045,6 +8046,7 @@ $isUnsigned $maxlength $maxlength + $setSelectSize $singleQuery $specialChars @@ -8104,7 +8106,6 @@ - @@ -14891,8 +14892,6 @@ $result $result $result - $result - $result diff --git a/templates/table/insert/column_row.twig b/templates/table/insert/column_row.twig index 73dd1963b9..1dcdeb1c1a 100644 --- a/templates/table/insert/column_row.twig +++ b/templates/table/insert/column_row.twig @@ -85,7 +85,7 @@ {% elseif column.is_binary or column.is_blob %} diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index af5cdc31b5..71a07c1f64 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -619,40 +619,6 @@ class InsertEditTest extends AbstractTestCase ); } - /** - * Test for getColumnSetValueAndSelectSize - */ - public function testGetColumnSetValueAndSelectSize(): void - { - $column = []; - $enumSetValues = ['a', '<']; - $result = $this->callFunction( - $this->insertEdit, - InsertEdit::class, - 'getColumnSetValueAndSelectSize', - [[], $enumSetValues], - ); - - $this->assertEquals( - [[['plain' => 'a', 'html' => 'a'], ['plain' => '<', 'html' => '<']], 2], - $result, - ); - - $column['values'] = [1, 2]; - $column['select_size'] = 3; - $result = $this->callFunction( - $this->insertEdit, - InsertEdit::class, - 'getColumnSetValueAndSelectSize', - [$column, $enumSetValues], - ); - - $this->assertEquals( - [[1, 2], 3], - $result, - ); - } - /** * Test for getHtmlInput */ From 3c6fbee97c731efab1e2cf8c7979e009f38ffdee Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Thu, 20 Apr 2023 13:29:09 +0100 Subject: [PATCH 07/15] getMaxUploadSize() should only return string Signed-off-by: Kamil Tekiela --- libraries/classes/InsertEdit.php | 26 +++++--------------------- phpstan-baseline.neon | 5 +++++ test/classes/InsertEditTest.php | 14 ++++---------- 3 files changed, 14 insertions(+), 31 deletions(-) diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index 75085c24a6..13c68c3b15 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -578,40 +578,24 @@ class InsertEdit /** * Retrieve the maximum upload file size - * - * @param string $pmaType column type - * @param int $biggestMaxFileSize biggest max file size for uploading - * - * @return mixed[] an html snippet and $biggest_max_file_size - * @psalm-return array{non-empty-string, int} */ - private function getMaxUploadSize(string $pmaType, int $biggestMaxFileSize): array + private function getMaxUploadSize(string $pmaType): string { // find maximum upload size, based on field type /** * @todo with functions this is not so easy, as you can basically * process any data with function like MD5 */ - $maxFieldSizes = [ + $maxFieldSize = match ($pmaType) { 'tinyblob' => 256, 'blob' => 65536, 'mediumblob' => 16777216, 'longblob' => 4294967296,// yeah, really - ]; + }; $thisFieldMaxSize = (int) $GLOBALS['config']->get('max_upload_size'); // from PHP max - if ($thisFieldMaxSize > $maxFieldSizes[$pmaType]) { - $thisFieldMaxSize = $maxFieldSizes[$pmaType]; - } - $htmlOutput = Util::getFormattedMaximumUploadSize($thisFieldMaxSize) . "\n"; - // do not generate here the MAX_FILE_SIZE, because we should - // put only one in the form to accommodate the biggest field - if ($thisFieldMaxSize > $biggestMaxFileSize) { - $biggestMaxFileSize = $thisFieldMaxSize; - } - - return [$htmlOutput, $biggestMaxFileSize]; + return Util::getFormattedMaximumUploadSize(min($thisFieldMaxSize, $maxFieldSize)) . "\n"; } /** @@ -2024,7 +2008,7 @@ class InsertEdit } if ($isUpload && $column['is_blob']) { - [$maxUploadSize] = $this->getMaxUploadSize($column['pma_type'], $biggestMaxFileSize); + $maxUploadSize = $this->getMaxUploadSize($column['pma_type']); } if (! empty($GLOBALS['cfg']['UploadDir'])) { diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index cf64841660..e6297f976f 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -4745,6 +4745,11 @@ parameters: count: 2 path: libraries/classes/InsertEdit.php + - + message: "#^Match expression does not handle remaining value\\: string$#" + count: 1 + path: libraries/classes/InsertEdit.php + - message: "#^Method PhpMyAdmin\\\\InsertEdit\\:\\:getFormParametersForInsertForm\\(\\) has parameter \\$whereClauses with no value type specified in iterable type array\\.$#" count: 1 diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index 71a07c1f64..d0ea88561c 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -699,13 +699,10 @@ class InsertEditTest extends AbstractTestCase $this->insertEdit, InsertEdit::class, 'getMaxUploadSize', - [$pmaType, 256], + [$pmaType], ); - $this->assertEquals( - ["(Max: 256B)\n", 256], - $result, - ); + $this->assertEquals("(Max: 256B)\n", $result); // case 2 $GLOBALS['config']->set('max_upload_size', 250); @@ -714,13 +711,10 @@ class InsertEditTest extends AbstractTestCase $this->insertEdit, InsertEdit::class, 'getMaxUploadSize', - [$pmaType, 20], + [$pmaType], ); - $this->assertEquals( - ["(Max: 250B)\n", 250], - $result, - ); + $this->assertEquals("(Max: 250B)\n", $result); } /** From 8367af19be95ddd5118fc63ddf08046692293b77 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Thu, 20 Apr 2023 13:38:56 +0100 Subject: [PATCH 08/15] Remove unused $biggestMaxFileSize param Not sure if it should have been a global, but judging by the rest of the code, I believe it's no longer needed. Signed-off-by: Kamil Tekiela --- .../Controllers/Table/ChangeController.php | 8 ---- libraries/classes/InsertEdit.php | 47 +++++++++---------- phpstan-baseline.neon | 2 +- psalm-baseline.xml | 5 -- test/classes/InsertEditTest.php | 5 -- 5 files changed, 22 insertions(+), 45 deletions(-) diff --git a/libraries/classes/Controllers/Table/ChangeController.php b/libraries/classes/Controllers/Table/ChangeController.php index 17f89440ed..cff2199672 100644 --- a/libraries/classes/Controllers/Table/ChangeController.php +++ b/libraries/classes/Controllers/Table/ChangeController.php @@ -62,7 +62,6 @@ class ChangeController extends AbstractController $GLOBALS['tabindex'] ??= null; $GLOBALS['tabindex_for_value'] ??= null; $GLOBALS['o_rows'] ??= null; - $GLOBALS['biggest_max_file_size'] ??= null; $GLOBALS['has_blob_field'] ??= null; $GLOBALS['jsvkey'] ??= null; $GLOBALS['vkey'] ??= null; @@ -183,7 +182,6 @@ class ChangeController extends AbstractController $GLOBALS['tabindex'] = 0; $GLOBALS['tabindex_for_value'] = 0; $GLOBALS['o_rows'] = 0; - $GLOBALS['biggest_max_file_size'] = 0; $GLOBALS['urlParams']['db'] = $GLOBALS['db']; $GLOBALS['urlParams']['table'] = $GLOBALS['table']; @@ -261,7 +259,6 @@ class ChangeController extends AbstractController $GLOBALS['table'], $GLOBALS['db'], $rowId, - $GLOBALS['biggest_max_file_size'], $GLOBALS['text_dir'], $GLOBALS['repopulate'], $GLOBALS['where_clause_array'], @@ -284,11 +281,6 @@ class ChangeController extends AbstractController 'is_numeric' => $isNumeric, ]); - if ($GLOBALS['biggest_max_file_size'] > 0) { - $htmlOutput .= '' . "\n"; - } - $htmlOutput .= ''; $htmlOutput .= $this->insertEdit->getHtmlForGisEditor(); diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index 13c68c3b15..561107db6b 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -1755,7 +1755,6 @@ class InsertEdit * @param string $table table * @param string $db database * @param int $rowId row id - * @param int $biggestMaxFileSize biggest max file size * @param string $defaultCharEditing default char editing mode which is stored in the config.inc.php script * @param string $textDir text direction * @param mixed[] $repopulate the data to be repopulated @@ -1781,7 +1780,6 @@ class InsertEdit string $table, string $db, int $rowId, - int $biggestMaxFileSize, string $defaultCharEditing, string $textDir, array $repopulate, @@ -2107,28 +2105,27 @@ class InsertEdit /** * Function to get html for each insert/edit row * - * @param mixed[] $urlParams url parameters - * @param mixed[][] $tableColumns table columns - * @param mixed[] $commentsMap comments map - * @param bool $timestampSeen whether timestamp seen - * @param ResultInterface $currentResult current result - * @param string $jsvkey javascript validation key - * @param string $vkey validation key - * @param bool $insertMode whether insert mode - * @param mixed[] $currentRow current row - * @param int $oRows row offset - * @param int $tabindex tab index - * @param int $columnsCnt columns count - * @param bool $isUpload whether upload - * @param mixed[] $foreigners foreigners - * @param int $tabindexForValue tab index offset for value - * @param string $table table - * @param string $db database - * @param int $rowId row id - * @param int $biggestMaxFileSize biggest max file size - * @param string $textDir text direction - * @param mixed[] $repopulate the data to be repopulated - * @param mixed[] $whereClauseArray the array of where clauses + * @param mixed[] $urlParams url parameters + * @param mixed[][] $tableColumns table columns + * @param mixed[] $commentsMap comments map + * @param bool $timestampSeen whether timestamp seen + * @param ResultInterface $currentResult current result + * @param string $jsvkey javascript validation key + * @param string $vkey validation key + * @param bool $insertMode whether insert mode + * @param mixed[] $currentRow current row + * @param int $oRows row offset + * @param int $tabindex tab index + * @param int $columnsCnt columns count + * @param bool $isUpload whether upload + * @param mixed[] $foreigners foreigners + * @param int $tabindexForValue tab index offset for value + * @param string $table table + * @param string $db database + * @param int $rowId row id + * @param string $textDir text direction + * @param mixed[] $repopulate the data to be repopulated + * @param mixed[] $whereClauseArray the array of where clauses */ public function getHtmlForInsertEditRow( array $urlParams, @@ -2149,7 +2146,6 @@ class InsertEdit string $table, string $db, int $rowId, - int $biggestMaxFileSize, string $textDir, array $repopulate, array $whereClauseArray, @@ -2196,7 +2192,6 @@ class InsertEdit $table, $db, $rowId, - $biggestMaxFileSize, $defaultCharEditing, $textDir, $repopulate, diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index e6297f976f..284dd39b02 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -4876,7 +4876,7 @@ parameters: path: libraries/classes/InsertEdit.php - - message: "#^Parameter \\#24 \\$whereClause of method PhpMyAdmin\\\\InsertEdit\\:\\:getHtmlForInsertEditFormColumn\\(\\) expects string, mixed given\\.$#" + message: "#^Parameter \\#23 \\$whereClause of method PhpMyAdmin\\\\InsertEdit\\:\\:getHtmlForInsertEditFormColumn\\(\\) expects string, mixed given\\.$#" count: 1 path: libraries/classes/InsertEdit.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 9f54d5d123..7442a2d60a 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -3271,7 +3271,6 @@ - @@ -3313,7 +3312,6 @@ - @@ -3349,9 +3347,6 @@ $isUpload - - 0]]> - diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index d0ea88561c..e8ecfb7f2a 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -2482,7 +2482,6 @@ class InsertEditTest extends AbstractTestCase 'table', 'db', 0, - 0, '', '', $repopulate, @@ -2542,7 +2541,6 @@ class InsertEditTest extends AbstractTestCase 'table', 'db', 0, - 0, '', '', $repopulate, @@ -2650,7 +2648,6 @@ class InsertEditTest extends AbstractTestCase 'table', 'db', 0, - 0, 'ltr', [], ['wc'], @@ -2729,7 +2726,6 @@ class InsertEditTest extends AbstractTestCase 'table', 'db', 0, - 0, '', [], ['wc'], @@ -2789,7 +2785,6 @@ class InsertEditTest extends AbstractTestCase 'table', 'db', 0, - 0, '', [], ['wc'], From 56d1eed3581d1d910e394e982ac493ee3c13dcb8 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Thu, 20 Apr 2023 13:46:20 +0100 Subject: [PATCH 09/15] Remove MAX_FILE_SIZE This is not used anywhere in frontend or backend. Signed-off-by: Kamil Tekiela --- js/src/drag_drop_import.ts | 2 -- libraries/classes/Controllers/Import/ImportController.php | 1 - psalm-baseline.xml | 1 - templates/import.twig | 2 -- templates/preferences/manage/main.twig | 1 - 5 files changed, 7 deletions(-) diff --git a/js/src/drag_drop_import.ts b/js/src/drag_drop_import.ts index 3c0801294d..e9e0df9de3 100644 --- a/js/src/drag_drop_import.ts +++ b/js/src/drag_drop_import.ts @@ -329,8 +329,6 @@ var DragDropImport = { fd.append('token', CommonParams.get('token')); fd.append('import_type', 'database'); // todo: method to find the value below - fd.append('MAX_FILE_SIZE', '4194304'); - // todo: method to find the value below fd.append('charset_of_file', 'utf-8'); // todo: method to find the value below fd.append('allow_interrupt', 'yes'); diff --git a/libraries/classes/Controllers/Import/ImportController.php b/libraries/classes/Controllers/Import/ImportController.php index e71e0b04e5..37abdc6b99 100644 --- a/libraries/classes/Controllers/Import/ImportController.php +++ b/libraries/classes/Controllers/Import/ImportController.php @@ -101,7 +101,6 @@ final class ImportController extends AbstractController $GLOBALS['format'] = $request->getParsedBodyParam('format', ''); $GLOBALS['import_type'] = $request->getParsedBodyParam('import_type'); $GLOBALS['is_js_confirmed'] = $request->getParsedBodyParam('is_js_confirmed'); - $GLOBALS['MAX_FILE_SIZE'] = $request->getParsedBodyParam('MAX_FILE_SIZE'); $GLOBALS['message_to_show'] = $request->getParsedBodyParam('message_to_show'); $GLOBALS['noplugin'] = $request->getParsedBodyParam('noplugin'); $GLOBALS['skip_queries'] = $request->getParsedBodyParam('skip_queries'); diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 7442a2d60a..4c6a5bbc80 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -2174,7 +2174,6 @@ - diff --git a/templates/import.twig b/templates/import.twig index dc63c91c06..1abfd6d0ec 100644 --- a/templates/import.twig +++ b/templates/import.twig @@ -42,7 +42,6 @@
-
@@ -69,7 +68,6 @@
{% elseif is_upload %} -
diff --git a/templates/preferences/manage/main.twig b/templates/preferences/manage/main.twig index ebe6a4a6b8..6bbbd1d224 100644 --- a/templates/preferences/manage/main.twig +++ b/templates/preferences/manage/main.twig @@ -14,7 +14,6 @@
{{ get_hidden_inputs() }} - From d10d11bf8f68afac971c2351dcf86413eca3ad95 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Thu, 20 Apr 2023 13:53:11 +0100 Subject: [PATCH 10/15] Add missing @psalm-return Signed-off-by: Kamil Tekiela --- libraries/classes/InsertEdit.php | 3 ++- phpstan-baseline.neon | 35 -------------------------------- psalm-baseline.xml | 7 ------- 3 files changed, 2 insertions(+), 43 deletions(-) diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index 561107db6b..8b4906bd3a 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -811,6 +811,7 @@ class InsertEdit * * @return mixed[] $real_null_value, $data, $special_chars, $backup_field, * $special_chars_encoded + * @psalm-return array{bool, string, string, string, string} */ private function getSpecialCharsAndBackupFieldForExistingRow( array $currentRow, @@ -1998,7 +1999,7 @@ class InsertEdit $isColumnProtectedBlob = ($GLOBALS['cfg']['ProtectBinary'] === 'blob' && $column['is_blob']) || ($GLOBALS['cfg']['ProtectBinary'] === 'all') || ($GLOBALS['cfg']['ProtectBinary'] === 'noblob' && ! $column['is_blob']); - if ($isColumnProtectedBlob && isset($data)) { + if ($isColumnProtectedBlob) { $blobSize = Util::formatByteDown(mb_strlen(stripslashes($data)), 3, 1); if ($blobSize !== null) { [$blobValue, $blobValueUnit] = $blobSize; diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 284dd39b02..da7dcd7db1 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -4815,11 +4815,6 @@ parameters: count: 1 path: libraries/classes/InsertEdit.php - - - message: "#^Parameter \\#1 \\$string of function stripslashes expects string, mixed given\\.$#" - count: 1 - path: libraries/classes/InsertEdit.php - - message: "#^Parameter \\#1 \\$string of function substr expects string, mixed given\\.$#" count: 1 @@ -4850,16 +4845,6 @@ parameters: count: 1 path: libraries/classes/InsertEdit.php - - - message: "#^Parameter \\#11 \\$specialCharsEncoded of method PhpMyAdmin\\\\InsertEdit\\:\\:getValueColumnForOtherDatatypes\\(\\) expects string, mixed given\\.$#" - count: 1 - path: libraries/classes/InsertEdit.php - - - - message: "#^Parameter \\#12 \\$data of method PhpMyAdmin\\\\InsertEdit\\:\\:getValueColumnForOtherDatatypes\\(\\) expects string, mixed given\\.$#" - count: 1 - path: libraries/classes/InsertEdit.php - - message: "#^Parameter \\#2 \\$column of method PhpMyAdmin\\\\ConfigStorage\\\\Relation\\:\\:searchColumnInForeigners\\(\\) expects string, mixed given\\.$#" count: 1 @@ -4880,31 +4865,11 @@ parameters: count: 1 path: libraries/classes/InsertEdit.php - - - message: "#^Parameter \\#3 \\$backupField of method PhpMyAdmin\\\\InsertEdit\\:\\:getValueColumnForOtherDatatypes\\(\\) expects string, mixed given\\.$#" - count: 1 - path: libraries/classes/InsertEdit.php - - message: "#^Parameter \\#3 \\$row of static method PhpMyAdmin\\\\Util\\:\\:getUniqueCondition\\(\\) expects array\\, mixed given\\.$#" count: 1 path: libraries/classes/InsertEdit.php - - - message: "#^Parameter \\#3 \\$specialChars of method PhpMyAdmin\\\\InsertEdit\\:\\:getHtmlInput\\(\\) expects string, mixed given\\.$#" - count: 1 - path: libraries/classes/InsertEdit.php - - - - message: "#^Parameter \\#4 \\$data of method PhpMyAdmin\\\\ConfigStorage\\\\Relation\\:\\:foreignDropdown\\(\\) expects string, mixed given\\.$#" - count: 1 - path: libraries/classes/InsertEdit.php - - - - message: "#^Parameter \\#7 \\$specialChars of method PhpMyAdmin\\\\InsertEdit\\:\\:getValueColumnForOtherDatatypes\\(\\) expects string, mixed given\\.$#" - count: 1 - path: libraries/classes/InsertEdit.php - - message: "#^Binary operation \"\\-\" between string and 1 results in an error\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 4c6a5bbc80..69e6931fde 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -7914,7 +7914,6 @@ - $backupField @@ -7949,9 +7948,6 @@ - $data - $data - $data @@ -7966,9 +7962,6 @@ $rows[$keyId] $singleQuery $singleQuery - $specialChars - $specialChars - $specialCharsEncoded getScripts()]]> From 9640a765560b337033dc0b52a23bc3559b16f90a Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Thu, 20 Apr 2023 14:05:14 +0100 Subject: [PATCH 11/15] Move $specialCharsEncoded out of getSpecialCharsAndBackupFieldForInsertingMode Signed-off-by: Kamil Tekiela --- libraries/classes/InsertEdit.php | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index 8b4906bd3a..8b71ebe163 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -895,8 +895,8 @@ class InsertEdit /** * display default values * - * @return mixed[] $real_null_value, $data, $special_chars, $special_chars_encoded - * @psalm-return array{bool, string, string, string} + * @return mixed[] $real_null_value, $data, $special_chars + * @psalm-return array{bool, string, string} */ private function getSpecialCharsAndBackupFieldForInsertingMode( string|null $defaultValue, @@ -921,9 +921,7 @@ class InsertEdit $specialChars = htmlspecialchars($defaultValue); } - $specialCharsEncoded = Util::duplicateFirstNewline($specialChars); - - return [$realNullValue, $defaultValue, $specialChars, $specialCharsEncoded]; + return [$realNullValue, $defaultValue, $specialChars]; } /** @@ -1855,8 +1853,8 @@ class InsertEdit $realNullValue, $data, $specialChars, - $specialCharsEncoded, ] = $this->getSpecialCharsAndBackupFieldForInsertingMode($tmp['Default'] ?? null, $tmp['True_Type']); + $specialCharsEncoded = Util::duplicateFirstNewline($specialChars); $backupField = ''; unset($tmp); } From 15f1fc44e76e6b9255be2f16e39e11e468c69dd6 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Thu, 20 Apr 2023 14:24:56 +0100 Subject: [PATCH 12/15] Unwind getSpecialCharsAndBackupFieldForInsertingMode() Signed-off-by: Kamil Tekiela --- libraries/classes/InsertEdit.php | 24 ++++++----------- psalm-baseline.xml | 7 ++--- test/classes/InsertEditTest.php | 45 ++++++++++++++------------------ 3 files changed, 32 insertions(+), 44 deletions(-) diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index 8b71ebe163..9cbfd8b6cc 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -894,17 +894,12 @@ class InsertEdit /** * display default values - * - * @return mixed[] $real_null_value, $data, $special_chars - * @psalm-return array{bool, string, string} */ - private function getSpecialCharsAndBackupFieldForInsertingMode( + private function getSpecialCharsForInsertingMode( string|null $defaultValue, string $trueType, - ): array { - $realNullValue = false; + ): string { if ($defaultValue === null) { - $realNullValue = true; $defaultValue = ''; } @@ -921,7 +916,7 @@ class InsertEdit $specialChars = htmlspecialchars($defaultValue); } - return [$realNullValue, $defaultValue, $specialChars]; + return $specialChars; } /** @@ -1844,19 +1839,16 @@ class InsertEdit } else { // (we are inserting) // display default values - $tmp = $column; + $defaultValue = $column['Default'] ?? null; if (isset($repopulate[$fieldHashMd5])) { - $tmp['Default'] = $repopulate[$fieldHashMd5]; + $defaultValue = $repopulate[$fieldHashMd5]; } - [ - $realNullValue, - $data, - $specialChars, - ] = $this->getSpecialCharsAndBackupFieldForInsertingMode($tmp['Default'] ?? null, $tmp['True_Type']); + $realNullValue = $defaultValue === null; + $data = (string) $defaultValue; + $specialChars = $this->getSpecialCharsForInsertingMode($defaultValue, $column['True_Type']); $specialCharsEncoded = Util::duplicateFirstNewline($specialChars); $backupField = ''; - unset($tmp); } $idindex = ($oRows * $columnsCnt) + $columnNumber + 1; diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 69e6931fde..472abfb93e 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -7927,6 +7927,7 @@ + @@ -7948,6 +7949,7 @@ + $defaultValue @@ -7962,8 +7964,6 @@ $rows[$keyId] $singleQuery $singleQuery - - getScripts()]]> $whereClause @@ -8026,6 +8026,8 @@ $currentValue $data + $defaultValue + $defaultValue $enumSelectedValue $enumValue $fieldsize @@ -8036,7 +8038,6 @@ $setSelectSize $singleQuery $specialChars - $transformedHtml $whereClause $whereClause diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index e8ecfb7f2a..4575b3b783 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -1162,26 +1162,26 @@ class InsertEditTest extends AbstractTestCase } /** - * Test for getSpecialCharsAndBackupFieldForInsertingMode + * Test for getSpecialCharsForInsertingMode * - * @param array $column Column parameters - * @param array $expected Expected result + * @param array $column Column parameters + * @param string $expected Expected result * @psalm-param array $column - * @psalm-param array $expected * - * @dataProvider providerForTestGetSpecialCharsAndBackupFieldForInsertingMode + * @dataProvider providerForTestGetSpecialCharsForInsertingMode */ - public function testGetSpecialCharsAndBackupFieldForInsertingMode( + public function testGetSpecialCharsForInsertingMode( array $column, - array $expected, + string $expected, ): void { $GLOBALS['cfg']['ProtectBinary'] = false; $GLOBALS['cfg']['ShowFunctionFields'] = true; - $result = (array) $this->callFunction( + /** @var string $result */ + $result = $this->callFunction( $this->insertEdit, InsertEdit::class, - 'getSpecialCharsAndBackupFieldForInsertingMode', + 'getSpecialCharsForInsertingMode', [$column['Default'] ?? null, $column['True_Type']], ); @@ -1189,46 +1189,41 @@ class InsertEditTest extends AbstractTestCase } /** - * Data provider for test getSpecialCharsAndBackupFieldForInsertingMode() + * Data provider for test getSpecialCharsForInsertingMode() * - * @return array, array}> + * @return array, string}> */ - public static function providerForTestGetSpecialCharsAndBackupFieldForInsertingMode(): array + public static function providerForTestGetSpecialCharsForInsertingMode(): array { return [ 'bit' => [ ['True_Type' => 'bit', 'Default' => 'b\'101\'', 'is_binary' => true], - [false, 'b\'101\'', '101', '101'], + '101', ], - 'char' => [['True_Type' => 'char', 'is_binary' => true], [true, '', '', '']], + 'char' => [['True_Type' => 'char', 'is_binary' => true], ''], 'time with CURRENT_TIMESTAMP value' => [ ['True_Type' => 'time', 'Default' => 'CURRENT_TIMESTAMP'], - [false, 'CURRENT_TIMESTAMP', 'CURRENT_TIMESTAMP', 'CURRENT_TIMESTAMP'], + 'CURRENT_TIMESTAMP', ], 'time with current_timestamp() value' => [ ['True_Type' => 'time', 'Default' => 'current_timestamp()'], - [false, 'current_timestamp()', 'current_timestamp()', 'current_timestamp()'], + 'current_timestamp()', ], 'time with no dot value' => [ ['True_Type' => 'time', 'Default' => '10'], - [false, '10', '10.000000', '10.000000'], + '10.000000', ], 'time with dot value' => [ ['True_Type' => 'time', 'Default' => '10.08'], - [false, '10.08', '10.080000', '10.080000'], + '10.080000', ], 'any text with escape text default' => [ ['True_Type' => 'text', 'Default' => '"lorem\"ipsem"'], - [false, '"lorem\"ipsem"', 'lorem"ipsem', 'lorem"ipsem'], + 'lorem"ipsem', ], 'varchar with html special chars' => [ ['True_Type' => 'varchar', 'Default' => 'hello world
lorem ipsem'], - [ - false, - 'hello world
lorem ipsem', - 'hello world<br><b>lorem</b> ipsem', - 'hello world<br><b>lorem</b> ipsem', - ], + 'hello world<br><b>lorem</b> ipsem', ], ]; } From a852acb3e95a938d214982d8208484204457909a Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Thu, 20 Apr 2023 14:49:47 +0100 Subject: [PATCH 13/15] Move $isInsertignore out of getParamsForUpdateOrInsert Signed-off-by: Kamil Tekiela --- .../classes/Controllers/Table/ReplaceController.php | 4 +++- libraries/classes/InsertEdit.php | 9 +++------ test/classes/InsertEditTest.php | 4 ++-- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/libraries/classes/Controllers/Table/ReplaceController.php b/libraries/classes/Controllers/Table/ReplaceController.php index ccacb56c30..3697d6250b 100644 --- a/libraries/classes/Controllers/Table/ReplaceController.php +++ b/libraries/classes/Controllers/Table/ReplaceController.php @@ -104,7 +104,9 @@ final class ReplaceController extends AbstractController /** * Prepares the update/insert of a row */ - [$loopArray, $usingKey, $isInsert, $isInsertignore] = $this->insertEdit->getParamsForUpdateOrInsert(); + [$loopArray, $usingKey, $isInsert] = $this->insertEdit->getParamsForUpdateOrInsert(); + + $isInsertignore = isset($_POST['submit_type']) && $_POST['submit_type'] === 'insertignore'; $GLOBALS['query'] = []; $valueSets = []; diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index 9cbfd8b6cc..ed68e79329 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -922,8 +922,8 @@ class InsertEdit /** * Prepares the update/insert of a row * - * @return mixed[] $loop_array, $using_key, $is_insert, $is_insertignore - * @psalm-return array{array, bool, bool, bool} + * @return mixed[] $loop_array, $using_key, $is_insert + * @psalm-return array{array, bool, bool} */ public function getParamsForUpdateOrInsert(): array { @@ -948,10 +948,7 @@ class InsertEdit $isInsert = true; } - $isInsertIgnore = isset($_POST['submit_type']) - && $_POST['submit_type'] === 'insertignore'; - - return [$loopArray, $usingKey, $isInsert, $isInsertIgnore]; + return [$loopArray, $usingKey, $isInsert]; } /** diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index 4575b3b783..9e0e9b08e2 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -1239,7 +1239,7 @@ class InsertEditTest extends AbstractTestCase $result = $this->insertEdit->getParamsForUpdateOrInsert(); $this->assertEquals( - [['LIMIT 1'], true, true, false], + [['LIMIT 1'], true, true], $result, ); @@ -1249,7 +1249,7 @@ class InsertEditTest extends AbstractTestCase $result = $this->insertEdit->getParamsForUpdateOrInsert(); $this->assertEquals( - [['a', 'c'], false, true, false], + [['a', 'c'], false, true], $result, ); } From 25b24966c11e2a2e8e7fe0f5b30bf003137ba581 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Thu, 20 Apr 2023 14:57:51 +0100 Subject: [PATCH 14/15] Move getParamsForUpdateOrInsert into controller Signed-off-by: Kamil Tekiela --- .../Controllers/Table/ReplaceController.php | 35 +++++++++++++- libraries/classes/InsertEdit.php | 33 ------------- phpstan-baseline.neon | 15 ++++-- psalm-baseline.xml | 6 +-- .../Table/ReplaceControllerTest.php | 48 +++++++++++++++++++ test/classes/InsertEditTest.php | 26 ---------- 6 files changed, 94 insertions(+), 69 deletions(-) diff --git a/libraries/classes/Controllers/Table/ReplaceController.php b/libraries/classes/Controllers/Table/ReplaceController.php index 3697d6250b..f6b796989d 100644 --- a/libraries/classes/Controllers/Table/ReplaceController.php +++ b/libraries/classes/Controllers/Table/ReplaceController.php @@ -25,6 +25,7 @@ use PhpMyAdmin\Transformations; use PhpMyAdmin\Util; use function __; +use function array_keys; use function array_values; use function class_exists; use function implode; @@ -104,7 +105,7 @@ final class ReplaceController extends AbstractController /** * Prepares the update/insert of a row */ - [$loopArray, $usingKey, $isInsert] = $this->insertEdit->getParamsForUpdateOrInsert(); + [$loopArray, $usingKey, $isInsert] = $this->getParamsForUpdateOrInsert(); $isInsertignore = isset($_POST['submit_type']) && $_POST['submit_type'] === 'insertignore'; @@ -506,4 +507,36 @@ final class ReplaceController extends AbstractController /** @psalm-suppress UnresolvableInclude */ require ROOT_PATH . Core::securePath($gotoInclude); } + + /** + * Prepares the update/insert of a row + * + * @return mixed[] $loop_array, $using_key, $is_insert + * @psalm-return array{array, bool, bool} + */ + private function getParamsForUpdateOrInsert(): array + { + if (isset($_POST['where_clause'])) { + // we were editing something => use the WHERE clause + $loopArray = is_array($_POST['where_clause']) + ? $_POST['where_clause'] + : [$_POST['where_clause']]; + $usingKey = true; + $isInsert = isset($_POST['submit_type']) + && ($_POST['submit_type'] === 'insert' + || $_POST['submit_type'] === 'showinsert' + || $_POST['submit_type'] === 'insertignore'); + } else { + // new row => use indexes + $loopArray = []; + if (! empty($_POST['fields'])) { + $loopArray = array_keys($_POST['fields']['multi_edit']); + } + + $usingKey = false; + $isInsert = true; + } + + return [$loopArray, $usingKey, $isInsert]; + } } diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index ed68e79329..0eb1f83dce 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -13,7 +13,6 @@ use PhpMyAdmin\Utils\Gis; use function __; use function array_fill; use function array_key_exists; -use function array_keys; use function array_merge; use function array_values; use function bin2hex; @@ -919,38 +918,6 @@ class InsertEdit return $specialChars; } - /** - * Prepares the update/insert of a row - * - * @return mixed[] $loop_array, $using_key, $is_insert - * @psalm-return array{array, bool, bool} - */ - public function getParamsForUpdateOrInsert(): array - { - if (isset($_POST['where_clause'])) { - // we were editing something => use the WHERE clause - $loopArray = is_array($_POST['where_clause']) - ? $_POST['where_clause'] - : [$_POST['where_clause']]; - $usingKey = true; - $isInsert = isset($_POST['submit_type']) - && ($_POST['submit_type'] === 'insert' - || $_POST['submit_type'] === 'showinsert' - || $_POST['submit_type'] === 'insertignore'); - } else { - // new row => use indexes - $loopArray = []; - if (! empty($_POST['fields'])) { - $loopArray = array_keys($_POST['fields']['multi_edit']); - } - - $usingKey = false; - $isInsert = true; - } - - return [$loopArray, $usingKey, $isInsert]; - } - /** * set $_SESSION for edit_next * diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index da7dcd7db1..5c2d8d27b4 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -2220,6 +2220,11 @@ parameters: count: 1 path: libraries/classes/Controllers/Table/ReplaceController.php + - + message: "#^Method PhpMyAdmin\\\\Controllers\\\\Table\\\\ReplaceController\\:\\:getParamsForUpdateOrInsert\\(\\) return type has no value type specified in iterable type array\\.$#" + count: 1 + path: libraries/classes/Controllers/Table/ReplaceController.php + - message: "#^PHPDoc tag @var for variable \\$relationFields has no value type specified in iterable type array\\.$#" count: 1 @@ -4755,11 +4760,6 @@ parameters: count: 1 path: libraries/classes/InsertEdit.php - - - message: "#^Method PhpMyAdmin\\\\InsertEdit\\:\\:getParamsForUpdateOrInsert\\(\\) return type has no value type specified in iterable type array\\.$#" - count: 1 - path: libraries/classes/InsertEdit.php - - message: "#^Parameter \\#1 \\$buffer of method PhpMyAdmin\\\\Plugins\\\\TransformationsPlugin\\:\\:applyTransformation\\(\\) expects string, mixed given\\.$#" count: 1 @@ -9305,6 +9305,11 @@ parameters: count: 7 path: test/classes/Controllers/Server/VariablesControllerTest.php + - + message: "#^PHPDoc tag @var for variable \\$result has no value type specified in iterable type array\\.$#" + count: 2 + path: test/classes/Controllers/Table/ReplaceControllerTest.php + - message: "#^Cannot cast mixed to string\\.$#" count: 3 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 472abfb93e..10a0782a9c 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -3837,6 +3837,7 @@ + $columnName $columnName $columnName @@ -3918,6 +3919,7 @@ + $multiEditColumnsPrev[$key] $multiEditColumnsType[$key] @@ -7913,7 +7915,6 @@ ] - @@ -8081,9 +8082,6 @@ $whereClause - - - diff --git a/test/classes/Controllers/Table/ReplaceControllerTest.php b/test/classes/Controllers/Table/ReplaceControllerTest.php index 185ba90e90..17dc5a51c5 100644 --- a/test/classes/Controllers/Table/ReplaceControllerTest.php +++ b/test/classes/Controllers/Table/ReplaceControllerTest.php @@ -187,4 +187,52 @@ class ReplaceControllerTest extends AbstractTestCase $output, ); } + + /** + * Test for getParamsForUpdateOrInsert + */ + public function testGetParamsForUpdateOrInsert(): void + { + $_POST['where_clause'] = 'LIMIT 1'; + $_POST['submit_type'] = 'showinsert'; + + $replaceController = new ReplaceController( + $this->createStub(ResponseRenderer::class), + $this->createStub(Template::class), + $this->createStub(InsertEdit::class), + $this->createStub(Transformations::class), + $this->createStub(Relation::class), + $this->createStub(DatabaseInterface::class), + ); + + /** @var array $result */ + $result = $this->callFunction( + $replaceController, + ReplaceController::class, + 'getParamsForUpdateOrInsert', + [], + ); + + $this->assertEquals( + [['LIMIT 1'], true, true], + $result, + ); + + // case 2 (else) + unset($_POST['where_clause']); + $_POST['fields']['multi_edit'] = ['a' => 'b', 'c' => 'd']; + + /** @var array $result */ + $result = $this->callFunction( + $replaceController, + ReplaceController::class, + 'getParamsForUpdateOrInsert', + [], + ); + + $this->assertEquals( + [['a', 'c'], false, true], + $result, + ); + } } diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index 9e0e9b08e2..dd5af54381 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -1228,32 +1228,6 @@ class InsertEditTest extends AbstractTestCase ]; } - /** - * Test for getParamsForUpdateOrInsert - */ - public function testGetParamsForUpdateOrInsert(): void - { - $_POST['where_clause'] = 'LIMIT 1'; - $_POST['submit_type'] = 'showinsert'; - - $result = $this->insertEdit->getParamsForUpdateOrInsert(); - - $this->assertEquals( - [['LIMIT 1'], true, true], - $result, - ); - - // case 2 (else) - unset($_POST['where_clause']); - $_POST['fields']['multi_edit'] = ['a' => 'b', 'c' => 'd']; - $result = $this->insertEdit->getParamsForUpdateOrInsert(); - - $this->assertEquals( - [['a', 'c'], false, true], - $result, - ); - } - /** * Test for setSessionForEditNext */ From 5cd6c166842d11ec6208ce5e2460d0d45d30074a Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Thu, 20 Apr 2023 16:14:44 +0100 Subject: [PATCH 15/15] Instead of returning array, use array cast Signed-off-by: Kamil Tekiela --- .../Controllers/Table/ReplaceController.php | 7 +++- libraries/classes/InsertEdit.php | 33 ++++++++----------- psalm-baseline.xml | 2 -- test/classes/InsertEditTest.php | 12 +++---- 4 files changed, 25 insertions(+), 29 deletions(-) diff --git a/libraries/classes/Controllers/Table/ReplaceController.php b/libraries/classes/Controllers/Table/ReplaceController.php index f6b796989d..6f1df84fa7 100644 --- a/libraries/classes/Controllers/Table/ReplaceController.php +++ b/libraries/classes/Controllers/Table/ReplaceController.php @@ -286,7 +286,12 @@ final class ReplaceController extends AbstractController // Builds the sql query if ($isInsert && $valueSets !== []) { - $GLOBALS['query'] = $this->insertEdit->buildSqlQuery($isInsertignore, $queryFields, $valueSets); + $GLOBALS['query'] = (array) $this->insertEdit->buildInsertSqlQuery( + $GLOBALS['table'], + $isInsertignore, + $queryFields, + $valueSets, + ); } elseif (empty($GLOBALS['query']) && ! isset($_POST['preview_sql']) && ! $rowSkipped) { // No change -> move back to the calling script // diff --git a/libraries/classes/InsertEdit.php b/libraries/classes/InsertEdit.php index 0eb1f83dce..6da47a7622 100644 --- a/libraries/classes/InsertEdit.php +++ b/libraries/classes/InsertEdit.php @@ -1008,29 +1008,24 @@ class InsertEdit } /** - * Builds the sql query + * Builds the SQL insert query * - * @param bool $isInsertIgnore $_POST['submit_type'] === 'insertignore' - * @param mixed[] $queryFields column names array - * @param mixed[] $valueSets array of query values + * @param bool $isInsertIgnore $_POST['submit_type'] === 'insertignore' + * @param string[] $queryFields column names array + * @param string[] $valueSets array of query values * - * @return mixed[] of query - * @psalm-return array{string} + * @todo move this to Query generator class */ - public function buildSqlQuery(bool $isInsertIgnore, array $queryFields, array $valueSets): array - { - if ($isInsertIgnore) { - $insertCommand = 'INSERT IGNORE '; - } else { - $insertCommand = 'INSERT '; - } - - return [ - $insertCommand . 'INTO ' - . Util::backquote($GLOBALS['table']) + public function buildInsertSqlQuery( + string $table, + bool $isInsertIgnore, + array $queryFields, + array $valueSets, + ): string { + return ($isInsertIgnore ? 'INSERT IGNORE ' : 'INSERT ') . 'INTO ' + . Util::backquote($table) . ' (' . implode(', ', $queryFields) . ') VALUES (' - . implode('), (', $valueSets) . ')', - ]; + . implode('), (', $valueSets) . ')'; } /** diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 10a0782a9c..13345003f9 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -7973,11 +7973,9 @@ $query - $queryFields $thisUrlParams $thisUrlParams $urlParams - $valueSets $whereClauseArray diff --git a/test/classes/InsertEditTest.php b/test/classes/InsertEditTest.php index dd5af54381..8fea1fc02c 100644 --- a/test/classes/InsertEditTest.php +++ b/test/classes/InsertEditTest.php @@ -1336,19 +1336,17 @@ class InsertEditTest extends AbstractTestCase */ public function testBuildSqlQuery(): void { - $GLOBALS['db'] = 'db'; - $GLOBALS['table'] = 'table'; $queryFields = ['a', 'b']; - $valueSets = [1, 2]; + $valueSets = ['1', '2']; $this->assertEquals( - ['INSERT IGNORE INTO `table` (a, b) VALUES (1), (2)'], - $this->insertEdit->buildSqlQuery(true, $queryFields, $valueSets), + 'INSERT IGNORE INTO `table` (a, b) VALUES (1), (2)', + $this->insertEdit->buildInsertSqlQuery('table', true, $queryFields, $valueSets), ); $this->assertEquals( - ['INSERT INTO `table` (a, b) VALUES (1), (2)'], - $this->insertEdit->buildSqlQuery(false, $queryFields, $valueSets), + 'INSERT INTO `table` (a, b) VALUES (1), (2)', + $this->insertEdit->buildInsertSqlQuery('table', false, $queryFields, $valueSets), ); }