From a91638bd387a320fb68e31f7bc0757a8c417c057 Mon Sep 17 00:00:00 2001
From: WMDE-Fisch <christoph.jauera@wikimedia.de>
Date: Tue, 30 Apr 2019 16:22:48 +0200
Subject: [PATCH] Let User::idFromName always return int or null

This patch makes sure, that idFromName always returns either an int
or null value. The $idCacheByName can contain string values so we cast
the values if necessary.

An alternative could have been to make sure that just int values go
into the cache. But since the cache is public the current approach
to seems to be better atm.

Change-Id: I3085d89b93db2b888c190ba193623b86dc93759a
---
 includes/user/User.php                   |  2 +-
 tests/phpunit/includes/user/UserTest.php | 34 ++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/includes/user/User.php b/includes/user/User.php
index ba6bb08a73..bcd444ee22 100644
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -914,7 +914,7 @@ class User implements IDBAccessObject, UserIdentity {
 		}
 
 		if ( !( $flags & self::READ_LATEST ) && array_key_exists( $name, self::$idCacheByName ) ) {
-			return self::$idCacheByName[$name];
+			return is_null( self::$idCacheByName[$name] ) ? null : (int)self::$idCacheByName[$name];
 		}
 
 		list( $index, $options ) = DBAccessObjectUtils::getDBOptions( $flags );
diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php
index f84be3f1c4..d41a1f8f46 100644
--- a/tests/phpunit/includes/user/UserTest.php
+++ b/tests/phpunit/includes/user/UserTest.php
@@ -1506,4 +1506,38 @@ class UserTest extends MediaWikiTestCase {
 		$updater->setContent( 'main', $content );
 		return $updater->saveRevision( CommentStoreComment::newUnsavedComment( $comment ) );
 	}
+
+	/**
+	 * @covers User::idFromName
+	 */
+	public function testExistingIdFromName() {
+		$this->assertTrue(
+			array_key_exists( $this->user->getName(), User::$idCacheByName ),
+			'Test user should already be in the id cache.'
+		);
+		$this->assertSame(
+			$this->user->getId(), User::idFromName( $this->user->getName() ),
+			'Id is correctly retreived from the cache.'
+		);
+		$this->assertSame(
+			$this->user->getId(), User::idFromName( $this->user->getName(), User::READ_LATEST ),
+			'Id is correctly retreived from the database.'
+		);
+	}
+
+	/**
+	 * @covers User::idFromName
+	 */
+	public function testNonExistingIdFromName() {
+		$this->assertFalse(
+			array_key_exists( 'NotExisitngUser', User::$idCacheByName ),
+			'Non exisitng user should not be in the id cache.'
+		);
+		$this->assertSame( null, User::idFromName( 'NotExisitngUser' ) );
+		$this->assertTrue(
+			array_key_exists( 'NotExisitngUser', User::$idCacheByName ),
+			'Username will be cached when requested once.'
+		);
+		$this->assertSame( null, User::idFromName( 'NotExisitngUser' ) );
+	}
 }
-- 
2.20.1