From 4dc1f5a1759bdf392504f4901748175b46f59a7e Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Mon, 29 Sep 2014 14:40:51 -0400
Subject: [PATCH] PasswordFactory::newFromPlaintext( null ) needs to work

Various code passes null around to mean "an invalid password". It
shouldn't all have to test for null and specially handle that.

This also fixes a codepath where User::$mNewpassword could get set to an
empty string rather than a password object, which would cause problems
later when anything else tries to use it.

Bug: 71421
Change-Id: Ib5f94b52c07e7dba89328b98fb43c86db95ee09f
---
 includes/User.php                       | 15 ++++-----------
 includes/password/PasswordFactory.php   |  6 +++++-
 tests/phpunit/includes/PasswordTest.php |  6 ++++++
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/includes/User.php b/includes/User.php
index 989701306d..e25bc1fc10 100644
--- a/includes/User.php
+++ b/includes/User.php
@@ -2351,11 +2351,7 @@ class User implements IDBAccessObject {
 		$this->setToken();
 
 		$passwordFactory = self::getPasswordFactory();
-		if ( $str === null ) {
-			$this->mPassword = $passwordFactory->newFromCiphertext( null );
-		} else {
-			$this->mPassword = $passwordFactory->newFromPlaintext( $str );
-		}
+		$this->mPassword = $passwordFactory->newFromPlaintext( $str );
 
 		$this->mNewpassword = $passwordFactory->newFromCiphertext( null );
 		$this->mNewpassTime = null;
@@ -2400,14 +2396,11 @@ class User implements IDBAccessObject {
 	public function setNewpassword( $str, $throttle = true ) {
 		$this->loadPasswords();
 
+		$this->mNewpassword = self::getPasswordFactory()->newFromPlaintext( $str );
 		if ( $str === null ) {
-			$this->mNewpassword = '';
 			$this->mNewpassTime = null;
-		} else {
-			$this->mNewpassword = self::getPasswordFactory()->newFromPlaintext( $str );
-			if ( $throttle ) {
-				$this->mNewpassTime = wfTimestampNow();
-			}
+		} elseif ( $throttle ) {
+			$this->mNewpassTime = wfTimestampNow();
 		}
 	}
 
diff --git a/includes/password/PasswordFactory.php b/includes/password/PasswordFactory.php
index 3b4ebb1af8..48d6866926 100644
--- a/includes/password/PasswordFactory.php
+++ b/includes/password/PasswordFactory.php
@@ -141,11 +141,15 @@ final class PasswordFactory {
 	 * If no existing object is given, make a new default object. If one is given, clone that
 	 * object. Then pass the plaintext to Password::crypt().
 	 *
-	 * @param string $password Plaintext password
+	 * @param string|null $password Plaintext password, or null for an invalid password
 	 * @param Password|null $existing Optional existing hash to get options from
 	 * @return Password
 	 */
 	public function newFromPlaintext( $password, Password $existing = null ) {
+		if ( $password === null ) {
+			return new InvalidPassword( $this, array( 'type' => '' ), null );
+		}
+
 		if ( $existing === null ) {
 			$config = $this->types[$this->default];
 			$obj = new $config['class']( $this, $config );
diff --git a/tests/phpunit/includes/PasswordTest.php b/tests/phpunit/includes/PasswordTest.php
index ceb794b592..5ad8aca676 100644
--- a/tests/phpunit/includes/PasswordTest.php
+++ b/tests/phpunit/includes/PasswordTest.php
@@ -30,4 +30,10 @@ class PasswordTest extends MediaWikiTestCase {
 
 		$this->assertFalse( $invalid1->equals( $invalid2 ) );
 	}
+
+	public function testInvalidPlaintext() {
+		$invalid = User::getPasswordFactory()->newFromPlaintext( null );
+
+		$this->assertInstanceOf( 'InvalidPassword', $invalid );
+	}
 }
-- 
2.20.1