Make WordPress Core

Opened 15 years ago

Closed 15 years ago

#10420 closed enhancement (fixed)

wp_mail fails to send to multiple recipients

Reported by: patmcnally's profile patmcnally Owned by: nacin's profile 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 15 years ago.
Splits first parameter to wp_mail on commas and calls AddAddress for each
10420_2.patch (483 bytes) - added by nacin 15 years ago.
Allow multiple recipients in wp_mail (array only)
10420_3.patch (982 bytes) - added by nacin 15 years ago.
Multiple recipients via array or comma-separated string
10420_4.patch (7.0 KB) - added by nacin 15 years ago.
Better handling of To: recipients; some cleanup

Download all attachments as: .zip

Change History (17)

#1 @patmcnally
15 years ago

  • Version changed from 2.8.1 to 2.9

@patmcnally
15 years ago

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

#2 @patmcnally
15 years ago

  • Keywords has-patch needs-testing added

#3 @Denis-de-Bernardy
15 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

#4 @pkthree
15 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 ) );
}

#5 follow-up: @nacin
15 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.

@nacin
15 years ago

Allow multiple recipients in wp_mail (array only)

#6 in reply to: ↑ 5 @nacin
15 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.

@nacin
15 years ago

Multiple recipients via array or comma-separated string

#7 @nacin
15 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.

@nacin
15 years ago

Better handling of To: recipients; some cleanup

#8 @nacin
15 years ago

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

#9 @janeforshort
15 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.

#10 @nacin
15 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.

#11 @chrisscott
15 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

#12 @nacin
15 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.

#13 @nacin
15 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.