manishsaraan / email-validator

email syntax validator npm module. fast and pretty robust
The Unlicense
435 stars 79 forks source link

Doesn't conform to the spec #14

Open Y-Less opened 8 years ago

Y-Less commented 8 years ago

These are valid, but unhandled, e-mail addresses:

name(nested (comment))@example.com
"my name"@example.com

See this link for more details:

http://haacked.com/archive/2007/08/21/i-knew-how-to-validate-an-email-address-until-i.aspx/

themrb commented 8 years ago

'Valid' is one thing, but 'sane' is another.

themrb commented 7 years ago

@jamen while that would be ideal, in the case of email the accepted use of email syntax has drifted from the spec to a simplified version.

For example, try using Gmail to send an email to one of the addresses listed above - I receive an error.

rcsheets commented 7 years ago

I don't see "doesn't conform to the spec" as a reasonable issue for this module. I'd suggest that the real problem is that it doesn't document the level of validation it performs, so users of the module have no particular basis for their expectations of the module's behavior. All we have to go on at a glance is "pretty robust" and "only checks form, not function". The examples in test.js provide a somewhat clearer picture, but what's really needed is better documentation.

Y-Less commented 7 years ago

The library is called "email-validator" - that implies to me that it should assert validity. To me being truly "valid" means exact and correct in every formal variation. The first post listed several addresses that are valid but that this check fails - that is not a validator, so either the code is wrong or the name is wrong. Maybe "email-approximator" or "email-(ad-hoc-informal-subset-of-valid)ator".

rcsheets commented 7 years ago

To me being truly "valid" means exact and correct in every formal variation.

Right. To you, that's what it means, having read just the name of the library. That's why it needs to spell out more clearly what it does. So that you'll have more to go on than just the name. Different people will undoubtedly have different expectations and assumptions. The only way to deal with that effectively is with documentation.

Sembiance commented 7 years ago

When I created this module over 4 years ago, I did it just to quickly validate an e-mail and went with the "good enough" approach for the project I was using it for. I see the merit in providing a full spec validation method, whether that's an additional method/option or just baked in.

In any case, I don't currently have any interest in working on improving this myself. I'm happy to merge any pull requests that come in and if anyone would like to take over the project I can transfer the GitHub repo and NPM module.

FrenchTechLead commented 7 years ago

Is it sane not to validate an email that finishes with a whitespace ?

daspete commented 6 years ago

@Meshredded what about trimming the input before validation? ;)

noway commented 5 years ago
> validator.validate("test@localhost")
false

this is not okay.

charlie-s commented 5 years ago

There's a surprising number of responses in projects like this and related StackOverflow questions where users point out that "it doesn't validate foo bar@10.0.0.1". I think there's a lot of confusion over what the intent of regex or little helper functions like this do – are they validating email spec or are they validating an email address to ensure deliverability from a public server?

For the former, the spec is pretty open and I haven't found a situation where I wanted to accept an email that conformed to edge cases. I don't write things like SMTP libraries, though.

For the latter, I'm not interested (e.g.) in sending ecommerce receipts to foo@localhost or user @aaaaaa.bbbbbb. I'm combatting the many users that punch in jsmith@gmail and jsmith@gmail.xom. As was pointed out earlier, the intent just needs to be made clear so that folks can use the right tool for the job. I know what the maintainer said about not using this anymore, just posting some thoughts.

noway commented 5 years ago

@charlie-s I had a customer once complain that writeme@dave.consultingservices (or something of that sort) wasn't getting accepted by the registration page.

It's just better to conform to the spec all the way, filter out the most egregious email strings which are certainly don't conform to the spec (&@&, etc), and let the rest be handled by your email sending API (SMTP connection to somewhere, Sendgrid, etc).

I've chosen https://www.npmjs.com/package/isemail since 6 days ago and am not looking back.

To be honest, this package does validate writeme@dave.consultingservices email specifically, but I just can't bring myself to rely on ad hoc regular expressions which are based on someone's understanding of how email addresses usually work, whether they are written by me or packaged by someone else. writeme@dave.consultingservices. for example, still routable, is not validated.

charlie-s commented 5 years ago

I would also complain if that address wasn't accepted – I'm not suggesting it shouldn't be, quite the opposite.

If you conform to the spec all the way, then you'd allow test@localhost on your registration page, or even $@a (which is valid). In the systems I work on, we send emails asynchronously via workers and don't require the user wait on the registration (or checkout page) before their email is sent (or rejected by SendGrid). Instead, we require a confirmation field and validate them as quickly as possible.

I'm also using https://www.npmjs.com/package/isemail but with the tld white/black list options to prevent jsmith@gmail.xom. There are a bunch of web services that exist to do validation, maybe I need to find the right one. MailGun has their own, and I use it, but it's unfortunately not cheap at scale.

I'm just kind of confused when I stumble upon these threads and inevitably find people wanting &@a to pass as valid.

noway commented 5 years ago

Fair enough for your constraints, totally an appropriate approach if you send emails asynchronously. In my case I err on the side of correctness over bandwidth. Also, I concede that $@a and &@a are valid, but &@& is indeed not though.