validator.normalizeEmail improvements

[ItalyPaleAle]

I made some improvements to validator.normalizeEmail:

1. It rejects (returning false) non valid email addresses. A sanitizer, indeed, should first of all make sure it's dealing with valid emails, in my opinion.
2. Since not all email providers are case-insensitive with the local part of the email address, an option was added to not lowercase that part for all hostnames (believe it or not, I know people with case-sensitive email addresses - and of course their company gave them uppercase letters inside! - this is 100% valid per RFC).
   However: the hostname is always lowercased and the local part of hosts known for being case-insensitive (currently, only gmail/googlemail) is always lowercased.
3. Added an option to normalize googlemail.com email addresses to gmail.com .

Please note: per issue #258 I had to remove a couple of existing test cases, as they were failing on validator.isEmail.

[ItalyPaleAle]

Actually, I'm also thinking (after reading comments to the pull request that created normalizeEmail) that it would be a good idea to create a flag to enable and disable host-specific normalizations, one for each host.

Suppose this scenario.

• Developers create a website and allow users to register and log in using their email address. The web site normalizes email addresses using chriso's validator.js and stores them in the database.
• User with email address [email protected] registers and starts using Developers' app
• After a while, chriso realizes that in the domain bar.com dots are ignored in the local part, so that [email protected] is the same as [email protected]. Chriso updates validator.js with this new, wonderful fix.
• Developers see a new version of validator.js and install it. Their tests still run successfully (they cannot write test suits for every domain!). However, they don't update their database, so User is still signed up as [email protected].
• When User tries to log in, the app normalizes his/her email address and gets [email protected]. The login fails because it doesn't match the value in the database!

So, my suggestion is to not apply host-specific normalizations by default. For each host that exists or is added, a new option flag has to be added.
If you agree, I'll also edit my patch for GMail. In the future, you'll have to be careful that changes normalizeEmail have to always be backward-compatible.

[chriso]

I agree with 1, 2 and 3. In fact I think it should always normalize googlemail.com => gmail.com without needing an option since AFAIK both are interchangeable.

I think being able to enable/disable host-specific migrations is getting too complicated. The situation you described is really outside the scope of the library.

[ItalyPaleAle]

Ok I will change the flag to automatically normalize googlemail.com . Please let me know if you need anything more to accept the pull request.

Speaking about the other issue, I understand your point and I agree that enabling/disabling each host could be complicated. However, I don't see the situation I described as outside the scope of the library; au contraire is exactly what I would use normalizeEmail() for.

I keep repeating to anyone that they always need to validate/sanitize every user input (that's why I've been using your validator.js :) ), and the email address should undergo the same treatment.

Most website use the email address as login id (I actually hate when I need to chose a different username for the login, personally), so they could/should validate that as well. And in case of an update to the library, this could break a lot of code!

I can bring three different proposals for this issue:

1. Make validateEmail only for GMail and never implement support for any other host in validator.js. The function should also be renamed to validateGMail in my opinion, in this case.
2. Write a warning note in the docs that this method may produce different results in the future, should other hosts be added. This won't solve the issue, but at least you could say "we told ya!"
3. Add another field, for example "hosts". This could be a string with all the hosts you want to parse, plus "all". The value will be "all" by default, but thanks to the warning developers can (and should) list which hosts they want to support.

Now it's your call :)

[chriso]

Thanks. I'd still prefer to keep it simple and use the same strategy that I use when I introduce any other breaking change: bump the major, as per semver.

If you could fix up a few things:

• Remove the googlemail_to_gmail flag entirely
• Put a space after if: if(foo) => if (foo)
• Use 4 spaces instead of tabs

[ItalyPaleAle]

Ok. So, shall I also rename the method to "normalizeGMail"?Anyways, it's thanksgiving weekend in Canada. I'll fix the patch on Monday

On Sat, Oct 11, 2014 at 3:18 AM, chriso [email protected] wrote:

Thanks. I'd still prefer to keep it simple and use the same strategy that I use when I introduce any other breaking change: bump the major, as per semver.
If you could fix up a few things:

• Remove the googlemail_to_gmail flag entirely
• Put a space after if: if(foo) => if (foo)

- Use 4 spaces instead of tabs

Reply to this email directly or view it on GitHub:
#320 (comment)

[chriso]

Leave it as normalizeEmail since other email addresses could still benefit from the lowercasing. Maybe just make a note in the README about how all email addresses are lowercased (the host part always, and the local part using an option), and then how gmail addresses have dots and the +extension removed. Happy thanksgiving! Enjoy your weekend.

[ItalyPaleAle]

Done

[chriso]

Perfect, thanks!

[chriso]

Available in 3.20.0.