Skip to content

Conversation

@MdAnowarHosen
Copy link

@MdAnowarHosen MdAnowarHosen commented Feb 12, 2025

Check the email is valid or not using Str::isEmail()
I've used here egulias/EmailValidator to check email validation

Now, we can check the email valid or not using Str::isEmail()

if email is valid then it's return true
else it's return false

Example:

$email = 'example@laravel.com';
Str::isEmail($email);

It's return true.

Another example:

$email = 'example.laravel';
Str::isEmail($email);

It's return false.

check the email is valid or not using Str::isEmail()
@shaedrich
Copy link
Contributor

Would be nice if this used egulias/EmailValidator like the validator 🤔

@MdAnowarHosen
Copy link
Author

MdAnowarHosen commented Feb 12, 2025

Would be nice if this used egulias/EmailValidator like the validator 🤔

@shaedrich egulias/EmailValidator showing me user@sub.-domain.com that email is valid.

but current code showing me that email is invalid.

What do you think?

@rodrigopedra
Copy link
Contributor

rodrigopedra commented Feb 12, 2025

I guess we should use whichever the default is for validating emails using the validator:

https://laravel.com/docs/11.x/validation#rule-email

That would be using egulias/email-validator with its RFCValidation.

It would be odd if a request passes validation for an email, and then Str::isEmail() fails.

Optionally, as the email validation rule allows, we could allow the developer to specify which validation style they want to use.

As for egulias/email-validator allowing user@sub.-domain.com, if you are sure that is invalid according to the many RFCs it is built against, you could open a bug report on their repo.

I truly don't know all the rules, per RCF this would be a valid email:

  • "John Doe"@example.com
    • user part can contain spaces as long it is quoted with double quotes

That passes with egulias/email-validator, but fails with filter_var.

Also, your implementation checks for the characters after the last dot on the domain part to have at least two characters, while the RFC allows emails using an IP:

  • jonh.doe@192.168.0.1

That would fail, as there is a single character after the last dot. (EDIT: actually, it already fails filter_var())

Of course, those are very rare cases, and I wouldn't allow them in any system I maintain (unless it is a hard-request from an enterprise customer).

Nonetheless, again, a developer relying on the default validation could be surprised if Str::isEmail() fails after validating a request.

TL;DR; my opinion is to be consistent with our current email validation rule.

@MdAnowarHosen
Copy link
Author

egulias/EmailValidator implemented to check email valid or not. Thanks @shaedrich , @rodrigopedra

Copy link
Contributor

@shaedrich shaedrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should use whichever the default is for validating emails using the validator:

https://laravel.com/docs/11.x/validation#rule-email

That would be using egulias/email-validator with its RFCValidation.

It would be odd if a request passes validation for an email, and then Str::isEmail() fails

egulias/EmailValidator implemented to check email valid or not. Thanks @shaedrich , @rodrigopedra

Yeah, this way, it's consistent 👍🏻

Comment on lines +2022 to +2027
public static function isEmail(string $value): bool
{
$validator = new EmailValidator();

return $validator->isValid($value, new RFCValidation());
}
Copy link
Contributor

@shaedrich shaedrich Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optionally, as the email validation rule allows, we could allow the developer to specify which validation style they want to use.

Suggested change
public static function isEmail(string $value): bool
{
$validator = new EmailValidator();
return $validator->isValid($value, new RFCValidation());
}
public static function isEmail(string $value, ?string $rule = null): bool
{
$validator = new EmailValidator();
return $validator->isValid($value, match ($rule) {
'strict' => new NoRFCWarningsValidation(),
'dns' => new DNSCheckValidation(),
'spoof' => new SpoofCheckValidation(),
'filter' => new FilterEmailValidation(),
default => new RFCValidation(),
});
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of a string, $rule could probably be made an enum 🤔

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shaedrich , i think it must be enum 100%

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The enum could even do the mapping itself:

enum EmailValidation
{
    case NoRfcWarnings;
    case DnsCheck;
    case SpoofCheck;
    case Filter;
    case Rfc;

    public function validation()
    {
        return match ($this) {
            self::NoRfc => new NoRFCWarningsValidation(),
            self::Dns => new DNSCheckValidation(),
            self::SpoofCheck => new SpoofCheckValidation(),
            self::Filter => new FilterEmailValidation(),
            default => new RFCValidation(),
        }
    }
}
Suggested change
public static function isEmail(string $value): bool
{
$validator = new EmailValidator();
return $validator->isValid($value, new RFCValidation());
}
public static function isEmail(string $value, ?EmailValidation $rule = null): bool
{
$validator = new EmailValidator();
return $validator->isValid($value, $rule?->validation() ?? new RFCValidation());
}

@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions!

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

Successfully merging this pull request may close these issues.

5 participants