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

validateEmail incorrectly tests results of std.net.isemail.isEmail() #1580

Closed
radcapricorn opened this issue Oct 1, 2016 · 2 comments
Closed

Comments

@radcapricorn
Copy link
Contributor

vibe.utils.validation.validateEmail tests the results of std.net.isemail.isEmail like this:

return isEmail(str) != EmailStatusCode.valid;

However, isEmail returns EmailStatus, not EmailStatusCode. Incidentally, EmailStatus aliases self to a bool property valid(), which allows the code above to compile. But the logic is reversed, since EmailStatusCode.valid == 0, although isEmail(str) == false when the e-mail is invalid.
Paired with Phobos fixes between versions, this leads to further inconsistencies at run time.
For example, with dmd 2.071.1 (when there was an issue in Phobos):

auto result = isEmail("[email protected]");
/*
result == EmailStatus
{
    valid: false
    localPart: a
    domainPart: b.c
    statusCode: error
}
*/
auto err = appender!string();
auto vibeResult = validateEmail(err, "[email protected]");
/*
vibeResult == true
*/

Even though isEmail() (incorrectly) returned an invalid status, vibe treats it as valid.
And with current dmd nightly:

auto result = isEmail("[email protected]");
/*
result == EmailStatus
{
    valid: true
    localPart: a
    domainPart: b.c
    statusCode: valid
}
*/
auto err = appender!string();
auto vibeResult = validateEmail(err, "[email protected]");
/*
vibeResult == false
err.data == "The email address is invalid."
*/

Vice-versa, isEmail() now correctly returns a valid status, but vibe treats it as invalid.

@radcapricorn
Copy link
Contributor Author

Note: Phobos issue mentioned above holds for Phobos included with dmd 2.071.2 as well. The proposed fix would effectively break validateEmail() for anyone not using nightly dmd/Phobos builds.

@s-ludwig
Copy link
Member

Fixed by #1582.

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

2 participants