Opened 3 years ago
Closed 3 years ago
#51030 closed defect (bug) (wontfix)
Error in wp_mail function
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Keywords: | has-patch has-unit-tests needs-testing dev-feedback 2nd-opinion | |
Focuses: | Cc: |
Description (last modified by )
Hello,
Currently I am developing a wordpress theme with a script to send a verification mail using wp_mail(). Since switching to wordpress version 5.5 my script no longer works. By trying PHP's mail() function it works.
By reinstalling the version of wordpress 5.4 with the wp_mail() function, it works again.
While performing the debugging, I get the following error:
[wp_mail_failed] => Array ( [0] => Invalid address: (From): wordpress@localhost )
Thanks for your feedback
Change History (14)
#2
@
3 years ago
- Component changed from General to Mail
- Description modified (diff)
- Milestone changed from Awaiting Review to 5.5.1
#4
@
3 years ago
Prior to 5.5 the wp_mail
function used PHPMailer's validator function to validate emails. By default, PHPMailer::$validator
variable was set to auto
, and PHPMailer would then attempt to use a somewhat complex regex to validate emails.
The PHPMailer auto
option was using pcre8 (if avaliable) and wordpress@localhost
would be valid in that case.
With the update to the newest version of PHPMailer (#41750) the auto
option is no longer available, and instead uses the php
validation option. Shortly after #41750 was closed, this proved to be an issue (#50720) so WP core moved to using our own is_email
function for validation.
Unfortunately, WordPress's is_email
function does not consider wordpress@localhost
to be a valid email, thus this ticket exists.
I think we may have been quick to jump to using our own is_email
function when another better option exists. The PHPMailer class includes a validation option called html5
which I think would work.
Some quick tests show the following statements to be true:
false=== is_email( 'wordpress@localhost' )
true === PHPMailer\PHPMailer\PHPMailer::validateAddress('wordpress@localhost','html5')
false=== PHPMailer\PHPMailer\PHPMailer::validateAddress('wordpress@localhost','php')
true === PHPMailer\PHPMailer\PHPMailer::validateAddress('wordpress@localhost','pcre8')
As of 5.5, in wp_mail
we assign $phpmailer::$validator
to a function which wraps is_email
.
Perhaps instead we should just do $phpmailer::$validator = 'html5'
?
Alternatively we improve is_email
to support emails with a domain that does not include a period.
#5
@
3 years ago
A test to add:
function test_phpmailer_validator() { $phpmailer = $GLOBALS['phpmailer']; $this->assertTrue( $phpmailer->validateAddress( 'foo@localhost' ), 'Assert PHPMailer accepts domains without a dot' ); }
#7
follow-up:
↓ 8
@
3 years ago
is_email
was used because the follow up ticket to PHPMailer 6 was that certain emails that were allowed by iz_email
were not accepted by PHPMailer, so using is_email() to validate the emails was sensible.
I doubt MTAs would accept root@localhost
as valid address in production use cases, so I still believe using is_email
provides a safe bar to validate email addresses.
In either case, is_email is mostly used for front end uses, so it should be strict to only accept real life email addresses as opposed to pure RFC addresses.
#8
in reply to:
↑ 7
@
3 years ago
Replying to ayeshrajans:
is_email is mostly used for front end uses, so it should be strict to only accept real life email addresses as opposed to pure RFC addresses.
I agree that is_email is the best choice for validating input from form fields where we'd expect a "normal" email address. (i.e. "front end uses")
But "behind the scenes" it should be acceptable for PHPMailer to allow email@localhost
. After all, this issue was created specifically because the updates in 5.5 broke functionality for some users.
If changing the validation for wp_mail to use PHPMailer's 'html5' validation instead of 'is_email' fixes the issue without creating any new issues, it feels like the right solution.
This ticket was mentioned in PR #490 on WordPress/wordpress-develop by khag7.
3 years ago
#9
- Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed
Trac ticket: https://core.trac.wordpress.org/ticket/51030
Includes some tests as well
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
3 years ago
#11
@
3 years ago
- Milestone changed from 5.5.1 to 5.6
As per today’s bug scrub: Moving to milestone 5.6 to give it more time.
#12
@
3 years ago
- Keywords needs-testing dev-feedback 2nd-opinion added
After reviewing discussion in the scrub, this ticket needs more discussion, testing, and investigation.
Here are some of the discussions:
Per joostdevalk:
let me get this straight, all this is about emailing a domain that has no dots in it?
i see why it’s annoying for a developer but it’s not like this has a live use case, right? or am i nuts?
Per Sergey:
Seems like it
Per johnbillion:
It could affect an intranet
Marking this ticket for dev-feedback
and 2nd-opinion
to get more eyes on it.
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
3 years ago
#14
@
3 years ago
- Milestone 5.6 deleted
- Resolution set to wontfix
- Status changed from new to closed
I recognize that this used to work and no longer does, but upon further reflection I think it's correct that it no longer does because that style of email address is not valid for SMTP. So I would say that by default, the current WordPress behavior is correct, and that should you be doing something more specialized like in your case, there is a "hoop" you have to jump through in the form of needing to use the is_email
filter with the domain_no_periods
context to get it to pass instead.
Hi @hichembelhadj - Welcome to WordPress Trac.
Are you instantiating PHPMailer on your own?
wp_mail
function should use the PHPMailer 6 library now, and a bug with the address validation was fixed recently: #50720.