diff --git a/UPGRADE.md b/UPGRADE.md index 2847c786413..8da236de70f 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,3 +1,17 @@ +# Upgrade to UNRELEASED + +## MINOR BC BREAK: escaped default values + +Default values will be automatically escaped. So default values must now be specified non-escaped. + +Before: + + $column->setDefault('Foo\\\\Bar\\\\Baz'); + +After: + + $column->setDefault('Foo\\Bar\\Baz'); + # Upgrade to 2.6 ## MINOR BC BREAK: `fetch()` and `fetchAll()` method signatures in `Doctrine\DBAL\Driver\ResultStatement` diff --git a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php index 29f0c6ce925..20e85d21d1c 100644 --- a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php @@ -2306,7 +2306,7 @@ public function getDefaultValueDeclarationSQL($field) return " DEFAULT '" . $this->convertBooleans($default) . "'"; } - return " DEFAULT '" . $default . "'"; + return " DEFAULT " . $this->quoteDefaultStringLiteral($default); } /** @@ -3570,4 +3570,16 @@ public function getStringLiteralQuoteCharacter() { return "'"; } + + /** + * Quote a literal string for usage as DEFAULT value. + * + * @param string $str + * + * @return string + */ + protected function quoteDefaultStringLiteral(string $str) : string + { + return $this->quoteStringLiteral($str); + } } diff --git a/lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php b/lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php index dbdf9bb6491..f0d7fba252b 100644 --- a/lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php @@ -1236,4 +1236,12 @@ private function isNumericType(Type $type) : bool { return $type instanceof IntegerType || $type instanceof BigIntType; } + + /** + * {@inheritDoc} + */ + protected function quoteDefaultStringLiteral(string $str) : string + { + return parent::quoteStringLiteral($str); + } } diff --git a/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php b/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php index 9038f29b8e3..d8d9ae5fe7c 100644 --- a/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php @@ -384,11 +384,12 @@ protected function _getPortableTableColumnDefinition($tableColumn) $length = null; break; case 'text': + case '_varchar': + case 'varchar': + $tableColumn['default'] = $this->unescapeDefaultValue($tableColumn['default']); $fixed = false; break; - case 'varchar': case 'interval': - case '_varchar': $fixed = false; break; case 'char': @@ -468,4 +469,22 @@ private function fixVersion94NegativeNumericDefaultValue($defaultValue) return $defaultValue; } + + /** + * Un-escape a default value as given by PostgreSQL + * + * @param null|string $default + * + * @return null|string + */ + private function unescapeDefaultValue(?string $default) : ?string + { + if ($default === null) { + return $default; + } + + $default = str_replace("''", "'", $default); + + return $default; + } } diff --git a/lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php b/lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php index f7075d9105d..4feecb820fe 100644 --- a/lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php @@ -308,8 +308,9 @@ protected function _getPortableTableColumnDefinition($tableColumn) $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 strings wrapped in single quotes and escaped, so we need to strip them + $default = preg_replace("/^'(.*)'$/s", '\1', $default); + $default = str_replace("''", "'", $default); } $notnull = (bool) $tableColumn['notnull']; diff --git a/tests/Doctrine/Tests/DBAL/Functional/Schema/MySqlSchemaManagerTest.php b/tests/Doctrine/Tests/DBAL/Functional/Schema/MySqlSchemaManagerTest.php index d6c93761274..be32a66cc2c 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/Schema/MySqlSchemaManagerTest.php +++ b/tests/Doctrine/Tests/DBAL/Functional/Schema/MySqlSchemaManagerTest.php @@ -442,39 +442,4 @@ public function testColumnDefaultValuesCurrentTimeAndDate() : void $diff = $comparator->diffTable($table, $onlineTable); 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->_sm->getDatabasePlatform(); - $this->_conn->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 = "CREATE TABLE test_column_defaults_with_create( - col1 VARCHAR(255) NULL DEFAULT {$platform->quoteStringLiteral($default)} - )"; - $this->_conn->query($sql); - $onlineTable = $this->_sm->listTableDetails("test_column_defaults_with_create"); - self::assertSame($default, $onlineTable->getColumn('col1')->getDefault()); - } } diff --git a/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php b/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php index ea73e275335..f4b54dd85cd 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php +++ b/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php @@ -1397,4 +1397,64 @@ public function testCreateAndListSequences() : void self::assertEquals($sequence2AllocationSize, $actualSequence2->getAllocationSize()); self::assertEquals($sequence2InitialValue, $actualSequence2->getInitialValue()); } + + /** + * 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 array + */ + private function getEscapedLiterals() : array + { + return [ + ['An ASCII NUL (X\'00\')', "foo\\0bar"], + ['Single quote, C-style', "foo\\'bar"], + ['Single quote, doubled-style', "foo''bar"], + ['Double quote, C-style', 'foo\\"bar'], + ['Double quote, double-style', 'foo""bar'], + ['Backspace', 'foo\\bbar'], + ['New-line', 'foo\\nbar'], + ['Carriage return', 'foo\\rbar'], + ['Tab', 'foo\\tbar'], + ['ASCII 26 (Control+Z)', 'foo\\Zbar'], + ['Backslash (\)', 'foo\\\\bar'], + ['Percent (%)', 'foo\\%bar'], + ['Underscore (_)', 'foo\\_bar'], + ]; + } + + private function createTableForDefaultValues() : void + { + $table = new Table('string_escaped_default_value'); + foreach ($this->getEscapedLiterals() as $i => $literal) { + $table->addColumn('field' . $i, 'string', ['default' => $literal[1]]); + } + + $table->addColumn('def_foo', 'string'); + $this->_sm->dropAndCreateTable($table); + } + + public function testEscapedDefaultValueCanBeIntrospected() : void + { + $this->createTableForDefaultValues(); + + $onlineTable = $this->_sm->listTableDetails('string_escaped_default_value'); + foreach ($this->getEscapedLiterals() as $i => $literal) { + self::assertSame($literal[1], $onlineTable->getColumn('field' . $i)->getDefault(), 'should be able introspect the value of default for: ' . $literal[0]); + } + } + + public function testEscapedDefaultValueCanBeInserted() : void + { + $this->createTableForDefaultValues(); + + $this->_conn->insert('string_escaped_default_value', ['def_foo' => 'foo']); + $row = $this->_conn->fetchAssoc('SELECT * FROM string_escaped_default_value'); + foreach ($this->getEscapedLiterals() as $i => $literal) { + self::assertSame($literal[1], $row['field' . $i], 'inserted default value should be the configured default value for: ' . $literal[0]); + } + } } diff --git a/tests/Doctrine/Tests/DBAL/Platforms/AbstractMySQLPlatformTestCase.php b/tests/Doctrine/Tests/DBAL/Platforms/AbstractMySQLPlatformTestCase.php index 54f49354c52..8eac1c1353b 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/AbstractMySQLPlatformTestCase.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/AbstractMySQLPlatformTestCase.php @@ -473,7 +473,7 @@ public function testNamedPrimaryKey() "ALTER TABLE mytable ADD PRIMARY KEY (foo)", ), $sql); } - + public function testAlterPrimaryKeyWithNewColumn() { $table = new Table("yolo"); @@ -483,7 +483,7 @@ public function testAlterPrimaryKeyWithNewColumn() $comparator = new Comparator(); $diffTable = clone $table; - + $diffTable->addColumn('pkc2', 'integer'); $diffTable->dropPrimaryKey(); $diffTable->setPrimaryKey(array('pkc1', 'pkc2')); @@ -495,7 +495,7 @@ public function testAlterPrimaryKeyWithNewColumn() 'ALTER TABLE yolo ADD PRIMARY KEY (pkc1, pkc2)', ), $this->_platform->getAlterTableSQL($comparator->diffTable($table, $diffTable)) - ); + ); } public function testInitializesDoctrineTypeMappings()