From f5cc3356087e7e9ec4956e8ee06bc909a81f0160 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Fri, 30 Nov 2018 22:45:20 -0800 Subject: [PATCH] Statement::fetchColumn() will throw an exception in the case of invalid index --- UPGRADE.md | 4 +++ lib/Doctrine/DBAL/Cache/ArrayStatement.php | 13 ++++++-- .../DBAL/Cache/ResultCacheStatement.php | 14 +++++++-- lib/Doctrine/DBAL/DBALException.php | 10 ++++++ .../DBAL/Driver/IBMDB2/DB2Statement.php | 8 ++++- .../DBAL/Driver/Mysqli/MysqliStatement.php | 8 ++++- .../DBAL/Driver/OCI8/OCI8Statement.php | 7 ++++- lib/Doctrine/DBAL/Driver/PDOStatement.php | 13 +++++++- .../SQLAnywhere/SQLAnywhereStatement.php | 5 ++- .../DBAL/Driver/SQLSrv/SQLSrvStatement.php | 7 ++++- phpstan.neon.dist | 5 +++ .../Tests/DBAL/Functional/StatementTest.php | 31 +++++++++++++++++++ 12 files changed, 115 insertions(+), 10 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 520fd393e49..d2b40120b32 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,9 @@ # Upgrade to 3.0 +## MINOR BC BREAK `Statement::fetchColumn()` with an invalid index. + +Similarly to `PDOStatement::fetchColumn()`, DBAL statements throw an exception in case of an invalid column index. + ## BC BREAK `Statement::execute()` with redundant parameters. Similarly to the drivers based on `pdo_pgsql` and `pdo_sqlsrv`, `OCI8Statement::execute()` and `MySQLiStatement::execute()` do not longer ignore redundant parameters. diff --git a/lib/Doctrine/DBAL/Cache/ArrayStatement.php b/lib/Doctrine/DBAL/Cache/ArrayStatement.php index 494876c935e..27ca4a83ccb 100644 --- a/lib/Doctrine/DBAL/Cache/ArrayStatement.php +++ b/lib/Doctrine/DBAL/Cache/ArrayStatement.php @@ -3,10 +3,12 @@ namespace Doctrine\DBAL\Cache; use ArrayIterator; +use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Driver\ResultStatement; use Doctrine\DBAL\FetchMode; use InvalidArgumentException; use IteratorAggregate; +use function array_key_exists; use function array_merge; use function array_values; use function count; @@ -130,7 +132,14 @@ public function fetchColumn($columnIndex = 0) { $row = $this->fetch(FetchMode::NUMERIC); - // TODO: verify that return false is the correct behavior - return $row[$columnIndex] ?? false; + if ($row === false) { + return false; + } + + if (! array_key_exists($columnIndex, $row)) { + throw DBALException::invalidColumnIndex($columnIndex, count($row)); + } + + return $row[$columnIndex]; } } diff --git a/lib/Doctrine/DBAL/Cache/ResultCacheStatement.php b/lib/Doctrine/DBAL/Cache/ResultCacheStatement.php index bd80dadb96e..66461976f75 100644 --- a/lib/Doctrine/DBAL/Cache/ResultCacheStatement.php +++ b/lib/Doctrine/DBAL/Cache/ResultCacheStatement.php @@ -4,14 +4,17 @@ use ArrayIterator; use Doctrine\Common\Cache\Cache; +use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Driver\ResultStatement; use Doctrine\DBAL\Driver\Statement; use Doctrine\DBAL\FetchMode; use InvalidArgumentException; use IteratorAggregate; +use function array_key_exists; use function array_merge; use function array_values; use function assert; +use function count; use function reset; /** @@ -179,8 +182,15 @@ public function fetchColumn($columnIndex = 0) { $row = $this->fetch(FetchMode::NUMERIC); - // TODO: verify that return false is the correct behavior - return $row[$columnIndex] ?? false; + if ($row === false) { + return false; + } + + if (! array_key_exists($columnIndex, $row)) { + throw DBALException::invalidColumnIndex($columnIndex, count($row)); + } + + return $row[$columnIndex]; } /** diff --git a/lib/Doctrine/DBAL/DBALException.php b/lib/Doctrine/DBAL/DBALException.php index 02cad00cc43..806e40f59fe 100644 --- a/lib/Doctrine/DBAL/DBALException.php +++ b/lib/Doctrine/DBAL/DBALException.php @@ -277,4 +277,14 @@ public static function typeNotFound($name) { return new self('Type to be overwritten ' . $name . ' does not exist.'); } + + public static function invalidColumnIndex(int $index, int $count) : self + { + return new self(sprintf( + 'Invalid column index %d. The statement result contains %d column%s.', + $index, + $count, + $count === 1 ? '' : 's' + )); + } } diff --git a/lib/Doctrine/DBAL/Driver/IBMDB2/DB2Statement.php b/lib/Doctrine/DBAL/Driver/IBMDB2/DB2Statement.php index d7cea000a0e..d3d0a454dad 100644 --- a/lib/Doctrine/DBAL/Driver/IBMDB2/DB2Statement.php +++ b/lib/Doctrine/DBAL/Driver/IBMDB2/DB2Statement.php @@ -2,6 +2,7 @@ namespace Doctrine\DBAL\Driver\IBMDB2; +use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Driver\Statement; use Doctrine\DBAL\Driver\StatementIterator; use Doctrine\DBAL\FetchMode; @@ -18,6 +19,7 @@ use const DB2_PARAM_FILE; use const DB2_PARAM_IN; use function array_change_key_case; +use function array_key_exists; use function count; use function db2_bind_param; use function db2_execute; @@ -338,7 +340,11 @@ public function fetchColumn($columnIndex = 0) return false; } - return $row[$columnIndex] ?? null; + if (! array_key_exists($columnIndex, $row)) { + throw DBALException::invalidColumnIndex($columnIndex, count($row)); + } + + return $row[$columnIndex]; } /** diff --git a/lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php b/lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php index 36a474fd488..9be2c3f167c 100644 --- a/lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php +++ b/lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php @@ -2,6 +2,7 @@ namespace Doctrine\DBAL\Driver\Mysqli; +use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Driver\Statement; use Doctrine\DBAL\Driver\StatementIterator; use Doctrine\DBAL\Exception\InvalidArgumentException; @@ -12,6 +13,7 @@ use mysqli_stmt; use function array_combine; use function array_fill; +use function array_key_exists; use function assert; use function count; use function feof; @@ -380,7 +382,11 @@ public function fetchColumn($columnIndex = 0) return false; } - return $row[$columnIndex] ?? null; + if (! array_key_exists($columnIndex, $row)) { + throw DBALException::invalidColumnIndex($columnIndex, count($row)); + } + + return $row[$columnIndex]; } /** diff --git a/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php b/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php index c1191bea4cf..3ab77fd1f07 100644 --- a/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php +++ b/lib/Doctrine/DBAL/Driver/OCI8/OCI8Statement.php @@ -2,6 +2,7 @@ namespace Doctrine\DBAL\Driver\OCI8; +use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Driver\Statement; use Doctrine\DBAL\Driver\StatementIterator; use Doctrine\DBAL\FetchMode; @@ -517,7 +518,11 @@ public function fetchColumn($columnIndex = 0) return false; } - return $row[$columnIndex] ?? null; + if (! array_key_exists($columnIndex, $row)) { + throw DBALException::invalidColumnIndex($columnIndex, count($row)); + } + + return $row[$columnIndex]; } /** diff --git a/lib/Doctrine/DBAL/Driver/PDOStatement.php b/lib/Doctrine/DBAL/Driver/PDOStatement.php index 8d935b55b39..e8f0a33e890 100644 --- a/lib/Doctrine/DBAL/Driver/PDOStatement.php +++ b/lib/Doctrine/DBAL/Driver/PDOStatement.php @@ -2,6 +2,7 @@ namespace Doctrine\DBAL\Driver; +use Doctrine\DBAL\DBALException; use Doctrine\DBAL\FetchMode; use Doctrine\DBAL\ParameterType; use IteratorAggregate; @@ -193,7 +194,17 @@ public function fetchAll($fetchMode = null, ...$args) public function fetchColumn($columnIndex = 0) { try { - return $this->stmt->fetchColumn($columnIndex); + $value = $this->stmt->fetchColumn($columnIndex); + + if ($value === null) { + $columnCount = $this->columnCount(); + + if ($columnIndex < 0 || $columnIndex >= $columnCount) { + throw DBALException::invalidColumnIndex($columnIndex, $columnCount); + } + } + + return $value; } catch (\PDOException $exception) { throw new PDOException($exception); } diff --git a/lib/Doctrine/DBAL/Driver/SQLAnywhere/SQLAnywhereStatement.php b/lib/Doctrine/DBAL/Driver/SQLAnywhere/SQLAnywhereStatement.php index b67b67de8a1..03139989be0 100644 --- a/lib/Doctrine/DBAL/Driver/SQLAnywhere/SQLAnywhereStatement.php +++ b/lib/Doctrine/DBAL/Driver/SQLAnywhere/SQLAnywhereStatement.php @@ -2,6 +2,7 @@ namespace Doctrine\DBAL\Driver\SQLAnywhere; +use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Driver\Statement; use Doctrine\DBAL\Driver\StatementIterator; use Doctrine\DBAL\FetchMode; @@ -284,7 +285,9 @@ public function fetchColumn($columnIndex = 0) return false; } - return $row[$columnIndex] ?? null; + if (! array_key_exists($columnIndex, $row)) { + throw DBALException::invalidColumnIndex($columnIndex, count($row)); + } } /** diff --git a/lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php b/lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php index 2229e442b54..56763ac7dcb 100644 --- a/lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php +++ b/lib/Doctrine/DBAL/Driver/SQLSrv/SQLSrvStatement.php @@ -2,6 +2,7 @@ namespace Doctrine\DBAL\Driver\SQLSrv; +use Doctrine\DBAL\DBALException; use Doctrine\DBAL\Driver\Statement; use Doctrine\DBAL\Driver\StatementIterator; use Doctrine\DBAL\FetchMode; @@ -410,7 +411,11 @@ public function fetchColumn($columnIndex = 0) return false; } - return $row[$columnIndex] ?? null; + if (! array_key_exists($columnIndex, $row)) { + throw DBALException::invalidColumnIndex($columnIndex, count($row)); + } + + return $row[$columnIndex]; } /** diff --git a/phpstan.neon.dist b/phpstan.neon.dist index e532fa5b918..428af1038d4 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -65,3 +65,8 @@ parameters: # https://github.com/doctrine/dbal/issues/3237 - '~^Call to an undefined method Doctrine\\DBAL\\Driver\\PDOStatement::nextRowset\(\)~' + + # https://github.com/phpstan/phpstan/pull/1886 + - + message: '~^Strict comparison using === between string|false and null will always evaluate to false\.~' + path: %currentWorkingDirectory%/lib/Doctrine/DBAL/Driver/PDOStatement.php diff --git a/tests/Doctrine/Tests/DBAL/Functional/StatementTest.php b/tests/Doctrine/Tests/DBAL/Functional/StatementTest.php index ad7f71018f9..0a455bd1313 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/StatementTest.php +++ b/tests/Doctrine/Tests/DBAL/Functional/StatementTest.php @@ -3,6 +3,7 @@ namespace Doctrine\Tests\DBAL\Functional; use Doctrine\DBAL\DBALException; +use Doctrine\DBAL\Driver\PDOConnection; use Doctrine\DBAL\Driver\PDOOracle\Driver as PDOOracleDriver; use Doctrine\DBAL\Driver\Statement; use Doctrine\DBAL\FetchMode; @@ -349,4 +350,34 @@ public function testExecWithRedundantParameters() : void self::expectException(DBALException::class); $stmt->execute([null]); } + + /** + * @throws DBALException + * + * @dataProvider nonExistingIndexProvider + */ + public function testFetchColumnNonExistingIndex(int $index) : void + { + if ($this->connection->getWrappedConnection() instanceof PDOConnection) { + $this->markTestSkipped('PDO supports this behavior natively but throws a different exception'); + } + + $platform = $this->connection->getDatabasePlatform(); + $query = $platform->getDummySelectSQL(); + $stmt = $this->connection->query($query); + + self::expectException(DBALException::class); + $stmt->fetchColumn($index); + } + + /** + * @return mixed[][] + */ + public static function nonExistingIndexProvider() : iterable + { + return [ + [1], + [-1], + ]; + } }