From ff5fb5ea3e2f2a4d103fcf2ca527ad11ac34c657 Mon Sep 17 00:00:00 2001 From: Antoine Musso Date: Thu, 27 Jan 2011 20:52:12 +0000 Subject: [PATCH] Bug 26948 - hyphens incorrectly interpreted as range This is the kind of easy to fix, hard to review bug. Email validation make uses of strings listing characters, those strings are then enclosed between brackets "[]". Inside brackets, the hyphen is used to describe a range of character [a-d] being a b c d. The string containing an unescaped hyphen, made JS/PHP validation to match the incorrect comma ",". * Backslash hyphen * JS,PHP: add tests for commas and hyphens in username or domain * JS: add var to rfc_1034_ldh_str * JS: minor code cleanup TESTS: php phpunit.php -c suite.xml --filter ValidEmail OK (13 tests, 32 assertions) Special:BlankPage?action=mwutiltest&debug=true Ran 66 tests. 66 passed test(s). 0 error(s). 0 partially passed test(s). --- includes/User.php | 8 ++++++-- resources/mediawiki.util/mediawiki.util.js | 6 +++--- resources/mediawiki.util/mediawiki.util.test.js | 11 +++++++++++ tests/phpunit/includes/UserIsValidEmailAddrTest.php | 9 +++++++++ 4 files changed, 29 insertions(+), 5 deletions(-) diff --git a/includes/User.php b/includes/User.php index db854a7ea5..2ef85d25e1 100644 --- a/includes/User.php +++ b/includes/User.php @@ -673,8 +673,12 @@ class User { if( !wfRunHooks( 'isValidEmailAddr', array( $addr, &$result ) ) ) { return $result; } - $rfc5322_atext = "a-z0-9!#$%&'*+-\/=?^_`{|}~" ; - $rfc1034_ldh_str = "a-z0-9-" ; + + // Please note strings below are enclosed in brackets [], this make the + // hyphen "-" a range indicator. Hence it is double backslashed below. + // See bug 26948 + $rfc5322_atext = "a-z0-9!#$%&'*+\\-\/=?^_`{|}~" ; + $rfc1034_ldh_str = "a-z0-9\\-" ; $HTML5_email_regexp = "/ ^ # start of string diff --git a/resources/mediawiki.util/mediawiki.util.js b/resources/mediawiki.util/mediawiki.util.js index 497bb33212..7cbc85e8e4 100644 --- a/resources/mediawiki.util/mediawiki.util.js +++ b/resources/mediawiki.util/mediawiki.util.js @@ -475,7 +475,7 @@ "|" / "}" / "~" */ - var rfc5322_atext = "a-z0-9!#$%&'*+-/=?^_`{|}~", + var rfc5322_atext = "a-z0-9!#$%&'*+\\-/=?^_`{|}~", /** * Next define the RFC 1034 'ldh-str' @@ -486,14 +486,14 @@ * ::= | "-" * ::= | */ - rfc1034_ldh_str = "a-z0-9-", + var rfc1034_ldh_str = "a-z0-9\\-", HTML5_email_regexp = new RegExp( // start of string '^' + // User part which is liberal :p - '[' + rfc5322_atext + '\\.' + ']' + '+' + '[' + rfc5322_atext + '\\.]+' + // 'at' '@' diff --git a/resources/mediawiki.util/mediawiki.util.test.js b/resources/mediawiki.util/mediawiki.util.test.js index fc28e7d136..43fe4fdb4a 100644 --- a/resources/mediawiki.util/mediawiki.util.test.js +++ b/resources/mediawiki.util/mediawiki.util.test.js @@ -273,6 +273,17 @@ mw.test.addTest( 'mw.util.validateEmail( "user@localhost" )', 'true (boolean)' ); + // testEmailWithCommasAreInvalids + mw.test.addTest( 'mw.util.validateEmail( "user,foo@example.org" )', + 'false (boolean)' ); + mw.test.addTest( 'mw.util.validateEmail( "userfoo@ex,ample.org" )', + 'false (boolean)' ); + // testEmailWithHyphens + mw.test.addTest( 'mw.util.validateEmail( "user-foo@example.org" )', + 'true (boolean)' ); + mw.test.addTest( 'mw.util.validateEmail( "userfoo@ex-ample.org" )', + 'true (boolean)' ); + // jQuery plugins mw.test.addHead( 'jQuery plugins' ); diff --git a/tests/phpunit/includes/UserIsValidEmailAddrTest.php b/tests/phpunit/includes/UserIsValidEmailAddrTest.php index 203dfaee3e..99bf718ee3 100644 --- a/tests/phpunit/includes/UserIsValidEmailAddrTest.php +++ b/tests/phpunit/includes/UserIsValidEmailAddrTest.php @@ -47,6 +47,15 @@ class UserIsValidEmailAddrTest extends MediaWikiTestCase { $this->invalid( "first last@mycompany" ); $this->invalid( "firstlast@my company" ); } + // bug 26948 : comma were matched by an incorrect regexp range + function testEmailWithCommasAreInvalids() { + $this->invalid( "user,foo@example.org" ); + $this->invalid( "userfoo@ex,ample.org" ); + } + function testEmailWithHyphens() { + $this->valid( "user-foo@example.org" ); + $this->valid( "userfoo@ex-ample.org" ); + } function testEmailDomainCanNotBeginWithDot() { $this->invalid( "user@." ); $this->invalid( "user@.localdomain" ); -- 2.20.1