From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 23 Nov 2017 10:17:14 +0000 (-0800)
Subject: Disallow setting DBO_IGNORE in Database for sanity
X-Git-Tag: 1.31.0-rc.0~1358^2
X-Git-Url: https://git.cyclocoop.org/%242?a=commitdiff_plain;h=e0805d32e4e00a530dd8445863b2bf9e51ce2f74;p=lhc%2Fweb%2Fwiklou.git

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
---

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 );
+	}
 }