PDO: Raise a proper exception if user or password is false (#6513)

| Q | A |------------- | ----------- | Type | bug | Fixed issues | #6512 #### Summary If `false` (or anything that is not a string) is passed as `user` or `password`, we run into a `TypeError` because we pass that value as-is to the constructor of PDO. This started to happen after we enabled strict types on our driver classes in 4.0. On 3.9, `false` would implicitly be cast to an empty string which is either desired or leads to more obscure connection errors. We could restore the behavior of 3.9 by adding explicit type casts to the two parameters. But since we don't document `false` as a valid value for either parameter, my preference would indeed be raising an exception.
doctrine · Sep 2, 2024 · ab82363 · ab82363
1 parent b9183ca
commit ab82363
Show file tree

Hide file tree

Showing 13 changed files with 170 additions and 10 deletions.
diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -63,6 +63,9 @@ parameters:
         -
             message: '~^Parameter #1 \$driverOptions of method Doctrine\\DBAL\\Tests\\Functional\\Driver\\Mysqli\\ConnectionTest\:\:getConnection\(\) expects array<string, mixed>, .* given\.$~'
             path: tests/Functional/Driver/Mysqli/ConnectionTest.php
+        -
+            message: '~^Parameter #1 \$params of method Doctrine\\DBAL\\Driver\:\:connect\(\) expects array~'
+            path: tests/Driver/PDO/*/DriverTest.php
 
         # DriverManagerTest::testDatabaseUrl() should be refactored as it's too dynamic.
         -

diff --git a/psalm.xml.dist b/psalm.xml.dist
@@ -134,6 +134,7 @@
         <InvalidArgument>
             <errorLevel type="suppress">
                 <!-- We're testing with invalid input here. -->
+                <file name="tests/Driver/PDO/*/DriverTest.php"/>
                 <file name="tests/Functional/Driver/Mysqli/ConnectionTest.php"/>
                 <file name="tests/Platforms/AbstractPlatformTestCase.php"/>
             </errorLevel>

diff --git a/src/Driver/PDO/Exception/InvalidConfiguration.php b/src/Driver/PDO/Exception/InvalidConfiguration.php
@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\DBAL\Driver\PDO\Exception;
+
+use Doctrine\DBAL\Driver\AbstractException;
+
+use function get_debug_type;
+use function sprintf;
+
+/** @psalm-immutable */
+final class InvalidConfiguration extends AbstractException
+{
+    public static function notAStringOrNull(string $key, mixed $value): self
+    {
+        return new self(sprintf(
+            'The %s configuration parameter is expected to be either a string or null, got %s.',
+            $key,
+            get_debug_type($value),
+        ));
+    }
+}
diff --git a/src/Driver/PDO/MySQL/Driver.php b/src/Driver/PDO/MySQL/Driver.php
@@ -7,10 +7,13 @@
 use Doctrine\DBAL\Driver\AbstractMySQLDriver;
 use Doctrine\DBAL\Driver\PDO\Connection;
 use Doctrine\DBAL\Driver\PDO\Exception;
+use Doctrine\DBAL\Driver\PDO\Exception\InvalidConfiguration;
 use PDO;
 use PDOException;
 use SensitiveParameter;
 
+use function is_string;
+
 final class Driver extends AbstractMySQLDriver
 {
     /**
@@ -26,6 +29,12 @@ public function connect(
             $driverOptions[PDO::ATTR_PERSISTENT] = true;
         }
 
+        foreach (['user', 'password'] as $key) {
+            if (isset($params[$key]) && ! is_string($params[$key])) {
+                throw InvalidConfiguration::notAStringOrNull($key, $params[$key]);
+            }
+        }
+
         $safeParams = $params;
         unset($safeParams['password']);
 

diff --git a/src/Driver/PDO/OCI/Driver.php b/src/Driver/PDO/OCI/Driver.php
@@ -7,10 +7,13 @@
 use Doctrine\DBAL\Driver\AbstractOracleDriver;
 use Doctrine\DBAL\Driver\PDO\Connection;
 use Doctrine\DBAL\Driver\PDO\Exception;
+use Doctrine\DBAL\Driver\PDO\Exception\InvalidConfiguration;
 use PDO;
 use PDOException;
 use SensitiveParameter;
 
+use function is_string;
+
 final class Driver extends AbstractOracleDriver
 {
     /**
@@ -26,6 +29,12 @@ public function connect(
             $driverOptions[PDO::ATTR_PERSISTENT] = true;
         }
 
+        foreach (['user', 'password'] as $key) {
+            if (isset($params[$key]) && ! is_string($params[$key])) {
+                throw InvalidConfiguration::notAStringOrNull($key, $params[$key]);
+            }
+        }
+
         $safeParams = $params;
         unset($safeParams['password']);
 

diff --git a/src/Driver/PDO/PgSQL/Driver.php b/src/Driver/PDO/PgSQL/Driver.php
@@ -7,10 +7,13 @@
 use Doctrine\DBAL\Driver\AbstractPostgreSQLDriver;
 use Doctrine\DBAL\Driver\PDO\Connection;
 use Doctrine\DBAL\Driver\PDO\Exception;
+use Doctrine\DBAL\Driver\PDO\Exception\InvalidConfiguration;
 use PDO;
 use PDOException;
 use SensitiveParameter;
 
+use function is_string;
+
 final class Driver extends AbstractPostgreSQLDriver
 {
     /**
@@ -26,6 +29,12 @@ public function connect(
             $driverOptions[PDO::ATTR_PERSISTENT] = true;
         }
 
+        foreach (['user', 'password'] as $key) {
+            if (isset($params[$key]) && ! is_string($params[$key])) {
+                throw InvalidConfiguration::notAStringOrNull($key, $params[$key]);
+            }
+        }
+
         $safeParams = $params;
         unset($safeParams['password']);
 

diff --git a/src/Driver/PDO/SQLSrv/Driver.php b/src/Driver/PDO/SQLSrv/Driver.php
@@ -9,10 +9,12 @@
 use Doctrine\DBAL\Driver\Exception;
 use Doctrine\DBAL\Driver\PDO\Connection as PDOConnection;
 use Doctrine\DBAL\Driver\PDO\Exception as PDOException;
+use Doctrine\DBAL\Driver\PDO\Exception\InvalidConfiguration;
 use PDO;
 use SensitiveParameter;
 
 use function is_int;
+use function is_string;
 use function sprintf;
 
 final class Driver extends AbstractSQLServerDriver
@@ -40,6 +42,12 @@ public function connect(
             $driverOptions[PDO::ATTR_PERSISTENT] = true;
         }
 
+        foreach (['user', 'password'] as $key) {
+            if (isset($params[$key]) && ! is_string($params[$key])) {
+                throw InvalidConfiguration::notAStringOrNull($key, $params[$key]);
+            }
+        }
+
         $safeParams = $params;
         unset($safeParams['password']);
 

diff --git a/src/Driver/PDO/SQLite/Driver.php b/src/Driver/PDO/SQLite/Driver.php
@@ -7,11 +7,13 @@
 use Doctrine\DBAL\Driver\AbstractSQLiteDriver;
 use Doctrine\DBAL\Driver\PDO\Connection;
 use Doctrine\DBAL\Driver\PDO\Exception;
+use Doctrine\DBAL\Driver\PDO\Exception\InvalidConfiguration;
 use PDO;
 use PDOException;
 use SensitiveParameter;
 
 use function array_intersect_key;
+use function is_string;
 
 final class Driver extends AbstractSQLiteDriver
 {
@@ -22,6 +24,12 @@ public function connect(
         #[SensitiveParameter]
         array $params,
     ): Connection {
+        foreach (['user', 'password'] as $key) {
+            if (isset($params[$key]) && ! is_string($params[$key])) {
+                throw InvalidConfiguration::notAStringOrNull($key, $params[$key]);
+            }
+        }
+
         try {
             $pdo = new PDO(
                 $this->constructPdoDsn(array_intersect_key($params, ['path' => true, 'memory' => true])),

diff --git a/tests/Driver/PDO/MySQL/DriverTest.php b/tests/Driver/PDO/MySQL/DriverTest.php
@@ -4,13 +4,31 @@
 
 namespace Doctrine\DBAL\Tests\Driver\PDO\MySQL;
 
-use Doctrine\DBAL\Driver as DriverInterface;
+use Doctrine\DBAL\Driver\PDO\Exception\InvalidConfiguration;
 use Doctrine\DBAL\Driver\PDO\MySQL\Driver;
 use Doctrine\DBAL\Tests\Driver\AbstractMySQLDriverTestCase;
 
 class DriverTest extends AbstractMySQLDriverTestCase
 {
-    protected function createDriver(): DriverInterface
+    public function testUserIsFalse(): void
+    {
+        $this->expectException(InvalidConfiguration::class);
+        $this->expectExceptionMessage(
+            'The user configuration parameter is expected to be either a string or null, got bool.',
+        );
+        $this->driver->connect(['user' => false]);
+    }
+
+    public function testPasswordIsFalse(): void
+    {
+        $this->expectException(InvalidConfiguration::class);
+        $this->expectExceptionMessage(
+            'The password configuration parameter is expected to be either a string or null, got bool.',
+        );
+        $this->driver->connect(['password' => false]);
+    }
+
+    protected function createDriver(): Driver
     {
         return new Driver();
     }

diff --git a/tests/Driver/PDO/OCI/DriverTest.php b/tests/Driver/PDO/OCI/DriverTest.php
@@ -4,13 +4,31 @@
 
 namespace Doctrine\DBAL\Tests\Driver\PDO\OCI;
 
-use Doctrine\DBAL\Driver as DriverInterface;
+use Doctrine\DBAL\Driver\PDO\Exception\InvalidConfiguration;
 use Doctrine\DBAL\Driver\PDO\OCI\Driver;
 use Doctrine\DBAL\Tests\Driver\AbstractOracleDriverTestCase;
 
 class DriverTest extends AbstractOracleDriverTestCase
 {
-    protected function createDriver(): DriverInterface
+    public function testUserIsFalse(): void
+    {
+        $this->expectException(InvalidConfiguration::class);
+        $this->expectExceptionMessage(
+            'The user configuration parameter is expected to be either a string or null, got bool.',
+        );
+        $this->driver->connect(['user' => false]);
+    }
+
+    public function testPasswordIsFalse(): void
+    {
+        $this->expectException(InvalidConfiguration::class);
+        $this->expectExceptionMessage(
+            'The password configuration parameter is expected to be either a string or null, got bool.',
+        );
+        $this->driver->connect(['password' => false]);
+    }
+
+    protected function createDriver(): Driver
     {
         return new Driver();
     }

diff --git a/tests/Driver/PDO/PgSQL/DriverTest.php b/tests/Driver/PDO/PgSQL/DriverTest.php
@@ -4,9 +4,9 @@
 
 namespace Doctrine\DBAL\Tests\Driver\PDO\PgSQL;
 
-use Doctrine\DBAL\Driver as DriverInterface;
 use Doctrine\DBAL\Driver\Connection;
 use Doctrine\DBAL\Driver\PDO;
+use Doctrine\DBAL\Driver\PDO\Exception\InvalidConfiguration;
 use Doctrine\DBAL\Driver\PDO\PgSQL\Driver;
 use Doctrine\DBAL\Tests\Driver\AbstractPostgreSQLDriverTestCase;
 use Doctrine\DBAL\Tests\TestUtil;
@@ -60,7 +60,25 @@ public function testConnectionDisablePreparesWhenDisablePreparesIsExplicitlyDefi
         );
     }
 
-    protected function createDriver(): DriverInterface
+    public function testUserIsFalse(): void
+    {
+        $this->expectException(InvalidConfiguration::class);
+        $this->expectExceptionMessage(
+            'The user configuration parameter is expected to be either a string or null, got bool.',
+        );
+        $this->driver->connect(['user' => false]);
+    }
+
+    public function testPasswordIsFalse(): void
+    {
+        $this->expectException(InvalidConfiguration::class);
+        $this->expectExceptionMessage(
+            'The password configuration parameter is expected to be either a string or null, got bool.',
+        );
+        $this->driver->connect(['password' => false]);
+    }
+
+    protected function createDriver(): Driver
     {
         return new Driver();
     }

diff --git a/tests/Driver/PDO/SQLSrv/DriverTest.php b/tests/Driver/PDO/SQLSrv/DriverTest.php
@@ -4,13 +4,31 @@
 
 namespace Doctrine\DBAL\Tests\Driver\PDO\SQLSrv;
 
-use Doctrine\DBAL\Driver as DriverInterface;
+use Doctrine\DBAL\Driver\PDO\Exception\InvalidConfiguration;
 use Doctrine\DBAL\Driver\PDO\SQLSrv\Driver;
 use Doctrine\DBAL\Tests\Driver\AbstractSQLServerDriverTestCase;
 
 class DriverTest extends AbstractSQLServerDriverTestCase
 {
-    protected function createDriver(): DriverInterface
+    public function testUserIsFalse(): void
+    {
+        $this->expectException(InvalidConfiguration::class);
+        $this->expectExceptionMessage(
+            'The user configuration parameter is expected to be either a string or null, got bool.',
+        );
+        $this->driver->connect(['user' => false]);
+    }
+
+    public function testPasswordIsFalse(): void
+    {
+        $this->expectException(InvalidConfiguration::class);
+        $this->expectExceptionMessage(
+            'The password configuration parameter is expected to be either a string or null, got bool.',
+        );
+        $this->driver->connect(['password' => false]);
+    }
+
+    protected function createDriver(): Driver
     {
         return new Driver();
     }

diff --git a/tests/Driver/PDO/SQLite/DriverTest.php b/tests/Driver/PDO/SQLite/DriverTest.php
@@ -4,13 +4,31 @@
 
 namespace Doctrine\DBAL\Tests\Driver\PDO\SQLite;
 
-use Doctrine\DBAL\Driver as DriverInterface;
+use Doctrine\DBAL\Driver\PDO\Exception\InvalidConfiguration;
 use Doctrine\DBAL\Driver\PDO\SQLite\Driver;
 use Doctrine\DBAL\Tests\Driver\AbstractSQLiteDriverTestCase;
 
 class DriverTest extends AbstractSQLiteDriverTestCase
 {
-    protected function createDriver(): DriverInterface
+    public function testUserIsFalse(): void
+    {
+        $this->expectException(InvalidConfiguration::class);
+        $this->expectExceptionMessage(
+            'The user configuration parameter is expected to be either a string or null, got bool.',
+        );
+        $this->driver->connect(['user' => false]);
+    }
+
+    public function testPasswordIsFalse(): void
+    {
+        $this->expectException(InvalidConfiguration::class);
+        $this->expectExceptionMessage(
+            'The password configuration parameter is expected to be either a string or null, got bool.',
+        );
+        $this->driver->connect(['password' => false]);
+    }
+
+    protected function createDriver(): Driver
     {
         return new Driver();
     }