Skip to content

Commit

Permalink
FIX PHP 8.1 support in MySQLiConnector::query errors (#10570)
Browse files Browse the repository at this point in the history 
* FIX PHP 8.1 support in MySQLiConnector::query errors

The default error reporting mode in PHP 8.1 has changed from using
errors reported on the connection handle to throwing
mysqli_sql_exception. query() makes no allowance for this, and
functions up the call stack expect to catch
Silverstripe\ORM\Connect\DatabaseException instead - resulting in the
MySQLi exception going all the way up to halt the system.

We can use a try, catch, and finally to retain backwards compatibility,
no matter which setting (e.g. PHP version default) someone has enabled.

* Move MySQLConnector test skip call into setUp()

As review feedback; marking the test as skipped in a private function
obfuscated where the call was happening and made it harder to skimread
the tests. Moving this into a setUp function makes it obvious the check
is run before each test case, and skipped if necessary.
  • Loading branch information
@NightJar
NightJar authored Jul 7, 2023
1 parent ab4802c commit 8c3ba81
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 54 deletions.
17 changes: 12 additions & 5 deletions src/ORM/Connect/MySQLiConnector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,19 @@ public function query($sql, $errorLevel = E_USER_ERROR)
{
$this->beforeQuery($sql);

// Benchmark query
$handle = $this->dbConn->query($sql ?? '', MYSQLI_STORE_RESULT);
$error = null;
$handle = null;

if (!$handle || $this->dbConn->error) {
$this->databaseError($this->getLastError(), $errorLevel, $sql);
return null;
try {
// Benchmark query
$handle = $this->dbConn->query($sql ?? '', MYSQLI_STORE_RESULT);
} catch (mysqli_sql_exception $e) {
$error = $e->getMessage();
} finally {
if (!$handle || $this->dbConn->error) {
$this->databaseError($error ?? $this->getLastError(), $errorLevel, $sql);
return null;
}
}

// Some non-select queries return true on success
Expand Down
111 changes: 62 additions & 49 deletions tests/php/ORM/MySQLiConnectorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,60 @@

namespace SilverStripe\ORM\Tests;

use mysqli_driver;
use SilverStripe\Dev\SapphireTest;
use SilverStripe\Dev\TestOnly;
use SilverStripe\ORM\Tests\MySQLiConnectorTest\MySQLiConnector;
use SilverStripe\ORM\Connect\DatabaseException;
use SilverStripe\ORM\DB;
use SilverStripe\ORM\Tests\MySQLiConnectorTest\MySQLiConnector;
use SilverStripe\Tests\ORM\Utf8\Utf8TestHelper;

/**
* @requires extension mysqli
*/
class MySQLiConnectorTest extends SapphireTest implements TestOnly
{
/**
* @dataProvider charsetProvider
*/
public function testConnectionCharsetControl($charset, $defaultCollation)
/** @var array project database settings configuration */
private $config = [];

private function getConnector(?string $charset = null, ?string $collation = null, bool $selectDb = false)
{
$config = DB::getConfig();
$config['charset'] = $charset;
$config = $this->config;

if ($charset) {
$config['charset'] = $charset;
}
if ($collation) {
$config['collation'] = $collation;
}

$config['database'] = 'information_schema';

$connector = new MySQLiConnector();
$connector->connect($config, $selectDb);

return $connector;
}

public function setUp(): void
{
parent::setUp();

$config = DB::getConfig();

if (strtolower(substr($config['type'] ?? '', 0, 5)) !== 'mysql') {
return $this->markTestSkipped('The test only relevant for MySQL');
$this->markTestSkipped("The test only relevant for MySQL - but $config[type] is in use");
}

$connector = new MySQLiConnector();
$connector->connect($config);
$this->config = $config;
}

/**
* @dataProvider charsetProvider
*/
public function testConnectionCharsetControl($charset, $defaultCollation)
{
$connector = $this->getConnector($charset);
$connection = $connector->getMysqliConnection();

$cset = $connection->get_charset();
Expand All @@ -47,17 +75,7 @@ public function testConnectionCharsetControl($charset, $defaultCollation)
*/
public function testConnectionCollationControl($charset, $defaultCollation, $customCollation)
{
$config = DB::getConfig();
$config['charset'] = $charset;
$config['collation'] = $customCollation;
$config['database'] = 'information_schema';

if (strtolower(substr($config['type'] ?? '', 0, 5)) !== 'mysql') {
return $this->markTestSkipped('The test only relevant for MySQL');
}

$connector = new MySQLiConnector();
$connector->connect($config);
$connector = $this->getConnector($charset, $customCollation);
$connection = $connector->getMysqliConnection();

$cset = $connection->get_charset();
Expand Down Expand Up @@ -101,20 +119,7 @@ public function charsetProvider()

public function testUtf8mb4GeneralCollation()
{
$charset = 'utf8mb4';
$collation = 'utf8mb4_general_ci';

$config = DB::getConfig();
$config['charset'] = $charset;
$config['collation'] = $collation;
$config['database'] = 'information_schema';

if (strtolower(substr($config['type'] ?? '', 0, 5)) !== 'mysql') {
return $this->markTestSkipped('The test only relevant for MySQL');
}

$connector = new MySQLiConnector();
$connector->connect($config, true);
$connector = $this->getConnector('utf8mb4', 'utf8mb4_general_ci', true);
$connection = $connector->getMysqliConnection();

$result = $connection->query(
Expand All @@ -127,20 +132,7 @@ public function testUtf8mb4GeneralCollation()

public function testUtf8mb4UnicodeCollation()
{
$charset = 'utf8mb4';
$collation = 'utf8mb4_unicode_ci';

$config = DB::getConfig();
$config['charset'] = $charset;
$config['collation'] = $collation;
$config['database'] = 'information_schema';

if (strtolower(substr($config['type'] ?? '', 0, 5)) !== 'mysql') {
return $this->markTestSkipped('The test only relevant for MySQL');
}

$connector = new MySQLiConnector();
$connector->connect($config, true);
$connector = $this->getConnector('utf8mb4', 'utf8mb4_unicode_ci', true);
$connection = $connector->getMysqliConnection();

$result = $connection->query(
Expand All @@ -151,4 +143,25 @@ public function testUtf8mb4UnicodeCollation()
$this->assertEquals('rßt', $result[0][0]);
$this->assertEquals('rst', $result[1][0]);
}

public function testQueryThrowsDatabaseErrorOnMySQLiError()
{
$connector = $this->getConnector();
$driver = new mysqli_driver();
// The default with PHP >= 8.0
$driver->report_mode = MYSQLI_REPORT_OFF;
$this->expectException(DatabaseException::class);
$connector = $this->getConnector(null, null, true);
$connector->query('force an error with invalid SQL');
}

public function testQueryThrowsDatabaseErrorOnMySQLiException()
{
$driver = new mysqli_driver();
// The default since PHP 8.1 - https://www.php.net/manual/en/mysqli-driver.report-mode.php
$driver->report_mode = MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT;
$this->expectException(DatabaseException::class);
$connector = $this->getConnector(null, null, true);
$connector->query('force an error with invalid SQL');
}
}

0 comments on commit 8c3ba81

Please sign in to comment.