Last modified: 2014-09-23 23:42:12 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 12581 - @ (at-sign) needs to be totally invalid for usernames
@ (at-sign) needs to be totally invalid for usernames
Status: NEW
Product: MediaWiki
Classification: Unclassified
User login and signup (Other open bugs)
All All
: Low normal with 1 vote (vote)
: ---
Assigned To: Nobody - You can work on this!
: patch, patch-reviewed
Depends on:
  Show dependency treegraph
Reported: 2008-01-10 19:35 UTC by Aryeh Gregor (not reading bugmail, please e-mail directly)
Modified: 2014-09-23 23:42 UTC (History)
7 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---

Possible patch, needs review (1.65 KB, patch)
2009-10-04 05:04 UTC, mac.med02
Different (936 bytes, patch)
2009-10-05 03:43 UTC, mac.med02

Description Aryeh Gregor (not reading bugmail, please e-mail directly) 2008-01-10 19:35:21 UTC
It's been forbidden to create accounts with at-signs in them for some time, because this conflicts with interwiki syntax and will cause problems for SUL.  More immediately, such users cannot be affected by Special:Userrights, which caused some confusion on enwiki earlier with the wider access that's been given.  A maintenance script runnable from update.php needs to find and rename all such users (with suitable warning), and @ should be made entirely invalid in usernames instead of just invalid on creation.
Comment 1 Aryeh Gregor (not reading bugmail, please e-mail directly) 2008-01-12 23:41:52 UTC
*** Bug 12602 has been marked as a duplicate of this bug. ***
Comment 2 Daniel Friesen 2008-02-27 04:33:55 UTC
Just a minor correction, but @ should be made invalid for use, not completely invalid.

There are 3 levels:
* 'valid' controlled by User::isValidUserName();
* 'useable' controlled by User::isUsableName() and inherits from 'valid';
* 'creatable' controlled by User::isCreatableName() and inherits from 'useable' and therefore 'valid' to;

A 'valid' username is one which does not contain characters such as / or other types of characters which could get broken or break something in the interface.

A 'useable' username is one which is 'valid' and also is not a blacklisted username (ie: For maintenance scripts).

A 'creatable' username is one which is 'useable' and therefore also 'valid' and also does not contain characters which should not be 'useable' however have been in prior use or have some 'useable' use under restricted circumstances and therefore are not put in the unusable category but still denied for creation.

The proposal would be to move the @ sign from it's current non-'createable' state to a non-'useable' state. A lot of interface methods test for 'valid' usernames, but still accept 'useable' usernames as input. If the @ sign were moved directly to non-'valid' then it would break the proposed upgrade to transwiki import by breaking ui parts where it should still be valid for checking things on users which were transwikied. For example, the API would no longer allow you to check the contributions of a transwikied user. In other words if someone imported a number of pages and you wanted to check out what imported contributions were made by the API would fail with a message "User name is not valid" instead of displaying the transwikied user's contributions.

So to restate, @ should be moved from it's current location in User::isCreatableName() into User::isUsableName(); NOT into User::isValidUserName();
Comment 3 Mike.lifeguard 2008-11-25 04:00:41 UTC
Severity -> normal as I don't think this is really an /enhancement/
Comment 4 mac.med02 2009-10-04 05:04:41 UTC
Created attachment 6619 [details]
Possible patch, needs review
Comment 5 Platonides 2009-10-04 17:54:56 UTC
You're applying all creation restrictions to usable names. That also means changing $wgInvalidUsernameCharacters semantics. I don't feel that's the right way.
Comment 6 mac.med02 2009-10-05 03:43:18 UTC
Created attachment 6625 [details]

Gave something different a shot. Hopefully this should work.
Comment 7 Aryeh Gregor (not reading bugmail, please e-mail directly) 2009-10-05 15:27:17 UTC
+		//Ensure that the username does not contain @ symbol
+		else if ( preg.match( '@', $name ) {
+			return false;
+		} 

Did you test this?  "preg.match" looks like a fatal error to me, plus you're missing a parenthesis, so that looks like a syntax error there as well.  You should at *least* test your patches with php -l before submitting . . .

Also, you should be able to use strpos() here, preg_match() is overkill.  And don't put whitespace between an if and elseif; it should be like

if ( foo ) {
} elseif ( bar ) {
	// comment

And finally, you've got a whole bunch of trailing whitespace here, on three separate lines -- please avoid that.

-		global $wgInvalidUsernameCharacters;
+		global $wgInvalidUsernameCharacters, $wgInvalidUsernameCharacters;
 			self::isUsableName( $name ) &&
 			// Registration-time character blacklisting...
 			!preg_match( '/[' . preg_quote( $wgInvalidUsernameCharacters, '/' ) . ']/', $name );

You don't seem to have changed anything in this part other than adding an unused global declaration, and adding trailing whitespace.

Other than that, this seems good, except that a) I don't know for sure if we actually want this anymore, I filed this bug based on something Brion said ages ago and his reasons might be obsoleted by SUL; b) if we do want it, we definitely need to work out a migration strategy before committing it.
Comment 8 Sumana Harihareswara 2011-11-04 19:05:04 UTC, did you see Aryeh's review?  Adding the "reviewed" keyword.

Note You need to log in before you can comment on or make changes to this bug.