Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#51030 closed defect (bug) (wontfix)

Error in wp_mail function

Reported by: hichembelhadj's profile hichembelhadj Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Mail Keywords: has-patch has-unit-tests needs-testing dev-feedback 2nd-opinion
Focuses: Cc:

Description (last modified by SergeyBiryukov)

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)

#1 @ayeshrajans
3 years ago

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.

Last edited 3 years ago by SergeyBiryukov (previous) (diff)

#2 @SergeyBiryukov
3 years ago

  • Component changed from General to Mail
  • Description modified (diff)
  • Milestone changed from Awaiting Review to 5.5.1

Related: [48645] / #50720.

#3 @desrosj
3 years ago

  • Keywords reporter-feedback added

#4 @khag7
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 @khag7
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' );
}

#6 @SergeyBiryukov
3 years ago

  • Keywords needs-patch needs-unit-tests added; reporter-feedback removed

#7 follow-up: @ayeshrajans
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 @khag7
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 @audrasjb
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 @hellofromTonya
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 @helen
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.

Note: See TracTickets for help on using tickets.