From a638d4eec23c9aebc7c7751ad7dacf2adb3193fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20B=C3=BCrk?= Date: Thu, 11 Apr 2024 13:31:05 +0200 Subject: [PATCH] [BUGFIX] Ensure correct default value normalization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Default value support for TEXT, JSON and BLOB fields has been added with #103578 by implementing the use of default value expression for MySQL. That required to add custom normalization on data schema reads to be comparable. MySQL requires to use a single-quote to quote a single quote in a value string, and due to the expression way this needs to be properly decoded now in two steps: * Revert escape sequences in the retrieved default value * Unquote the unescaped retrieved default value JSON field defaults shows a similar issue for double quotes in the json value and can be fixed in the same way. Added test revealed, that Doctrine DBAL has an issue with double single-quotes for PostgreSQL too. To fix this the issue has been reported [1] and a pull-request provided [2]. This change ensure correct unescaping and unquoting of the retrieved column default value for TEXT, JSON and BLOB column types for MySQL connections, enriched with further tests. The extended PostreSQLSchemaManager now clones method `_getPortableTableColumnDefinition()` to incorporate the bugfix directly until a fixed Doctrine DBAL version has been released. [1] https://github.com/doctrine/dbal/issues/6357 [2] https://github.com/doctrine/dbal/pull/6358 Resolves: #103610 Related: #103578 Releases: main Change-Id: Icb39cdb8c87ae7907f84e5c38adcde4ef545ed1b Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/83745 Tested-by: Stefan Bürk Tested-by: Garvin Hicking Reviewed-by: Christian Kuhn Reviewed-by: Stefan Bürk Reviewed-by: Garvin Hicking Tested-by: core-ci Tested-by: Christian Kuhn --- .../SchemaManager/MySQLSchemaManager.php | 13 +- .../SchemaManager/PostgreSQLSchemaManager.php | 192 +++++++++++++++++- ...lueSupportForTEXTBLOBAndJSONFieldTypes.rst | 15 ++ ...a-object-value-with-double-quote-value.csv | 3 + ...a-object-value-with-single-quote-value.csv | 3 + ...a-object-value-with-double-quote-value.sql | 9 + ...a-object-value-with-single-quote-value.sql | 9 + ...t-value-string-with-single-quote-value.csv | 3 + ...t-value-string-with-single-quote-value.sql | 9 + .../Database/Schema/SchemaMigratorTest.php | 27 +++ 10 files changed, 281 insertions(+), 2 deletions(-) create mode 100644 Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/Assertions/json-not-null-default-data-object-value-with-double-quote-value.csv create mode 100644 Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/Assertions/json-not-null-default-data-object-value-with-single-quote-value.csv create mode 100644 Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/json-not-null-default-data-object-value-with-double-quote-value.sql create mode 100644 Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/json-not-null-default-data-object-value-with-single-quote-value.sql create mode 100644 Tests/Functional/Database/Fixtures/TextFieldDefaultValue/Assertions/text-not-null-default-value-string-with-single-quote-value.csv create mode 100644 Tests/Functional/Database/Fixtures/TextFieldDefaultValue/text-not-null-default-value-string-with-single-quote-value.sql diff --git a/Classes/Database/Schema/SchemaManager/MySQLSchemaManager.php b/Classes/Database/Schema/SchemaManager/MySQLSchemaManager.php index da833a7746..5a84e0d4ea 100644 --- a/Classes/Database/Schema/SchemaManager/MySQLSchemaManager.php +++ b/Classes/Database/Schema/SchemaManager/MySQLSchemaManager.php @@ -67,6 +67,11 @@ class MySQLSchemaManager extends DoctrineMySQLSchemaManager "''" => "'", ]; + private const MYSQL_UNQUOTE_SEQUENCES = [ + "\\'" => "'", + '\\"' => '"', + ]; + /** * Gets Table Column Definition. * @@ -117,7 +122,13 @@ protected function getMySQLTextAndBlobColumnDefault(string|null $columnDefault): return ''; } if (preg_match("/^\\\'(.*)\\\'$/", trim($columnDefault), $matches) === 1) { - return strtr($matches[1], self::MYSQL_ESCAPE_SEQUENCES); + return strtr( + strtr($matches[1], self::MYSQL_ESCAPE_SEQUENCES), + // MySQL saves quoted single-quote as escaped single-quote in the INFORMATION SCHEMA table, even + // if it has been provided with double-quote quoting and is inconsistent for itself and enforces + // a additional unquoting after the un-escaping step + self::MYSQL_UNQUOTE_SEQUENCES + ); } return $columnDefault; } diff --git a/Classes/Database/Schema/SchemaManager/PostgreSQLSchemaManager.php b/Classes/Database/Schema/SchemaManager/PostgreSQLSchemaManager.php index 028ee9fb4f..1414b610df 100644 --- a/Classes/Database/Schema/SchemaManager/PostgreSQLSchemaManager.php +++ b/Classes/Database/Schema/SchemaManager/PostgreSQLSchemaManager.php @@ -19,6 +19,8 @@ use Doctrine\DBAL\Platforms\PostgreSQLPlatform as DoctrinePostgreSQLPlatform; use Doctrine\DBAL\Schema\Column; +use Doctrine\DBAL\Types\JsonType; +use Doctrine\DBAL\Types\Type; /** * Extending the doctrine PostgreSQLSchemaManager to integrate additional processing stuff @@ -50,6 +52,194 @@ protected function _getPortableTableColumnDefinition(array $tableColumn): Column /** @var DoctrinePostgreSQLPlatform $platform */ $platform = $this->platform; return $this->processCustomDoctrineTypesColumnDefinition($tableColumn, $platform) - ?? parent::_getPortableTableColumnDefinition($tableColumn); + // @todo Use parent::_getPortableTableColumnDefinition($tableColumn) after fixed Doctrine DBAL version has + // been required as minimum version, see https://github.com/doctrine/dbal/issues/6357 + ?? $this->fixedGetPortableTableColumnDefinition($tableColumn); + } + + /** + * Gets Table Column Definition. + * + * @param array $tableColumn + * + * @todo Remove along with PostgreSQMSchemaManager::parseDefaultExpression() after fixed Doctrine DBAL version + * has been required as minimum version, see https://github.com/doctrine/dbal/issues/6357 + */ + protected function fixedGetPortableTableColumnDefinition(array $tableColumn): Column + { + $tableColumn = array_change_key_case($tableColumn, CASE_LOWER); + + $length = null; + + if ( + in_array(strtolower($tableColumn['type']), ['varchar', 'bpchar'], true) + && preg_match('/\((\d*)\)/', $tableColumn['complete_type'], $matches) === 1 + ) { + $length = (int)$matches[1]; + } + + $autoincrement = $tableColumn['attidentity'] === 'd'; + + $matches = []; + + assert(array_key_exists('default', $tableColumn)); + assert(array_key_exists('complete_type', $tableColumn)); + + if ($tableColumn['default'] !== null) { + if (preg_match("/^['(](.*)[')]::/", $tableColumn['default'], $matches) === 1) { + $tableColumn['default'] = $matches[1]; + } elseif (preg_match('/^NULL::/', $tableColumn['default']) === 1) { + $tableColumn['default'] = null; + } + } + + if ($length === -1 && isset($tableColumn['atttypmod'])) { + $length = $tableColumn['atttypmod'] - 4; + } + + if ((int)$length <= 0) { + $length = null; + } + + $fixed = false; + + if (! isset($tableColumn['name'])) { + $tableColumn['name'] = ''; + } + + $precision = null; + $scale = 0; + $jsonb = null; + + $dbType = strtolower($tableColumn['type']); + if ( + $tableColumn['domain_type'] !== null + && $tableColumn['domain_type'] !== '' + && ! $this->platform->hasDoctrineTypeMappingFor($tableColumn['type']) + ) { + $dbType = strtolower($tableColumn['domain_type']); + $tableColumn['complete_type'] = $tableColumn['domain_complete_type']; + } + + $type = $this->platform->getDoctrineTypeMapping($dbType); + + switch ($dbType) { + case 'smallint': + case 'int2': + case 'int': + case 'int4': + case 'integer': + case 'bigint': + case 'int8': + $length = null; + break; + + case 'bool': + case 'boolean': + if ($tableColumn['default'] === 'true') { + $tableColumn['default'] = true; + } + + if ($tableColumn['default'] === 'false') { + $tableColumn['default'] = false; + } + + $length = null; + break; + + case 'json': // Added to parse default expression for json fields too. + case 'text': + case '_varchar': + case 'varchar': + $tableColumn['default'] = $this->parseDefaultExpression($tableColumn['default']); + break; + + case 'char': + case 'bpchar': + $fixed = true; + break; + + case 'float': + case 'float4': + case 'float8': + case 'double': + case 'double precision': + case 'real': + case 'decimal': + case 'money': + case 'numeric': + if ( + preg_match( + '([A-Za-z]+\(([0-9]+),([0-9]+)\))', + $tableColumn['complete_type'], + $match, + ) === 1 + ) { + $precision = (int)$match[1]; + $scale = (int)$match[2]; + $length = null; + } + + break; + + case 'year': + $length = null; + break; + + // PostgreSQL 9.4+ only + case 'jsonb': + $jsonb = true; + break; + } + + if ( + is_string($tableColumn['default']) && preg_match( + "('([^']+)'::)", + $tableColumn['default'], + $match, + ) === 1 + ) { + $tableColumn['default'] = $match[1]; + } + + $options = [ + 'length' => $length, + 'notnull' => (bool)$tableColumn['isnotnull'], + 'default' => $tableColumn['default'], + 'precision' => $precision, + 'scale' => $scale, + 'fixed' => $fixed, + 'autoincrement' => $autoincrement, + ]; + + if (isset($tableColumn['comment'])) { + $options['comment'] = $tableColumn['comment']; + } + + $column = new Column($tableColumn['field'], Type::getType($type), $options); + + if (! empty($tableColumn['collation'])) { + $column->setPlatformOption('collation', $tableColumn['collation']); + } + + if ($column->getType() instanceof JsonType) { + $column->setPlatformOption('jsonb', $jsonb); + } + + return $column; + } + + /** + * Parses a default value expression as given by PostgreSQL + * @todo Remove along with PostgreSQMSchemaManager::fixedGetPortableTableColumnDefinition() after fixed Doctrine + * DBAL version has been required as minimum version, see https://github.com/doctrine/dbal/issues/6357 + */ + private function parseDefaultExpression(?string $default): ?string + { + if ($default === null) { + return $default; + } + + return str_replace("''", "'", $default); } } diff --git a/Documentation/Changelog/13.1/Feature-103578-AddDatabaseDefaultValueSupportForTEXTBLOBAndJSONFieldTypes.rst b/Documentation/Changelog/13.1/Feature-103578-AddDatabaseDefaultValueSupportForTEXTBLOBAndJSONFieldTypes.rst index a398efa178..ff6d2c2310 100644 --- a/Documentation/Changelog/13.1/Feature-103578-AddDatabaseDefaultValueSupportForTEXTBLOBAndJSONFieldTypes.rst +++ b/Documentation/Changelog/13.1/Feature-103578-AddDatabaseDefaultValueSupportForTEXTBLOBAndJSONFieldTypes.rst @@ -43,6 +43,21 @@ Example ] ); +Advanced example with value quoting +----------------------------------- + +.. code-block:: sql + :caption: EXT:my_extension/ext_tables.sql + + CREATE TABLE a_textfield_test_table + ( + # JSON object default value containting single quote in json field + field1 JSON NOT NULL DEFAULT '{"key1": "value1", "key2": 123, "key3": "value with a '' single quote"}', + + # JSON object default value containing double-quote in json field + field2 JSON NOT NULL DEFAULT '{"key1": "value1", "key2": 123, "key3": "value with a \" double quote"}', + ); + Impact ====== diff --git a/Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/Assertions/json-not-null-default-data-object-value-with-double-quote-value.csv b/Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/Assertions/json-not-null-default-data-object-value-with-double-quote-value.csv new file mode 100644 index 0000000000..5a09fdd068 --- /dev/null +++ b/Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/Assertions/json-not-null-default-data-object-value-with-double-quote-value.csv @@ -0,0 +1,3 @@ +"a_textfield_test_table", +,"uid","pid","testfield", +,1,0,"{""key1"": ""value1"", ""key2"": 123, ""key3"": ""value with a \" double quote""}", diff --git a/Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/Assertions/json-not-null-default-data-object-value-with-single-quote-value.csv b/Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/Assertions/json-not-null-default-data-object-value-with-single-quote-value.csv new file mode 100644 index 0000000000..21a4a56f6a --- /dev/null +++ b/Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/Assertions/json-not-null-default-data-object-value-with-single-quote-value.csv @@ -0,0 +1,3 @@ +"a_textfield_test_table", +,"uid","pid","testfield", +,1,0,"{""key1"": ""value1"", ""key2"": 123, ""key3"": ""value with a ' single quote""}", diff --git a/Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/json-not-null-default-data-object-value-with-double-quote-value.sql b/Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/json-not-null-default-data-object-value-with-double-quote-value.sql new file mode 100644 index 0000000000..a023ee6b65 --- /dev/null +++ b/Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/json-not-null-default-data-object-value-with-double-quote-value.sql @@ -0,0 +1,9 @@ +CREATE TABLE a_textfield_test_table +( + uid INT(11) UNSIGNED NOT NULL AUTO_INCREMENT, + pid INT(11) UNSIGNED DEFAULT '0' NOT NULL, + testfield JSON NOT NULL DEFAULT '{"key1": "value1", "key2": 123, "key3": "value with a \" double quote"}', + + PRIMARY KEY (uid), + KEY parent (pid) +); diff --git a/Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/json-not-null-default-data-object-value-with-single-quote-value.sql b/Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/json-not-null-default-data-object-value-with-single-quote-value.sql new file mode 100644 index 0000000000..e94eaabd21 --- /dev/null +++ b/Tests/Functional/Database/Fixtures/JsonFieldDefaultValue/json-not-null-default-data-object-value-with-single-quote-value.sql @@ -0,0 +1,9 @@ +CREATE TABLE a_textfield_test_table +( + uid INT(11) UNSIGNED NOT NULL AUTO_INCREMENT, + pid INT(11) UNSIGNED DEFAULT '0' NOT NULL, + testfield JSON NOT NULL DEFAULT '{"key1": "value1", "key2": 123, "key3": "value with a '' single quote"}', + + PRIMARY KEY (uid), + KEY parent (pid) +); diff --git a/Tests/Functional/Database/Fixtures/TextFieldDefaultValue/Assertions/text-not-null-default-value-string-with-single-quote-value.csv b/Tests/Functional/Database/Fixtures/TextFieldDefaultValue/Assertions/text-not-null-default-value-string-with-single-quote-value.csv new file mode 100644 index 0000000000..ffb832e3a1 --- /dev/null +++ b/Tests/Functional/Database/Fixtures/TextFieldDefaultValue/Assertions/text-not-null-default-value-string-with-single-quote-value.csv @@ -0,0 +1,3 @@ +"a_textfield_test_table", +,"uid","pid","testfield", +,1,0,"default-value with a single ' quote", diff --git a/Tests/Functional/Database/Fixtures/TextFieldDefaultValue/text-not-null-default-value-string-with-single-quote-value.sql b/Tests/Functional/Database/Fixtures/TextFieldDefaultValue/text-not-null-default-value-string-with-single-quote-value.sql new file mode 100644 index 0000000000..1afbb3865c --- /dev/null +++ b/Tests/Functional/Database/Fixtures/TextFieldDefaultValue/text-not-null-default-value-string-with-single-quote-value.sql @@ -0,0 +1,9 @@ +CREATE TABLE a_textfield_test_table +( + uid INT(11) UNSIGNED NOT NULL AUTO_INCREMENT, + pid INT(11) UNSIGNED DEFAULT '0' NOT NULL, + testfield TEXT NOT NULL DEFAULT 'default-value with a single '' quote', + + PRIMARY KEY (uid), + KEY parent (pid) +); diff --git a/Tests/Functional/Database/Schema/SchemaMigratorTest.php b/Tests/Functional/Database/Schema/SchemaMigratorTest.php index 1727a8ed2e..eae845e2f4 100644 --- a/Tests/Functional/Database/Schema/SchemaMigratorTest.php +++ b/Tests/Functional/Database/Schema/SchemaMigratorTest.php @@ -419,6 +419,15 @@ public static function textFieldDefaultValueTestDataProvider(): \Generator 'expectedNotNull' => true, 'expectDefaultValue' => true, ]; + yield 'text not null default value string with single quote value' => [ + 'fixtureFileName' => 'text-not-null-default-value-string-with-single-quote-value.sql', + 'table' => 'a_textfield_test_table', + 'fieldName' => 'testfield', + 'assertionFileName' => 'text-not-null-default-value-string-with-single-quote-value.csv', + 'expectedDefaultValue' => "default-value with a single ' quote", + 'expectedNotNull' => true, + 'expectDefaultValue' => true, + ]; } #[DataProvider('textFieldDefaultValueTestDataProvider')] @@ -508,6 +517,24 @@ public static function jsonFieldDefaultValueTestDataProvider(): \Generator 'expectedNotNull' => false, 'expectDefaultValue' => true, ]; + yield 'json not null default data object value containing single-quote value' => [ + 'fixtureFileName' => 'json-not-null-default-data-object-value-with-single-quote-value.sql', + 'table' => 'a_textfield_test_table', + 'fieldName' => 'testfield', + 'assertionFileName' => 'json-not-null-default-data-object-value-with-single-quote-value.csv', + 'expectedDefaultValue' => '{"key1": "value1", "key2": 123, "key3": "value with a \' single quote"}', + 'expectedNotNull' => true, + 'expectDefaultValue' => true, + ]; + yield 'json not null default data object value containing double-quote value' => [ + 'fixtureFileName' => 'json-not-null-default-data-object-value-with-double-quote-value.sql', + 'table' => 'a_textfield_test_table', + 'fieldName' => 'testfield', + 'assertionFileName' => 'json-not-null-default-data-object-value-with-double-quote-value.csv', + 'expectedDefaultValue' => '{"key1": "value1", "key2": 123, "key3": "value with a \" double quote"}', + 'expectedNotNull' => true, + 'expectDefaultValue' => true, + ]; } #[DataProvider('jsonFieldDefaultValueTestDataProvider')]