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

EmailValidator should use filter_var in strict mode if Egulias library is missing #25674

Closed
ebuildy opened this issue Jan 3, 2018 · 3 comments

Comments

@ebuildy
Copy link
Contributor

ebuildy commented Jan 3, 2018

Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? yes/no
Symfony version 4.0.0

Egulias\EmailValidator\EmailValidator is not really perfect (toto@toto.___c is valid whereas it's not for HTML5 or filter_var), so I propose to use filter_var with FILTER_VALIDATE_EMAIL if Egulias is not installed:

EmailValidator:

if (!class_exists('\Egulias\EmailValidator\EmailValidator')) {
                if (filter_var($value, FILTER_VALIDATE_EMAIL) === false) {
                    $this->context->buildViolation($constraint->message)
                        ->setParameter('{{ value }}', $this->formatValue($value))
                        ->setCode(Email::INVALID_FORMAT_ERROR)
                        ->addViolation();

                    return;
                }
            }
@jvasseur
Copy link
Contributor

jvasseur commented Jan 3, 2018

Having the behavior of the constraint change based on the presence of a dependency looks like a bad idea.

We could add another validation mode that is based on FILTER_VALIDATE_EMAIL but I'm not sure if it's worth it since FILTER_VALIDATE_EMAIL doesn't match any standard (that's why we stopped using it).

@ebuildy
Copy link
Contributor Author

ebuildy commented Jan 4, 2018

Yes you are right, I should declare the bug on the library Egulias\EmailValidator instead.

We are using FILTER_VALIDATE_EMAIL in lot of projects, do you have sources saying it's bad?

Thanks you, I am closing the issue so.

@ebuildy ebuildy closed this as completed Jan 4, 2018
@jvasseur
Copy link
Contributor

jvasseur commented Jan 4, 2018

@ebuildy you can look at the references in the PR that removed the use of filter_var : #9140

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