From ab82363d7ba838a5919500237c62a3b4092f462e Mon Sep 17 00:00:00 2001
From: "Alexander M. Turek" <me@derrabus.de>
Date: Mon, 2 Sep 2024 17:02:20 +0200
Subject: [PATCH] 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.
---
 phpstan.neon.dist                             |  3 +++
 psalm.xml.dist                                |  1 +
 .../PDO/Exception/InvalidConfiguration.php    | 23 +++++++++++++++++++
 src/Driver/PDO/MySQL/Driver.php               |  9 ++++++++
 src/Driver/PDO/OCI/Driver.php                 |  9 ++++++++
 src/Driver/PDO/PgSQL/Driver.php               |  9 ++++++++
 src/Driver/PDO/SQLSrv/Driver.php              |  8 +++++++
 src/Driver/PDO/SQLite/Driver.php              |  8 +++++++
 tests/Driver/PDO/MySQL/DriverTest.php         | 22 ++++++++++++++++--
 tests/Driver/PDO/OCI/DriverTest.php           | 22 ++++++++++++++++--
 tests/Driver/PDO/PgSQL/DriverTest.php         | 22 ++++++++++++++++--
 tests/Driver/PDO/SQLSrv/DriverTest.php        | 22 ++++++++++++++++--
 tests/Driver/PDO/SQLite/DriverTest.php        | 22 ++++++++++++++++--
 13 files changed, 170 insertions(+), 10 deletions(-)
 create mode 100644 src/Driver/PDO/Exception/InvalidConfiguration.php

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
index 164caf3ae56..2a04d32d39c 100644
--- 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
index 56d83d38e34..af28777385c 100644
--- 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
new file mode 100644
index 00000000000..4f9d9c2d415
--- /dev/null
+++ 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
index 963fef04bad..fad4b55954c 100644
--- 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
index 4c02de134a1..45f3ea25b01 100644
--- 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
index a267e8407fc..39e8a9452f1 100644
--- 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
index a3cae9785f4..7950c41a5e5 100644
--- 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
index 74194a59c38..4bef08bf997 100644
--- 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
index f56bad7f904..82168baac6f 100644
--- 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
index 9cf86eaf9fa..3d16b288e79 100644
--- 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
index cb4b3ad9aa0..5f6cdbc71a1 100644
--- 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
index 3d3fda54699..37b8d57fb7b 100644
--- 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
index 88bc29d20c5..b75e9f4ba6e 100644
--- 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();
     }