Skip to content

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

Merged
hueniverse merged 1 commit intohapijs:masterfrom
EndangeredMassa:email-error
May 22, 2014
Merged

Bug Hunt: double-quoted local-parts of email addresses cause validation failure#282
hueniverse merged 1 commit intohapijs:masterfrom
EndangeredMassa:email-error

Conversation

@EndangeredMassa
Copy link
Contributor

@EndangeredMassa EndangeredMassa commented Apr 23, 2014

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

hueniverse commented Apr 23, 2014

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
Merge pull request #282 from EndangeredMassa/email-error
f3937a2 
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

hueniverse commented May 22, 2014

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

@hueniverse
Copy link
Contributor

hueniverse commented May 22, 2014

@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
Replace email parser. Fixes #282
2eb1519
@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.

Reviewers

No reviews

Assignees

@hueniverse hueniverse

Labels

None yet

Projects

None yet

Milestone

4.3.0

Development

Successfully merging this pull request may close these issues.

3 participants

@EndangeredMassa @hueniverse @skeggse