Runtime: System.ComponentModel.DataAnnotations EmailAddressAttribute behaviour differences by framework/execution context

[chrisaut]

The following test fails when run via xUnitConsole/ConsoleApp/nCrunch, but succeeds (as it should) when run via the VS Test Explorer/inside an MVC app:

```c#
using Xunit;
using System.ComponentModel.DataAnnotations;
using System.Collections.Generic;

namespace ncrunchRepo
{
public class Class1
{
[Fact]
public void Bad()
{
var result = Test(new TestClass { Email = "[email protected]" });
Assert.NotEmpty(result);
}

private List Test(TestClass @object)
{
var context = new ValidationContext(@object, serviceProvider: null, items: null);
var results = new List();
Validator.TryValidateObject(
@object, context, results,
validateAllProperties: true
);
return results;
}
}

public class TestClass
{
[EmailAddress]
public string Email { get; set; }
}
}
```

Remco from NCrunch has validated that this issues does not appear prior to net472.
Here is result of different frameworks:

Note, while I use nCrunch to test this, the same behavior can be seen running with xUnit console runner or a simple Console Application, vs behavior in VSTestExplorer/MVC app.

The correct behavior I think is that this should pass (validation error), which I'm seeing on our production system when run as 472 inside an mvc app. The tests for it pass in VS Test Runner, but fail under any other runner.

Did the validation of EmailAttribute change recently?

[chrisaut]

It looks this the behavior was changed in this commit: https://github.com/dotnet/corefx/commit/070e282397b21450f80a20028c5e5eff10ec46a4#diff-a0a5196896f9688dcfac159ebdaf7d8f

which removed the regexes, and now simply checks if there is an '@' that is not in first or last place. The commit messages says this is due to:

Microsoft Security Bulletin MS15-101, CVE-2015-2526 - Removing the re… 
…gexes

for EmailAddressAttribute, PhoneAttribute and UrlAttribute and update
RegularExpressionAttribute to allow user to specify a timeout.

This gives information here: https://docs.microsoft.com/en-us/security-updates/SecurityBulletins/2015/ms15-101#mvc-denial-of-service-vulnerability---cve-2015-2526

So I guess this is by design? So essentially now EmailAddressAttribute is kind of useless, as even "1@2" will pass validation. Is this really the best we can do given the SecurityBulletins? Was the issue here the usage of Regexes in general that caused the vulnerability?

[ajcvickers]

Closing since this change was by-design, as mentioned above.

[weitzhandler]

1. Please update EmailAddressAttribute documentation accordingly.
2. Why are you sealing all these classes?

[weitzhandler]

How about adding an EmailValidationOptions flags enum to let devs choose from email validaiton types, where the None is the current implementations?
I'm willing to contribute on this.

We might want to complicate this a further by having a EmailValidationOptions.Custom flag validates against regex pattern.

Similar debate about the other validation attributes like phone etc.