Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix escaping database name for SqlServerAdapter::dropDatabase #2287

Merged
merged 5 commits into from
Jun 5, 2024
Merged

Fix escaping database name for SqlServerAdapter::dropDatabase #2287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c49452b
Fix escaping database name for SqlServerAdapter::dropDatabase
MasterOdin Jun 4, 2024
2cb3c35
Merge branch '0.x' into fix-database-escaped-characters
MasterOdin Jun 4, 2024
90414ae
tweaks
MasterOdin Jun 5, 2024
93f4f99
add testcase
MasterOdin Jun 5, 2024
8dd0ce0
style
MasterOdin Jun 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions src/Phinx/Db/Adapter/SqlServerAdapter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ public function quoteTableName(string $tableName): string
*/
public function quoteColumnName(string $columnName): string
{
return '[' . str_replace(']', '\]', $columnName) . ']';
return '[' . str_replace(']', ']]', $columnName) . ']';
}

/**
Expand Down Expand Up @@ -1176,7 +1176,7 @@ public function createDatabase(string $name, array $options = []): void
{
$databaseName = $this->quoteColumnName($name);
if (isset($options['collation'])) {
$this->execute(sprintf('CREATE DATABASE %s COLLATE [%s]', $databaseName, $options['collation']));
$this->execute(sprintf('CREATE DATABASE %s COLLATE %s', $databaseName, $options['collation']));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why the collate argument had brackets around it as it's not an identifier and shouldn't be created as such. I also tried ' and " and it didn't work either.

This has probably been broken for a long time, and I guess no one is creating databases with specific collations.

} else {
$this->execute(sprintf('CREATE DATABASE %s', $databaseName));
}
Expand Down Expand Up @@ -1204,12 +1204,13 @@ public function hasDatabase(string $name): bool
*/
public function dropDatabase(string $name): void
{
$sql = <<<SQL
USE master;
IF EXISTS(select * from sys.databases where name=N'$name')
ALTER DATABASE [$name] SET SINGLE_USER WITH ROLLBACK IMMEDIATE;
DROP DATABASE [$name];
SQL;
$sql = sprintf(
'USE master;
IF EXISTS(select * from sys.databases where name=N\'' . $name . '\')
ALTER DATABASE %1$s SET SINGLE_USER WITH ROLLBACK IMMEDIATE;
DROP DATABASE %1$s;',
Comment on lines +1210 to +1211
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By using %1$s, I'm able to replace both instances with a single value. Downside is that cannot use " around the string like do in other places. Could use a heredoc, and then run the $sql through sprintf on a separate line, but that didn't feel in line with other places sprintf is used.

$this->quoteColumnName($name),
);
$this->execute($sql);
$this->createdTables = [];
}
Expand Down
15 changes: 13 additions & 2 deletions tests/Phinx/Db/Adapter/SqlServerAdapterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,20 @@ public function testQuoteTableName()
$this->assertEquals('[test_table]', $this->adapter->quoteTableName('test_table'));
}

public function testQuoteColumnName()
public function columnNameDataProvider(): array
{
$this->assertEquals('[test_column]', $this->adapter->quoteColumnName('test_column'));
return [
['test_column', '[test_column]'],
['test_col[u]mn', '[test_col[u]]mn]'],
];
}

/**
* @dataProvider columnNameDataProvider
*/
public function testQuoteColumnName(string $columnName, string $expected)
{
$this->assertEquals($expected, $this->adapter->quoteColumnName($columnName));
}

public function testCreateTable()
Expand Down
Loading