WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 8 months ago

#19847 new defect (bug)

wp_mail $from_name field is removing an extra character from the name

Reported by: hakanca Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.3.1
Component: Mail Keywords: has-patch dev-feedback
Focuses: Cc:

Description

If the headers are sent as a string to wp_mail, parsing of $from_name field removes one required character at the end of the name. Hence causing a name field of "Hakan" to show up as "Haka" in the received email.

Line 257 of pluggable.php is causing this error.

$from_name = substr( $content, 0, strpos( $content, '<' ) - 1 );

Removing "-1" from substr function call solves the problem.

$from_name = substr( $content, 0, strpos( $content, '<' ));

Attachments (2)

19847.patch (572 bytes) - added by SergeyBiryukov 2 years ago.
19847.2.diff (567 bytes) - added by MikeHansenMe 18 months ago.
same patch edits now on new lines.

Download all attachments as: .zip

Change History (14)

comment:1 follow-up: kawauso2 years ago

  • Keywords reporter-feedback added

Are you passing in the From value without a space?

i.e. Hakan<hakan@…>

If not, can you provide an example to reproduce?

Also, please see patching for Windows and *nix

Last edited 2 years ago by kawauso (previous) (diff)

comment:2 in reply to: ↑ 1 hakanca2 years ago

  • Keywords reporter-feedback removed

It doesn't really matter if we have a space there or not. strpos returns the numeric position of the found string; in our case "<" character. substr returns as many characters specified by its length parameter; in our case the number we get from strpos. Both functions start counting from "0" unless we specify otherwise; which is our case. So in the following cases:

Hakan<hakan@…> --> strpos will return 5; the index position of "<" in the original string starting from "0". So substr will return "Hakan" since we are returning 5 characters from the beginning.

If, as you asked, we have a space between the name and email like below:

Hakan <hakan@…> --> then strpos will return 6, and substr will also include 6 characters and return "Hakan " -including the space-. Since we are already trimming the space in the returned string 2 lines later in the code with

$from_name = trim( $from_name );

There is no need to subtract 1 from the length of the string, since we never run into problem of accidentally including that "<" character. I think that was the false logic behind adding that -1 to substr call in the first place.

By removing that "-1" we are actually making the code more flexible, by accommodating both versions. (i.e. with or without space) And to the best of my knowledge, PHP has no problem sending the email in either case.(Tested on my server)

As for making patches to the Wordpress core, I am not an expert PHP coder per se. I'd rather leave it to people who know what they are doing. :)


Replying to kawauso:

Are you passing in the From value without a space?

i.e. Hakan<hakan@…>

If not, can you provide an example to reproduce?

Also, please see patching for Windows and *nix

SergeyBiryukov2 years ago

comment:3 MikeHansenMe18 months ago

  • Cc mdhansen@… added

Unable to reproduce the problem. Has it been fixed elsewhere?

MikeHansenMe18 months ago

same patch edits now on new lines.

comment:4 SergeyBiryukov18 months ago

  • Keywords 3.6-early added
  • Milestone changed from Awaiting Review to Future Release

To reproduce, leave out the space between the name and the e-mail address:

$headers = 'From: My Name<myname@mydomain.com>' . "\r\n";
wp_mail( 'test@test.com', 'subject', 'message', $headers );

The resulting name will be cut:

From: My Nam <myname@mydomain.com>

Considerations in comment:2 are valid.

comment:5 bpetty15 months ago

  • Keywords 3.6-early removed
  • Milestone changed from Future Release to 3.6

comment:6 kovshenin15 months ago

19847.2.diff applies cleanly and seems reasonable.

comment:7 follow-up: MattyRob15 months ago

RFC emails standards demand that there is a space in an email address between the "User name" and the <local-part@domain-part> of an email address.

If emails are passed without the space then it is being done wrong. I'm just asking then if we should really be 'fixing' this?

It kind of feels that we should but the standards are there for a reason aren't they?

comment:8 mordauk13 months ago

  • Cc pippin@… added

comment:9 in reply to: ↑ 7 ; follow-up: DrewAPicture12 months ago

  • Keywords dev-feedback added

Replying to MattyRob:

RFC emails standards demand that there is a space in an email address between the "User name" and the <local-part@domain-part> of an email address.

Can you provide a reference link?

If emails are passed without the space then it is being done wrong. I'm just asking then if we should really be 'fixing' this?

If it's considered the standard, I'm kind of with you that maybe this is just a case of _doing_it_wrong() and not something that needs "fixing"

comment:10 in reply to: ↑ 9 MattyRob12 months ago

Replying to DrewAPicture:

Replying to MattyRob:

RFC emails standards demand that there is a space in an email address between the "User name" and the <local-part@domain-part> of an email address.

Can you provide a reference link?

http://tools.ietf.org/html/rfc5322#section-3.4

I think this is the document I was referring to.

The pertinent sections seem to build on each other so:

name-addr = [display-name] angle-addr

name-addr is what we are defining here with display name being the define human readable name and angle-addr being the email address in angled brackets. The definition for angle-addr is:

[CFWS] "<" addr-spec ">" [CFWS]

Where adds-spec is:

local-part "@" domain

I think I've also read somewhere that a FWS must be included between the display-name and angle-addr but I can't find that right now. I'll post again if I stumble upon it.

comment:11 ryan10 months ago

  • Milestone changed from 3.6 to Future Release

comment:12 avryl8 months ago

  • Component changed from General to Mail
Note: See TracTickets for help on using tickets.