From 43959d9926e1f4a55246f7001b6882de172aace3 Mon Sep 17 00:00:00 2001 From: Mike Fellows Date: Fri, 11 Aug 2017 10:36:19 -0700 Subject: [PATCH] Remove the 'persistent' config option for Sqlserver connections - The PDO::ATTR_PERSISTENT connection configuration option is not allowed by the SQL Server PHP driver. In newer versions of the driver an exception is thrown if that option is set. In older versions of the driver it triggered a heap corruption bug which usually caused intermittent crashes. --- src/Database/Driver/Sqlserver.php | 15 +++- .../Database/Driver/SqlserverTest.php | 70 ++++++++++++++++++- 2 files changed, 80 insertions(+), 5 deletions(-) diff --git a/src/Database/Driver/Sqlserver.php b/src/Database/Driver/Sqlserver.php index 3d7531cf9b7..de65f4aa309 100644 --- a/src/Database/Driver/Sqlserver.php +++ b/src/Database/Driver/Sqlserver.php @@ -35,7 +35,6 @@ class Sqlserver extends Driver * @var array */ protected $_baseConfig = [ - 'persistent' => false, 'host' => 'localhost\SQLEXPRESS', 'username' => '', 'password' => '', @@ -54,8 +53,14 @@ class Sqlserver extends Driver ]; /** - * Establishes a connection to the database server + * Establishes a connection to the database server. * + * Please note that the PDO::ATTR_PERSISTENT attribute is not supported by + * the SQL Server PHP PDO drivers. As a result you cannot use the + * persistent config option when connecting to a SQL Server (for more + * information see: https://github.com/Microsoft/msphpsql/issues/65). + * + * @throws \InvalidArgumentException if an unsupported setting is in the driver config * @return bool true on success */ public function connect() @@ -64,8 +69,12 @@ public function connect() return true; } $config = $this->_config; + + if (isset($config['persistent']) && $config['persistent']) { + throw new \InvalidArgumentException('Config setting "persistent" cannot be set to true, as the Sqlserver PDO driver does not support PDO::ATTR_PERSISTENT'); + } + $config['flags'] += [ - PDO::ATTR_PERSISTENT => $config['persistent'], PDO::ATTR_EMULATE_PREPARES => false, PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION ]; diff --git a/tests/TestCase/Database/Driver/SqlserverTest.php b/tests/TestCase/Database/Driver/SqlserverTest.php index ff80aa93338..080dd251437 100644 --- a/tests/TestCase/Database/Driver/SqlserverTest.php +++ b/tests/TestCase/Database/Driver/SqlserverTest.php @@ -103,7 +103,6 @@ public function testConnectionConfigCustom() { $this->skipIf($this->missingExtension, 'pdo_sqlsrv is not installed.'); $config = [ - 'persistent' => false, 'host' => 'foo', 'username' => 'Administrator', 'password' => 'blablabla', @@ -121,7 +120,6 @@ public function testConnectionConfigCustom() $expected = $config; $expected['flags'] += [ - PDO::ATTR_PERSISTENT => false, PDO::ATTR_EMULATE_PREPARES => false, PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, PDO::SQLSRV_ATTR_ENCODING => 'a-language' @@ -159,6 +157,74 @@ public function testConnectionConfigCustom() $driver->connect(); } + /** + * Test connecting to Sqlserver with persistent set to false + * + * @return void + */ + public function testConnectionPersistentFalse() + { + $this->skipIf($this->missingExtension, 'pdo_sqlsrv is not installed.'); + $config = [ + 'persistent' => false, + 'host' => 'foo', + 'username' => 'Administrator', + 'password' => 'blablabla', + 'database' => 'bar', + 'encoding' => 'a-language', + ]; + $driver = $this->getMockBuilder('Cake\Database\Driver\Sqlserver') + ->setMethods(['_connect', 'connection']) + ->setConstructorArgs([$config]) + ->getMock(); + $dsn = 'sqlsrv:Server=foo;Database=bar;MultipleActiveResultSets=false'; + + $expected = $config; + $expected['flags'] = [ + PDO::ATTR_EMULATE_PREPARES => false, + PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, + PDO::SQLSRV_ATTR_ENCODING => 'a-language' + ]; + $expected['attributes'] = []; + $expected['settings'] = []; + $expected['init'] = []; + $expected['app'] = null; + $expected['connectionPooling'] = null; + $expected['failoverPartner'] = null; + $expected['loginTimeout'] = null; + $expected['multiSubnetFailover'] = null; + + $driver->expects($this->once())->method('_connect') + ->with($dsn, $expected); + + $driver->connect(); + } + + /** + * Test if attempting to connect with the driver throws an exception when + * using an invalid config setting. + * + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage Config setting "persistent" cannot be set to true, as the Sqlserver PDO driver does not support PDO::ATTR_PERSISTENT + * @return void + */ + public function testConnectionPersistentTrueException() + { + $this->skipIf($this->missingExtension, 'pdo_sqlsrv is not installed.'); + $config = [ + 'persistent' => true, + 'host' => 'foo', + 'username' => 'Administrator', + 'password' => 'blablabla', + 'database' => 'bar', + ]; + $driver = $this->getMockBuilder('Cake\Database\Driver\Sqlserver') + ->setMethods(['_connect', 'connection']) + ->setConstructorArgs([$config]) + ->getMock(); + $driver->connect(); + } + /** * Test select with limit only and SQLServer2012+ *