Merge pull request #2960 from morozov/issues/2850

Handle default values as values, not SQL expressions

UPGRADE.md

 # Upgrade to 2.10
 
+## MINOR BC BREAK: Default values are no longer handled as SQL expressions
+
+They are converted to SQL literals (e.g. escaped). Clients must now specify default values in their initial form, not in the form of an SQL literal (e.g. escaped).
+
+Before:
+
+    $column->setDefault('Foo\\\\Bar\\\\Baz');
+
+After:
+
+    $column->setDefault('Foo\\Bar\\Baz');
+
 ## Deprecated `Type::*` constants
 
 The constants for built-in types have been moved from `Doctrine\DBAL\Types\Type` to a separate class `Doctrine\DBAL\Types\Types`.

lib/Doctrine/DBAL/Platforms/AbstractPlatform.php

@@ -2311,7 +2311,7 @@ abstract class AbstractPlatform
             return " DEFAULT '" . $this->convertBooleans($default) . "'";
         }
 
-        return " DEFAULT '" . $default . "'";
+        return ' DEFAULT ' . $this->quoteStringLiteral($default);
     }
 
     /**

lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php

@@ -1629,7 +1629,7 @@ SQL
             return " DEFAULT '" . $this->convertBooleans($field['default']) . "'";
         }
 
-        return " DEFAULT '" . $field['default'] . "'";
+        return ' DEFAULT ' . $this->quoteStringLiteral($field['default']);
     }
 
     /**

lib/Doctrine/DBAL/Schema/DB2SchemaManager.php

@@ -6,10 +6,11 @@ use Doctrine\DBAL\Types\Type;
 use const CASE_LOWER;
 use function array_change_key_case;
 use function is_resource;
+use function preg_match;
+use function str_replace;
 use function strpos;
 use function strtolower;
 use function substr;
-use function trim;
 
 /**
  * IBM Db2 Schema Manager.
@@ -47,7 +48,11 @@ class DB2SchemaManager extends AbstractSchemaManager
         $default = null;
 
         if ($tableColumn['default'] !== null && $tableColumn['default'] !== 'NULL') {
-            $default = trim($tableColumn['default'], "'");
+            $default = $tableColumn['default'];
+
+            if (preg_match('/^\'(.*)\'$/s', $default, $matches)) {
+                $default = str_replace("''", "'", $matches[1]);
+            }
         }
 
         $type = $this->_platform->getDoctrineTypeMapping($tableColumn['typename']);

lib/Doctrine/DBAL/Schema/MySqlSchemaManager.php

@@ -13,17 +13,36 @@ use function assert;
 use function explode;
 use function is_string;
 use function preg_match;
-use function str_replace;
-use function stripslashes;
 use function strpos;
 use function strtok;
 use function strtolower;
+use function strtr;
 
 /**
  * Schema manager for the MySql RDBMS.
  */
 class MySqlSchemaManager extends AbstractSchemaManager
 {
+    /**
+     * @see https://mariadb.com/kb/en/library/string-literals/#escape-sequences
+     */
+    private const MARIADB_ESCAPE_SEQUENCES = [
+        '\\0' => "\0",
+        "\\'" => "'",
+        '\\"' => '"',
+        '\\b' => "\b",
+        '\\n' => "\n",
+        '\\r' => "\r",
+        '\\t' => "\t",
+        '\\Z' => "\x1a",
+        '\\\\' => '\\',
+        '\\%' => '%',
+        '\\_' => '_',
+
+        // Internally, MariaDB escapes single quotes using the standard syntax
+        "''" => "'",
+    ];
+
     /**
      * {@inheritdoc}
      */
@@ -219,7 +238,7 @@ class MySqlSchemaManager extends AbstractSchemaManager
         }
 
         if (preg_match('/^\'(.*)\'$/', $columnDefault, $matches)) {
-            return stripslashes(str_replace("''", "'", $matches[1]));
+            return strtr($matches[1], self::MARIADB_ESCAPE_SEQUENCES);
         }
 
         switch ($columnDefault) {

lib/Doctrine/DBAL/Schema/OracleSchemaManager.php

@@ -13,6 +13,7 @@ use function array_values;
 use function assert;
 use function preg_match;
 use function sprintf;
+use function str_replace;
 use function strpos;
 use function strtolower;
 use function strtoupper;
@@ -144,8 +145,10 @@ class OracleSchemaManager extends AbstractSchemaManager
         }
 
         if ($tableColumn['data_default'] !== null) {
-            // Default values returned from database are enclosed in single quotes.
-            $tableColumn['data_default'] = trim($tableColumn['data_default'], "'");
+            // Default values returned from database are represented as literal expressions
+            if (preg_match('/^\'(.*)\'$/s', $tableColumn['data_default'], $matches)) {
+                $tableColumn['data_default'] = str_replace("''", "'", $matches[1]);
+            }
         }
 
         if ($tableColumn['data_precision'] !== null) {

lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php

@@ -21,7 +21,6 @@ use function preg_match;
 use function preg_replace;
 use function sprintf;
 use function str_replace;
-use function stripos;
 use function strlen;
 use function strpos;
 use function strtolower;
@@ -330,11 +329,9 @@ class PostgreSqlSchemaManager extends AbstractSchemaManager
             $autoincrement           = true;
         }
 
-        if (preg_match("/^['(](.*)[')]::.*$/", $tableColumn['default'], $matches)) {
+        if (preg_match("/^['(](.*)[')]::/", $tableColumn['default'], $matches)) {
             $tableColumn['default'] = $matches[1];
-        }
-
-        if (stripos($tableColumn['default'], 'NULL') === 0) {
+        } elseif (preg_match('/^NULL::/', $tableColumn['default'])) {
             $tableColumn['default'] = null;
         }
 
@@ -395,11 +392,12 @@ class PostgreSqlSchemaManager extends AbstractSchemaManager
                 $length = null;
                 break;
             case 'text':
-                $fixed = false;
-                break;
+            case '_varchar':
             case 'varchar':
+                $tableColumn['default'] = $this->parseDefaultExpression($tableColumn['default']);
+                $fixed                  = false;
+                break;
             case 'interval':
-            case '_varchar':
                 $fixed = false;
                 break;
             case 'char':
@@ -479,4 +477,16 @@ class PostgreSqlSchemaManager extends AbstractSchemaManager
 
         return $defaultValue;
     }
+
+    /**
+     * Parses a default value expression as given by PostgreSQL
+     */
+    private function parseDefaultExpression(?string $default) : ?string
+    {
+        if ($default === null) {
+            return $default;
+        }
+
+        return str_replace("''", "'", $default);
+    }
 }

lib/Doctrine/DBAL/Schema/SQLServerSchemaManager.php

@@ -16,7 +16,6 @@ use function sprintf;
 use function str_replace;
 use function strpos;
 use function strtok;
-use function trim;
 
 /**
  * SQL Server Schema Manager.
@@ -107,7 +106,7 @@ class SQLServerSchemaManager extends AbstractSchemaManager
             'length'        => $length === 0 || ! in_array($type, ['text', 'string']) ? null : $length,
             'unsigned'      => false,
             'fixed'         => (bool) $fixed,
-            'default'       => $default !== 'NULL' ? $default : null,
+            'default'       => $default,
             'notnull'       => (bool) $tableColumn['notnull'],
             'scale'         => $tableColumn['scale'],
             'precision'     => $tableColumn['precision'],
@@ -124,10 +123,18 @@ class SQLServerSchemaManager extends AbstractSchemaManager
         return $column;
     }
 
-    private function parseDefaultExpression(string $value) : string
+    private function parseDefaultExpression(string $value) : ?string
     {
-        while (preg_match('/^\((.*)\)$/', $value, $matches)) {
-            $value = trim($matches[1], "'");
+        while (preg_match('/^\((.*)\)$/s', $value, $matches)) {
+            $value = $matches[1];
+        }
+
+        if ($value === 'NULL') {
+            return null;
+        }
+
+        if (preg_match('/^\'(.*)\'$/s', $value, $matches)) {
+            $value = str_replace("''", "'", $matches[1]);
         }
 
         if ($value === 'getdate()') {

lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php

@@ -324,10 +324,14 @@ class SqliteSchemaManager extends AbstractSchemaManager
         if ($default === 'NULL') {
             $default = null;
         }
+
         if ($default !== null) {
-            // SQLite returns strings wrapped in single quotes, so we need to strip them
-            $default = preg_replace("/^'(.*)'$/", '\1', $default);
+            // SQLite returns the default value as a literal expression, so we need to parse it
+            if (preg_match('/^\'(.*)\'$/s', $default, $matches)) {
+                $default = str_replace("''", "'", $matches[1]);
+            }
         }
+
         $notnull = (bool) $tableColumn['notnull'];
 
         if (! isset($tableColumn['name'])) {

tests/Doctrine/Tests/DBAL/Functional/Schema/DefaultValueTest.php

+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\DBAL\Functional\Schema;
+
+use Doctrine\DBAL\Schema\Table;
+use Doctrine\Tests\DbalFunctionalTestCase;
+use function sprintf;
+
+class DefaultValueTest extends DbalFunctionalTestCase
+{
+    /** @var bool */
+    private static $initialized = false;
+
+    protected function setUp() : void
+    {
+        parent::setUp();
+
+        if (self::$initialized) {
+            return;
+        }
+
+        self::$initialized = true;
+
+        $table = new Table('default_value');
+        $table->addColumn('id', 'integer');
+
+        foreach (self::columnProvider() as [$name, $default]) {
+            $table->addColumn($name, 'string', [
+                'default' => $default,
+                'notnull' => false,
+            ]);
+        }
+
+        $this->connection->getSchemaManager()
+            ->dropAndCreateTable($table);
+
+        $this->connection->insert('default_value', ['id' => 1]);
+    }
+
+    /**
+     * @dataProvider columnProvider
+     */
+    public function testEscapedDefaultValueCanBeIntrospected(string $name, $expectedDefault) : void
+    {
+        self::assertSame(
+            $expectedDefault,
+            $this->connection
+                ->getSchemaManager()
+                ->listTableDetails('default_value')
+                ->getColumn($name)
+                ->getDefault()
+        );
+    }
+
+    /**
+     * @dataProvider columnProvider
+     */
+    public function testEscapedDefaultValueCanBeInserted(string $name, $expectedDefault) : void
+    {
+        $value = $this->connection->fetchColumn(
+            sprintf('SELECT %s FROM default_value', $name)
+        );
+
+        self::assertSame($expectedDefault, $value);
+    }
+
+    /**
+     * Returns potential escaped literals from all platforms combined.
+     *
+     * @see https://dev.mysql.com/doc/refman/5.7/en/string-literals.html
+     * @see http://www.sqlite.org/lang_expr.html
+     * @see https://www.postgresql.org/docs/9.6/static/sql-syntax-lexical.html#SQL-SYNTAX-STRINGS-ESCAPE
+     *
+     * @return mixed[][]
+     */
+    public static function columnProvider() : iterable
+    {
+        return [
+            'Single quote' => [
+                'single_quote',
+                "foo'bar",
+            ],
+            'Single quote, doubled' => [
+                'single_quote_doubled',
+                "foo''bar",
+            ],
+            'Double quote' => [
+                'double_quote',
+                'foo"bar',
+            ],
+            'Double quote, doubled' => [
+                'double_quote_doubled',
+                'foo""bar',
+            ],
+            'Backspace' => [
+                'backspace',
+                "foo\x08bar",
+            ],
+            'New line' => [
+                'new_line',
+                "foo\nbar",
+            ],
+            'Carriage return' => [
+                'carriage_return',
+                "foo\rbar",
+            ],
+            'Tab' => [
+                'tab',
+                "foo\tbar",
+            ],
+            'Substitute' => [
+                'substitute',
+                "foo\x1abar",
+            ],
+            'Backslash' => [
+                'backslash',
+                'foo\\bar',
+            ],
+            'Backslash, doubled' => [
+                'backslash_doubled',
+                'foo\\\\bar',
+            ],
+            'Percent' => [
+                'percent_sign',
+                'foo%bar',
+            ],
+            'Underscore' => [
+                'underscore',
+                'foo_bar',
+            ],
+            'NULL string' => [
+                'null_string',
+                'NULL',
+            ],
+            'NULL value' => [
+                'null_value',
+                null,
+            ],
+            'SQL expression' => [
+                'sql_expression',
+                "'; DROP DATABASE doctrine --",
+            ],
+            'No double conversion' => [
+                'no_double_conversion',
+                "\\'",
+            ],
+        ];
+    }
+}

tests/Doctrine/Tests/DBAL/Functional/Schema/MySqlSchemaManagerTest.php

@@ -11,8 +11,6 @@ use Doctrine\DBAL\Schema\Table;
 use Doctrine\DBAL\Types\Type;
 use Doctrine\DBAL\Types\Types;
 use Doctrine\Tests\Types\MySqlPointType;
-use function implode;
-use function sprintf;
 
 class MySqlSchemaManagerTest extends SchemaManagerFunctionalTestCase
 {
@@ -517,45 +515,6 @@ class MySqlSchemaManagerTest extends SchemaManagerFunctionalTestCase
         self::assertFalse($diff, 'Tables should be identical with column defauts time and date.');
     }
 
-    /**
-     * Ensure default values (un-)escaping is properly done by mysql platforms.
-     * The test is voluntarily relying on schema introspection due to current
-     * doctrine limitations. Once #2850 is landed, this test can be removed.
-     *
-     * @see https://dev.mysql.com/doc/refman/5.7/en/string-literals.html
-     */
-    public function testEnsureDefaultsAreUnescapedFromSchemaIntrospection() : void
-    {
-        $platform = $this->schemaManager->getDatabasePlatform();
-        $this->connection->query('DROP TABLE IF EXISTS test_column_defaults_with_create');
-
-        $escapeSequences = [
-            "\\0",          // An ASCII NUL (X'00') character
-            "\\'",
-            "''",    // Single quote
-            '\\"',
-            '""',    // Double quote
-            '\\b',          // A backspace character
-            '\\n',          // A new-line character
-            '\\r',          // A carriage return character
-            '\\t',          // A tab character
-            '\\Z',          // ASCII 26 (Control+Z)
-            '\\\\',         // A backslash (\) character
-            '\\%',          // A percent (%) character
-            '\\_',          // An underscore (_) character
-        ];
-
-        $default = implode('+', $escapeSequences);
-
-        $sql = sprintf(
-            'CREATE TABLE test_column_defaults_with_create(col1 VARCHAR(255) NULL DEFAULT %s)',
-            $platform->quoteStringLiteral($default)
-        );
-        $this->connection->query($sql);
-        $onlineTable = $this->schemaManager->listTableDetails('test_column_defaults_with_create');
-        self::assertSame($default, $onlineTable->getColumn('col1')->getDefault());
-    }
-
     public function testEnsureTableOptionsAreReflectedInMetadata() : void
     {
         $this->connection->query('DROP TABLE IF EXISTS test_table_metadata');