Log warning and show error on empty username

Historically it seems that if Linker::userLink or friends were passed an
empty username (probably due to an incorrect database entry), they would
produce bogus output, e.g., an <a> with no contents or a link to the
invalid page "User_talk:" or similar.

In b6e1e99bec we replaced an occurrence of Title::makeTitle() (no
safety checks!) with creating a TitleValue, which asserts in its
constructor that the title text is not empty. This made such pages fail
an assertion and stop displaying at all.

Now there's a proper check for the error. Such cases will log a
production error and return "(no username available)".

Bug: T222529
Change-Id: Id65bdf9666b0d16e5553b8f38c7cf8fce2e37a25

RELEASE-NOTES-1.34

@@ -55,7 +55,10 @@ For notes on 1.33.x and older releases, see HISTORY.
 * …
 
 === Bug fixes in 1.34 ===
-* …
+* (T222529) If a log entry or page revision is recorded in the database with an
+  empty username, attempting to display it will log an error and return a "no
+  username available" to the user instead of silently displaying nothing or
+  invalid links.
 
 === Action API changes in 1.34 ===
 * The 'recenteditcount' response property from action=query list=allusers,

includes/Linker.php

@@ -894,6 +894,12 @@ class Linker {
 	 * @since 1.16.3. $altUserName was added in 1.19.
 	 */
 	public static function userLink( $userId, $userName, $altUserName = false ) {
+		if ( $userName === '' ) {
+			wfLogWarning( __METHOD__ . ' received an empty username. Are there database errors ' .
+				'that need to be fixed?' );
+			return wfMessage( 'empty-username' )->parse();
+		}
+
 		$classes = 'mw-userlink';
 		$page = null;
 		if ( $userId == 0 ) {
@@ -936,6 +942,12 @@ class Linker {
 		$userId, $userText, $redContribsWhenNoEdits = false, $flags = 0, $edits = null,
 		$useParentheses = true
 	) {
+		if ( $userText === '' ) {
+			wfLogWarning( __METHOD__ . ' received an empty username. Are there database errors ' .
+				'that need to be fixed?' );
+			return ' ' . wfMessage( 'empty-username' )->parse();
+		}
+
 		global $wgUser, $wgDisableAnonTalk, $wgLang;
 		$talkable = !( $wgDisableAnonTalk && $userId == 0 );
 		$blockable = !( $flags & self::TOOL_LINKS_NOBLOCK );
@@ -1018,6 +1030,12 @@ class Linker {
 	 * @return string HTML fragment with user talk link
 	 */
 	public static function userTalkLink( $userId, $userText ) {
+		if ( $userText === '' ) {
+			wfLogWarning( __METHOD__ . ' received an empty username. Are there database errors ' .
+				'that need to be fixed?' );
+			return wfMessage( 'empty-username' )->parse();
+		}
+
 		$userTalkPage = new TitleValue( NS_USER_TALK, strtr( $userText, ' ', '_' ) );
 		$moreLinkAttribs['class'] = 'mw-usertoollinks-talk';
 
@@ -1034,6 +1052,12 @@ class Linker {
 	 * @return string HTML fragment with block link
 	 */
 	public static function blockLink( $userId, $userText ) {
+		if ( $userText === '' ) {
+			wfLogWarning( __METHOD__ . ' received an empty username. Are there database errors ' .
+				'that need to be fixed?' );
+			return wfMessage( 'empty-username' )->parse();
+		}
+
 		$blockPage = SpecialPage::getTitleFor( 'Block', $userText );
 		$moreLinkAttribs['class'] = 'mw-usertoollinks-block';
 
@@ -1049,6 +1073,12 @@ class Linker {
 	 * @return string HTML fragment with e-mail user link
 	 */
 	public static function emailLink( $userId, $userText ) {
+		if ( $userText === '' ) {
+			wfLogWarning( __METHOD__ . ' received an empty username. Are there database errors ' .
+				'that need to be fixed?' );
+			return wfMessage( 'empty-username' )->parse();
+		}
+
 		$emailPage = SpecialPage::getTitleFor( 'Emailuser', $userText );
 		$moreLinkAttribs['class'] = 'mw-usertoollinks-mail';
 		return self::link( $emailPage,

languages/i18n/en.json

@@ -2711,6 +2711,7 @@
 	"blocklink": "block",
 	"unblocklink": "unblock",
 	"change-blocklink": "change block",
+	"empty-username": "(no username available)",
 	"contribslink": "contribs",
 	"emaillink": "send email",
 	"autoblocker": "Autoblocked because your IP address has been recently used by \"[[User:$1|$1]]\".\nThe reason given for $1's block is \"$2\"",

languages/i18n/qqq.json

@@ -2918,6 +2918,7 @@
 	"blocklink": "Display name for a link that, when selected, leads to a form where a user can be blocked. Used in page history and recent changes pages. Example: \"''UserName (Talk | contribs | '''block''')''\".\n\nUsed as link title in [[Special:Contributions]] and in [[Special:DeletedContributions]].\n\nSee also:\n* {{msg-mw|Sp-contributions-talk}}\n* {{msg-mw|Change-blocklink}}\n* {{msg-mw|Unblocklink}}\n* {{msg-mw|Sp-contributions-blocklog}}\n* {{msg-mw|Sp-contributions-uploads}}\n* {{msg-mw|Sp-contributions-logs}}\n* {{msg-mw|Sp-contributions-deleted}}\n* {{msg-mw|Sp-contributions-userrights}}\n{{Identical|Block}}",
 	"unblocklink": "Used as link title in [[Special:Contributions]] and in [[Special:DeletedContributions]].\n\nSee also:\n* {{msg-mw|Sp-contributions-talk}}\n* {{msg-mw|change-blocklink}}\n* {{msg-mw|blocklink}}\n* {{msg-mw|sp-contributions-blocklog}}\n* {{msg-mw|sp-contributions-uploads}}\n* {{msg-mw|sp-contributions-logs}}\n* {{msg-mw|sp-contributions-deleted}}\n* {{msg-mw|sp-contributions-userrights}}\n{{Identical|Unblock}}",
 	"change-blocklink": "Used to name the link on [[Special:Log]].\n\nAlso used as link title in [[Special:Contributions]] and in [[Special:DeletedContributions]].\n\nSee also:\n* {{msg-mw|Sp-contributions-talk}}\n* {{msg-mw|unblocklink}}\n* {{msg-mw|blocklink}}\n* {{msg-mw|sp-contributions-blocklog}}\n* {{msg-mw|sp-contributions-uploads}}\n* {{msg-mw|sp-contributions-logs}}\n* {{msg-mw|sp-contributions-deleted}}\n* {{msg-mw|sp-contributions-userrights}}",
+	"empty-username": "If no username is available to display for a log or history entry, such as because of an incorrect database entry, this message is displayed in place of the links to the user page/talk/contribs/etc.",
 	"contribslink": "Short for \"contributions\". Used as display name for a link to user contributions on history pages, [[Special:RecentChanges]], [[Special:Watchlist]], etc.\n{{Identical|Contribution}}",
 	"emaillink": "Used as display name for a link to send an e-mail to a user in the user tool links. Example: \"(Talk | contribs | block | send e-mail)\".\n\n{{Identical|E-mail}}",
 	"autoblocker": "Used in [[Special:Block]].\n* $1 - target username\n* $2 - reason",

tests/phpunit/includes/LinkerTest.php

@@ -14,11 +14,16 @@ class LinkerTest extends MediaWikiLangTestCase {
 			'wgArticlePath' => '/wiki/$1',
 		] );
 
-		$this->assertEquals(
-			$expected,
-			Linker::userLink( $userId, $userName, $altUserName ),
-			$msg
-		);
+		// We'd also test the warning, but injecting a mock logger into a static method is tricky.
+		if ( $userName === '' ) {
+			Wikimedia\suppressWarnings();
+		}
+		$actual = Linker::userLink( $userId, $userName, $altUserName );
+		if ( $userName === '' ) {
+			Wikimedia\restoreWarnings();
+		}
+
+		$this->assertEquals( $expected, $actual, $msg );
 	}
 
 	public static function provideCasesForUserLink() {
@@ -29,6 +34,9 @@ class LinkerTest extends MediaWikiLangTestCase {
 		# - optional altUserName
 		# - optional message
 		return [
+			# Empty name (T222529)
+			'Empty username, userid 0' => [ '(no username available)', 0, '' ],
+			'Empty username, userid > 0' => [ '(no username available)', 73, '' ],
 
 			# ## ANONYMOUS USER ########################################
 			[
@@ -87,6 +95,118 @@ class LinkerTest extends MediaWikiLangTestCase {
 		];
 	}
 
+	/**
+	 * @dataProvider provideUserToolLinks
+	 * @covers Linker::userToolLinks
+	 * @param string $expected
+	 * @param int $userId
+	 * @param string $userText
+	 */
+	public function testUserToolLinks( $expected, $userId, $userText ) {
+		// We'd also test the warning, but injecting a mock logger into a static method is tricky.
+		if ( $userText === '' ) {
+			Wikimedia\suppressWarnings();
+		}
+		$actual = Linker::userToolLinks( $userId, $userText );
+		if ( $userText === '' ) {
+			Wikimedia\restoreWarnings();
+		}
+
+		$this->assertSame( $expected, $actual );
+	}
+
+	public static function provideUserToolLinks() {
+		return [
+			// Empty name (T222529)
+			'Empty username, userid 0' => [ ' (no username available)', 0, '' ],
+			'Empty username, userid > 0' => [ ' (no username available)', 73, '' ],
+		];
+	}
+
+	/**
+	 * @dataProvider provideUserTalkLink
+	 * @covers Linker::userTalkLink
+	 * @param string $expected
+	 * @param int $userId
+	 * @param string $userText
+	 */
+	public function testUserTalkLink( $expected, $userId, $userText ) {
+		// We'd also test the warning, but injecting a mock logger into a static method is tricky.
+		if ( $userText === '' ) {
+			Wikimedia\suppressWarnings();
+		}
+		$actual = Linker::userTalkLink( $userId, $userText );
+		if ( $userText === '' ) {
+			Wikimedia\restoreWarnings();
+		}
+
+		$this->assertSame( $expected, $actual );
+	}
+
+	public static function provideUserTalkLink() {
+		return [
+			// Empty name (T222529)
+			'Empty username, userid 0' => [ '(no username available)', 0, '' ],
+			'Empty username, userid > 0' => [ '(no username available)', 73, '' ],
+		];
+	}
+
+	/**
+	 * @dataProvider provideBlockLink
+	 * @covers Linker::blockLink
+	 * @param string $expected
+	 * @param int $userId
+	 * @param string $userText
+	 */
+	public function testBlockLink( $expected, $userId, $userText ) {
+		// We'd also test the warning, but injecting a mock logger into a static method is tricky.
+		if ( $userText === '' ) {
+			Wikimedia\suppressWarnings();
+		}
+		$actual = Linker::blockLink( $userId, $userText );
+		if ( $userText === '' ) {
+			Wikimedia\restoreWarnings();
+		}
+
+		$this->assertSame( $expected, $actual );
+	}
+
+	public static function provideBlockLink() {
+		return [
+			// Empty name (T222529)
+			'Empty username, userid 0' => [ '(no username available)', 0, '' ],
+			'Empty username, userid > 0' => [ '(no username available)', 73, '' ],
+		];
+	}
+
+	/**
+	 * @dataProvider provideEmailLink
+	 * @covers Linker::emailLink
+	 * @param string $expected
+	 * @param int $userId
+	 * @param string $userText
+	 */
+	public function testEmailLink( $expected, $userId, $userText ) {
+		// We'd also test the warning, but injecting a mock logger into a static method is tricky.
+		if ( $userText === '' ) {
+			Wikimedia\suppressWarnings();
+		}
+		$actual = Linker::emailLink( $userId, $userText );
+		if ( $userText === '' ) {
+			Wikimedia\restoreWarnings();
+		}
+
+		$this->assertSame( $expected, $actual );
+	}
+
+	public static function provideEmailLink() {
+		return [
+			// Empty name (T222529)
+			'Empty username, userid 0' => [ '(no username available)', 0, '' ],
+			'Empty username, userid > 0' => [ '(no username available)', 73, '' ],
+		];
+	}
+
 	/**
 	 * @dataProvider provideCasesForFormatComment
 	 * @covers Linker::formatComment