Skip to content
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

Email address validation sometimes fails when it should not #1

Open
asalant opened this issue Apr 22, 2010 · 2 comments
Open

Email address validation sometimes fails when it should not #1

asalant opened this issue Apr 22, 2010 · 2 comments

Comments

@asalant
Copy link
Owner

asalant commented Apr 22, 2010

[Copied from old Trac hosting at http://bikekitchen.trac.cvsdude.com/membership/ticket/11. Reported by Tim Ledlie 2009-10-29.]

An issue came up at the greeter desk today about freehub. A volunteer was entering in a new member, and when she clicked "create", she got an error message saying the email address was invalid. I looked at the email address and it looked fine to me (no typos in the "@gmail.com" part of the email address). Another guy who often greets chimed in and said this often happens to him, and he has no other recourse but to delete the email address and save the new person without record of the address. (Obviously, it would have been helpful if he had opened a ticket like this one.)

How is freehub deciding that this is an invalid email address? I tested it out, and it seems to do validation of the domain and make sure the email address is well-formed (has a username followed by "@" followed by a domain), but does not do any error checking on the username (I'm not even sure this is possible without sending a test message and checking if it bounces).

In any case, I think freehub should give a more descriptive error message than: "Email is invalid." Better would be "Email has invalid domain (the part after the @). Make sure you spelled it right." or "Email missing @ character" or "Invalid character in email address. The following characters are not valid in any email address: ()[];:,<>". There should be one descriptive error message for every possible type of invalid email address.

In addition to better error messages, I think there might be other problems with the email validation code. Today, I looked carefully at the email address that freehub said was invalid, and I could see no problems with it -- it had a username with just letters in it, an @, and then gmail.com. It is possible that there was whitespace at the beginning or end -- I have since tested this and found that freehub says the email address is invalid if there is a space on either side of the address -- this is an easy fix.

This seems like an important issue b/c capturing patrons' email addresses is important, and we don't want confused greeter volunteers just leaving out the email address b/c they have no other way to get Freehub to let them create a new Person. I think it's great there is validation of email addresses to help prevent typos, but the email validation doesn't seem quite right yet.

I'd be happy to help further with this, just let me know.

Thanks Alon,

Tim Ledlie

@asalant
Copy link
Owner Author

asalant commented Apr 22, 2010

[Then Alon replied:]

I've pushed a fix for white space in email addresses. First name, last name and email are now stripped of white space before validation.

It's likely that there is still a remaining issue. I took a look at the docs ( http://github.com/heycarsten/email-veracity) for the plugin I am using to validate emails and found this insightful note:

== A Note About The ActiveRecord? Plugin

It's dead! Why? Determining the validity of an email address based on a lookup of its domain is a good idea, but basing it off of one single test done inline with a request is not. A name server might be down, shit happens, and you might be snubbing a totally legitimate person from using your product — not good.

I feel a better solution is to check the pattern of the email address and perhaps check it against the blacklist on creation. The other tests should be done in the background over the period of many days and be added to a log. A report can then be performed and you can statistically determine the addresses that are most likely false and take appropriate action from that point. ==

So the original author questions the approach. While it has been helpful defense against invalid/mistyped email addresses, it sounds like it is rejecting valid ones too. That's no good.

@asalant
Copy link
Owner Author

asalant commented Apr 22, 2010

[Then Tim replied:]

Yeah, his comment makes a lot of sense. If you wanted to continue using the validation code, I still think you could more gracefully handle errors. It looks like there are three types of errors: Error, MalformedEmailAddressError?, and DomainResourcesTimeoutError?. Depending on what error is returned, your code could act accordingly.

But I also think it would be fine to bag using email-veracity and just do some simple error checking like he suggests. Instead of a domain blacklist, it makes more sense to me to have a big whitelist, and if the given domain isn't in the whitelist, print a notice like "you may have mistyped the email domain (the part after the '@'). Please confirm it's correct". But then let them save it if they confirm.

asalant pushed a commit that referenced this issue Jun 23, 2021
Add email search function to Person model
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant