From d6b989b5506c4afc5f4da47d5cbf394e278738c2 Mon Sep 17 00:00:00 2001 From: daniel Date: Mon, 28 May 2018 21:19:11 +0200 Subject: [PATCH 1/1] Use "Unknown user" instead of an empty user name. This changes the user name to "User unknown" when constructing a RevisionRecord from a database row that has an empty ar_user_text resp rev_user_text field. This may cause "User unknown" to be written to the database, if the RevisionRecord is used as the basis for a new revision that is being created, particularly during undeletion. Since "Unknown user" is listed in $wgReservedUsernames, this should never lead to conflicts with actual user names. It is assumed that empty ar_user_text and rev_user_text fields will be fixed during migration to the new actor based database schema. Bug: T195692 Change-Id: I506c513b019778d83741e47f0d11093f5ab67a54 --- includes/DefaultSettings.php | 2 +- includes/Storage/RevisionStore.php | 8 +- .../Storage/RevisionStoreDbTestBase.php | 76 +++++++++++++++++++ 3 files changed, 81 insertions(+), 5 deletions(-) diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 343e80d6f5..8abff9792e 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -4833,7 +4833,7 @@ $wgReservedUsernames = [ 'Maintenance script', // Maintenance scripts which perform editing, image import script 'Template namespace initialisation script', // Used in 1.2->1.3 upgrade 'ScriptImporter', // Default user name used by maintenance/importSiteScripts.php - 'Unknown user', // Used in WikiImporter when importing revisions with no author + 'Unknown user', // Used in WikiImporter and RevisionStore for revisions with no author 'msg:double-redirect-fixer', // Automatic double redirect fix 'msg:usermessage-editor', // Default user for leaving user messages 'msg:proxyblocker', // For $wgProxyList and Special:Blockme (removed in 1.22) diff --git a/includes/Storage/RevisionStore.php b/includes/Storage/RevisionStore.php index 879f3229d3..61b428f13c 100644 --- a/includes/Storage/RevisionStore.php +++ b/includes/Storage/RevisionStore.php @@ -1711,8 +1711,8 @@ class RevisionStore $row->ar_actor ?? null ); } catch ( InvalidArgumentException $ex ) { - wfWarn( __METHOD__ . ': ' . $ex->getMessage() ); - $user = new UserIdentityValue( 0, '', 0 ); + wfWarn( __METHOD__ . ': ' . $title->getPrefixedDBkey() . ': ' . $ex->getMessage() ); + $user = new UserIdentityValue( 0, 'Unknown user', 0 ); } $db = $this->getDBConnectionRefForQueryFlags( $queryFlags ); @@ -1759,8 +1759,8 @@ class RevisionStore $row->rev_actor ?? null ); } catch ( InvalidArgumentException $ex ) { - wfWarn( __METHOD__ . ': ' . $ex->getMessage() ); - $user = new UserIdentityValue( 0, '', 0 ); + wfWarn( __METHOD__ . ': ' . $title->getPrefixedDBkey() . ': ' . $ex->getMessage() ); + $user = new UserIdentityValue( 0, 'Unknown user', 0 ); } $db = $this->getDBConnectionRefForQueryFlags( $queryFlags ); diff --git a/tests/phpunit/includes/Storage/RevisionStoreDbTestBase.php b/tests/phpunit/includes/Storage/RevisionStoreDbTestBase.php index 3e042f9816..910cdc401a 100644 --- a/tests/phpunit/includes/Storage/RevisionStoreDbTestBase.php +++ b/tests/phpunit/includes/Storage/RevisionStoreDbTestBase.php @@ -17,6 +17,7 @@ use MediaWiki\Storage\RevisionRecord; use MediaWiki\Storage\RevisionStore; use MediaWiki\Storage\SlotRecord; use MediaWiki\Storage\SqlBlobStore; +use MediaWiki\User\UserIdentityValue; use MediaWikiTestCase; use PHPUnit_Framework_MockObject_MockObject; use Revision; @@ -1050,6 +1051,81 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase { /** * @covers \MediaWiki\Storage\RevisionStore::newRevisionFromArchiveRow + */ + public function testNewRevisionFromArchiveRow_no_user() { + $store = MediaWikiServices::getInstance()->getRevisionStore(); + + $row = (object)[ + 'ar_id' => '1', + 'ar_page_id' => '2', + 'ar_namespace' => '0', + 'ar_title' => 'Something', + 'ar_rev_id' => '2', + 'ar_text_id' => '47', + 'ar_timestamp' => '20180528192356', + 'ar_minor_edit' => '0', + 'ar_deleted' => '0', + 'ar_len' => '78', + 'ar_parent_id' => '0', + 'ar_sha1' => 'deadbeef', + 'ar_comment_text' => 'whatever', + 'ar_comment_data' => null, + 'ar_comment_cid' => null, + 'ar_user' => '0', + 'ar_user_text' => '', // this is the important bit + 'ar_actor' => null, + 'ar_content_format' => null, + 'ar_content_model' => null, + ]; + + \Wikimedia\suppressWarnings(); + $record = $store->newRevisionFromArchiveRow( $row ); + \Wikimedia\suppressWarnings( true ); + + $this->assertInstanceOf( RevisionRecord::class, $record ); + $this->assertInstanceOf( UserIdentityValue::class, $record->getUser() ); + $this->assertSame( 'Unknown user', $record->getUser()->getName() ); + } + + /** + * @covers \MediaWiki\Storage\RevisionStore::newRevisionFromRow + */ + public function testNewRevisionFromRow_no_user() { + $store = MediaWikiServices::getInstance()->getRevisionStore(); + $title = Title::newFromText( __METHOD__ ); + + $row = (object)[ + 'rev_id' => '2', + 'rev_page' => '2', + 'page_namespace' => '0', + 'page_title' => $title->getText(), + 'rev_text_id' => '47', + 'rev_timestamp' => '20180528192356', + 'rev_minor_edit' => '0', + 'rev_deleted' => '0', + 'rev_len' => '78', + 'rev_parent_id' => '0', + 'rev_sha1' => 'deadbeef', + 'rev_comment_text' => 'whatever', + 'rev_comment_data' => null, + 'rev_comment_cid' => null, + 'rev_user' => '0', + 'rev_user_text' => '', // this is the important bit + 'rev_actor' => null, + 'rev_content_format' => null, + 'rev_content_model' => null, + ]; + + \Wikimedia\suppressWarnings(); + $record = $store->newRevisionFromRow( $row, 0, $title ); + \Wikimedia\suppressWarnings( true ); + + $this->assertNotNull( $record ); + $this->assertNotNull( $record->getUser() ); + $this->assertNotEmpty( $record->getUser()->getName() ); + } + + /** * @covers \MediaWiki\Storage\RevisionStore::insertRevisionOn */ public function testInsertRevisionOn_archive() { -- 2.20.1