valid_email_address() should use egulias/EmailValidator and become deprecated

+++ b/core/includes/common.inc
@@ -345,17 +346,15 @@ function drupal_get_destination() {
+  $validator = new EmailValidator(TRUE);

No constructor argument is needed, I think.

Noted that it isn't pass Chrome's client-side validation. (didn't test other browsers yet)

https://github.com/egulias/EmailValidator/blob/master/tests/egulias/Test...

+++ b/core/includes/common.inc
@@ -345,17 +346,15 @@ function drupal_get_destination() {
+  $validator = new EmailValidator();
+  return $validator->isValid($mail);

return new EmailValidator()->isValid($mail);

I am thinking this should be a service. That would allow someone to specify a different validation class. What do you think?

@cilefen: If my understanding of this page is correct, I guess we can just remove the constraint we have added #2278353-89: Update to Symfony 2.5. I think we will just have to make sure we pass in strict as true and it will use our new email validator class. Is this correct?

Why do we need an instance instead of a static method?

If it's a service I guess the motivation is so that people can swap the implementation?

So, can we deprecate the function?

Are the other methods in the class generally used? If not maybe the static wrapper could be like

 \Drupal::emailIsValid($mail)

As a direct replacement for the function

Are we sure we really want to add a method onto the drupal static class? This is something we should avoid here, because validating a mail is not a often used operation if you ask me.

I think it totally makes sense to use the same email validation mechanism everywhere.

Patch looks good, and gave it a quick test drive on a system form where valid_email_address() is used.

Yep having two different results from validating email address is confusing. Committed 2cdd0e2 and pushed to 8.0.x. Thanks!

Thank you for adding the beta evaluation to the issue summary.

Crossposted:

Are there any e-mail addresses that would have been allowed by filter_var() that won't be allowed by this library?

This could have implications for the migration path if so. Don't see it discussed in any of the previous issues.

+            "require": {
+                "php": ">=5.3.2"
+            },

I think we'd have to have a fallback for sites on PHP 5.2.

So is there really no hope of this getting fixed/improved in PHP itself?

I agree with #47 that PHP 5.2.5 has to be supported because of the Drupal System requirements.
Somehow we should find a good or at least better solution ASAP because of issues like #1427516: valid_email_address() rejects valid email addresses and can't wait for things to happen in issues sleeping like https://bugs.php.net/bug.php?id=69140.

I think the currently only valuable solution would be to use a custom regex for php versions below 5.3.2 and the solution from the patch in #45 for higher versions. What's your oppinon?

This blog post shows the problems: http://markonphp.com/properly-validate-email-address-php/

There's no change record for this?

We need a fallback for PHP 5.2, but that doesn't necessarily mean we need to hold up this issue on a fix for PHP 5.2.

In other words, one possible fallback would be to just keep calling filter_var() for older PHP versions, and only load up this library and use it if the PHP version is new enough.

Great! +1 for #51

I just ran into this issue for a test suite for valid email addresses from a api vendor.

All of that to say +1 for #51. Fall back to existing functionality if the php version is too old.

Updated remaining tasks in issue summary and marking as needs work.

After testing this with wikipedia's like of strange valid email addresses, only two didn't work.

"very.(),:;<>[]\".VERY.\"very@\\ \"very\".unusual"@strange.example.com
"()<>[]:,;@\\\"!#$%&'*+-/=?^_`{}| ~.a"@example.org

Would this be considered and up stream issue?

Good, so all that is left to do is put in the 5.2 fallback.

#50: We are working on filling in the missing Change Record for this as part of #2873881: [meta] Add Change Record to @deprecated for common.inc at DrupalCon Vienna. The draft CR can be found at [#2912661]. We are all newbies to this so please be gentle :-)

Published the CR for the 8.x branch. https://www.drupal.org/node/2912661

If this needs backport to D7, it should be Patch (to be ported) until the D7 maintainers create a separate D7 issue for it, and then it can be marked back to Fixed against the 8.x branch it was committed to.

For #49 / #51 about the 5.2 fallback I think we also could create a separate followup and link it here, but it's a D7-only issue since D8 requires 5.5.9.

Haven't we been working on the D7 backport since #40?

Yes, the backport has been in progress here for a couple years (the issue is currently filed against D7), but a separate issue could be created also and then this one closed.

I do recall that this patch was relatively close to being ready though.

diff --git a/composer.json b/composer.json
new file mode 100644
index 0000000..f4240d7
--- /dev/null
+++ b/composer.json
@@ -0,0 +1,5 @@
+{
+    "require": {
+        "egulias/email-validator": "^1.2"
+    }
+}

That said, I am really not sure about including a composer.json in Drupal 7 core - won't that cause problems for people who are already using Composer to manage their sites?

This is mostly just a one-off library, so would it be possible to include the library manually (like we normally would) without having to mess with Composer?

+1 #68

In attendance of the official D7 backport release, I just wanted to share my efforts in an alternative solution using xautoload using the Libraries API to autoload the necessary classes from Egulias' email validator, currently at version 2.1.7.

Still struggling with the expected availability of the necessary classes though, and reported here in xautoload doc support
https://www.drupal.org/node/1976234#comment-13021989

Tried to follow the class instantiation approach as applied in patch #45, but without success either :

function valid_email_address($mail) {
  if (module_exists('libraries') && libraries_load('EmailValidator')) {
    $validator = new Egulias\EmailValidator\EmailValidator;
    return $validator->isValid($mail, new Egulias\EmailValidator\Validation\RFCValidation);
 }
  return (bool)filter_var($mail, FILTER_VALIDATE_EMAIL);
}

Attached is a patch that adds the needed libraries from doctrine and egulias to the mics folder.
It loads the classes when valid_email_address() is invoked.
It uses EmailValidator() when php >= 5.5 and falls back filter_var() otherwise.

Please review.

Changed the invalid mail test of openid as explained in #45

Hi sanduhrs,

Thanks for providing this working patch.
So from the valid_email_address() point of view my test passed.

There's one side effect I noticed during 1 of my tests where after successful registration of numéros@mailinator.com user, a subsequent registration of numeros@mailinator.com was refused by core user module.

Inside user_account_form_validate() function, the entered email address is also validated against existing users.
For the above case numeros@mailinator.com is seen as an existing user because of the unCOLLATEd query on utf8_general_ci collated mail column.

After applying my attached patch on user module, the validation passed without an error message but for some unclear reason the registration does not complete successful (user_register_submit handler not triggered).

Small side remark with your patch #74:

I tried to replace your class includes with a recursive iterator, but the order in which this happens seems to be relevant for this EmailValidator plugin.

Here's my suggestion for improvement, in case the plugin should receive new php files in the future :

  include_once $directory . '/Exception/InvalidEmail.php';
  include_once $directory . '/Parser/Parser.php';
  include_once $directory . '/Validation/EmailValidation.php';
  include_once $directory . '/Validation/RFCValidation.php';
  include_once $directory . '/Warning/Warning.php';
  $dir_iterator = new RecursiveDirectoryIterator($directory);
  $iterator = new RecursiveIteratorIterator($dir_iterator);
  foreach ($iterator as $file) {
    if ($file->isFile() && substr($file->getPathname(), -3) == 'php') {
      include_once($file->getPathname());
    }
  }

I'll take some time next week to further investigate my new issue.
Regards,

Fixed patch.

The issue in #76 is out of scope of this patch IMHO.
The same behavior can be observed in D8/D9.

So the patch in #74 stands, but should be updated with the latest version of egulias/EmailValidator:3.1.2

Some test are failing on PHP 7.4 and higher with patch #74. The new library needs to be checked/updated.

Trying to access array offset on value of type null

See: https://www.drupal.org/pift-ci-job/2363706 and others.

And I also agree that the change in patch #76 is not relevant to this issue (not mention that this change is not working correctly in DBs other than MySQL/MariaDB).

Maybe the new proposed hook in #2966195: valid_email_address() should be easily overridable will better suit the current D7 status and we would not need to include 3rd party library into D7 core?

I think it would be better to close this as Fixed in D8 and for D7 use the flexible approach proposed by the mentioned issue, where sites can potentially add whatever library they want and D7 core does not need to maintain it. What do you think?