wp_mail() problem with Reply-To header

[pavelevap]

I am using in email headers also Reply-To:

$headers .= "Reply-To: \"$name\" <$email>\r\n";

But there is a problem when name contains special non-English characters.

Example (from email headers):

Name: Žlutý kůň

Reply-To: =?UTF-8?Q? "=C5=BDlut=C3=BD_k=C5=AF=C5=88" _ <my@email.com>,
  ?=@example.com

I think that it is somehow related to wp_mail() function, because I also tried to use SMTP plugin and it worked well:

Reply-To: =?UTF-8?Q?"=C5=BDlut=C3=BD_k=C5=AF=C5=88"_<my@email.com>?=

Also when I use simply mail() instead of wp_mail() then everything works well. Problem can be seen only for Reply-To header, all others work well (for example From).

I am using WP 3.4.1.

[szepe.viktor]

it helped me:

wp-includes/pluggable.php

243a244
>               $repto = array();
298a300,302
>                                       case 'reply-to':
>                                               $repto = array_merge( (array) $repto, explode( ',', $content ) );
>                                               break;
403a408,425
>       if ( !empty( $repto ) ) {
>               foreach ( (array) $repto as $recipient) {
>                       try {
>                               // Break $recipient into name and address parts if in the format "Foo <bar@baz.com>"
>                               $recipient_name = '';
>                               if( preg_match( '/(.*)<(.+)>/', $recipient, $matches ) ) {
>                                       if ( count( $matches ) == 3 ) {
>                                               $recipient_name = $matches[1];
>                                               $recipient = $matches[2];
>                                       }
>                               }
>                               $phpmailer->AddReplyTo( $recipient, $recipient_name );
>                       } catch ( phpmailerException $e ) {
>                               continue;
>                       }
>               }
>       }
>

[kitchin]

Reply-to should not have a Name at all, just a list of simple email addresses, according to this person: ​http://wordpress.org/support/topic/contact-form-possible-bug

[iandunn]

Replying to kitchin:

Reply-to should not have a Name at all, just a list of simple email addresses, according to this person: ​http://wordpress.org/support/topic/contact-form-possible-bug

I don't think that's what they were saying. The issue in that thread was that the address format was "foo@example.org <foo@example.org>", where the e-mail address itself is set in the first part rather than a name.

If I'm reading the ​RFC 2822 correctly, the Reply-To header can accept an address-list just like a To or From header, so an entry like "Jane Doe <jane@example.org>" is perfectly acceptable.

I was able to reproduce this with:

wp_mail( 'me@example.org', '#21659-core test', 'reproducing', array( 'Reply-To: Lukáš LastName <foo@example.org>' ) );

...which results in:

Reply-To: =?UTF-8?Q?Luk=C3=A1=C5=A1_LastName_ <me@example.org>,
	?=@macenzie.local

[bpetty]

Replying to iandunn:

Reply-To: =?UTF-8?Q?Luk=C3=A1=C5=A1_LastName_ <me@example.org>,
	?=@macenzie.local

I'm not sure where the "@macenzie.local" part is coming from. Obviously it's your actual test installation, but what I mean is that I'm not sure how it's being added to the Reply-To header, directly after the encoded-word part of the header.

Are you testing with latest trunk? PHPMailer was updated recently to 5.2.7 (see #25560). When I test with latest trunk with this update, here are the verbatim headers that are generated:

Date: Tue, 11 Mar 2014 23:02:15 +0000
Return-Path: <wordpress@wordpress.local>
From: WordPress <wordpress@wordpress.local>
Message-ID: <fa3d3980eb4c3f1f691dbb073688b95a@wordpress.local>
X-Priority: 3
X-Mailer: PHPMailer 5.2.7 (https://github.com/PHPMailer/PHPMailer/)
Reply-To: =?UTF-8?Q?Luk=C3=A1=C5=A1_LastName_<foo@example.org>?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

That's generated when I also called wp_mail as described above:

wp_mail('bar@example.org', '#21659-core test', 'reproducing',
		array( 'Reply-To: Lukáš LastName <foo@example.org>' ));

Everything looks correct with my headers. Those are passed directly to mail() from there.

[iandunn]

Replying to bpetty:

I'm not sure where the "@macenzie.local" part is coming from. Obviously it's your actual test installation, but what I mean is that I'm not sure how it's being added to the Reply-To header, directly after the encoded-word part of the header.

Yeah, that made me curious too. macenzie.local is the local hostname of the server, rather than the domain name of the virtual host. I've traced the headers from wp_mail() through PHPMailer to mail() and they look fine the entire time, even though they're malformed in the message I actually receive.

So, that makes me wonder if maybe the Reply-To header string isn't being encoded properly, and sendmail is adding the extra ",[carriage return][tab][local hostname]"bit.

The string that PHPMailer sends to mail() looks like:

Reply-To: =?UTF-8?Q?Luk=C3=A1=C5=A1_<ian.dunn@example.org>?=

That looks correct to me, but so far I haven't been able to find an RFC or anything that specifies what the format should be.

Are you testing with latest trunk? PHPMailer was updated recently to 5.2.7 (see #25560). When I test with latest trunk with this update, here are the verbatim headers that are generated:

I originally encountered this on a production server running 3.8.1, and then reproduced it on my local environment with trunk.

If it is happening at the server level, then it would make sense that some MTAs would handle it gracefully while others exhibit the behavior I'm seeing.

[iandunn]

Although, I did try to reproduce it in PHPMailer directly, and couldn't, so it seems like it may be influenced by something in wp_mail().

[bpetty]

Replying to iandunn:

That looks correct to me, but so far I haven't been able to find an RFC or anything that specifies what the format should be.

​RFC 2047 describes UTF-8 encoded headers (also see #18521). As far as I can see, encoded-words are allowed in To, From, CC, BCC (and thus likely Reply-To headers). Although, looking through here again, it does mention that "An 'encoded-word' MUST NOT appear in any portion of an 'addr-spec'." I think this means that instead of this:

Reply-To: =?UTF-8?Q?Luk=C3=A1=C5=A1_LastName_<foo@example.org>?=

It's possible that it's *supposed* to be:

Reply-To: =?UTF-8?Q?Luk=C3=A1=C5=A1_LastName?= <foo@example.org>

In other words, I think it's possible that the actual email address is supposed to never be included in an encoded-word section.

This would certainly mean it's a bug in PHPMailer upstream then if that's correct.

[iandunn]

Ah, yeah, that sounds like it might be it.

But I think it still might be on our end. When I run ​this in PHPMailer directly it generates the correct header:

Reply-To: =?UTF-8?B?THVrw6HFoQ==?= <info@example.com>

[bpetty]

I think I'm right about that last comment. This isn't a bug in PHPMailer. PHPMailer defines a function specifically for this purpose rather than adding it as a custom header, see $phpmailer->addReplyTo(). Addresses added in that manner are encoded correctly. So if I use this instead:

$phpmailer->addReplyTo('foo@example.org', 'Lukáš LastName');

Here's how the header is encoded:

Date: Wed, 12 Mar 2014 00:15:26 +0000
Return-Path: <wordpress@wordpress.local>
From: WordPress <wordpress@wordpress.local>
Reply-To: =?UTF-8?Q?Luk=C3=A1=C5=A1_LastName?= <foo@example.org>
Message-ID: <dae3fd6da29145b1e00c124e9013f0df@wordpress.local>
X-Priority: 3
X-Mailer: PHPMailer 5.2.7 (https://github.com/PHPMailer/PHPMailer/)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This would seem to indicate then that setting a Reply-To custom header is not the appropriate (or properly supported) method of setting a Reply-To address.

[bpetty]

So... I'm just going to suggest a close as invalid since it is possible to still make this work in 3rd party code. Here's one kind of weird way to do it anyway, but this is assuming you don't want to plug in you're own custom wp_mail() function:

$my_plugin_replyto_address = 'foo@example.org';
$my_plugin_replyto_name = 'Lukáš LastName';

add_action('phpmailer_init', 'my_plugin_add_reply_to');
function my_plugin_add_reply_to( $phpmailer ) {
	global $my_plugin_replyto_address, $my_plugin_replyto_name;
	$phpmailer->addReplyTo($my_plugin_replyto_address, $my_plugin_replyto_name);
}

wp_mail(...);

If someone wants to expand core wp_mail() to detect 'Reply-To' headers, and automatically do this itself, that might be nice, but it seems like this isn't so much a "bug" as it is intentional behavior.

[iandunn]

It seems like a bug to me, since Core is using AddCustomHeader() instead the appropriate method, and that causes an undesirable result. I think plugin authors have a reasonable expectation that they can add a Reply-To header without any extra fuss.

I don't see any drawback to making wp_mail() use AddReplyTo(), and if we do that then it would "just work" for everybody. If we don't, then any plugin dev who wants to set a Reply-To will potentially get bit by this; and if they do, then they'll have to spend a lot of time figuring out what happened and how to work around it.

I attached a first attempt at a patch in case others think it'd be a positive change.

[iandunn]

Alternate style in case this is considered more readable

[bpetty]

I could go for either, but you're first patch does seem a little more concise.

[iandunn]

I looked further into ​the problem that kitchin mentioned in comment 4, and initially thought it was a PHPMailer bug, but ​it turned out to be the same issue as this ticket -- that we're using AddCustomHeader() instead of AddReplyTo().

That also appears to be the cause of ​a similar problem kitchen reported on the support forums.

The address methods like AddAddress() and AddReplyTo() do some filtering on the addresses to make sure that they're compliant with the RFCs, but AddCustomHeader() doesn't because it rightfully assumes that the appropriate methods will be called when dealing with headers containing addresses.

So, there are potentially other problems being caused by using AddCustomHeader() instead of AddReplyTo(). Any fatal errors that would be corrected by the filtering in AddReplyTo() are currently failing.

[szepe.viktor]

Do you see my working patch?
https://core.trac.wordpress.org/ticket/21659#comment:2

[iandunn]

Replying to szepe.viktor:

Do you see my working patch?
https://core.trac.wordpress.org/ticket/21659#comment:2

Yeah, the patches I added are essentially the same thing, but they're a little bit cleaner IMO since they loop through the three header types and run the same block of code, rather than duplicating it multiple times. You should get props for the original patch, though.

[szepe.viktor]

Yes, My patch was written for a time till the next WP update.

[skijash]

This patch still hasn't been applied?
I'm having the same problem now in wordpress 4.0.

[kitchin]

Note upstream PHPMailer agrees this is a WP bug. ​https://github.com/PHPMailer/PHPMailer/issues/193

Note @skijash wp_mail is pluggable so you can redefine it in a plugin or theme if you want to for now.

Replying to skijash:

This patch still hasn't been applied?
I'm having the same problem now in wordpress 4.0.

[skijash]

I know, but I'm wondering why this hasn't been resolved in two years. It seems like a pretty big bug to me.

[nevis2us]

This bug still exists in 4.1.1.
Wasted several hours to write the exact same patch mentioned in #comment:2.

[iandunn]

Refreshed the patch and added unit tests.

I also refactored a few things to keep the function DRY, but I'm happy to back that out if it feels like too much.

I also went ahead and updated wp_mail() to use $phpmailer->setFrom(), rather than accessing the object members directly. I think that's argubably within the scope of the ticket, since the underlying bug here is that wp_mail() is treating addresses as if they're generic strings. In reality, they require special handling, and using the methods PHPMailer provides will run them through the appropriate processing and avoid other potential bugs. I'm happy to remove that from the patch too, though, if it seems like too much.

@stephenharris, I'd love to get any feedback you have on this.

[stephenharris]

It looks good: it fixes the bug, passes the tests and removes some duplicate code. My only concern is that using variable variables is slightly cryptic and difficult to follow. Could to, cc , bcc and reply_to be keys in an array rather than variable names, avoiding the need for$$address_header?

Additionally my personal preference would be to use an explicit switch statement rather than $header_method_map and call_user_func - it's slightly more lines, but it reads a little more easily:

 try {
        // Break $recipient into name and address parts if in the format "Foo <bar@baz.com>"
       $recipient_name = '';

       if ( preg_match( '/(.*)<(.+)>/', $address, $matches ) ) {
              if ( count( $matches ) == 3 ) {
                     $recipient_name = $matches[1];
                     $address        = $matches[2];
              }
       }

       switch ( $address_header ) {
              case 'to':
                     $phpmailer->AddAddress( $address, $recipient_name );
                     break;
              case 'cc':
                     $phpmailer->AddCc( $address, $recipient_name );
                     break;
              case 'bcc':
                     $phpmailer->AddBcc( $address, $recipient_name );
                     break;
              case 'reply_to':
                     $phpmailer->AddReplyTo( $address, $recipient_name );
                     break;
       }
} catch ( phpmailerException $e ) {
       continue;
}

I'd agree that none of the changes in that patch are out-of-scope.

[iandunn]

Ah, you're right, that is much more readable. 21659.4.diff implements that.

I didn't see a good way to keep the cases in switch ( strtolower( $name ) ) DRY without variable-variables, so I just switched that back to what it was doing previously.

$cc, $bcc, and $reply_to initialization was moved outside the if/else so that they're guaranteed to exist when they're called later by compact().

I also changed the $address_headers loop to continue early if the array is empty, so we can have 1 less layer of indentation.

How does that look?

[stephenharris]

Replying to iandunn:

$cc, $bcc, and $reply_to initialization was moved outside the if/else so that they're guaranteed to exist when they're called later by compact().

Good catch. Looks all good to me.

[boonebgorges]

In 38058:

Mail: Improve handling of UTF-8 address headers.

Previously, wp_mail() implemented Reply-To as a generic header, using
PHPMailer's addCustomHeader(). As such, the email address portion of
the header was being incorrectly encoded when the name portion
contained UTF-8 characters. Switching to PHPMailer's more specific
addReplyTo() method fixes the issue.

For greater readability, the handling of all address-related headers
(To, CC, BCC, Reply-To) has been standardized.

Props szepe.viktor, iandunn, bpetty, stephenharris.
Fixes #21659.

[iandunn]

Related #37736