WordPress.org

Make WordPress Core

Opened 20 months ago

Last modified 5 weeks ago

#21659 new defect (bug)

wp_mail() problem with Reply-To header

Reported by: pavelevap Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.4.1
Component: Mail Keywords: has-patch
Focuses: Cc:

Description

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.

Attachments (2)

21659.diff (2.8 KB) - added by iandunn 5 weeks ago.
21659.alt.diff (2.8 KB) - added by iandunn 5 weeks ago.
Alternate style in case this is considered more readable

Download all attachments as: .zip

Change History (21)

comment:1 avryl8 months ago

  • Component changed from General to Mail

comment:2 szepe.viktor7 months ago

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;
>                       }
>               }
>       }
>
Last edited 7 months ago by szepe.viktor (previous) (diff)

comment:3 husobj5 months ago

  • Cc ben@… added

comment:4 follow-up: kitchin4 months ago

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

comment:5 in reply to: ↑ 4 ; follow-up: iandunn5 weeks ago

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

comment:6 in reply to: ↑ 5 ; follow-up: bpetty5 weeks ago

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.

comment:7 in reply to: ↑ 6 ; follow-up: iandunn5 weeks ago

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.

comment:8 iandunn5 weeks ago

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().

comment:9 in reply to: ↑ 7 bpetty5 weeks ago

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.

comment:10 iandunn5 weeks ago

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>

comment:11 bpetty5 weeks ago

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.

comment:12 bpetty5 weeks ago

  • Keywords close added

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.

iandunn5 weeks ago

comment:13 iandunn5 weeks ago

  • Keywords has-patch added

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.

iandunn5 weeks ago

Alternate style in case this is considered more readable

comment:14 bpetty5 weeks ago

  • Keywords close removed

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

comment:15 bpetty5 weeks ago

  • Milestone changed from Awaiting Review to Future Release

comment:16 iandunn5 weeks ago

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.

comment:18 in reply to: ↑ 17 iandunn5 weeks ago

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.

comment:19 szepe.viktor5 weeks ago

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

Note: See TracTickets for help on using tickets.