From e0805d32e4e00a530dd8445863b2bf9e51ce2f74 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Thu, 23 Nov 2017 02:17:14 -0800 Subject: [PATCH] Disallow setting DBO_IGNORE in Database for sanity In the off chance something called this, it would break all sorts of code that expects that either query result functions either succeed or throw an error. Callers are not expected to have to check if the result of a query is meaningful or false due to an error. Change-Id: I0b4fe1403f55a399ffd40817ed12f857087d6f83 --- includes/libs/rdbms/database/Database.php | 17 ++++++++++++--- .../rdbms/database/DatabaseMysqlBaseTest.php | 21 +++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php index e04566eb49..5edf3fddde 100644 --- a/includes/libs/rdbms/database/Database.php +++ b/includes/libs/rdbms/database/Database.php @@ -461,9 +461,12 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware protected function ignoreErrors( $ignoreErrors = null ) { $res = $this->getFlag( self::DBO_IGNORE ); if ( $ignoreErrors !== null ) { - $ignoreErrors - ? $this->setFlag( self::DBO_IGNORE ) - : $this->clearFlag( self::DBO_IGNORE ); + // setFlag()/clearFlag() do not allow DBO_IGNORE changes for sanity + if ( $ignoreErrors ) { + $this->mFlags |= self::DBO_IGNORE; + } else { + $this->mFlags &= ~self::DBO_IGNORE; + } } return $res; @@ -621,6 +624,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware } public function setFlag( $flag, $remember = self::REMEMBER_NOTHING ) { + if ( ( $flag & self::DBO_IGNORE ) ) { + throw new \UnexpectedValueException( "Modifying DBO_IGNORE is not allowed." ); + } + if ( $remember === self::REMEMBER_PRIOR ) { array_push( $this->priorFlags, $this->mFlags ); } @@ -628,6 +635,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware } public function clearFlag( $flag, $remember = self::REMEMBER_NOTHING ) { + if ( ( $flag & self::DBO_IGNORE ) ) { + throw new \UnexpectedValueException( "Modifying DBO_IGNORE is not allowed." ); + } + if ( $remember === self::REMEMBER_PRIOR ) { array_push( $this->priorFlags, $this->mFlags ); } diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php index 456447f01c..461ef09291 100644 --- a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php +++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php @@ -27,6 +27,7 @@ use Wikimedia\Rdbms\TransactionProfiler; use Wikimedia\Rdbms\DatabaseDomain; use Wikimedia\Rdbms\MySQLMasterPos; use Wikimedia\Rdbms\DatabaseMysqlBase; +use Wikimedia\Rdbms\Database; /** * Fake class around abstract class so we can call concrete methods. @@ -368,4 +369,24 @@ class DatabaseMysqlBaseTest extends PHPUnit_Framework_TestCase { [ 1000.77 ], ]; } + + /** + * @expectedException UnexpectedValueException + * @covers Wikimedia\Rdbms\Database::setFlag + */ + public function testDBOIgnoreSet() { + $db = new FakeDatabaseMysqlBase(); + + $db->setFlag( Database::DBO_IGNORE ); + } + + /** + * @expectedException UnexpectedValueException + * @covers Wikimedia\Rdbms\Database::clearFlag + */ + public function testDBOIgnoreClear() { + $db = new FakeDatabaseMysqlBase(); + + $db->clearFlag( Database::DBO_IGNORE ); + } } -- 2.20.1