Skip to content

Commit 94500da

Browse files
dereuromarkdereuromark
authored
Backport bug fixes from 5.x branch (#1005)
This backports several important bug fixes from recent 5.x PRs: **From PR #1001 (Fix release readiness issues for 5.x):** - Fix copy-paste bug in Migrator::shouldDropTables() using $messages['down'] instead of $messages['missing'] - Fix uninitialized $command property in Migrations.php - Fix weak equality in Table::saveData() (use !== instead of !=) - Replace assert() with explicit RuntimeException in BaseSeed for production safety - Fix DumpCommand using non-existent $io->error() method (should be $io->err()) - Replace unsafe addslashes() with proper driver escaping (schemaValue()) for column comments in MysqlAdapter::getRenameColumnInstructions() **From PR #1002 (Quote database names in PostgreSQL and SQL Server adapters):** - PostgresAdapter: Quote database name and charset in createDatabase() - PostgresAdapter: Quote database name in dropDatabase() - SqlserverAdapter: Use quoteSchemaName() instead of manual brackets in createDatabase() and dropDatabase() - SqlserverAdapter: Fix SQL injection vulnerability in dropDatabase() **From PR #1003 (Improve SQL quoting and fix docblock issues):** - SqlserverAdapter: Use quoteString() for sp_rename parameters in getRenameTableInstructions() and getRenameColumnInstructions() - PostgresAdapter/SqlserverAdapter: Use quoteColumnName() for foreign key column definitions instead of hard-coded double quotes
1 parent d0f5e3c commit 94500da

File tree

8 files changed

+60
-40
lines changed

8 files changed

+60
-40
lines changed

‎src/BaseSeed.php‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,9 @@ public function shouldExecute(): bool
204204
public function call(string $seeder, array $options = []): void
205205
{
206206
$io = $this->getIo();
207-
assert($io !== null, 'Requires ConsoleIo');
207+
if ($io === null) {
208+
throw new RuntimeException('ConsoleIo is required for calling other seeders.');
209+
}
208210
$io->out('');
209211
$io->out(
210212
' ====' .
@@ -245,7 +247,9 @@ protected function runCall(string $seeder, array $options = []): void
245247
'connection' => $options['connection'] ?? $connection,
246248
]);
247249
$io = $this->getIo();
248-
assert($io !== null, 'Missing ConsoleIo instance');
250+
if ($io === null) {
251+
throw new RuntimeException('ConsoleIo is required for running seeders.');
252+
}
249253
$manager = $factory->createManager($io);
250254
$manager->seed($seeder);
251255
}

‎src/Command/DumpCommand.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ public function execute(Arguments $args, ConsoleIo $io): ?int
141141

142142
return self::CODE_SUCCESS;
143143
}
144-
$io->error("An error occurred while writing dump file `{$filePath}`");
144+
$io->err("<error>An error occurred while writing dump file `{$filePath}`</error>");
145145

146146
return self::CODE_ERROR;
147147
}

‎src/Db/Adapter/MysqlAdapter.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,9 @@ protected function getRenameColumnInstructions(string $tableName, string $column
616616
foreach ($rows as $row) {
617617
if (strcasecmp($row['Field'], $columnName) === 0) {
618618
$null = $row['Null'] === 'NO' ? 'NOT NULL' : 'NULL';
619-
$comment = isset($row['Comment']) ? ' COMMENT ' . '\'' . addslashes($row['Comment']) . '\'' : '';
619+
$comment = isset($row['Comment']) && $row['Comment'] !== ''
620+
? ' COMMENT ' . $this->getConnection()->getDriver()->schemaValue($row['Comment'])
621+
: '';
620622

621623
// create the extra string by also filtering out the DEFAULT_GENERATED option (MySQL 8 fix)
622624
$extras = array_filter(

‎src/Db/Adapter/PostgresAdapter.php‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -972,7 +972,11 @@ public function getPhinxType(string $sqlType): string
972972
public function createDatabase(string $name, array $options = []): void
973973
{
974974
$charset = $options['charset'] ?? 'utf8';
975-
$this->execute(sprintf("CREATE DATABASE %s WITH ENCODING = '%s'", $name, $charset));
975+
$this->execute(sprintf(
976+
'CREATE DATABASE %s WITH ENCODING = %s',
977+
$this->quoteSchemaName($name),
978+
$this->quoteString($charset),
979+
));
976980
}
977981

978982
/**
@@ -995,7 +999,7 @@ public function hasDatabase(string $name): bool
995999
public function dropDatabase($name): void
9961000
{
9971001
$this->disconnect();
998-
$this->execute(sprintf('DROP DATABASE IF EXISTS %s', $name));
1002+
$this->execute(sprintf('DROP DATABASE IF EXISTS %s', $this->quoteSchemaName($name)));
9991003
$this->createdTables = [];
10001004
$this->connect();
10011005
}
@@ -1091,10 +1095,11 @@ protected function getForeignKeySqlDefinition(ForeignKey $foreignKey, string $ta
10911095
$constraintName = $foreignKey->getName() ?: (
10921096
$parts['table'] . '_' . implode('_', $foreignKey->getColumns()) . '_fkey'
10931097
);
1098+
$columnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getColumns()));
1099+
$refColumnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getReferencedColumns()));
10941100
$def = ' CONSTRAINT ' . $this->quoteColumnName($constraintName) .
1095-
' FOREIGN KEY ("' . implode('", "', $foreignKey->getColumns()) . '")' .
1096-
" REFERENCES {$this->quoteTableName($foreignKey->getReferencedTable()->getName())} (\"" .
1097-
implode('", "', $foreignKey->getReferencedColumns()) . '")';
1101+
' FOREIGN KEY (' . $columnList . ')' .
1102+
' REFERENCES ' . $this->quoteTableName($foreignKey->getReferencedTable()->getName()) . ' (' . $refColumnList . ')';
10981103
if ($foreignKey->getOnDelete()) {
10991104
$def .= " ON DELETE {$foreignKey->getOnDelete()}";
11001105
}

‎src/Db/Adapter/SqlserverAdapter.php‎

Lines changed: 37 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -248,9 +248,9 @@ protected function getRenameTableInstructions(string $tableName, string $newTabl
248248
{
249249
$this->updateCreatedTableName($tableName, $newTableName);
250250
$sql = sprintf(
251-
"EXEC sp_rename '%s', '%s'",
252-
$tableName,
253-
$newTableName,
251+
'EXEC sp_rename %s, %s',
252+
$this->quoteString($tableName),
253+
$this->quoteString($newTableName),
254254
);
255255

256256
return new AlterInstructions([], [$sql]);
@@ -404,23 +404,21 @@ protected function getRenameColumnInstructions(string $tableName, string $column
404404

405405
$oldConstraintName = "DF_{$tableName}_{$columnName}";
406406
$newConstraintName = "DF_{$tableName}_{$newColumnName}";
407-
$sql = <<<SQL
408-
IF (OBJECT_ID('$oldConstraintName', 'D') IS NOT NULL)
407+
$sql = sprintf(
408+
'IF (OBJECT_ID(%s, \'D\') IS NOT NULL)
409409
BEGIN
410-
EXECUTE sp_rename N'%s', N'%s', N'OBJECT'
411-
END
412-
SQL;
413-
$instructions->addPostStep(sprintf(
414-
$sql,
415-
$oldConstraintName,
416-
$newConstraintName,
417-
));
410+
EXECUTE sp_rename %s, %s, N\'OBJECT\'
411+
END',
412+
$this->quoteString($oldConstraintName),
413+
$this->quoteString($oldConstraintName),
414+
$this->quoteString($newConstraintName),
415+
);
416+
$instructions->addPostStep($sql);
418417

419418
$instructions->addPostStep(sprintf(
420-
"EXECUTE sp_rename N'%s.%s', N'%s', 'COLUMN' ",
421-
$tableName,
422-
$columnName,
423-
$newColumnName,
419+
'EXECUTE sp_rename %s, %s, N\'COLUMN\'',
420+
$this->quoteString($tableName . '.' . $columnName),
421+
$this->quoteString($newColumnName),
424422
));
425423

426424
return $instructions;
@@ -970,12 +968,17 @@ public function getPhinxType(string $sqlType): string
970968
*/
971969
public function createDatabase(string $name, array $options = []): void
972970
{
971+
$quotedName = $this->quoteSchemaName($name);
973972
if (isset($options['collation'])) {
974-
$this->execute(sprintf('CREATE DATABASE [%s] COLLATE [%s]', $name, $options['collation']));
973+
$this->execute(sprintf(
974+
'CREATE DATABASE %s COLLATE %s',
975+
$quotedName,
976+
$this->quoteSchemaName($options['collation']),
977+
));
975978
} else {
976-
$this->execute(sprintf('CREATE DATABASE [%s]', $name));
979+
$this->execute(sprintf('CREATE DATABASE %s', $quotedName));
977980
}
978-
$this->execute(sprintf('USE [%s]', $name));
981+
$this->execute(sprintf('USE %s', $quotedName));
979982
}
980983

981984
/**
@@ -997,12 +1000,16 @@ public function hasDatabase(string $name): bool
9971000
*/
9981001
public function dropDatabase(string $name): void
9991002
{
1000-
$sql = <<<SQL
1001-
USE master;
1002-
IF EXISTS(select * from sys.databases where name=N'$name')
1003-
ALTER DATABASE [$name] SET SINGLE_USER WITH ROLLBACK IMMEDIATE;
1004-
DROP DATABASE [$name];
1005-
SQL;
1003+
$quotedName = $this->quoteSchemaName($name);
1004+
$sql = sprintf(
1005+
'USE master;
1006+
IF EXISTS(select * from sys.databases where name=%s)
1007+
ALTER DATABASE %s SET SINGLE_USER WITH ROLLBACK IMMEDIATE;
1008+
DROP DATABASE %s;',
1009+
$this->quoteString($name),
1010+
$quotedName,
1011+
$quotedName,
1012+
);
10061013
$this->execute($sql);
10071014
$this->createdTables = [];
10081015
}
@@ -1061,10 +1068,12 @@ protected function getIndexSqlDefinition(Index $index, string $tableName): strin
10611068
protected function getForeignKeySqlDefinition(ForeignKey $foreignKey, string $tableName): string
10621069
{
10631070
$constraintName = $foreignKey->getName() ?: $tableName . '_' . implode('_', $foreignKey->getColumns());
1071+
$columnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getColumns()));
1072+
$refColumnList = implode(', ', array_map($this->quoteColumnName(...), $foreignKey->getReferencedColumns()));
10641073

10651074
$def = ' CONSTRAINT ' . $this->quoteColumnName($constraintName);
1066-
$def .= ' FOREIGN KEY ("' . implode('", "', $foreignKey->getColumns()) . '")';
1067-
$def .= " REFERENCES {$this->quoteTableName($foreignKey->getReferencedTable()->getName())} (\"" . implode('", "', $foreignKey->getReferencedColumns()) . '")';
1075+
$def .= ' FOREIGN KEY (' . $columnList . ')';
1076+
$def .= ' REFERENCES ' . $this->quoteTableName($foreignKey->getReferencedTable()->getName()) . ' (' . $refColumnList . ')';
10681077
if ($foreignKey->getOnDelete()) {
10691078
$def .= " ON DELETE {$foreignKey->getOnDelete()}";
10701079
}

‎src/Db/Table.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,7 @@ public function saveData(): void
793793
$c = array_keys($row);
794794
foreach ($this->getData() as $row) {
795795
$k = array_keys($row);
796-
if ($k != $c) {
796+
if ($k !== $c) {
797797
$bulk = false;
798798
break;
799799
}

‎src/Migrations.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ class Migrations
6464
*
6565
* @var string
6666
*/
67-
protected string $command;
67+
protected string $command = '';
6868

6969
/**
7070
* Stub input to feed the manager class since we might not have an input ready when we get the Manager using

‎src/TestSuite/Migrator.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ protected function shouldDropTables(Migrations $migrations, array $options): boo
199199
if (!empty($messages['missing'])) {
200200
$hasProblems = true;
201201
$output[] = 'Applied but missing migrations:';
202-
$output = array_merge($output, array_map($itemize, $messages['down']));
202+
$output = array_merge($output, array_map($itemize, $messages['missing']));
203203
$output[] = '';
204204
}
205205
if ($output) {

0 commit comments

Comments
 (0)