Merge branch 'REL1_24' of ssh://gerrit.wikimedia.org:29418/mediawiki/core into REL1_241.24.0-rc.1

Change-Id: Ib91ada28aefa31026cfa6eda6902e18604377c7b

3 files changed, 197 insertions, 36 deletions

diff --git a/includes/specialpage/SpecialPageFactory.php b/includes/specialpage/SpecialPageFactory.php
index c60e03ae2c67..679492ae06b6 100644
--- a/includes/specialpage/SpecialPageFactory.php
+++ b/includes/specialpage/SpecialPageFactory.php
@@ -262,10 +262,9 @@ class SpecialPageFactory {
 
 	/**
 	 * Initialise and return the list of special page aliases.  Returns an object with
-	 * properties which can be accessed $obj->pagename - each property is an array of
-	 * aliases; the first in the array is the canonical alias.  All registered special
-	 * pages are guaranteed to have a property entry, and for that property array to
-	 * contain at least one entry (English fallbacks will be added if necessary).
+	 * properties which can be accessed $obj->pagename - each property name is an
+	 * alias, with the value being the canonical name of the special page. All
+	 * registered special pages are guaranteed to map to themselves.
 	 * @return object
 	 */
 	private static function getAliasListObject() {
@@ -273,21 +272,44 @@ class SpecialPageFactory {
 			global $wgContLang;
 			$aliases = $wgContLang->getSpecialPageAliases();
 
-			$missingPages = self::getPageList();
-
 			self::$aliases = array();
+			$keepAlias = array();
+
+			// Force every canonical name to be an alias for itself.
+			foreach ( self::getPageList() as $name => $stuff ) {
+				$caseFoldedAlias = $wgContLang->caseFold( $name );
+				self::$aliases[$caseFoldedAlias] = $name;
+				$keepAlias[$caseFoldedAlias] = 'canonical';
+			}
+
 			// Check for $aliases being an array since Language::getSpecialPageAliases can return null
 			if ( is_array( $aliases ) ) {
 				foreach ( $aliases as $realName => $aliasList ) {
-					foreach ( $aliasList as $alias ) {
-						self::$aliases[$wgContLang->caseFold( $alias )] = $realName;
+					$aliasList = array_values( $aliasList );
+					foreach ( $aliasList as $i => $alias ) {
+						$caseFoldedAlias = $wgContLang->caseFold( $alias );
+
+						if ( isset( self::$aliases[$caseFoldedAlias] ) &&
+							$realName === self::$aliases[$caseFoldedAlias]
+						) {
+							// Ignore same-realName conflicts
+							continue;
+						}
+
+						if ( !isset( $keepAlias[$caseFoldedAlias] ) ) {
+							self::$aliases[$caseFoldedAlias] = $realName;
+							if ( !$i ) {
+								$keepAlias[$caseFoldedAlias] = 'first';
+							}
+						} elseif ( !$i ) {
+							wfWarn( "First alias '$alias' for $realName conflicts with " .
+								"{$keepAlias[$caseFoldedAlias]} alias for " .
+								self::$aliases[$caseFoldedAlias]
+							);
+						}
 					}
-					unset( $missingPages->$realName );
 				}
 			}
-			foreach ( $missingPages as $name => $stuff ) {
-				self::$aliases[$wgContLang->caseFold( $name )] = $name;
-			}
 
 			// Cast to object: func()[$key] doesn't work, but func()->$key does
 			self::$aliases = (object)self::$aliases;
@@ -620,29 +642,42 @@ class SpecialPageFactory {
 	public static function getLocalNameFor( $name, $subpage = false ) {
 		global $wgContLang;
 		$aliases = $wgContLang->getSpecialPageAliases();
+		$aliasList = self::getAliasListObject();
 
-		if ( isset( $aliases[$name][0] ) ) {
-			$name = $aliases[$name][0];
-		} else {
-			// Try harder in case someone misspelled the correct casing
+		// Find the first alias that maps back to $name
+		if ( isset( $aliases[$name] ) ) {
 			$found = false;
-			// Check for $aliases being an array since Language::getSpecialPageAliases can return null
+			foreach ( $aliases[$name] as $alias ) {
+				$caseFoldedAlias = $wgContLang->caseFold( $alias );
+				$caseFoldedAlias = str_replace( ' ', '_', $caseFoldedAlias );
+				if ( isset( $aliasList->$caseFoldedAlias ) &&
+					$aliasList->$caseFoldedAlias === $name
+				) {
+					$name = $alias;
+					$found = true;
+					break;
+				}
+			}
+			if ( !$found ) {
+				wfWarn( "Did not find a usable alias for special page '$name'. " .
+					"It seems all defined aliases conflict?" );
+			}
+		} else {
+			// Check if someone misspelled the correct casing
 			if ( is_array( $aliases ) ) {
 				foreach ( $aliases as $n => $values ) {
 					if ( strcasecmp( $name, $n ) === 0 ) {
 						wfWarn( "Found alias defined for $n when searching for " .
 							"special page aliases for $name. Case mismatch?" );
-						$name = $values[0];
-						$found = true;
-						break;
+						return self::getLocalNameFor( $n, $subpage );
 					}
 				}
 			}
-			if ( !$found ) {
-				wfWarn( "Did not find alias for special page '$name'. " .
-					"Perhaps no aliases are defined for it?" );
-			}
+
+			wfWarn( "Did not find alias for special page '$name'. " .
+				"Perhaps no aliases are defined for it?" );
 		}
+
 		if ( $subpage !== false && !is_null( $subpage ) ) {
 			$name = "$name/$subpage";
 		}
diff --git a/languages/Language.php b/languages/Language.php
index 115a918ffc62..ad198c35cc6d 100644
--- a/languages/Language.php
+++ b/languages/Language.php
@@ -731,6 +731,8 @@ class Language {
 	}
 
 	/**
+	 * @deprecated since 1.24, doesn't handle conflicting aliases. Use
+	 *   SpecialPageFactory::getLocalNameFor instead.
 	 * @param string $name
 	 * @return string
 	 */
diff --git a/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php b/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php
index b1d425728135..779fa558b222 100644
--- a/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php
+++ b/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php
@@ -22,6 +22,12 @@
  */
 class SpecialPageFactoryTest extends MediaWikiTestCase {
 
+	protected function tearDown() {
+		parent::tearDown();
+
+		SpecialPageFactory::resetList();
+	}
+
 	public function newSpecialAllPages() {
 		return new SpecialAllPages();
 	}
@@ -41,7 +47,6 @@ class SpecialPageFactoryTest extends MediaWikiTestCase {
 	 */
 	public function testGetPage( $spec, $shouldReuseInstance ) {
 		$this->mergeMwGlobalArrayValue( 'wgSpecialPages', array( 'testdummy' => $spec ) );
-
 		SpecialPageFactory::resetList();
 
 		$page = SpecialPageFactory::getPage( 'testdummy' );
@@ -49,53 +54,172 @@ class SpecialPageFactoryTest extends MediaWikiTestCase {
 
 		$page2 = SpecialPageFactory::getPage( 'testdummy' );
 		$this->assertEquals( $shouldReuseInstance, $page2 === $page, "Should re-use instance:" );
-
-		SpecialPageFactory::resetList();
 	}
 
 	public function testGetNames() {
 		$this->mergeMwGlobalArrayValue( 'wgSpecialPages', array( 'testdummy' => 'SpecialAllPages' ) );
-
 		SpecialPageFactory::resetList();
+
 		$names = SpecialPageFactory::getNames();
 		$this->assertInternalType( 'array', $names );
 		$this->assertContains( 'testdummy', $names );
-		SpecialPageFactory::resetList();
 	}
 
 	public function testResolveAlias() {
 		$this->setMwGlobals( 'wgContLang', Language::factory( 'de' ) );
-
 		SpecialPageFactory::resetList();
 
 		list( $name, $param ) = SpecialPageFactory::resolveAlias( 'Spezialseiten/Foo' );
 		$this->assertEquals( 'Specialpages', $name );
 		$this->assertEquals( 'Foo', $param );
-
-		SpecialPageFactory::resetList();
 	}
 
 	public function testGetLocalNameFor() {
 		$this->setMwGlobals( 'wgContLang', Language::factory( 'de' ) );
-
 		SpecialPageFactory::resetList();
 
 		$name = SpecialPageFactory::getLocalNameFor( 'Specialpages', 'Foo' );
 		$this->assertEquals( 'Spezialseiten/Foo', $name );
-
-		SpecialPageFactory::resetList();
 	}
 
 	public function testGetTitleForAlias() {
 		$this->setMwGlobals( 'wgContLang', Language::factory( 'de' ) );
-
 		SpecialPageFactory::resetList();
 
 		$title = SpecialPageFactory::getTitleForAlias( 'Specialpages/Foo' );
 		$this->assertEquals( 'Spezialseiten/Foo', $title->getText() );
 		$this->assertEquals( NS_SPECIAL, $title->getNamespace() );
+	}
 
+	/**
+	 * @dataProvider provideTestConflictResolution
+	 */
+	public function testConflictResolution(
+		$test, $aliasesList, $alias, $expectedName, $expectedAlias, $expectWarnings
+	) {
+		global $wgContLang;
+		$lang = clone $wgContLang;
+		$lang->mExtendedSpecialPageAliases = $aliasesList;
+		$this->setMwGlobals( 'wgContLang', $lang );
+		$this->setMwGlobals( 'wgSpecialPages',
+			array_combine( array_keys( $aliasesList ), array_keys( $aliasesList ) )
+		);
 		SpecialPageFactory::resetList();
+
+		// Catch the warnings we expect to be raised
+		$warnings = array();
+		$this->setMwGlobals( 'wgDevelopmentWarnings', true );
+		set_error_handler( function ( $errno, $errstr ) use ( &$warnings ) {
+			if ( preg_match( '/First alias \'[^\']*\' for .*/', $errstr ) ||
+				preg_match( '/Did not find a usable alias for special page .*/', $errstr )
+			) {
+				$warnings[] = $errstr;
+				return true;
+			}
+			return false;
+		} );
+		$reset = new ScopedCallback( 'restore_error_handler' );
+
+		list( $name, /*...*/ ) = SpecialPageFactory::resolveAlias( $alias );
+		$this->assertEquals( $expectedName, $name, "$test: Alias to name" );
+		$result = SpecialPageFactory::getLocalNameFor( $name );
+		$this->assertEquals( $expectedAlias, $result, "$test: Alias to name to alias" );
+
+		$gotWarnings = count( $warnings );
+		if ( $gotWarnings !== $expectWarnings ) {
+			$this->fail( "Expected $expectWarnings warning(s), but got $gotWarnings:\n" .
+				join( "\n", $warnings )
+			);
+		}
+	}
+
+	/**
+	 * @dataProvider provideTestConflictResolution
+	 */
+	public function testConflictResolutionReversed(
+		$test, $aliasesList, $alias, $expectedName, $expectedAlias, $expectWarnings
+	) {
+		// Make sure order doesn't matter by reversing the list
+		$aliasesList = array_reverse( $aliasesList );
+		return $this->testConflictResolution(
+			$test, $aliasesList, $alias, $expectedName, $expectedAlias, $expectWarnings
+		);
+	}
+
+	public function provideTestConflictResolution() {
+		return array(
+			array(
+				'Canonical name wins',
+				array( 'Foo' => array( 'Foo', 'Bar' ), 'Baz' => array( 'Foo', 'BazPage', 'Baz2' ) ),
+				'Foo',
+				'Foo',
+				'Foo',
+				1,
+			),
+
+			array(
+				'Doesn\'t redirect to a different special page\'s canonical name',
+				array( 'Foo' => array( 'Foo', 'Bar' ), 'Baz' => array( 'Foo', 'BazPage', 'Baz2' ) ),
+				'Baz',
+				'Baz',
+				'BazPage',
+				1,
+			),
+
+			array(
+				'Canonical name wins even if not aliased',
+				array( 'Foo' => array( 'FooPage' ), 'Baz' => array( 'Foo', 'BazPage', 'Baz2' ) ),
+				'Foo',
+				'Foo',
+				'FooPage',
+				1,
+			),
+
+			array(
+				'Doesn\'t redirect to a different special page\'s canonical name even if not aliased',
+				array( 'Foo' => array( 'FooPage' ), 'Baz' => array( 'Foo', 'BazPage', 'Baz2' ) ),
+				'Baz',
+				'Baz',
+				'BazPage',
+				1,
+			),
+
+			array(
+				'First local name beats non-first',
+				array( 'First' => array( 'Foo' ), 'NonFirst' => array( 'Bar', 'Foo' ) ),
+				'Foo',
+				'First',
+				'Foo',
+				0,
+			),
+
+			array(
+				'Doesn\'t redirect to a different special page\'s first alias',
+				array(
+					'Foo' => array( 'Foo' ),
+					'First' => array( 'Bar' ),
+					'Baz' => array( 'Foo', 'Bar', 'BazPage', 'Baz2' )
+				),
+				'Baz',
+				'Baz',
+				'BazPage',
+				1,
+			),
+
+			array(
+				'Doesn\'t redirect wrong even if all aliases conflict',
+				array(
+					'Foo' => array( 'Foo' ),
+					'First' => array( 'Bar' ),
+					'Baz' => array( 'Foo', 'Bar' )
+				),
+				'Baz',
+				'Baz',
+				'Baz',
+				2,
+			),
+
+		);
 	}
 
 }