PleaseStand has uploaded a new change for review. https://gerrit.wikimedia.org/r/198661
Change subject: [WIP] Optionally log warnings generated by MySQL ...................................................................... [WIP] Optionally log warnings generated by MySQL There have been multiple cases in which strings are silently truncated when they are stored in the database. When fields are binary fields, this can result in ill-formed UTF-8. Under MySQL's strict mode, enabled by default in recent versions[1], an error would occur. However, strict mode is not enabled in WMF production. MediaWiki even turns it off by default! Fortunately, MySQL generates a warning even when strict mode is disabled. In fact, it even does for some other cases as well. If we log these warnings, we can discover and fix the bugs that cause them, without breaking the site by immediately enabling strict mode. This change adds an option to log warnings generated by successful queries to a new log group "DBWarnings". This is done only when using the mysqli extension; the original mysql extension was designed for MySQL 3 and thus lacks a sane way to determine whether a particular query generated any warnings. For some queries (e.g. INSERT IGNORE in MariaDB), certain warnings are expected and are thus not logged. The list of error codes to avoid logging is configurable. [1]: In MySQL 5.6+, the sample my.cnf file specifies a sql_mode containing STRICT_TRANS_TABLES; in 5.7.5+, the default sql_mode contains that option. Change-Id: I3314c6ac0bcf4beb19fb95c3a2ca61d5d46b44ae --- M includes/DefaultSettings.php M includes/db/DatabaseMysqli.php 2 files changed, 54 insertions(+), 2 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/61/198661/1 diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 1df809e..4b0fb99 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -1931,6 +1931,22 @@ */ $wgDBWindowsAuthentication = false; +/** + * Set to true to log warnings reported by the database, for successful + * queries only, to the "DBWarnings" log group. + * + * Currently only works for MySQL connections using the mysqli PHP extension. + */ +$wgDBLogWarnings = false; + +/** + * List of MySQL error codes to ignore when logging warnings + * (see $wgDBLogWarnings). + */ +$wgDBIgnoredMysqlWarnings = array( + 1062, // ER_DUP_ENTRY, see note in <https://mariadb.com/kb/en/mariadb/ignore/> +); + /**@}*/ # End of DB settings } /************************************************************************//** diff --git a/includes/db/DatabaseMysqli.php b/includes/db/DatabaseMysqli.php index ad12e19..30fe645 100644 --- a/includes/db/DatabaseMysqli.php +++ b/includes/db/DatabaseMysqli.php @@ -30,14 +30,46 @@ */ class DatabaseMysqli extends DatabaseMysqlBase { /** + * @var int + */ + protected $affectedRows = 0; + + /** * @param string $sql * @return resource */ protected function doQuery( $sql ) { - if ( $this->bufferResults() ) { + $bufferResults = $this->bufferResults(); + if ( $bufferResults ) { $ret = $this->mConn->query( $sql ); } else { $ret = $this->mConn->query( $sql, MYSQLI_USE_RESULT ); + } + + // Get the number of affected rows now; if we end up performing + // a SHOW WARNINGS query, that number would otherwise be lost. + $this->affectedRows = $this->mConn->affected_rows; + + global $wgDBLogWarnings, $wgDBIgnoredMysqlWarnings; + if ( $wgDBLogWarnings ) { + // If the query succeeded yet generated one or more warnings, log + // any that we are configured to log. We don't do this for unbuffered + // queries; there aren't many of them, and we would have to defer + // SHOW WARNINGS until the result set has been freed. + if ( $ret && $this->mConn->warning_count && ( $bufferResults || $ret === true ) ) { + $messagesToLog = array(); + $warnRet = $this->mConn->query( 'SHOW WARNINGS' ); + while ( $warnRet && ( $warning = $warnRet->fetch_row() ) ) { + list( $level, $code, $message ) = $warning; + if ( $level !== 'Note' && !in_array( $code, $wgDBIgnoredMysqlWarnings ) ) { + $messagesToLog[] = "$level (Code $code): $message"; + } + } + + if ( $messagesToLog ) { + wfDebugLog( 'DBWarnings', "$sql\n" . implode( "\n", $messagesToLog ) ); + } + } } return $ret; @@ -152,7 +184,11 @@ * @return int */ function affectedRows() { - return $this->mConn->affected_rows; + if ( !$this->mConn ) { + throw new DBUnexpectedError( $this, "DB connection was already closed." ); + } + + return $this->affectedRows; // saved in doQuery() } /** -- To view, visit https://gerrit.wikimedia.org/r/198661 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3314c6ac0bcf4beb19fb95c3a2ca61d5d46b44ae Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: PleaseStand <[email protected]> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
