WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#10420 closed enhancement (fixed)

wp_mail fails to send to multiple recipients

Reported by: patmcnally Owned by: nacin
Milestone: 3.0 Priority: normal
Severity: normal Version: 2.9
Component: Mail Keywords: has-patch tested
Focuses: Cc:

Description

The function wp_mail as defined in wp-includes/pluggable.php does not correctly handle multiple recipients when they are passed in as a comma separated string to the first parameter($to). This is evident when using any SMTP server that strictly adheres to section 4.1.1.3 of rfc2821 (such as gmail).

For example when the first parameter to wp_mail is "foo@…, bar@…" this string is passed directly to the SMTP server in the first and only RCPT command. Expected behavior is two separate RCPT commands, one for each email address.

phpmailer supports the required functionality, however $phpmailer->AddAddress needs to be called multiple times for each email address contained in the first parameter ($to). Instead AddAddress is called only once, borking any attempts to send to multiple recipients when using gmail's SMTP server.

I would submit a patch, however this is my first time investigating both php and Wordpress and fear that I could only do more harm than good.

Attachments (4)

10420-pluggable.php.diff (562 bytes) - added by patmcnally 6 years ago.
Splits first parameter to wp_mail on commas and calls AddAddress for each
10420_2.patch (483 bytes) - added by nacin 6 years ago.
Allow multiple recipients in wp_mail (array only)
10420_3.patch (982 bytes) - added by nacin 6 years ago.
Multiple recipients via array or comma-separated string
10420_4.patch (7.0 KB) - added by nacin 6 years ago.
Better handling of To: recipients; some cleanup

Download all attachments as: .zip

Change History (17)

comment:1 @patmcnally6 years ago

  • Version changed from 2.8.1 to 2.9

@patmcnally6 years ago

Splits first parameter to wp_mail on commas and calls AddAddress for each

comment:2 @patmcnally6 years ago

  • Keywords has-patch needs-testing added

comment:3 @Denis-de-Bernardy6 years ago

  • Keywords changed from wp_mail, smtp, mail, has-patch, needs-testing to has-patch needs-testing wp_mail, smtp, mail
  • Milestone changed from Unassigned to 2.9
  • Type changed from defect (bug) to enhancement

+1 to the idea, personally

comment:4 @pkthree6 years ago

patmcnally's idea gets my +1

You could also support both comma separated lists and arrays in the $to variable with this:

// Set destination address(es)
// Accept either a comma-separated list or an array
$to = explode( ',', implode( ',', (array) $to ) );
foreach ( $to as $to_recipient ) {
    $phpmailer->AddAddress( trim( $to_recipient ) );
}

comment:5 follow-up: @nacin6 years ago

  • Keywords needs-patch added; has-patch needs-testing removed

+1 with caveats.

Exploding by commas would cause issues with "Doe, John" <johndoe@example.com> becoming "Doe and John" <johndoe@example.com>.

Patch attached to handle an array only. I would think it should also explode by commas outside of quotation marks, but this is a start.

I'm looking at how the function handles CC and BCC headers and I see they currently explode by commas without regard for quotation marks. That may be enough to classify this as a bug. The function also doesn't appear to support CC: johndoe@example.com\nCC:janedoe@example.com as headers, which does work for PHP's mail.

I'll try to generate some test cases and build a patch to fix this.

@nacin6 years ago

Allow multiple recipients in wp_mail (array only)

comment:6 in reply to: ↑ 5 @nacin6 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

Replying to nacin:

Exploding by commas would cause issues with "Doe, John" <johndoe@example.com> becoming "Doe and John" <johndoe@example.com>.

So, wp_mail isn't configured to handle display names of To, CC, or BCC recipients, so you can explode by commas. I've been working on a patch to allow display names. There are a few other quirks I'm noticing as well I'm going to try to patch up. In the meantime:

Attached is a patch along the lines of pkthree's suggestion to allow a comma-separated list or an array for the $to parameter.

@nacin6 years ago

Multiple recipients via array or comma-separated string

comment:7 @nacin6 years ago

New, probably last, patch:

  • Handles multiple To: recipients via array or comma-separated string. Also checks if $to is empty first, as it could be a message with only CC and BCC recipients.
  • Per RFC 2822: Headers should be separated with \r\n, not \n. wp_mail now supports both so headers that work on mail work here. CC: john@example.com\r\nCC:doe@example.com now works as well.
  • Some whitespace and conventions cleanup.

Display name handling is a bit of an edge case, so I haven't bothered with that here, but I'd like to see multiple recipient handling make it into core.

@nacin6 years ago

Better handling of To: recipients; some cleanup

comment:8 @nacin6 years ago

  • Keywords changed from has-patch, needs-testing, wp_mail, smtp, mail to has-patch needs-testing wp_mail smtp mail

comment:9 @janeforshort6 years ago

  • Milestone changed from 2.9 to Future Release

Punting because we were already past feature freeze when recent patch added. Consider for 3.0.

comment:10 @nacin6 years ago

  • Keywords needs-testing wp_mail smtp mail removed
  • Milestone changed from Future Release to 3.0

Patch #4 adds multiple recipient handling to wp_mail(). It also introduces a few improvements in how it handles headers, which brings wp_mail() in line with RFC 2822 and PHP's mail().

Changes outlined above.

comment:11 @chrisscott5 years ago

  • Keywords tested added

Tested on r13319 and works fine for the test case of a comma separated string of multiple addresses or an array of addresses.

Testing with 'foo@example.com,bar@example.com', 'foo@example.com, bar@example.com' (space after comma) and array('foo@example.com', 'bar@example.com') all give the following To: header:

To: foo@example.com, bar@example.com

comment:12 @nacin5 years ago

  • Owner set to nacin
  • Status changed from new to reviewing

I guess I should clean up this patch and get it in.

It could probably use some testing on different kinds of CC and BCC headers. See this comment.

comment:13 @nacin5 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

(In [13456]) Allow multiple To: recipients in wp_mail(). Improve handling of \r\n in headers and multiple CC/BCC headers. fixes #10420

Note: See TracTickets for help on using tickets.