diff --git a/includes/api/ApiMessageTrait.php b/includes/api/ApiMessageTrait.php index 3cc2a6720dc..18269f217c5 100644 --- a/includes/api/ApiMessageTrait.php +++ b/includes/api/ApiMessageTrait.php @@ -66,6 +66,7 @@ trait ApiMessageTrait { 'importuploaderrorpartial' => 'partialupload', 'importuploaderrorsize' => 'filetoobig', 'importuploaderrortemp' => 'notempdir', + 'ipb-block-not-found' => 'alreadyblocked', 'ipb_already_blocked' => 'alreadyblocked', 'ipb_blocked_as_range' => 'blockedasrange', 'ipb_cant_unblock' => 'cantunblock', diff --git a/includes/block/DatabaseBlockStore.php b/includes/block/DatabaseBlockStore.php index 891c6adbc67..2bbe63a51f9 100644 --- a/includes/block/DatabaseBlockStore.php +++ b/includes/block/DatabaseBlockStore.php @@ -1098,17 +1098,29 @@ class DatabaseBlockStore { // Update bt_count field in existing target, if there is one if ( $isUser ) { $targetConds = [ 'bt_user' => $targetUserId ]; + $targetLockKey = $dbw->getDomainID() . ':block:u:' . $targetUserId; } else { $targetConds = [ 'bt_address' => $targetAddress, 'bt_auto' => $isAuto, ]; + $targetLockKey = $dbw->getDomainID() . ':block:' . + ( $isAuto ? 'a' : 'i' ) . ':' . $targetAddress; } $condsWithCount = $targetConds; if ( $expectedTargetCount !== null ) { $condsWithCount['bt_count'] = $expectedTargetCount; } + $dbw->lock( $targetLockKey, __METHOD__ ); + $func = __METHOD__; + $dbw->onTransactionCommitOrIdle( + static function () use ( $dbw, $targetLockKey, $func ) { + $dbw->unlock( $targetLockKey, "$func.closure" ); + }, + __METHOD__ + ); + // This query locks the index gap when the target doesn't exist yet, // so there is a risk of throttling adjacent block insertions, // especially on small wikis which have larger gaps. If this proves to @@ -1126,6 +1138,7 @@ class DatabaseBlockStore { ->select( 'bt_id' ) ->from( 'block_target' ) ->where( $targetConds ) + ->forUpdate() ->caller( __METHOD__ ) ->fetchFieldValues(); if ( count( $ids ) > 1 ) { diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 45431a1cec8..be107df1ab9 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -2664,7 +2664,7 @@ "blockipsuccesssub": "Block succeeded", "blockipsuccesstext": "[[Special:Contributions/$1|$1]] has been blocked.
\nSee the [[Special:BlockList|block list]] to review blocks.", "ipb-empty-block": "The block submitted has no restrictions enabled.", - "ipb-block-not-found": "The block could not be made, but no existing block was found for \"$1\". If this problem persists, please [https://www.mediawiki.org/wiki/Special:MyLanguage/Help_talk:Blocking_users report it].", + "ipb-block-not-found": "The block could not be made, probably because another administrator tried to block this user at the same time. Check the block status and try again.", "ipb-blockingself": "You are about to block yourself! Are you sure you want to do that?", "ipb-confirmhideuser": "You are about to block a user with \"hide user\" enabled. This will suppress the user's name in all lists and log entries. Are you sure you want to do that?", "ipb-confirmaction": "If you are sure you really want to do it, please check the \"{{int:ipb-confirm}}\" field at the bottom.", diff --git a/tests/api-testing/action/Block.js b/tests/api-testing/action/Block.js new file mode 100644 index 00000000000..8053137494b --- /dev/null +++ b/tests/api-testing/action/Block.js @@ -0,0 +1,29 @@ +'use strict'; + +const { action, assert } = require( 'api-testing' ); + +describe( 'Block', () => { + const ip = '::' + Math.floor( Math.random() * 65534 ).toString( 16 ); + it( 'should not allow multiblocks without newblock (T389028)', async () => { + const mindy = await action.mindy(); + const token = await mindy.token(); + const promises = [ + mindy.request( { action: 'block', user: ip, token: token }, 'POST' ), + mindy.request( { action: 'block', user: ip, token: token }, 'POST' ) + ]; + const res = await Promise.all( promises ); + assert.lengthOf( res, 2 ); + assert.equal( res[ 0 ].status, 200 ); + assert.equal( res[ 1 ].status, 200 ); + + const goodIndex = 'block' in res[ 0 ].body ? 0 : 1; + const goodBody = res[ goodIndex ].body; + const badBody = res[ +!goodIndex ].body; + + assert.property( goodBody, 'block' ); + assert.isOk( goodBody.block.id ); + + assert.property( badBody, 'error' ); + assert.equal( badBody.error.code, 'alreadyblocked' ); + } ); +} );