Opened 4 years ago
Last modified 3 months ago
#55821 new defect (bug)
`is_email()` does not follow PHP FILTER_VALIDATE_EMAIL rules, when an email has double period (..)
| Reported by: | khokansardar | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Awaiting Review |
| Component: | Formatting | Version: | 5.9.3 |
| Severity: | normal | Keywords: | has-test-info has-patch has-unit-tests |
| Cc: | Focuses: |
Description (last modified by )
If there has a typo in an email address like there has double period .. in an email address ( abc..def@… ). is_email() function return valid email when we have use like -
is_email( 'abc..def@xyz.com' )
Where it should not return as valid email address. Which is working with PHP FILTER_VALIDATE_EMAIL checking. When we are checking the same kind of email with below code it return boolean false.
filter_var( 'abc..def@xyz.com', FILTER_VALIDATE_EMAIL )
Expected behaviour should be like this. When use is_email() function it should respect like what PHP does.
Related
- #31992 where the change in email address parsing was introduced
Change History (13)
This ticket was mentioned in PR #4631 on WordPress/wordpress-develop by @lopo.
3 years ago
#6
- Keywords has-patch has-unit-tests added; needs-patch removed
Added a check for double periods on the local part of the email address (taking inspiration from the same check on the domain part) + added unit test cases for both the local and the domain part (which was missing).
Trac ticket: https://core.trac.wordpress.org/ticket/55821
This ticket was mentioned in PR #12188 on WordPress/wordpress-develop by @nimeshatxecurify.
3 months ago
#8
## Description
is_email( 'abc..def@xyz.com' ) returns the address as valid, even though the local part contains consecutive dots. PHP's filter_var( …, FILTER_VALIDATE_EMAIL ) rejects it, and so does every SMTP server, because the address is undeliverable. The same applies to a leading dot (.abc@xyz.com) and a trailing dot (abc.@xyz.com).
Since [62225]/#31992, is_email() delegates to WP_Email_Address::from_string(), whose local-part patterns are derived from the WHATWG <input type=email> character set. That set places . in the character class with no positional constraint ([…]+), so any arrangement of dots passes. The domain side already validates correctly (each label must be bookended by an alphanumeric), so this only affected the local part.
This PR restructures the local part as a proper dot-atom, mirroring the existing domain-label design already in the class: a dot may only separate non-empty atoms. Leading, trailing, and consecutive dots are now rejected, while all previously valid addresses — including the intended Unicode addresses from #31992 (grå@grå.org) — continue to validate.
## Why reject these
This change is not a departure from #31992 — it implements a decision that ticket's discussion already reached. While reviewing test cases on #31992, @agulbra noted:
to..to@couc.ouandto.@couc.ouare invalid according to RFC 5321, therefore WordPress cannot send mail to them. Rejecting these is a good idea.
and @peteresnick — the author of RFC 5322 — confirmed:
Also invalid according to 5322 section 3 (the more restrictive) syntax.
The WHATWG character set was adopted as the *basis* for the allowed characters, but the agreed intent was to be at least as strict as the syntaxes used to *generate* and *deliver* mail, which forbid empty atoms in the local part. The shipped regex used the WHATWG […]+ shorthand verbatim and did not enforce that structure; this PR closes that gap.
## Implementation
- Add
LOCAL_PART_ATOM_ASCIIandLOCAL_PART_ATOM_UNICODEconstants (the previous local-part character sets, minus the dot). - Assemble
LOCAL_PART_ASCII_REGEX/LOCAL_PART_UNICODE_REGEXasatom (?:\. atom)*, the same dot-separated structure already used byDOMAIN_ASCII_REGEX/DOMAIN_UNICODE_REGEX.
## Testing instructions
- Apply the patch.
- Confirm the following now return
false:is_email( 'abc..def@xyz.com' ); // consecutive dots is_email( '.abc@xyz.com' ); // leading dot is_email( 'abc.@xyz.com' ); // trailing dot
- Confirm normal addresses still validate:
is_email( 'bob@example.com' ); is_email( 'bill+ted@example.com' ); is_email( 'a.b.c@example.com' ); is_email( 'grå@grå.org' );
- Or run the test suite:
npm run test:php -- --group 55821 npm run test:php -- --filter 'IsEmail|EmailAddress|Antispambot|SanitizeEmail'
### Unit tests
- tests/formatting/isEmail.php: Moved
..@example.comfrom the valid provider to the invalid provider, added the leading/trailing/consecutive-dot cases, and tagged the test with@ticket 55821. - tests/wp-email-address/wpEmailAddress.php: New
@ticket 55821test asserting rejection in both unicode and ascii modes. - tests/formatting/antispambot.php: Relabeled a now-inaccurate data-set key (
antispambot()is obfuscation-only; behavior unchanged).
Trac ticket: https://core.trac.wordpress.org/ticket/55821
## Use of AI Tools
AI assistance: Yes — used to investigate the regression, cross-reference the #31992 discussion, draft the fix, and write the tests. All changes were reviewed and verified by me.
#10
@
3 months ago
I tested the newer patch from GitHub PR 12188: https://github.com/WordPress/wordpress-develop/pull/12188 locally.
I also checked current trunk before testing. The issue still exists there. For example, the current is_email() tests still include ..@example.com as a valid email case, and the current WP_Email_Address local-part regex still allows dots without checking dot-atom placement.
Environment
- wordpress-develop PR branch:
55821-double-dot-fix - Docker:
29.5.3 - PHPUnit:
9.6.34 - Single site test run
Test results
npm run test:php -- --group 55821
Passed: 25 tests, 30 assertions.
npm run test:php -- --filter 'IsEmail|EmailAddress|Antispambot|SanitizeEmail'
Passed: 149 tests, 242 assertions.
composer lint:errors on the 4 touched files
Passed: 4 / 4 files.
What I checked
- The PR rejects emails with invalid dots in the local part.
- This includes leading dot, trailing dot, consecutive dots, only dots, and single dot local part.
- The related email formatting tests are also passing, including
is_email(),WP_Email_Address,antispambot(), andsanitize_email().
Based on this local automated testing, GitHub PR 12188 looks good to me.
This ticket was mentioned in Slack in #core by amykamala. View the logs.
3 months ago
#12
@
3 months ago
thanks @nimeshatxecurify — I expected a lot of cases to arise with the change.
one note though: Google happily delivers email to localparts containing a leading or trailing dot. this lies, I think, at the heart of the complexity of email addressing. it still rejects successive dots, but allows the email addresses this change proposes to reject.
the WHATWG spec intentionally allows these addresses, meaning they should be allowed in a browser. are there any email servers or services which deliver to localparts with successive dots?
I want us to be careful about diverging from correspondence with the browsers since that is the point of integration between the WordPress backend and the interface that users see. at times, we can apply additional rules, which is fine, but there is great benefit in applying those rules after understanding the spec value first.
for example, we have an is_email() filter that could be used to further restrict the email addresses that WordPress allows, but right now a browser will accept all of these and allow a site visitor or admin to enter them.
this is not me saying the request in this ticket is wrong (apart from that I do recommend we avoid using filter_var() as a standard, and at that, avoid using filter_var() anywhere). I merely want to make sure we’re thorough in the choices we make, as diversions from browser behavior introduces new opportunities to leave site visitors hanging and to exploit divergence between how a browser and how WordPress understands a given input.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Testing Instructions
These steps define how to reproduce the issue, and indicate the expected behavior.
Steps to Reproduce
</html>https://snippi.com/s/8gji8lpemail address is validis displayedExpected Results
When testing a patch to validate it works as expected:
email address is not valid.Additionaly
We could change the
abc..def@xyz.comto other not valid emails and see the results.