Problem/Motivation

Split from #3284422: [META] Symfony 6.2 compatibility.

Drupal sets 'strict' mode on the Email constraint via a subclass, but it doesn't do this on the validator. Does this mean we've been validating in loose mode the whole time?

#3284422-30: [META] Symfony 6.2 compatibility contains a working patch, we just need to remove the unrelated hunks and post it here. Then decide between 'strict' and 'html5'.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#11 3307736-11.patch1.61 KBcatch
#11 3307736-interdiff.txt774 bytescatch
#10 3307736-10.patch1.62 KBcatch
#10 3307736-interdiff.txt1003 bytescatch
#2 3307736.patch1.47 KBcatch
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Title: EmailValidator defaults to 'loose' mode which is deprecated » EmailValidator defaults to 'loose' mode which is deprecated in Symfony 6.2
Status: Active » Needs review
FileSize
3307736.patch1.47 KB
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs review » Reviewed & tested by the community

No idea how tests could be improved but it's ready

Could use explanation about why we are switching to html5 instead of 4.1(strict) but core already did this switch years ago

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

I think we need a follow-up to figure out why the constraint and validator can go out of sync and maybe also to reconcile the email validator service which uses egulias directly with the Symfony validator.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

Filed follow-up #3307810: Figure out how email contraint bound to egulias email validator

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at QuantCDN, Salsa Digital commented
Issue tags: +Symfony 6.2

Tagging

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Issue tags: +Drupal 10 beta blocker
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Status: Reviewed & tested by the community » Needs work

This class override can be removed when we update to SF7.

We'll never use the default mode in Drupal because our constraint is hardcoded to use the strict mode... see \Drupal\Core\Validation\Plugin\Validation\Constraint\EmailConstraint

So I think here we should default to strict too. We should make the class final and add an @internal that documents that this class will be removed when we depend on SF7.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented

Also setting to strict forces the \Symfony\Component\Validator\Constraints\EmailValidator to use the EguliasEmailValidator validator in the same way that Drupal does.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Needs work » Needs review
FileSize
3307736-interdiff.txt1003 bytes
3307736-10.patch1.62 KB

I am fine with final/internal and defaulting to strict. But... if we default to strict, we can't remove this in Drupal 11 because Symfony's default is HTML5. Just made the language around removal a bit vague to get around this.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
FileSize
3307736-interdiff.txt774 bytes
3307736-11.patch1.61 KB

Discussed a bit more with @alexpott. Given we ignore the default anyway in practice, if we go with HTML5, we can remove the sublass in 11.x, so make our lives easier.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC as #8 addresed

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed c3a7a32c5d to 10.1.x and 4a67a410b0 to 10.0.x. Thanks!

diff --git a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/EmailValidator.php b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/EmailValidator.php
index 7f2a6facbb..a39d4c6a7a 100644
--- a/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/EmailValidator.php
+++ b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/EmailValidator.php
@@ -11,7 +11,7 @@
  * Overrides the symfony validator to use the HTML 5 setting.
  *
  * @internal Exists only to override the constructor to avoid a deprecation
- * in Symfony 6, and will be removed in drupal:11.0.0.
+ *   in Symfony 6, and will be removed in drupal:11.0.0.
  */
 final class EmailValidator extends SymfonyEmailValidator {

Fixed the @internal indentation on commit.

  • alexpott committed c3a7a32 on 10.1.x 
    Issue #3307736 by catch, alexpott: EmailValidator defaults to 'loose'...

  • alexpott committed 4a67a41 on 10.0.x 
    Issue #3307736 by catch, alexpott: EmailValidator defaults to 'loose'...

Status: Fixed » Closed (fixed)

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