Problem/Motivation

Drupal currently depends on egulias/email-validator:^2.0 but v3 of this library was released in 2020 and v2 is EOL in January 2022.

The only breaking changes according to https://github.com/egulias/EmailValidator/blob/3.x/CHANGELOG.md are:

  • PHP version upgraded to match Symfony's (as of 12/2020).
  • DNSCheckValidation now fails for missing MX records. While the RFC argues that the existence of only A records to be valid, starting in v3 they will be considered invalid.
  • Emails domain part are now intenteded to be RFC 1035 compliant, rendering previous valid emails (e.g example@examp&) invalid.

None of these appear to be breaking changes in terms of the code, just in terms of improving functionality and rejecting edge case email addresses that were previously considered valid. As far as I know we don't use DNSCheckValidation.

Steps to reproduce

Proposed resolution

Allow egulias/email-validator:^2.1.22|^3.0.

Remaining tasks

Decide whether we can do this in a minor version.

User interface changes

API changes

Data model changes

Release notes snippet

egulias/email-validator has been updated from version 2 to version 3.1.2, because version 2 will not have security coverage in two months. This upgrade will result in stricter validation of e-mail addresses, for example example@examp& will now be rejected, when it was previously treated as valid with egulias/email-validator 2.

If an invalid e-mail address has been entered into the system, it will fail validation the next time it is saved, for example when editing a user account. When this happens, the user will only be able to continue with saving the form by changing the e-mail address to a valid format. No automatic changes to user data will be made. Where user registration requires an e-mail verification step, it is very unlikely that any existing user accounts will be affected, since they must have been able to receive an e-mail in order to register successfully in the first place. Additionally, most modern browsers provide client-side validation on e-mail form inputs. However, this could affect test content on local development environments and other edge cases.

If a site experiences problems from this update, it is still possible to downgrade back to egulias/email-validator 2 via composer (assuming that you're not using drupal/core-recommended.

Additionally, the API for the EmailParser class provided by this library has changed:

Before:

$emailParser = new EmailParser(new EmailLexer());
$result = $emailParser->parse('test@test'); // result is array with

After:

$emailParser = new EmailParser(new EmailLexer());
$emailParser->parse('email@test'); // result is an object that implements Egulias\EmailValidator\Result\Result interface
$emailParser->getDomainPart(); // this is how you get the domain part
CommentFileSizeAuthor
#7 3238763-7.patch4.48 KBlongwave
#5 3238763-5.patch4.47 KBlongwave
#2 3238763-2.patch4.47 KBlongwave

Comments

longwave created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
StatusFileSize
new 3238763-2.patch4.47 KB
krzysztof domański’s picture

krzysztof domański
Location Poland
commented
Priority: Normal » Major

EmailValidator v2.1.x EOL 01/2022
See https://github.com/egulias/EmailValidator

spokje’s picture

spokje
Primary language Dutch
commented

31/10/2021 egulias/EmailValidator version 3.1.2 was released, current patch uses 3.1.1.

I think we want to use the latest available release if we're going to update?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
StatusFileSize
new 3238763-5.patch4.47 KB 
$ composer-lock-diff --no-links
+-------------------------+--------+-------+
| Production Changes      | From   | To    |
+-------------------------+--------+-------+
| egulias/email-validator | 2.1.25 | 3.1.2 |
+-------------------------+--------+-------+
spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs review » Needs work

That somehow displeases the TestBot gods... :/

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
StatusFileSize
new 3238763-7.patch4.48 KB

Collided with #3061074: egulias/EmailValidator prior to 2.1.22 allows addresses with a space in the domain part :)

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Priority: Major » Critical
Issue summary: View changes

This is critical if the version we are using becomes unsupported in two months.

spokje’s picture

spokje
Primary language Dutch
commented
Priority: Critical » Major
Status: Needs review » Reviewed & tested by the community

Talk about Just-NOT-In-Time patching.... ;)

RTBC for me if TestBot returns green.

spokje’s picture

spokje
Primary language Dutch
commented
Priority: Major » Critical

Restoring priority

catch’s picture

catch
he/him
Primary language English
commented
Issue tags: +Needs release manager review

Based on the issue summary this looks like a case of 'major for them but not for us' and probably fine to commit, but tagging for RM review to get a second opinion.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

Emails domain part are now intenteded to be RFC 1035 compliant, rendering previous valid emails (e.g example@examp&) invalid.

I might be wrong, but this means that if people have e.g. "example@examp&" in an email field they will no longer be able to save that entity from the UI (or in genereal with validation) unchanged, right? Not at all opposed to this change, but if I'm right it would probably be good to mention this in the release notes, maybe even with a little example SQL snippet for people to check if this affects them?

krzysztof domański’s picture

krzysztof domański
Location Poland
commented

I might be wrong, but this means that if people have e.g. "example@examp&" in an email field they will no longer be able to save that entity from the UI (or in genereal with validation) unchanged, right?

That's right, it affects existing content. Tested locally.

However, this applies to invalid e-mail addresses. Invalid email should be corrected. IMO this is acceptable behavior.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

We have bumped point versions of egulias/email-validator in the past which will have also tweaked the rules for invalid addresses from time to time, e.g. in 9.1.0 we went from 2.1.17 to 2.1.22 and as such "example@email .com" was valid in 9.0.0 but invalid in 9.1.0.

Not saying that was the right thing to do without a release note, but we didn't even call out this package explicitly in the release notes that I can see.

  • catch committed d87c168 on 9.4.x 
    Issue #3238763 by longwave, Spokje, Krzysztof Domański, tstoeckler,...

  • catch committed f325f8e on 9.3.x 
    Issue #3238763 by longwave, Spokje, Krzysztof Domański, tstoeckler,...

catch credited quietone.

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs release manager review +9.3.0 release notes

Discussed with quietone and we both think this should go in.

I've added a short release note, improvements welcome.

Committed/pushed to 9.4.x and cherry-picked to 9.3.x, thanks!

xjm’s picture

xjm
she/her
Primary language English
commented
Issue summary: View changes
Status: Fixed » Needs work
Issue tags: +Needs release note

Thanks all for catching this in time!

I think we need to do a little more work on the release note for this because of the disruption. For example:

  1. What were the previous results of using one of these invalid email addresses in various usecases?
  2. If a Drupal account was created with an email address that's no longer allowed following the fix, what happens to it?
  3. If a previously invalid email was data in an email field, or an email for a webform, what impact will this have on that data? Will it fail validation on the field when the content is updated? Etc.

We should give some recommendations on what sites should do WRT to checking and fixing any data integrity issues that arise from this, as well as mention the workaround of using the older version (and not core/recommended) until such issues can be addressed.

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs release note

Took a pass at a more comprehensive release note.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Also worth noting that invalid email addresses are already difficult to enter via UI because most web browsers perform stricter validation on <input type="email"> - for example I can't enter example@examp& in Chrome or Firefox as that is flagged client-side before Drupal even gets to see it, so these edge case email addresses would already have been difficult to use.

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes

Thanks, tried to incorporate that.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

matroskeen’s picture

matroskeen
Primary language Ukrainian
Location 🇺🇦 Ukraine, Lutsk
commented
Status: Needs review » Needs work

I think it's worth mentioning that there were more breaking changes in the library.

This library also provides an email parser, that is used at least by one contrib module and probably custom modules as well.
Here is the issue: #3258005: Handle a new version of egulias/email-validator and EmailParser return results.

Before:

$emailParser = new EmailParser(new EmailLexer());
$result = $emailParser->parse('test@test'); // result is array with

After:

$emailParser = new EmailParser(new EmailLexer());
$emailParser->parse('email@test'); // result is an object that implements Egulias\EmailValidator\Result\Result interface
$emailParser->getDomainPart(); // this is how you get the domain part

Here is a commit, where this change was introduced: https://github.com/egulias/EmailValidator/commit/5455d688c1354af2a481ce3...

P.S. http://grep.xnddx.ru/search?text=EmailParser&filename= says there is one more module affected, which is still in dev version, so no big deal 👌

Moving to Needs Work to update the release snippet.

catch’s picture

catch
he/him
Primary language English
commented
Version: 9.4.x-dev » 9.3.x-dev
Issue summary: View changes
Status: Needs work » Fixed

Added an extra snippet to the release notes here. I'll also update the 9.3.0 release notes with what's here. Going to go ahead and mark this fixed again.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Filed #3298343: upgrade egulias/email-validator to 3.2.1 to support PHP 8.2 to drop support for v2 as it's EOL now