5.8 Email validation changes

[arjasco]

The email validation package thats now being used in 5.8 clearly covers a lot more cases now as it matches a range of RFC's, which is a good thing, I guess!

As this is the case, periods (.) are no longer required which is technically correct but the likelihood of any of the methods that developers are using to send email support a domain without a period is really slim, unless you're using it internally as it will be matching a hostname.

Tinker:

even ' @ ' is valid, again it's correct as per the RFC's but highly unusual..

The package is customisable, you can add your own validators. However, there is no way to customise the behaviour within Laravel without extending the validator and implement your own validation check.

I personally wouldn't consider requiring a period in the domain part of the email address as "special case". Users could easily enter "JohnDoe@yahoo" in their user profile for example, passes validation (if you're not using another service to go beyond, DNS check etc) and then an issue only gets noticed when emails are failing to send as they forgot to supply .com..

Would there be a consideration to make this customisable? Just feels like we are covering highly unusual cases now, which if that was your need then maybe bring in a package specifically as such the one being used 🤷‍♂️

[staudenmeir]

How do we implement this? Add an option to the email rule? Or have two separate rules?

[arjasco]

So i think the options are quite limited in what can be done other than the two points you've mentioned. Introducing another validation rule just feels a bit odd to me. The documentation will need to be updated to make it clear the difference between the two.

The new library being used (egulias/EmailValidator) allows you to create custom validators. I see it's already been queried about validating a TLD and the author replied giving an example of creating a new validator in conjunction with the RFC validator.

However this would mean creating a new class in the laravel framework source but extending the interface from the library 🤢so thats a clear no go.

Then yeah the other possibility is a rule with an option like email:simple or similar and just use filter_var if this is present.

[Sladewill]

Why not do it in the same way that Unique and Exists rules are done with parameters?

[felorhik]

With just RFC validation like the referenced issue above, invalid domains in an email validate.

So something like something@invaliddomain would validate as true. In most cases this is fine, since at the lowest level of validation that would be correct for some systems, and the validation would block that out (assuming things like something@localhost).

I ran into this issue when testing a form that created customers in stripe, Stripe requires a correct domain, so it would lead to the validation passing, then an exception would be thrown by Stripe.

There is now another validator in https://github.com/egulias/EmailValidator that can allow you to check for a correct domain as well, but that would require a change to the validator, and would make it more strict which could cause issues in certain circumstances as well.

In order to handle more use cases would be variables attached or something like @arjasco has brought up, but maybe like email:rfc,dns with standard email being the same as email:rfc (so it works the same as it currently does).

This would allow alternate validators to be added like SpoofCheckValidation so email:rfc,dns,spoof or even go further and add custom validators to the email validator like:

'rules' => [
   'email' => [
      'rfc',
      'dns',
      new CustomEmailValidation
   ]
]

I ended up just adding a custom rule to use instead of email for now, here is the function that implements the dns validator as well.

public function passes($attribute, $value)
    {
        if (! is_string($value) && ! (is_object($value) && method_exists($value, '__toString'))) {
            return false;
        }

        return (new EmailValidator)->isValid($value, new MultipleValidationWithAnd([
            new RFCValidation,
            new DNSCheckValidation
        ]));
    }

[sf-steve]

Urg, this is causing us issues - whilst technically valid, emails without periods in the domain cause mailgun to throw an exception. In every case in the logs, this is a result of user error. Is there a simple way to revert this behavior - i know we can find all instances of 'email' in validators, and replace it with a custom validator, but i would prefer to revert the behavior if possible - we have no desire to support these type of emails, and i dont want to have to check all future code additions for accidental usuage of 'email' in validators

[ppelgrims]

It would be lovely to pass custom implementations of EmailValidation, we can add the class as a parameter as suggested above, maybe something like this?

        $validations = collect($parameters)
            ->unique()
            ->map(function ($validation) {
                if ($validation === 'rfc') {
                    return new RFCValidation();
                } elseif ($validation === 'strict') {
                    return new NoRFCWarningsValidation();
                } elseif ($validation === 'dns') {
                    return new DNSCheckValidation();
                } elseif ($validation === 'spoof') {
                    return new SpoofCheckValidation();
                } elseif ($validation === 'filter') {
                    return new FilterEmailValidation();
+              } elseif ($validation instanceof EmailValidation) {
+                   return $validation;
                }
            })
            ->values()
            ->all() ?: [new RFCValidation()];

Thoughts?

[Braunson]

@ppelgrims seems like a great solution the issue at hand. It'll need some more tweaking but is eloquent. I'm curious why the framework would implement the Egulias package, specifically point that out in the docs but not allow the ability to include a custom EmailValidator (that implements Egulia's) package alongside the rest of the email flags.