Reject usernames with namespace or interwiki prefixes

User::getCanonicalName silently stripped namepace/iw prefixes,
resulting in weird behavior (e.g. entering 'Template:Foo' in the
registration form username field was accepted, but user 'Foo' was
created). Explicitly reject such names instead.

Also remove leading whitespace from test cases testing for something
else.

Bug: T94656
Change-Id: Idfb22f998cb069fca24abcf5809afd6e0324b0ae

diff --git a/includes/user/User.php b/includes/user/User.php
index 04eba97..c1e096b 100644 (file)
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -1070,9 +1070,9 @@ class User implements IDBAccessObject {
                // Clean up name according to title rules,
                // but only when validation is requested (bug 12654)
                $t = ( $validate !== false ) ?
-                       Title::newFromText( $name ) : Title::makeTitle( NS_USER, $name );
+                       Title::newFromText( $name, NS_USER ) : Title::makeTitle( NS_USER, $name );
                // Check for invalid titles
-               if ( is_null( $t ) ) {
+               if ( is_null( $t ) || $t->getNamespace() !== NS_USER || $t->isExternal() ) {
                        return false;
                }
 

diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php
index 88bb328..c9b6929 100644 (file)
--- a/tests/phpunit/includes/user/UserTest.php
+++ b/tests/phpunit/includes/user/UserTest.php
@@ -342,30 +342,46 @@ class UserTest extends MediaWikiTestCase {
         * @covers User::getCanonicalName()
         * @dataProvider provideGetCanonicalName
         */
-       public function testGetCanonicalName( $name, $expectedArray, $msg ) {
+       public function testGetCanonicalName( $name, $expectedArray ) {
+               // fake interwiki map for the 'Interwiki prefix' testcase
+               $this->mergeMwGlobalArrayValue( 'wgHooks', [
+                       'InterwikiLoadPrefix' => [
+                               function ( $prefix, &$iwdata ) {
+                                       if ( $prefix === 'interwiki' ) {
+                                               $iwdata = [
+                                                       'iw_url' => 'http://example.com/',
+                                                       'iw_local' => 0,
+                                                       'iw_trans' => 0,
+                                               ];
+                                               return false;
+                                       }
+                               },
+                       ],
+               ] );
+
                foreach ( $expectedArray as $validate => $expected ) {
                        $this->assertEquals(
                                $expected,
-                               User::getCanonicalName( $name, $validate === 'false' ? false : $validate ),
-                               $msg . ' (' . $validate . ')'
-                       );
+                               User::getCanonicalName( $name, $validate === 'false' ? false : $validate ), $validate );
                }
        }
 
        public static function provideGetCanonicalName() {
                return [
-                       [ ' Trailing space ', [ 'creatable' => 'Trailing space' ], 'Trailing spaces' ],
-                       // @todo FIXME: Maybe the creatable name should be 'Talk:Username' or false to reject?
-                       [ 'Talk:Username', [ 'creatable' => 'Username', 'usable' => 'Username',
-                               'valid' => 'Username', 'false' => 'Talk:Username' ], 'Namespace prefix' ],
-                       [ ' name with # hash', [ 'creatable' => false, 'usable' => false ], 'With hash' ],
-                       [ 'Multi  spaces', [ 'creatable' => 'Multi spaces',
-                               'usable' => 'Multi spaces' ], 'Multi spaces' ],
-                       [ 'lowercase', [ 'creatable' => 'Lowercase' ], 'Lowercase' ],
-                       [ 'in[]valid', [ 'creatable' => false, 'usable' => false, 'valid' => false,
-                               'false' => 'In[]valid' ], 'Invalid' ],
-                       [ 'with / slash', [ 'creatable' => false, 'usable' => false, 'valid' => false,
-                               'false' => 'With / slash' ], 'With slash' ],
+                       'Leading space' => [ ' Leading space', [ 'creatable' => 'Leading space' ] ],
+                       'Trailing space ' => [ 'Trailing space ', [ 'creatable' => 'Trailing space' ] ],
+                       'Namespace prefix' => [ 'Talk:Username', [ 'creatable' => false, 'usable' => false,
+                               'valid' => false, 'false' => 'Talk:Username' ] ],
+                       'Interwiki prefix' => [ 'interwiki:Username', [ 'creatable' => false, 'usable' => false,
+                               'valid' => false, 'false' => 'Interwiki:Username' ] ],
+                       'With hash' => [ 'name with # hash', [ 'creatable' => false, 'usable' => false ] ],
+                       'Multi spaces' => [ 'Multi  spaces', [ 'creatable' => 'Multi spaces',
+                               'usable' => 'Multi spaces' ] ],
+                       'Lowercase' => [ 'lowercase', [ 'creatable' => 'Lowercase' ] ],
+                       'Invalid character' => [ 'in[]valid', [ 'creatable' => false, 'usable' => false,
+                               'valid' => false, 'false' => 'In[]valid' ] ],
+                       'With slash' => [ 'with / slash', [ 'creatable' => false, 'usable' => false, 'valid' => false,
+                               'false' => 'With / slash' ] ],
                ];
        }