r80694 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r80693‎ | r80694 | r80695 >
Date:18:01, 21 January 2011
Author:hashar
Status:ok (Comments)
Tags:
Comment:
Hack invalid w3 spec to validate @localhost email

In r75682, I have implemented a PHP function to validate email address
based on bug 22449. Siebrand pointed a w3.org specification which I
implemented. The spec is bugged since it requires a domain and a top
level domain!

I could either make the first part optional or alter the second part
to require 0 to x elements. I choose the later: s/+/*/

Should fix bug 22449 for good.

TESTS:

Added testEmailDoesNotNeedATopLevelDomain:

Made following emails valid:
user.@localdaomin
.@localdomain
user@a

Test output (please add more):
$ php phpunit.php -c suite.xml --filter alidEmail --tap
TAP version 13
ok 1 - UserIsValidEmailAddrTest::testEmailWellKnownUserAtHostDotTldAreValid
ok 2 - UserIsValidEmailAddrTest::testEmailWithUpperCaseCharactersAreValid
ok 3 - UserIsValidEmailAddrTest::testEmailWithAPlusInUserName
ok 4 - UserIsValidEmailAddrTest::testEmailDoesNotNeedATopLevelDomain
ok 5 - UserIsValidEmailAddrTest::testEmailWithWhiteSpacesBeforeOrAfterAreInvalids
ok 6 - UserIsValidEmailAddrTest::testEmailWithWhiteSpacesAreInvalids
ok 7 - UserIsValidEmailAddrTest::testEmailDomainCanNotBeginWithDot
ok 8 - UserIsValidEmailAddrTest::testEmailWithFunnyCharacters
ok 9 - UserIsValidEmailAddrTest::testEmailTopLevelDomainCanBeNumerical
ok 10 - UserIsValidEmailAddrTest::testEmailWithoutAtSignIsInvalid
ok 11 - UserIsValidEmailAddrTest::testEmailWithOneCharacterDomainIsValid
1..11
Modified paths:
  • /trunk/phase3/includes/User.php (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/UserIsValidEmailAddrTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/includes/UserIsValidEmailAddrTest.php
@@ -3,6 +3,7 @@
44 class UserIsValidEmailAddrTest extends MediaWikiTestCase {
55
66 private function checkEmail( $addr, $expected = true, $msg = '') {
 7+ if( $msg == '' ) { $msg = "Testing $addr"; }
78 $this->assertEquals(
89 $expected,
910 User::isValidEmailAddr( $addr ),
@@ -30,6 +31,11 @@
3132 $this->valid( 'user+sub@example.com' );
3233 $this->valid( 'user+@example.com' );
3334 }
 35+ function testEmailDoesNotNeedATopLevelDomain() {
 36+ $this->valid( "user@localhost" );
 37+ $this->valid( "FooBar@localdomain" );
 38+ $this->valid( "nobody@mycompany" );
 39+ }
3440 function testEmailWithWhiteSpacesBeforeOrAfterAreInvalids() {
3541 $this->invalid( " user@host.com" );
3642 $this->invalid( "user@host.com " );
@@ -45,8 +51,8 @@
4652 $this->invalid( "user@." );
4753 $this->invalid( "user@.localdomain" );
4854 $this->invalid( "user@localdomain." );
49 - $this->invalid( "user.@localdomain" );
50 - $this->invalid( ".@localdomain" );
 55+ $this->valid( "user.@localdomain" );
 56+ $this->valid( ".@localdomain" );
5157 $this->invalid( ".@a............" );
5258 }
5359 function testEmailWithFunnyCharacters() {
@@ -58,7 +64,7 @@
5965 function testEmailWithoutAtSignIsInvalid() {
6066 $this->invalid( 'useràexample.com' );
6167 }
62 - function testEmailWithOneCharacterDomainIsInvalid() {
63 - $this->invalid( 'user@a' );
 68+ function testEmailWithOneCharacterDomainIsValid() {
 69+ $this->valid( 'user@a' );
6470 }
6571 }
Index: trunk/phase3/includes/User.php
@@ -660,7 +660,7 @@
661661 [$rfc5322_atext\\.]+ # user part which is liberal :p
662662 @ # 'apostrophe'
663663 [$rfc1034_ldh_str]+ # First domain part
664 - (\\.[$rfc1034_ldh_str]+)+ # Following part prefixed with a dot
 664+ (\\.[$rfc1034_ldh_str]+)* # Following part prefixed with a dot
665665 $ # End of string
666666 /ix" ; // case Insensitive, eXtended
667667

Follow-up revisions

RevisionCommit summaryAuthorDate
r807221.17: MFT r80324, r80326, r80328, r80339, r80350, r80351, r80355, r80358, r80...catrope23:00, 21 January 2011
r80918Email validation for @localhost address + tests...hashar21:13, 24 January 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r75627Lame user side email validator using JQuery....hashar21:01, 28 October 2010
r75682Follow up r75627. Implements r75670 in PHP to validate emails....hashar22:03, 29 October 2010

Comments

#Comment by Hashar (talk | contribs)   18:04, 21 January 2011

This one MUST be in 1.17 release tarball.

Status & tagging log