From: daniel Date: Tue, 19 Dec 2017 16:11:08 +0000 (+0100) Subject: Prevent write operations to database replicas. X-Git-Tag: 1.31.0-rc.0~1162^2 X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=36f4daf32c591d6b7e2435629fc6e431398b641a Prevent write operations to database replicas. Bug: T183265 Change-Id: I8e17644d1b447416adee18e42cf0122b52a80b22 --- diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php index 7f0718c952..1c9cad58cb 100644 --- a/includes/libs/rdbms/database/Database.php +++ b/includes/libs/rdbms/database/Database.php @@ -236,6 +236,12 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware /** @var TransactionProfiler */ protected $trxProfiler; + /** + * @var bool Whether writing is allowed on this connection. + * Should be false for connections to replicas. + */ + protected $allowWrite = true; + /** * Constructor and database handle and attempt to connect to the DB server * @@ -277,6 +283,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware $this->connLogger = $params['connLogger']; $this->queryLogger = $params['queryLogger']; $this->errorLogger = $params['errorLogger']; + $this->allowWrite = empty( $params['noWrite'] ); // Set initial dummy domain until open() sets the final DB/prefix $this->currentDomain = DatabaseDomain::newUnspecified(); @@ -908,6 +915,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware } if ( $isWrite ) { + if ( !$this->allowWrite ) { + throw new DBError( + $this, + 'Write operations are not allowed on this database connection!' + ); + } + # In theory, non-persistent writes are allowed in read-only mode, but due to things # like https://bugs.mysql.com/bug.php?id=33669 that might not work anyway... $reason = $this->getReadOnlyReason(); @@ -2951,6 +2965,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware } final public function begin( $fname = __METHOD__, $mode = self::TRANSACTION_EXPLICIT ) { + if ( !$this->allowWrite ) { + throw new DBError( + $this, + 'Write operations are not allowed on this database connection!' + ); + } + // Protect against mismatched atomic section, transaction nesting, and snapshot loss if ( $this->mTrxLevel ) { if ( $this->mTrxAtomicLevels ) { @@ -3012,6 +3033,14 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware } final public function commit( $fname = __METHOD__, $flush = '' ) { + if ( !$this->allowWrite ) { + // we should never get here, since begin() would already throw + throw new DBError( + $this, + 'Write operations are not allowed on this database connection!' + ); + } + if ( $this->mTrxLevel && $this->mTrxAtomicLevels ) { // There are still atomic sections open. This cannot be ignored $levels = implode( ', ', $this->mTrxAtomicLevels ); diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php index 86c43350a0..b565b3b0ef 100644 --- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php @@ -87,6 +87,9 @@ interface ILoadBalancer { /** @var int DB handle should have DBO_TRX disabled and the caller will leave it as such */ const CONN_TRX_AUTO = 1; + /** Disable writing for the given connection. Used internally. Do not use with DB_MASTER! */ + const CONN_NO_WRITE = 2; + /** * Construct a manager of IDatabase connection objects * diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php index a9eaa9974e..eb288dd63d 100644 --- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php @@ -645,6 +645,12 @@ class LoadBalancer implements ILoadBalancer { $oldConnsOpened = $this->connsOpened; // connections open now if ( $i == self::DB_MASTER ) { + if ( $flags & self::CONN_NO_WRITE ) { + throw new InvalidArgumentException( + 'Cannot set CONN_NO_WRITE flag on master connection!' + ); + } + $i = $this->getWriterIndex(); } else { # Try to find an available server in any the query groups (in order) @@ -655,6 +661,9 @@ class LoadBalancer implements ILoadBalancer { break; } } + + // Request no-write connection, even if $i == $this->getWriterIndex(). + $flags |= self::CONN_NO_WRITE; } # Operation-based index @@ -671,6 +680,9 @@ class LoadBalancer implements ILoadBalancer { $this->reportConnectionError(); return null; // not reached } + + // Request no-write connection, even if $i == $this->getWriterIndex(). + $flags |= self::CONN_NO_WRITE; } # Now we have an explicit index into the servers array @@ -791,6 +803,13 @@ class LoadBalancer implements ILoadBalancer { // a) those are usually set to implicitly use transaction rounds via DBO_TRX // b) those must support the use of explicit transaction rounds via beginMasterChanges() $autoCommit = ( ( $flags & self::CONN_TRX_AUTO ) == self::CONN_TRX_AUTO ); + $noWrite = ( ( $flags & self::CONN_NO_WRITE ) == self::CONN_NO_WRITE ); + + if ( $noWrite && $i === $this->getWriterIndex() ) { + // We can't disable writes on the master connection! + // TODO: Wrap the master connection, so write operations fail! + $noWrite = false; + } if ( $domain !== false ) { // Connection is to a foreign domain @@ -807,6 +826,7 @@ class LoadBalancer implements ILoadBalancer { // Open a new connection $server = $this->mServers[$i]; $server['serverIndex'] = $i; + $server['noWrite'] = $noWrite; $server['autoCommitOnly'] = $autoCommit; $conn = $this->reallyOpenConnection( $server, false ); $host = $this->getServerName( $i ); @@ -863,6 +883,13 @@ class LoadBalancer implements ILoadBalancer { $dbName = $domainInstance->getDatabase(); $prefix = $domainInstance->getTablePrefix(); $autoCommit = ( ( $flags & self::CONN_TRX_AUTO ) == self::CONN_TRX_AUTO ); + $noWrite = ( ( $flags & self::CONN_NO_WRITE ) == self::CONN_NO_WRITE ); + + if ( $noWrite && $i === $this->getWriterIndex() ) { + // We can't disable writes on the master connection! + // TODO: Wrap the master connection, so write operations fail! + $noWrite = false; + } if ( $autoCommit ) { $connFreeKey = self::KEY_FOREIGN_FREE_NOROUND; @@ -910,6 +937,7 @@ class LoadBalancer implements ILoadBalancer { $server['foreignPoolRefCount'] = 0; $server['foreign'] = true; $server['autoCommitOnly'] = $autoCommit; + $server['noWrite'] = $noWrite; $conn = $this->reallyOpenConnection( $server, $dbName ); if ( !$conn->isOpen() ) { $this->connLogger->warning( __METHOD__ . ": connection error for $i/$domain" );