Skip to content

Commit

Permalink
Merge branch 'hotfix/44' into develop
Browse files Browse the repository at this point in the history
Forward port #44
  • Loading branch information
michalbundyra committed Jan 14, 2020
2 parents a0de9fe + be9c372 commit 1c5de74
Show file tree
Hide file tree
Showing 18 changed files with 315 additions and 86 deletions.
15 changes: 15 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,23 @@ matrix:
env:
- DEPS=lowest
- php: 7.3
services:
- mysql
env:
- DEPS=latest
- TEST_INTEGRATION=true
- TESTS_LAMINAS_DB_ADAPTER_DRIVER_MYSQL=true
- TESTS_LAMINAS_DB_ADAPTER_DRIVER_MYSQL_HOSTNAME=127.0.0.1
- php: 7.3
services:
- postgresql
addons:
postgresql: "9.6"
env:
- DEPS=latest
- TEST_INTEGRATION=true
- TESTS_LAMINAS_DB_ADAPTER_DRIVER_PGSQL=true
- TESTS_LAMINAS_DB_ADAPTER_DRIVER_PGSQL_HOSTNAME=127.0.0.1

before_install:
- if [[ $TEST_COVERAGE != 'true' ]]; then phpenv config-rm xdebug.ini || return 0 ; fi
Expand Down
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ All notable changes to this project will be documented in this file, in reverse

### Fixed

- Nothing.
- [#44](https://github.com/laminas/laminas-db/pull/44) fixes preventing \PDO disconnect in some `Laminas\Db\Adapter\Platform\AbstractPlatform` inheritors (Mysql, Oracle, Postgresql, SqlServer).
Using \PDO as a driver in Platform is not recommended. It can be used safely when wrapped by `Laminas\Db\Adapter\Driver\Pdo\Pdo`.

## 2.11.1 - 2020-01-10

Expand Down
48 changes: 28 additions & 20 deletions src/Adapter/Platform/Mysql.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ class Mysql extends AbstractPlatform
protected $quoteIdentifierTo = '``';

/**
* @var \mysqli|\PDO
* @var \mysqli|\PDO|Pdo\Pdo|Mysqli\Mysqli
*/
protected $resource = null;
protected $driver = null;

/**
* NOTE: Include dashes for MySQL only, need tests for others platforms
Expand Down Expand Up @@ -60,7 +60,7 @@ public function setDriver($driver)
|| ($driver instanceof \mysqli)
|| ($driver instanceof \PDO && $driver->getAttribute(\PDO::ATTR_DRIVER_NAME) == 'mysql')
) {
$this->resource = $driver;
$this->driver = $driver;
return $this;
}

Expand Down Expand Up @@ -90,32 +90,40 @@ public function quoteIdentifierChain($identifierChain)
*/
public function quoteValue($value)
{
if ($this->resource instanceof DriverInterface) {
$this->resource = $this->resource->getConnection()->getResource();
}
if ($this->resource instanceof \mysqli) {
return '\'' . $this->resource->real_escape_string($value) . '\'';
}
if ($this->resource instanceof \PDO) {
return $this->resource->quote($value);
}
return parent::quoteValue($value);
$quotedViaDriverValue = $this->quoteViaDriver($value);

return $quotedViaDriverValue !== null ? $quotedViaDriverValue : parent::quoteValue($value);
}

/**
* {@inheritDoc}
*/
public function quoteTrustedValue($value)
{
if ($this->resource instanceof DriverInterface) {
$this->resource = $this->resource->getConnection()->getResource();
$quotedViaDriverValue = $this->quoteViaDriver($value);

return $quotedViaDriverValue !== null ? $quotedViaDriverValue : parent::quoteTrustedValue($value);
}

/**
* @param string $value
* @return string|null
*/
protected function quoteViaDriver($value)
{
if ($this->driver instanceof DriverInterface) {
$resource = $this->driver->getConnection()->getResource();
} else {
$resource = $this->driver;
}
if ($this->resource instanceof \mysqli) {
return '\'' . $this->resource->real_escape_string($value) . '\'';

if ($resource instanceof \mysqli) {
return '\'' . $resource->real_escape_string($value) . '\'';
}
if ($this->resource instanceof \PDO) {
return $this->resource->quote($value);
if ($resource instanceof \PDO) {
return $resource->quote($value);
}
return parent::quoteTrustedValue($value);

return null;
}
}
14 changes: 8 additions & 6 deletions src/Adapter/Platform/Oracle.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,18 @@ public function quoteIdentifierChain($identifierChain)
public function quoteValue($value)
{
if ($this->resource instanceof DriverInterface) {
$this->resource = $this->resource->getConnection()->getResource();
$resource = $this->resource->getConnection()->getResource();
} else {
$resource = $this->resource;
}

if ($this->resource) {
if ($this->resource instanceof PDO) {
return $this->resource->quote($value);
if ($resource) {
if ($resource instanceof PDO) {
return $resource->quote($value);
}

if (get_resource_type($this->resource) == 'oci8 connection'
|| get_resource_type($this->resource) == 'oci8 persistent connection'
if (get_resource_type($resource) == 'oci8 connection'
|| get_resource_type($resource) == 'oci8 persistent connection'
) {
return "'" . addcslashes(str_replace("'", "''", $value), "\x00\n\r\"\x1a") . "'";
}
Expand Down
48 changes: 28 additions & 20 deletions src/Adapter/Platform/Postgresql.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ class Postgresql extends AbstractPlatform
protected $quoteIdentifierTo = '""';

/**
* @var resource|\PDO
* @var resource|\PDO|Pdo\Pdo|Pgsql\Pgsql
*/
protected $resource = null;
protected $driver = null;

/**
* @param null|\Laminas\Db\Adapter\Driver\Pgsql\Pgsql|\Laminas\Db\Adapter\Driver\Pdo\Pdo|resource|\PDO $driver
Expand All @@ -49,7 +49,7 @@ public function setDriver($driver)
|| (is_resource($driver) && (in_array(get_resource_type($driver), ['pgsql link', 'pgsql link persistent'])))
|| ($driver instanceof \PDO && $driver->getAttribute(\PDO::ATTR_DRIVER_NAME) == 'pgsql')
) {
$this->resource = $driver;
$this->driver = $driver;
return $this;
}

Expand Down Expand Up @@ -80,32 +80,40 @@ public function quoteIdentifierChain($identifierChain)
*/
public function quoteValue($value)
{
if ($this->resource instanceof DriverInterface) {
$this->resource = $this->resource->getConnection()->getResource();
}
if (is_resource($this->resource)) {
return '\'' . pg_escape_string($this->resource, $value) . '\'';
}
if ($this->resource instanceof \PDO) {
return $this->resource->quote($value);
}
return 'E' . parent::quoteValue($value);
$quotedViaDriverValue = $this->quoteViaDriver($value);

return $quotedViaDriverValue !== null ? $quotedViaDriverValue : ('E' . parent::quoteValue($value));
}

/**
* {@inheritDoc}
*/
public function quoteTrustedValue($value)
{
if ($this->resource instanceof DriverInterface) {
$this->resource = $this->resource->getConnection()->getResource();
$quotedViaDriverValue = $this->quoteViaDriver($value);

return $quotedViaDriverValue !== null ? $quotedViaDriverValue : ('E' . parent::quoteTrustedValue($value));
}

/**
* @param string $value
* @return string|null
*/
protected function quoteViaDriver($value)
{
if ($this->driver instanceof DriverInterface) {
$resource = $this->driver->getConnection()->getResource();
} else {
$resource = $this->driver;
}
if (is_resource($this->resource)) {
return '\'' . pg_escape_string($this->resource, $value) . '\'';

if (is_resource($resource)) {
return '\'' . pg_escape_string($resource, $value) . '\'';
}
if ($this->resource instanceof \PDO) {
return $this->resource->quote($value);
if ($resource instanceof \PDO) {
return $resource->quote($value);
}
return 'E' . parent::quoteTrustedValue($value);

return null;
}
}
21 changes: 13 additions & 8 deletions src/Adapter/Platform/SqlServer.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,13 @@ public function quoteIdentifierChain($identifierChain)
*/
public function quoteValue($value)
{
if ($this->resource instanceof DriverInterface) {
$this->resource = $this->resource->getConnection()->getResource();
$resource = $this->resource;

if ($resource instanceof DriverInterface) {
$resource = $resource->getConnection()->getResource();
}
if ($this->resource instanceof \PDO) {
return $this->resource->quote($value);
if ($resource instanceof \PDO) {
return $resource->quote($value);
}
trigger_error(
'Attempting to quote a value in ' . __CLASS__ . ' without extension/driver support '
Expand All @@ -107,12 +109,15 @@ public function quoteValue($value)
*/
public function quoteTrustedValue($value)
{
if ($this->resource instanceof DriverInterface) {
$this->resource = $this->resource->getConnection()->getResource();
$resource = $this->resource;

if ($resource instanceof DriverInterface) {
$resource = $resource->getConnection()->getResource();
}
if ($this->resource instanceof \PDO) {
return $this->resource->quote($value);
if ($resource instanceof \PDO) {
return $resource->quote($value);
}

return '\'' . str_replace('\'', '\'\'', $value) . '\'';
}
}
7 changes: 6 additions & 1 deletion test/integration/Adapter/Driver/Mysqli/ConnectionTest.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
<?php

/**
* @see https://github.com/laminas/laminas-db for the canonical source repository
* @copyright https://github.com/laminas/laminas-db/blob/master/COPYRIGHT.md
* @license https://github.com/laminas/laminas-db/blob/master/LICENSE.md New BSD License
*/

namespace LaminasIntegrationTest\Db\Adapter\Driver\Mysqli;

use Laminas\Db\Adapter\Driver\Mysqli\Connection;
Expand All @@ -11,7 +17,6 @@
*/
class ConnectionTest extends TestCase
{

use TraitSetup;

public function testConnectionOk()
Expand Down
10 changes: 10 additions & 0 deletions test/integration/Adapter/Driver/Mysqli/TableGatewayTest.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
<?php

/**
* @see https://github.com/laminas/laminas-db for the canonical source repository
* @copyright https://github.com/laminas/laminas-db/blob/master/COPYRIGHT.md
* @license https://github.com/laminas/laminas-db/blob/master/LICENSE.md New BSD License
*/

namespace LaminasIntegrationTest\Db\Adapter\Driver\Mysqli;

use Laminas\Db\Adapter\Adapter;
Expand Down Expand Up @@ -27,6 +33,8 @@ public function testSelectWithEmptyCurrentWithBufferResult()
$rowset = $tableGateway->select('id = 0');

$this->assertNull($rowset->current());

$adapter->getDriver()->getConnection()->disconnect();
}

/**
Expand All @@ -46,5 +54,7 @@ public function testSelectWithEmptyCurrentWithoutBufferResult()
$rowset = $tableGateway->select('id = 0');

$this->assertNull($rowset->current());

$adapter->getDriver()->getConnection()->disconnect();
}
}
79 changes: 79 additions & 0 deletions test/integration/Adapter/Driver/Pdo/AbstractAdapterTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
<?php

/**
* @see https://github.com/laminas/laminas-db for the canonical source repository
* @copyright https://github.com/laminas/laminas-db/blob/master/COPYRIGHT.md
* @license https://github.com/laminas/laminas-db/blob/master/LICENSE.md New BSD License
*/

namespace LaminasIntegrationTest\Db\Adapter\Driver\Pdo;

use Laminas\Db\Adapter\Adapter;
use PHPUnit\Framework\TestCase;

/**
* @property Adapter $adapter
*/
abstract class AbstractAdapterTest extends TestCase
{
const DB_SERVER_PORT = null;

/**
* @covers \Laminas\Db\Adapter\Adapter::__construct()
*/
public function testConnection()
{
$this->assertInstanceOf(Adapter::class, $this->adapter);
}

public function testDriverDisconnectAfterQuoteWithPlatform()
{
$isTcpConnection = $this->isTcpConnection();

$this->adapter->getDriver()->getConnection()->connect();

self::assertTrue($this->adapter->getDriver()->getConnection()->isConnected());
if ($isTcpConnection) {
self::assertTrue($this->isConnectedTcp());
}

$this->adapter->getDriver()->getConnection()->disconnect();

self::assertFalse($this->adapter->getDriver()->getConnection()->isConnected());
if ($isTcpConnection) {
self::assertFalse($this->isConnectedTcp());
}

$this->adapter->getDriver()->getConnection()->connect();

self::assertTrue($this->adapter->getDriver()->getConnection()->isConnected());
if ($isTcpConnection) {
self::assertTrue($this->isConnectedTcp());
}

$this->adapter->getPlatform()->quoteValue('test');

$this->adapter->getDriver()->getConnection()->disconnect();

self::assertFalse($this->adapter->getDriver()->getConnection()->isConnected());
if ($isTcpConnection) {
self::assertFalse($this->isConnectedTcp());
}
}

protected function isConnectedTcp()
{
$mypid = getmypid();
$dbPort = static::DB_SERVER_PORT;
$lsof = shell_exec("lsof -i -P -n | grep $dbPort | grep $mypid");

return $lsof !== null;
}

protected function isTcpConnection()
{
return $this->getHostname() !== 'localhost';
}

abstract protected function getHostname();
}
Loading

0 comments on commit 1c5de74

Please sign in to comment.