[BUGFIX] Ensure correct default value normalization

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] doctrine/dbal#6357 [2] doctrine/dbal#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 <stefan@buerk.tech> Tested-by: Garvin Hicking <gh@faktor-e.de> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Garvin Hicking <gh@faktor-e.de> Tested-by: core-ci <typo3@b13.com> Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
TYPO3-CMS · Apr 16, 2024 · a638d4e · a638d4e
1 parent 3ce932d
commit a638d4e
Show file tree

Hide file tree

Showing 10 changed files with 281 additions and 2 deletions.
diff --git 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
@@ -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<string, mixed> $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/...1/Feature-103578-AddDatabaseDefaultValueSupportForTEXTBLOBAndJSONFieldTypes.rst b/...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/...faultValue/Assertions/json-not-null-default-data-object-value-with-double-quote-value.csv b/...faultValue/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/...faultValue/Assertions/json-not-null-default-data-object-value-with-single-quote-value.csv b/...faultValue/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/...JsonFieldDefaultValue/json-not-null-default-data-object-value-with-double-quote-value.sql b/...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/...JsonFieldDefaultValue/json-not-null-default-data-object-value-with-single-quote-value.sql b/...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/...eldDefaultValue/Assertions/text-not-null-default-value-string-with-single-quote-value.csv b/...eldDefaultValue/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/...ures/TextFieldDefaultValue/text-not-null-default-value-string-with-single-quote-value.sql b/...ures/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
@@ -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')]