Remove dns lookup functionality and remove punycode dependency

[WesTyler]

Following up the discussion from #17

At the moment, it seems that dns checking is superfluous to the goals of this module, and getting rid of it would also mean we wouldn't need the output of punycode. We'd need to figure out how to calculate what a punycoded domain's length would be, though.

In support of Unicode internationalized email addresses, punycode was added in order to convert the normalized Unicode domain to ASCII-only text for dns lookups checking for valid MX records on the domain. Per the comments above, dns checking should be removed along with the dependency upon the punycode module; some method for calculating punycoded domain lengths should be implemented.

@skeggse is this an accurate summary? Also, is it acceptable to bring punycode in as a devDep for testing that the domain length calculation is accurate?

[skeggse]

Yes to both.

This will also fix behavior that I forgot to add/request tests for (maybe you already got this from the other conversation): we need to length-check what the domain would be after punycoding, rather than checking what it would be after utf8 encoding.

[WesTyler]

Sorry, ended up having some Felicity maintenance take up most of my day today. This is on my radar ASAP.

[WesTyler]

@skeggse I'm working on this off and on for the next couple of days. I have a question regarding a specific test case that I hope you can weigh in on.

test@io is (prior to removing dns) considered a valid email because the dns.resolveMX returns an entry for the io domain. This short-circuits a logic path later in the code here because of the dnsPositive variable that I am now removing.

So, the question is: should test@io still be considered a valid email? If so, what is the intent of this condition now?

[WesTyler]

It seems from RFC 5321 section 2.3.5 that io is a valid top-level domain. That brings up my second issue with the elementCount === 0 check.

[skeggse]

test@io is intentionally valid, and should continue to be classified as such.

I believe the referenced code path is specifically for test@.

[WesTyler]

Got it. Thank you

[WesTyler]

Hm, it looks like test@ and test@\u0000 are not making it to that code block. They are getting caught here or here first.

[skeggse]

Maybe push up what you have - as long as there's an error produced when the email address is invalid, we can figure out the correct classification.

[WesTyler]

Okay, @skeggse , sorry for the crazy long delay. Finally getting back around to this.

I feel like I'm 99% there, but I have a clarification in changed behavior directly related to removing DNS MX lookups that I want to check with you.

Per the discussion above, test@io should be valid because it is a valid top-level domain per RFC 5321. I updated the code to allow this, but now it allows non-top-level domains such as test@org. I'm not seeing a way around this since the MX lookups will no longer be happening. Do you?

[WesTyler]

tl;dr if we are considering test@io to be valid I think test@org should be as well.

[WesTyler]

Also, I ported over the minimum code from punycode to calculate the punycode length without adding the dependency. My question now is re:

we need to length-check what the domain would be after punycoding, rather than checking what it would be after utf8 encoding

This will mean some Unicode domains with a Buffer.byteLength well over the threshold will now be considered valid since the punycode algorithm saves a ton of bytes for small-delta codepoints. This is expected and acceptable, right?

For example:

Buffer.byteLength('ñoñó郵件ñoñó郵件.ñoñó郵件ñoñoñó郵件ñoñó.郵件ñoñó郵件ñoñó郵件.ñoñó郵件ñoñó郵件ñoñó郵件.ñoñó郵件ñoñó郵件.ñoñó郵件ñoñó郵件.ñoñó郵件ñoñó郵件.ñoñó郵件ñoñó郵件.ñoñó郵件ñoñó郵件.oñó郵件ñoñó郵件ñoñó郵件.商務');
// 316
PunyLength('ñoñó郵件ñoñó郵件.ñoñó郵件ñoñoñó郵件ñoñó.郵件ñoñó郵件ñoñó郵件.ñoñó郵件ñoñó郵件ñoñó郵件.ñoñó郵件ñoñó郵件.ñoñó郵件ñoñó郵件.ñoñó郵件ñoñó郵件.ñoñó郵件ñoñó郵件.ñoñó郵件ñoñó郵件.oñó郵件ñoñó郵件ñoñó郵件.商務');
// 173

[EDIT] According to RFC 5890 Section 2.3.2.1 I think that this should be acceptable and expected behavior. The RFC is specifically referring to domain label length, but I think the intent applies more broadly to the entire domain:

expansion of
the A-label form to a U-label may produce strings that are much
longer than the normal 63 octet DNS limit (potentially up to 252
characters) due to the compression efficiency of the Punycode
algorithm. Such extended-length U-labels are valid from the
standpoint of IDNA, but caution should be exercised as shorter limits
may be imposed by some applications

thought that does sound like I need to add logic and tests to look at the punycoded length of the domain labels rather than just character length. Thoughts?

[WesTyler]

I went ahead with my assumptions and pushed a new commit on the open PR. Let me know if I chose incorrectly :)

[skeggse]

Yay! #153 is shipping in 3.0.0.

[WesTyler]

Awesome! :D 👍