Hi,
Today I found out that EmailAddressAttribute is quite bad.
First of all, now email string can contain only one @ char.
RFC 2821 specifies Abc@[email protected] as valid address (https://haacked.com/archive/2007/08/21/i-knew-how-to-validate-an-email-address-until-i.aspx/), so I think that we need better validation.
Second, mail address [email protected] is not valid, but EmailAddressAttribute recognizes it as valid.
I've prepared some tests (https://github.com/dotnet/corefx/pull/32717).
Can we solve it with regular expressions? I can provide new implementation in next few days.
This is a pretty difficult problem to solve, and I don't think that a (very complicated) regex is the answer. In System.Uri we implement email address parsing that isn't perfect but that is generally "good enough", and I think we should take the same approach here.
Are you actually running into issues caused by these specific cases?
I ran into issues with this too, see my my comments here: https://github.com/dotnet/corefx/issues/32688#issuecomment-428780729
@rmkerr I don't agree that regex is not the answer. Regex was removed in 2015 (details in dotnet/runtime#27569) and until that moment it worked like a charm.
@chrisaut I totally agree. EmailAddressAttribute is useless now.
@rmkerr Our tester found out this. For him, it is unacceptable that he can register a user with an invalid email address. And for me, it is not "good enough". Even in spring they use regex for it (https://github.com/Baeldung/spring-security-registration/blob/master/src/main/java/org/baeldung/validation/EmailValidator.java)
If it is that difficult problem maybe you should remove this attribute?
@galczo5 You do realise that the expression used in your linked file is not correct aka. RFC compliant?
@2called-chaos
Yup. But it's better than current solution. I found regex in referencesource repo too: https://github.com/Microsoft/referencesource/blob/master/System.ComponentModel.DataAnnotations/DataAnnotations/EmailAddressAttribute.cs
For me, if we want to keep it regex free, we should implement additional email address domain checking and it will be great. We should make very good documentation for it.
@rmkerr do you mean use(no RegEx involved)
https://github.com/dotnet/corefx/blob/a10890f4ffe0fadf090c922578ba0e606ebdd16c/src/Common/src/System/Net/Mail/MailAddressParser.cs
https://github.com/dotnet/corefx/blob/a10890f4ffe0fadf090c922578ba0e606ebdd16c/src/System.Net.Http/src/System/Net/Http/Headers/HeaderUtilities.cs#L404
Hi,
I think that the current implementation is not enough and it should be improved. Invalid addresses as '.@.' or '[email protected]' are valid for the current validator.
IMHO the safest way to validate an email address is to actually send an email and verify it came through.
@conradreuter, agreed. But in most cases you want to verify it without sending an email, that's what the old RegEx was for. Why not use it again?
That highly depends on what you specify the word "valid" to mean.
If you want to make sure that the address conforms to RFC2822, then sure, a complex RegEx might be the way to go.
But for all practical purposes I can think of, "valid" means that somebody is actually able to receive an email via this address. A quick sanity check 脿 la _contains @ and something before and after_ might be completely sufficient.
@conradreuter I agree that simple check is ok, but current validation is not enough. A user can misspell an email address: instead of "[email protected]", the user types "user@userLcom" and it is a problem.
If I'm not mistaken, according to RFC2822 this is a valid email address.
(Just like _root@localhost_ is a valid email address)
But for all practical purposes I can think of, "valid" means that somebody is actually able to receive an email via this address. A quick sanity check 脿 la contains @ and something before and after might be completely sufficient.
Speaking on this point, I've had multiple sites reject my primary email address as "invalid". (Including one unsubscribe box, which is a quick way to get marked as spam)
Maybe EmailAddressAttribute should implement more than one validation algorithm.
Default one that is the same as current and second more complex that will check for domain i email.
What do you think?
Maybe EmailAddressAttribute should implement more than one validation algorithm.
I think we can simply have a better implementation without RegEx (for security reason https://github.com/dotnet/corefx/pull/4319). On codebase there is already similar code.
This is not something we plan to implement. The check is intentionally naive because doing something infallible is very hard. The email really should be validated in some other way, such as through an email confirmation flow where an email is actually sent. The validation attribute is designed only to catch egregiously wrong values such as for a U.I.
Most helpful comment
This is not something we plan to implement. The check is intentionally naive because doing something infallible is very hard. The email really should be validated in some other way, such as through an email confirmation flow where an email is actually sent. The validation attribute is designed only to catch egregiously wrong values such as for a U.I.