Skip to content

Bug Hunt: double-quoted local-parts of email addresses cause validation failure #282

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

Merged
merged 1 commit into from
May 22, 2014
Merged

Bug Hunt: double-quoted local-parts of email addresses cause validation failure #282

merged 1 commit into from
May 22, 2014

Conversation

EndangeredMassa
Copy link
Contributor

According to RFC2822 Section 3.4.1, email addresses can use double-quoted local-parts. Joi erronously calls this an error case.

This, of course, depends on your definition of a valid email address. If you want to be strictly RFC-compliant, then this is an error. If you want to be more realistic, I don't think this is a huge deal. In fact, RFC 5321 says that "all quoted forms MUST be treated as equivalent, and the sending system SHOULD transmit the form that uses the minimum quoting possible.". So, while technically valid, hosts are discouraged from using it. It's up to you whether or not this qualifies for the Bug Hunt.

It also fails on üñîçøðé@üñîçøðé.com, if that's a more acceptable failure case.

Test output using joi@4.0.0:

$ npm test

> joi@4.0.0 test /home/sean/demo/joi
> make test-cov



  ..................................................
  ..................................................
  ..................................................
  ..................................................
  ..................................................
  ..................................................
  .........................{ [Error: value must be a valid email]
  details: 
   [ { message: 'value must be a valid email',
       path: 'value',
       type: 'string.email' } ],
  _object: '"van"@walmartlabs.com',
  annotate: [Function] }
x........................
  ............................................

 1 of 394 tests failed:

  326) string #validate should validate email:

      actual expected

      truefalse

      at /home/sean/demo/joi/test/helper.js:35:37


 No global variable leaks detected.

 Coverage: 100.00%
make: *** [test-cov] Error 1
npm ERR! Test failed.  See above for more details.
npm ERR! not ok code 0

@hueniverse
Copy link
Contributor

Can you use unicode directly without encoding it per RFC 3987?

I need to look into the quotes part.

@skeggse
Copy link

skeggse commented Apr 30, 2014

The following RFC's are all related to whether an email address is valid.

http://tools.ietf.org/html/rfc5321
http://tools.ietf.org/html/rfc5322
http://tools.ietf.org/html/rfc4291
http://tools.ietf.org/html/rfc1123#section-2.1
http://tools.ietf.org/html/rfc2821
http://tools.ietf.org/html/rfc2822
http://tools.ietf.org/html/rfc3696

Regular expressions aren't necessarily the right solution to parse all of these, so I ported the is_email PHP function over to Node (node-isemail) for the purposes of RFC-compliant email address validation, with optional DNS record verification. The module passes all the test-cases.

Edit: that said, it looks like isemail doesn't support unicode because they're not valid as per the reference rfc's. Unicode needs to be serialized to ASCII so that it can be used in an email address. This is done automatically in Nodemailer, using the built-in punycode module and mimelib.

hueniverse pushed a commit that referenced this pull request May 22, 2014
Bug Hunt: double-quoted local-parts of email addresses cause validation failure
@hueniverse hueniverse merged commit f3937a2 into hapijs:master May 22, 2014
@hueniverse hueniverse added this to the 4.3.0 milestone May 22, 2014
@hueniverse hueniverse self-assigned this May 22, 2014
@hueniverse
Copy link
Contributor

Confirmed. $50
@EndangeredMassa please email me to arrange for delivery of your award.

@hueniverse
Copy link
Contributor

@skeggse thanks for the module. We'll be using it. Hopefully you can push a 1.x soon so that the API is locked.

hueniverse pushed a commit that referenced this pull request May 22, 2014
@EndangeredMassa EndangeredMassa deleted the email-error branch May 22, 2014 14:14
@skeggse
Copy link

skeggse commented May 27, 2014

@hueniverse pushed 1.0.0, locked API.

@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants