checkDNS can't work since it's making isemail asynchronous.

Would you recommend supporting a subset of the options and mapping those to the ones expected by isemail?

checkDNS seems to be the one exception Joi can't support at the moment, we'll revisit later if Joi changes enough so that it becomes feasible.

@nlindley can you make the change about checkDNS ?

@Marsup Sure. I thought you wanted to hold off until async could be supported. I’ll get you something either this afternoon or tomorrow depending on how much time I have available.

@nlindley May I suggest you validate the passed options yourself immediately, to catch errors at configuration time?

I have updated the branch which disallows checkDNS.

@kanongil If you would prefer to whitelist options that are known to not cause problems, I can do that instead.

I should also mention, isemail does a pretty good job of either coercing options or throwing a TypeError.

@nlindley I was not really concerned about how well isemail handles the options. I suggested the extra check because the options can then be validated when configuring the schema, rather than when it is used. A brief glance tells me that Joi pre-validates most (all?) params using Hoek.assert(), though in this case Joi.assert() might be more appropriate.

@kanongil All right. I’ll get it fixed up in the morning.

I’ve pushed another update. I’ve used Joi to validate the isemail options. Doing this is more complicated, so let me know if you want to go a different route, or if there are style changes needing made.

isemail seems to be doing a far more complex validation of its input, we won't be able to guarantee it doesn't fail, I'm wondering if it's worth it at all. Wouldn't it be a better solution to do a test run on a generated email ?

I’m not sure that would keep somebody from trying to set checkDNS to true. To be honest, we’re not even worrying about the options any more, and if somebody really needs it (at least in Hapi), they can always write a custom validate() function, which supports async, that uses isemail directly.

@kanongil I think we're gonna rollback that change, this seems like an unnecessary pain for joi.

No problem – it was merely a suggestion towards improving the user experience (at the expense of some additional complexity).

Sidenote: Does joi handle the exceptions that can be thrown from isemail? or is it up to the caller to catch those errors? It is not apparent that validate can fail with a thrown error.

@nlindley sorry for the back and forth, I finally took time to read in details about isemail.

I think we'll go with this :

• Deny checkDNS like you did
• Do a basic number check for minDomainAtoms with Hoek (number > 0)
• Same thing for tldWhitelist (object or array)
• Same thing for errorLevel (number > 0 or boolean), you'll have to check in this case that the result === 0

@kanongil is right about errors, isemail seems to throw a few by looking at its code, we should be defensive about that. What worries me is that these cases are not even tested in that lib.

Hopefully I got this right this time :)

Sounds good. minDomainAtoms can actually be 0, so I'm checking that it is >= 0. The check you propose for errorLevel is a little more strict than what isemail requires, but I’d be surprised if that causes anybody problems.

Nevermind about the minDomainAtoms. I missed the coercion in the if statement of isemail.

Stricter in what way ?

I was mistaken. See my "Nevermind" comment.

@Marsup Actually, errorLevel is a little stricter. I got confused on which property you were asking about.

In isemail, if typeof errorLevel is not a number, it gets the double-bang (!!errorLevel) treatment to coerce the value to a boolean.

I don't see why that module does that, let's keep it strict.

The assertions get a little lengthy. Let me know if the latest commit is more of what you were thinking.

@nlindley I took over and fixed a last few details, thanks a lot for your work.

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.