From c30b04958083cfca0f20d63474f00d03a8c03112 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 24 Oct 2015 03:22:41 -0700 Subject: [PATCH] Add $wgMaxUserDBWriteDuration to limit user-generated transactions This is a low level catch-all net for huge updates that still slip through. Features that let users add/remove arbitrarily many rows to lists of arbitrary size can easily result in high lag due to strange usage patterns or deliberate attacks. Also removed duplicate 'autochange-username' JSON key. Bug: T95501 Change-Id: I58a91ca23cae528ef1954d2d78c8f0a90681983e --- RELEASE-NOTES-1.27 | 2 ++ autoload.php | 1 + includes/DefaultSettings.php | 9 +++++++++ includes/MediaWiki.php | 19 +++++++++++++++++-- includes/db/DatabaseError.php | 6 ++++++ languages/i18n/en.json | 2 +- languages/i18n/qqq.json | 1 + 7 files changed, 37 insertions(+), 3 deletions(-) diff --git a/RELEASE-NOTES-1.27 b/RELEASE-NOTES-1.27 index 8d0853ed6e..ead82f3bb2 100644 --- a/RELEASE-NOTES-1.27 +++ b/RELEASE-NOTES-1.27 @@ -87,6 +87,8 @@ production. * Added CentralIdLookup, a service that allows extensions needing a concept of "central" users to get that without having to know about specific central authentication extensions. +* $wgMaxUserDBWriteDuration added to limit huge user-generated transactions. + Regular web request transactions that takes longer than this are aborted. === External library changes in 1.27 === ==== Upgraded external libraries ==== diff --git a/autoload.php b/autoload.php index 685849b368..e3545849ce 100644 --- a/autoload.php +++ b/autoload.php @@ -289,6 +289,7 @@ $wgAutoloadLocalClasses = array( 'DBQueryError' => __DIR__ . '/includes/db/DatabaseError.php', 'DBReadOnlyError' => __DIR__ . '/includes/db/DatabaseError.php', 'DBSiteStore' => __DIR__ . '/includes/site/DBSiteStore.php', + 'DBTransactionError' => __DIR__ . '/includes/db/DatabaseError.php', 'DBUnexpectedError' => __DIR__ . '/includes/db/DatabaseError.php', 'DataUpdate' => __DIR__ . '/includes/deferred/DataUpdate.php', 'Database' => __DIR__ . '/includes/db/Database.php', diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index e76b627cf6..ee8c8c7b65 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -7811,6 +7811,15 @@ $wgSearchRunSuggestedQuery = true; */ $wgPopularPasswordFile = __DIR__ . '/../serialized/commonpasswords.cdb'; +/* + * Max time (in seconds) a user-generated transaction can spend in writes. + * If exceeded, the transaction is rolled back with an error instead of being committed. + * + * @var int|bool Disabled if false + * @since 1.27 + */ +$wgMaxUserDBWriteDuration = false; + /** * For really cool vim folding this needs to be at the end: * vim: foldmarker=@{,@} foldmethod=marker diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php index ecfd8f87aa..c4ea536436 100644 --- a/includes/MediaWiki.php +++ b/includes/MediaWiki.php @@ -505,9 +505,25 @@ class MediaWiki { // Either all DBs should commit or none ignore_user_abort( true ); - // Commit all changes and record ChronologyProtector positions + $config = $context->getConfig(); + $factory = wfGetLBFactory(); + // Check if any transaction was too big + $limit = $config->get( 'MaxUserDBWriteDuration' ); + $factory->forEachLB( function ( LoadBalancer $lb ) use ( $limit ) { + $lb->forEachOpenConnection( function ( IDatabase $db ) use ( $limit ) { + $time = $db->pendingWriteQueryDuration(); + if ( $limit > 0 && $time > $limit ) { + throw new DBTransactionError( + $db, + wfMessage( 'transaction-duration-limit-exceeded', $time, $limit )->plain() + ); + } + } ); + } ); + // Commit all changes $factory->commitMasterChanges(); + // Record ChronologyProtector positions $factory->shutdown(); wfDebug( __METHOD__ . ': all transactions committed' ); @@ -517,7 +533,6 @@ class MediaWiki { // Set a cookie to tell all CDN edge nodes to "stick" the user to the // DC that handles this POST request (e.g. the "master" data center) $request = $context->getRequest(); - $config = $context->getConfig(); if ( $request->wasPosted() && $factory->hasOrMadeRecentMasterChanges() ) { $expires = time() + $config->get( 'DataCenterUpdateStickTTL' ); $request->response()->setCookie( 'UseDC', 'master', $expires, array( 'prefix' => '' ) ); diff --git a/includes/db/DatabaseError.php b/includes/db/DatabaseError.php index 78d26ae94b..4993eacd83 100644 --- a/includes/db/DatabaseError.php +++ b/includes/db/DatabaseError.php @@ -464,3 +464,9 @@ class DBReadOnlyError extends DBExpectedError { return $this->msg( 'readonly', 'Database is locked' ); } } + +/** + * @ingroup Database + */ +class DBTransactionError extends DBExpectedError { +} diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 32e9b00685..b9b1382ac0 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -316,6 +316,7 @@ "databaseerror-query": "Query: $1", "databaseerror-function": "Function: $1", "databaseerror-error": "Error: $1", + "transaction-duration-limit-exceeded": "In order to avoid creating high replication lag, this transaction was aborted because the write duration ($1) exceeded the $2 second limit.\nIf you are changing many items at once, trying doing multiple smaller operations instead.", "laggedslavemode": "Warning: Page may not contain recent updates.", "readonly": "Database locked", "enterlockreason": "Enter a reason for the lock, including an estimate of when the lock will be released", @@ -2631,7 +2632,6 @@ "spam_blanking": "All revisions contained links to $1, blanking", "spam_deleting": "All revisions contained links to $1, deleting", "simpleantispam-label": "Anti-spam check.\nDo not fill this in!", - "autochange-username": "MediaWiki automatic change", "pageinfo-header": "-", "pageinfo-title": "Information for \"$1\"", "pageinfo-not-current": "Sorry, it's impossible to provide this information for old revisions.", diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index 15538e636b..c93c2eb3a9 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -490,6 +490,7 @@ "databaseerror-query": "Identifies, in the list of technical details, the [[wikipedia:SQL|SQL]] statement that failed.\nParameters:\n* $1 - SQL statement (shown within a box)\n{{Identical|Query}}", "databaseerror-function": "Identifies, in the list of technical details, the function that tried to execute the database query.\nParameters:\n* $1 - Name of function\n{{Identical|Function}}", "databaseerror-error": "Identifies, in the list of technical details, the error message the database server returned.\nParameters:\n* $1 - Error message from the database server, probably in English\n{{Identical|Error}}", + "transaction-duration-limit-exceeded": "Plain text error shown when DB updates take too long. Parameters:\n* $1 - time spent in database updates\n* $2 - maximum time allowed in database updates", "laggedslavemode": "Used as warning when getting the timestamp of the latest version, if in LaggedSlaveMode.", "readonly": "Used as title of error message when database is locked.", "enterlockreason": "For developers when locking the database", -- 2.20.1