From 64c18da1a9f14cc72c99bdeb44a9db498f9ec802 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Wed, 3 Jan 2018 13:15:59 -0800 Subject: [PATCH] Reworked the tests and fixed other platforms --- UPGRADE.md | 4 +- .../DBAL/Platforms/SQLServerPlatform.php | 2 +- lib/Doctrine/DBAL/Schema/DB2SchemaManager.php | 9 +- .../DBAL/Schema/MySqlSchemaManager.php | 25 ++- .../DBAL/Schema/OracleSchemaManager.php | 7 +- .../DBAL/Schema/PostgreSqlSchemaManager.php | 7 +- .../DBAL/Schema/SQLServerSchemaManager.php | 17 +- .../DBAL/Schema/SqliteSchemaManager.php | 9 +- .../Functional/Schema/DefaultValueTest.php | 151 ++++++++++++++++++ .../Schema/MySqlSchemaManagerTest.php | 41 ----- .../SchemaManagerFunctionalTestCase.php | 61 ------- 11 files changed, 208 insertions(+), 125 deletions(-) create mode 100644 tests/Doctrine/Tests/DBAL/Functional/Schema/DefaultValueTest.php diff --git a/UPGRADE.md b/UPGRADE.md index 60bc45c4915..91e57013d29 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,8 +1,8 @@ # Upgrade to 2.10 -## MINOR BC BREAK: escaped default values +## MINOR BC BREAK: Default values are no longer handled as SQL expressions -Default values will be automatically escaped. So default values must now be specified non-escaped. +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: diff --git a/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php b/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php index a2e68049d68..dc9f88da327 100644 --- a/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php @@ -1629,7 +1629,7 @@ public function getDefaultValueDeclarationSQL($field) return " DEFAULT '" . $this->convertBooleans($field['default']) . "'"; } - return " DEFAULT '" . $field['default'] . "'"; + return ' DEFAULT ' . $this->quoteStringLiteral($field['default']); } /** diff --git a/lib/Doctrine/DBAL/Schema/DB2SchemaManager.php b/lib/Doctrine/DBAL/Schema/DB2SchemaManager.php index 0ae7aba2cf4..201d6015b96 100644 --- a/lib/Doctrine/DBAL/Schema/DB2SchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/DB2SchemaManager.php @@ -6,10 +6,11 @@ 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 @@ protected function _getPortableTableColumnDefinition($tableColumn) $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']); diff --git a/lib/Doctrine/DBAL/Schema/MySqlSchemaManager.php b/lib/Doctrine/DBAL/Schema/MySqlSchemaManager.php index fb957f247ff..5e0368509f7 100644 --- a/lib/Doctrine/DBAL/Schema/MySqlSchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/MySqlSchemaManager.php @@ -13,17 +13,36 @@ 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 @@ private function getMariaDb1027ColumnDefault(MariaDb1027Platform $platform, ?str } if (preg_match('/^\'(.*)\'$/', $columnDefault, $matches)) { - return stripslashes(str_replace("''", "'", $matches[1])); + return strtr($matches[1], self::MARIADB_ESCAPE_SEQUENCES); } switch ($columnDefault) { diff --git a/lib/Doctrine/DBAL/Schema/OracleSchemaManager.php b/lib/Doctrine/DBAL/Schema/OracleSchemaManager.php index 0a3e76b1c51..0e9700a616d 100644 --- a/lib/Doctrine/DBAL/Schema/OracleSchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/OracleSchemaManager.php @@ -13,6 +13,7 @@ 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 @@ protected function _getPortableTableColumnDefinition($tableColumn) } 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) { diff --git a/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php b/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php index 509b3641f6f..d84077d5268 100644 --- a/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/PostgreSqlSchemaManager.php @@ -21,7 +21,6 @@ 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 @@ protected function _getPortableTableColumnDefinition($tableColumn) $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; } diff --git a/lib/Doctrine/DBAL/Schema/SQLServerSchemaManager.php b/lib/Doctrine/DBAL/Schema/SQLServerSchemaManager.php index bd6b02cb9a0..31ec6fffd50 100644 --- a/lib/Doctrine/DBAL/Schema/SQLServerSchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/SQLServerSchemaManager.php @@ -16,7 +16,6 @@ use function str_replace; use function strpos; use function strtok; -use function trim; /** * SQL Server Schema Manager. @@ -107,7 +106,7 @@ protected function _getPortableTableColumnDefinition($tableColumn) '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 @@ protected function _getPortableTableColumnDefinition($tableColumn) 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()') { diff --git a/lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php b/lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php index 94c3e936fbd..66d3c8503d5 100644 --- a/lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php +++ b/lib/Doctrine/DBAL/Schema/SqliteSchemaManager.php @@ -324,11 +324,14 @@ protected function _getPortableTableColumnDefinition($tableColumn) if ($default === 'NULL') { $default = null; } + if ($default !== null) { - // 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); + // 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'])) { diff --git a/tests/Doctrine/Tests/DBAL/Functional/Schema/DefaultValueTest.php b/tests/Doctrine/Tests/DBAL/Functional/Schema/DefaultValueTest.php new file mode 100644 index 00000000000..a65305a2e13 --- /dev/null +++ b/tests/Doctrine/Tests/DBAL/Functional/Schema/DefaultValueTest.php @@ -0,0 +1,151 @@ +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', + "\\'", + ], + ]; + } +} diff --git a/tests/Doctrine/Tests/DBAL/Functional/Schema/MySqlSchemaManagerTest.php b/tests/Doctrine/Tests/DBAL/Functional/Schema/MySqlSchemaManagerTest.php index 41754e3e672..558159b7fa4 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/Schema/MySqlSchemaManagerTest.php +++ b/tests/Doctrine/Tests/DBAL/Functional/Schema/MySqlSchemaManagerTest.php @@ -11,8 +11,6 @@ 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 @@ public function testColumnDefaultValuesCurrentTimeAndDate() : void 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'); diff --git a/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php b/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php index 15c99c0395a..aeb9f4c90dd 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php +++ b/tests/Doctrine/Tests/DBAL/Functional/Schema/SchemaManagerFunctionalTestCase.php @@ -1495,67 +1495,6 @@ public function testCreateAndListSequences() : void 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 mixed[][] - */ - private function getEscapedLiterals() : iterable - { - 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->schemaManager->dropAndCreateTable($table); - } - - public function testEscapedDefaultValueCanBeIntrospected() : void - { - $this->createTableForDefaultValues(); - - $onlineTable = $this->schemaManager->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->connection->insert('string_escaped_default_value', ['def_foo' => 'foo']); - - foreach ($this->getEscapedLiterals() as $i => $literal) { - $value = $this->connection->fetchColumn('SELECT field' . $i . ' FROM string_escaped_default_value'); - self::assertSame($literal[1], $value, 'inserted default value should be the configured default value for: ' . $literal[0]); - } - } - /** * @group #3086 */