Page MenuHomePhabricator

Problems with e-mail validation script
Closed, ResolvedPublic

Description

As said in Bug 26910:

The variable rfc5322_atext contains "...+-/...". This doesn't seem right
(RegExp would also catch ",") and so "-" should be backslashed (double
backslashed).

Also I think that var keyword should be placed before rfc1034_ldh_str as the
comment above it makes the code less clear.

And a minor code cleanup - change this:
'[' + rfc5322_atext + '\\.' + ']' + '+'
to this (shorter and clearer):
'[' + rfc5322_atext + '\\.]+'

BTW there is an error message here:
http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/resources/mediawiki.util/mediawiki.util.js?view=markup&pathrev=79924

Maybe this is because of the the character next to "}" in rfc5322_atext which I
see as a highlight question mark or as "Ń" (depending on codepage). Not sure
what it is. Probably should be encoded in some other way.


Version: 1.17.x
Severity: minor

Details

Reference
bz26948

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:12 PM
bzimport added a project: MediaWiki-Email.
bzimport set Reference to bz26948.

Self notes:

Since rfc5322_atext is used inside brackets, the hyphen - is interpreted as a range operator (need to be checked in both JS and PHP version + tests).

I already removed the incorrect character in r79952.

Nux detected three issues:

  1. backslash the dash "-" in rfc5322_atext (check both JS & PHP versions)
  2. code cleanup in JS version
  3. invalid utf8 character

I plan to fix and update tests for 1 & 2 next week. 3 is the only one fixed by r79952. I would then make sure they get reviewed by poking someone on IRC and tag them for 1.17 merge.

Being minor changes, they can be handled by someone else if there is any emergency.

Lowering severity to "minor".

Fixed in r81101
Marked revision for inclusion in 1.17