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).

diff --git a/includes/User.php b/includes/User.php
index db854a7..2ef85d2 100644 (file)
--- 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 497bb33..7cbc85e 100644 (file)
--- 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 @@
                         *      <let-dig-hyp> ::= <let-dig> | "-"
                         *      <let-dig> ::= <letter> | <digit>
                         */
-                               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 fc28e7d..43fe4fd 100644 (file)
--- 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 203dfae..99bf718 100644 (file)
--- 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" );