Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discrepancies between isValid and normalizeEmail #825

Open
rathboma opened this issue May 3, 2018 · 4 comments
Open

Discrepancies between isValid and normalizeEmail #825

rathboma opened this issue May 3, 2018 · 4 comments
Labels

Comments

@rathboma
Copy link
Contributor

rathboma commented May 3, 2018

So there are situations where isEmail will return true, but normalizeEmail will return false.

For example:

[email protected] -> isEmail returns true, normalizeEmail returns false.

Really this comes down to a behavior that I think the library should enforce:

If isEmail returns true, normalizeEmail should be able to return a value

Currently normalizeEmail can return false, which seems wrong. It feels like if normalizeEmail can't do what it needs to it should return the original string or null?

Extend isEmail?

Currently normalizeEmail is actually doing domain-specific email validation as well as normalizing the email. For example [email protected] is syntactically valid, but invalid for gmail. Maybe isEmail should also be extended to contain this domain-specific logic to bring it into line with what normalizeEmail is doing.

@chriso
Copy link
Collaborator

chriso commented May 4, 2018

Agreed, this is confusing behavior of normalizeEmail(). It should always succeed, provided the input email is valid. It should be isEmail() that rejects those particular emails (starting with +). We could add domain-specific validation to isEmail(), but I wonder if [email protected] is even valid (in the general case)?

@chriso chriso added the 🐛 bug label May 4, 2018
@tux-tn
Copy link
Member

tux-tn commented May 13, 2018

@chriso i want to work on this issue but i have some questions:

  • [email protected] is not working because normalizeEmail is considering it as a subaddress, should we return [email protected] or return false/null in that case?
  • Should we check for Email providers limitations in normalizeEmail and isEmail, for example gmail is limiting the local part of the address to a minimum of 6 characters and a maximum of 30 character and the first character needs to be alphabetic.
  • Many RFC 5322 compliant email addresses cannont be used in gmail/yahoo/.. Should we remove invalid characters like $ or % ?

@chriso
Copy link
Collaborator

chriso commented May 14, 2018

@tux-tn [email protected] is an invalid gmail address. I think we should add domain-specific validation given how ubiquitous gmail addresses are.

@twelve17
Copy link

twelve17 commented Jul 9, 2018

According to this post, there are gmail addresses out in the wild that have a username part less than six characters. It's an edge case, but it's possible we ran into one of these today, and now I am faced with having to disable email validation altogether because it is trying to be too clever.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants