Upgrade mismatched begin() warnings to exceptions
[lhc/web/wiklou.git] / includes / db / Database.php
index 351d438..92e89b0 100644 (file)
@@ -859,8 +859,12 @@ abstract class DatabaseBase implements IDatabase {
 
                # Try reconnecting if the connection was lost
                if ( false === $ret && $this->wasErrorReissuable() ) {
-                       # Transaction is gone, like it or not
-                       $hadTrx = $this->mTrxLevel; // possible lost transaction
+                       # Transaction is gone; this can mean lost writes or REPEATABLE-READ snapshots
+                       $hadTrx = $this->mTrxLevel;
+                       # T127428: for non-write transactions, a disconnect and a COMMIT are similar:
+                       # neither changed data and in both cases any read snapshots are reset anyway.
+                       $isNoopCommit = ( !$this->writesOrCallbacksPending() && $sql === 'COMMIT' );
+                       # Update state tracking to reflect transaction loss
                        $this->mTrxLevel = 0;
                        $this->mTrxIdleCallbacks = []; // bug 65263
                        $this->mTrxPreCommitCallbacks = []; // bug 65263
@@ -874,12 +878,12 @@ abstract class DatabaseBase implements IDatabase {
                                $msg = __METHOD__ . ": lost connection to $server; reconnected";
                                wfDebugLog( 'DBPerformance', "$msg:\n" . wfBacktrace( true ) );
 
-                               if ( $hadTrx || $this->mNamedLocksHeld ) {
+                               if ( ( $hadTrx && !$isNoopCommit ) || $this->mNamedLocksHeld ) {
                                        # Leave $ret as false and let an error be reported.
                                        # Callers may catch the exception and continue to use the DB.
                                        $this->reportQueryError( $lastError, $lastErrno, $sql, $fname, $tempIgnore );
                                } else {
-                                       # Should be safe to silently retry (no trx and thus no callbacks)
+                                       # Should be safe to silently retry (no trx/callbacks/locks)
                                        $startTime = microtime( true );
                                        $ret = $this->doQuery( $commentedSql );
                                        $queryRuntime = microtime( true ) - $startTime;
@@ -1222,6 +1226,7 @@ abstract class DatabaseBase implements IDatabase {
                return '';
        }
 
+       // See IDatabase::select for the docs for this function
        public function select( $table, $vars, $conds = '', $fname = __METHOD__,
                $options = [], $join_conds = [] ) {
                $sql = $this->selectSQLText( $table, $vars, $conds, $fname, $options, $join_conds );
@@ -1554,8 +1559,8 @@ abstract class DatabaseBase implements IDatabase {
                                                // Special-case single values, as IN isn't terribly efficient
                                                // Don't necessarily assume the single key is 0; we don't
                                                // enforce linear numeric ordering on other arrays here.
-                                               $value = array_values( $value );
-                                               $list .= $field . " = " . $this->addQuotes( $value[0] );
+                                               $value = array_values( $value )[0];
+                                               $list .= $field . " = " . $this->addQuotes( $value );
                                        } else {
                                                $list .= $field . " IN (" . $this->makeList( $value ) . ") ";
                                        }
@@ -1664,6 +1669,8 @@ abstract class DatabaseBase implements IDatabase {
         * themselves. Pass the canonical name to such functions. This is only needed
         * when calling query() directly.
         *
+        * @note This function does not sanitize user input. It is not safe to use
+        *   this function to escape user input.
         * @param string $name Database table name
         * @param string $format One of:
         *   quoted - Automatically pass the table name through addIdentifierQuotes()
@@ -1840,7 +1847,7 @@ abstract class DatabaseBase implements IDatabase {
                if ( !$alias || (string)$alias === (string)$name ) {
                        return $name;
                } else {
-                       return $name . ' AS ' . $alias; // PostgreSQL needs AS
+                       return $name . ' AS ' . $this->addIdentifierQuotes( $alias ); // PostgreSQL needs AS
                }
        }
 
@@ -1977,6 +1984,8 @@ abstract class DatabaseBase implements IDatabase {
         * Returns if the given identifier looks quoted or not according to
         * the database convention for quoting identifiers .
         *
+        * @note Do not use this to determine if untrusted input is safe.
+        *   A malicious user can trick this function.
         * @param string $name
         * @return bool
         */
@@ -2580,17 +2589,13 @@ abstract class DatabaseBase implements IDatabase {
                        } elseif ( !$this->mTrxAutomatic ) {
                                // We want to warn about inadvertently nested begin/commit pairs, but not about
                                // auto-committing implicit transactions that were started by query() via DBO_TRX
-                               $msg = "$fname: Transaction already in progress (from {$this->mTrxFname}), " .
-                                       " performing implicit commit!";
-                               wfWarn( $msg );
-                               wfLogDBError( $msg,
-                                       $this->getLogContext( [
-                                               'method' => __METHOD__,
-                                               'fname' => $fname,
-                                       ] )
+                               throw new DBUnexpectedError(
+                                       $this,
+                                       "$fname: Transaction already in progress (from {$this->mTrxFname}), " .
+                                               " performing implicit commit!"
                                );
                        } else {
-                               // if the transaction was automatic and has done write operations
+                               // The transaction was automatic and has done write operations
                                if ( $this->mTrxDoneWrites ) {
                                        wfDebug( "$fname: Automatic transaction with writes in progress" .
                                                " (from {$this->mTrxFname}), performing implicit commit!\n"