wp_mail(): Address header parsing is not RFC-5322 complient and fails on quoted-string when including a "<", ">" or ","

[bhujagendra]

Problem

The following From field values result in an error, although both comply with RFC 2822:

Case 1: quoted printable:

• Input: From: "test<stage" <test@example.com>
• Result:
  • $from_name: "test
  • $from_email: stage" <test@example.com

Case 2: multiple from addresses:

• Input: From: test <test@example.com>, User <user@example.com>
• Result:
  • $from_name: test
  • $from_email: test@example.com, User <user@example.com

Case 3: similar problem for to, cc, bcc, and reply_to fields!

• Input: To: test <test@example.com>, "Joe, Doe" <joe.doe@example.com>
• Result:
  • address 1: test <test@example.com>
  • address 2: "Joe
  • address 3: Doe" <joe.doe@example.com>

Reason

The ​php mail() documentation does not explicitly state that the From header needs to comply with RFC 2822. But it does say so for the To header. Furthermore, the current implementation does already support the name-addr-​syntax (i.e. [display-name] angle-addr), which was not allowed in the preceding ​RFC 822.

However, wp_mail does ignore the facts

• that display-name may include < (case #1 above), and
• that the From field may actually include multiple addresses.

RFC 2822 Specification

• From field value is mailbox-list ​section 3.6.2
• mailbox-list is a single mailbox, or two or more comma-separated mailbox, mailbox ​section 3.4
• mailbox is either just an addr-spec (e-mail address) or a name-addr ​section 3.4
• display-name is a phrase ​section 3.4
• phrase is a word and hence a quoted-string ​section 3.2.6
• quoted-string ​(i.e. a string in double quotes) may include any US-ASCII prinatable character except for the double quote (") and the backslash (\). And as such also the <.

Current Implementation

The parsing of the field simply splits the two parts of the name-addr at the position of the first occurrence of <, see line 2 of the code in question ​(pluggable.php:301-319):

<?php
case 'from':
        $bracket_pos = strpos( $content, '<' );
        if ( false !== $bracket_pos ) {
                // Text before the bracketed email is the "From" name.
                if ( $bracket_pos > 0 ) {
                        $from_name = substr( $content, 0, $bracket_pos );
                        $from_name = str_replace( '"', '', $from_name );
                        $from_name = trim( $from_name );
                }

                $from_email = substr( $content, $bracket_pos + 1 );
                $from_email = str_replace( '>', '', $from_email );
                $from_email = trim( $from_email );

                // Avoid setting an empty $from_email.
        } elseif ( '' !== trim( $content ) ) {
                $from_email = trim( $content );
        }
        break;

Similarly, for the to, cc, bcc, and reply_to fields:

• ​Splitting the field value at every comma (which may be quoted)
• ​Ignoring possible quotes when extracting name and address from the different values

Discussion

Unfortunately there is no simple solution:

• Correct parsing - according to the standard outlined in RFC 2822 - would probably require the use of a real parser (as opposed to searching the entire string for certain tokens).
  • PHPMailer::parseAddresses() ​method cannot be used, as it lacks the same shortcoming, if the IMAP extension is not installed.
  • ​mail-mime-parser could work as an alternative. But it's a whole other level of complexity and only supports php v8+
• Even if correct parsing would be supported and $from_name as well as $from_email correctly assigned, this would only work for single-mailbox entries. Multiple from addresses are not supported by
  • the the wp_mail_from-​filter and wp_mail_from_name-​filter
  • the PHPMailer\PHPMailer\PHPMailer's createHeader ​method

Weak implementation

It must be noted, that the current implementation has repeatedly had bugs and regressions (although it has been quiet around this in the last years):

• #23243
• #19847
• #18463
• #17305
• #1593

Pragmatic proposal

• Introduce a new filter wp_mail_from_raw at the beginning of the From field parsing:

  <?php
  /**
   * Filters the raw "From" header field before parsing it.
   *
   * @since 7.6.2
   *
   * @param string $content Form header content.
   * @param string $header Full raw header.
   */
  $content = apply_filters( 'wp_mail_from_raw', $content, $header );
  

  This would allow a user (or plugin author) to mitigate the issue, if it happens on a given site. But it would save them the effort of parsing the headers entirely (as would be required with wp_mail).
• Alternatively, the filter could be called for all headers in ​line 300

  <?php
  
  $origName = $name; // used later for the default case in line 347
  $name = strtolower( $name );
  
  /**
   * Filters the raw header fields before parsing them.
   *
   * @since 7.6.2
   *
   * @param string $content Header field's content.
   * @param string $name Header field's name.
   * @param string $header Full raw header.
   */
  $content = apply_filters( 'wp_mail_raw_header', $content, $name, $header );
  
  /**
   * Filters the raw header fields before parsing them.
   *
   * @since 7.6.2
   *
   * @param string $content Header field's content.
   * @param string $name Header field's name.
   * @param string $header Full raw header.
   */
  $content = apply_filters( "wp_mail_header_$name", $content, $header );
  
  // ...
  
  // Line 347 needs to be updated:
  // $headers[ trim( $name ) ] = trim( $content );
  $headers[ $origName ] = $content; // both values are already trimmed in lines 296 & 297
  

• Potentially use a RFC-compliant parser to avoid future errors with quoted address names at least in the other address fields
• Use an instance of an object implementing ArrayAccess and Stringable. This can be used to store one or a set of addresses/names in the $from_email/wp_mail_from and $from_name/wp_mail_from_name values and is sent through the respective filters. By this it is basically backwards compatible.
• Parse the Sender header, add a filter and default it to the first address of a multi-address From field of the default wordpress from address.
• Enhance the PHPMailer::setFrom() method to accept $address and $name to be arrays. And therefore also the corresponting class fields $From and $FromName (maybe through a PR to ​PHPMailer)

Happy to help with a PR. I would just love to know what would be accepted in there.

Thanks,
Bhujagendra

[SirLouen]

We have to divide this into 3 bugs or enhancements (I would have created 3 tickets, one for each one), and probably we could find an answer for each one individually instead of treating them as a single problem (although, by the end I think that there could be a single solution that could benefit all 3 scenarios).

Also, these would have avoided this macro questions and answers that are ultra-tiring for most reviewers that will prefer to skip for the future in most cases.

These problems have been here pretty much since the integration of PHPMailer in [4946]

Firstly, about parsing the various forms like From: "test<stage" <test@example.com>

Replying to bhujagendra:

The ​php mail() documentation does not explicitly state that the From header needs to comply with RFC 2822. But it does say so for the To header. Furthermore, the current implementation does already support the name-addr-​syntax (i.e. [display-name] angle-addr), which was not allowed in the preceding ​RFC 822.

mail doesn't specify anything about the from because it doesn't have any specific limitations regarding the headers which are left to the final decision of the MTA (and you can pretty set whatever custom header you like).

This doesn't mean that any development should or should not take care of this. In the case of PHPMailer it provides a way of taking care of the From: part from the headers.

Remember that in wp_mail the From: parsing from Headers ​you mention here was declared legacy [5639] and ideally not to be manipulated from there, but from the hook wp_mail_from (similar fashion as using any other raw thing, instead of the provided functionality, which is causing some troubles in other areas)

So basically, using Headers is a bad idea, and still PHPMailer will fail with such format if you directly use the hook:
PHPMailer Error: Invalid address: (From): "test<stage" <test@example.com>

So technically, this should be a thing that ​should be discussed upstream. Plus, with just one From, without a Sender header, you cannot really bypass the addFrom method from PHPMailer if you want to actually send the email and it to be accepted by any MTA (with many additional problems I'm explaining after).

Second, the multiple From: issue:

This is a little incremental to the first point, because now, we can actually introduce a Sender header, so we could play around From: a little more.

Let's say we go into the wp_mail_from and we directly pass:

add_filter('wp_mail_from', function($original_email_address) {
     return 'test <test@example.com>, User <user@example.com>'; 
});

What PHPMailer will reject is that From format. It might ultimately reject any of the forms you are thinking for the From:.

Moreover, this is one of the least recommended practices, ​as many servers will reject this kind of header.

Although it's true that the specification permits this according to ​RFC 5322, we need to add the Sender field in the header.

So yes, we could add everything in the Custom Headers, but the idea moves towards handling everything without a full custom email in the PHPMailer fashion. Or you could do something like this:

add_action('phpmailer_init', function ($phpmailer) {
	$phpmailer->addCustomHeader( 'From', 'User <user@example.com>' );
	$phpmailer->addCustomHeader( 'Sender', 'test <sender@example.com>' );
});

add_filter('wp_mail_from', function() {
	return 'test@example.com'; 
});

add_filter('wp_mail_from_name', function() {
	return 'test'; 
});

To achieve the same behaviour (although, if you set a From: in the format like in the first part, it will fail with the same PHPMailer error)

Third, about the To:, CC:… parsing

Here, as in the first, I agree that the parsing is completely insufficient.

But in this case, we don't really have an easy way to work around it. So I think is the main bug that should be sorted out. Although I have to agree with you that despite the Header part is legacy, sorting it out should also be done (and remove it from "legacy" because playing around with the Headers should be completely functional).

Personally, I would approach this with a specific sanitization function, extracting it from wp_mail, and using such to parse each of the potential areas where an email address could be present compliant with RFC 2822

So I invite you to work on a possible fix (also unit tests will be 100% needed for this fix)

[jdeep]

I would like to work on the third issue regarding To:/CC: header parsing.

[SirLouen]

Go for it @jdeep, I'm reviewing your other patches today.

[jdeep]

@SirLouen @bhujagendra,

Yesterday, I took some time to read RFC 5322 and 2822. WordPress Core supporting fully RFC-5322 compliant email headers does seem like a very ambitious feature. However, I feel this is much suited to be implemented properly (without IMAP extension) in ​PHPMailer::parseAddresses() itself rather than in WordPress. Naively splitting on , isn't the best way to handle headers with multiple addresses.

Instead of trying to implement a new parser inside WordPress, I think it is better to make the PHP fallback in PHPMailer more RFC 5322–aware.

But that also means it will not be fixed soon and will probably takes a few months to years to get the latest PHPMailer implementation in WordPress.

In the meantime, I’ll try putting together a WordPress patch that handles at least the quoted-comma case. This is meant to be a temporary fix till the time we get a better implementation in PHPMailer itself.

[SirLouen]

@jdeep you mean for the whole parsing part using the PHPMailer static method?

Many of the these functions were implemented, overlapping PHPMailer, for many reasons (mostly because they were implemented actually before even they existed in PHPMailer, like the case of parseAddresses). In fact most of wp_mail is just a reimplementation of almost the whole PHPMailer API. This provides some degree of easiness to migrate in case PHPMailer happens to derail at some point. Nowadays I would rather choose to extend PHPMailer, rather than rewrite the same code, and definitely never inherit anything.

The parseAddresses from PHPMailer implements both options, with the imap PHP Extension, and without it. It's not a bad idea to simply replace everything with this ngl, especially since we are already mostly depending on PHPMailer atm, instead of implementing a full parsing flawed logic as we are currently doing.

I feel this is much suited to be implemented properly

I don't understand this: Why do you think ​its not implemented propertly here?

[jdeep]

I feel this is much suited to be implemented properly

I don't understand this: Why do you think ​its not implemented propertly here?

As @bhujagendra pointed out, the main issue is in ​this line which naively splits the string on commas
which breaks valid address strings like:

To: test <test@example.com>, "Joe, Doe" <joe.doe@example.com>.

The comma inside the quotation should not be broken to form two separate addresses.

[SirLouen]

Replying to jdeep:

The comma inside the quotation should not be broken to form two separate addresses.

Definitely building for this and testing for every single possibility that is included in the RFC is very complex. In fact we were particularly discussing this recently in one of the latest test meetings, as this also applies for other validation methods like the email validation itself, but in this case we keep choosing to validate by ourselves.

The problem as I commented on my first reply, is that this report is two or three folded.

So this goes into two levels: the email validation itself (case 1) and the multiple email scenario (cases 2 and 3) which are two separate issues, although, its true as I said, that they could be interfering with each other (like the case of opening angle brackets, or the commas, quotes, within other emails as a whole), but they are two separate issues.

The first part, accepting or not multiple From, and how to handle them, and how to introduce the Sender header, if we are going to be handling this, leads to a discussion on how to approach this. Personally, I would rather to have this discussion upstream (PHPMailer), as there are more dedicate people into the mailing world before getting to a conclusion here. I'm going to take note of this and move it as soon as I can.

As @bhujagendra pointed out, the main issue is in this line which naively splits the string on commas which breaks valid address strings like:

So here we will be for now focusing on the second part, the multiple address validation (which still, as you said also could have a solution upstream). It seems that @bhujagendra was warning of a failed scenario without imap extension in parseAddresses, but to be sincere, I did not spot it. Before proceeding with a patch here, let me add some unit test and build an environment without imap for PHPMailer, and let me prove results upstream.

So give me a day or two and I will report back and see if its worthy to work on our own parsing solution. If you want to provide some code to showcase a possible option, go on with it, but after reading this with more detail I advocate for wiping all and include a better maintained and more comprehensive function for this purpose (although I have to admit that is not too well maintained in the PHPMailer end, so its not the ultimate solution, but still it has more eyeballs on it).

[jdeep]

Replying to SirLouen:

Alright.

[SirLouen]

@jdeep

After thinking about this problem I don't really see any point on going beyond imap support. Its like: you want advanced features? Install imap extension, don't ask me to upgrade to a fully featured RFC822 parser for something that is going to be used by maybe 0.01% of the WP population.

Currently parseAddresses support full rfc822 address splitting in all the variants. In fact I'm proposing to PHPMailer to water down the non-imap-ext version as much as possible. We cannot reinvent the wheel. So, since we have access to the latest version of PHPMailer which happens to be integrated as an External Library, and it has parseAddresses, lets use it.

Now I'm thinking if we could take a check based on the code, to create a Site Health check to recommend php-imap extension installed based on something we pick from code because the sole use of this is through custom mail headers, its so niche that its undetectable.

But definitely the addresses parsing in the headers part should leave. It's excessively simple and for pretty much all simple use-cases, parseAddresses does the trick (even without the imap version)

For the other part of the ticket, the multiple From addresses, I'm doing a bit more research. For now, from the native support perspective this would not be feasible as setFrom doesnt support multiple Froms. And from the Headers perspective, we are having a massive trouble with the former, as our Header parsing options are disastrous.

So lets take one thing at a time. Maybe we can open a second ticket in the future to handle the multiple From part. For now lets try to fix the first as I commented in the beginning (with the parseAddresses support in exchange on the weak logic for parsing headers in the current code)

We also need unit tests for this.

[jdeep]

Replying to SirLouen:

@jdeep

After thinking about this problem I don't really see any point on going beyond imap support. Its like: you want advanced features? Install imap extension, don't ask me to upgrade to a fully featured RFC822 parser for something that is going to be used by maybe 0.01% of the WP population.

Yes that works too.

Currently parseAddresses support full rfc822 address splitting in all the variants. In fact I'm proposing to PHPMailer to water down the non-imap-ext version as much as possible. We cannot reinvent the wheel. So, since we have access to the latest version of PHPMailer which happens to be integrated as an External Library, and it has parseAddresses, lets use it.

So basically a refactor of existing code to use parseAddresses.

So lets take one thing at a time. Maybe we can open a second ticket in the future to handle the multiple From part. For now lets try to fix the first as I commented in the beginning (with the parseAddresses support in exchange on the weak logic for parsing headers in the current code)

We also need unit tests for this.

Right. Let me add unit tests to my other patches and then I will start refactoring with parseAddresses here.

Current implementation uses naive approach of
splitting email address headers at ,. This makes perfectly valid headers like
"Jane, Doe" <jane@example.com> break. But this is a perfectly valid RFC-5322 email address header.

We should use PHPMailer's parseAddresses() function which uses IMAP extension (if available) which addresses this issue instead of the current implementation.

Trac ticket: #62940

[jdeep]

Replying to SirLouen:

@jdeep

So lets take one thing at a time. Maybe we can open a second ticket in the future to handle the multiple From part. For now lets try to fix the first as I commented in the beginning (with the parseAddresses support in exchange on the weak logic for parsing headers in the current code)

We also need unit tests for this.

For the unit tests, how should we approach them? Since parseAddresses() relies on the IMAP extension, should the tests be written to verify headers against fully RFC-5322-compliant cases only when the IMAP extension is available?

Also, how would this behave in the CI pipelines—would we need to ensure the IMAP extension is installed there?

[SirLouen]

Hey @jdeep

First, I can't remember the use of a closure outside a hook in Core.
I've been doing a search, and I have found none, so despite your proposed patch is good, we should avoid the closure. I would prefer not to add a function either, because, ideally, I would like in the near future to encapsulate the wp_mail function, so a method with private visibility would be ideal for this case, but adding a function now will only bring future back-compat dread. So I feel a litte in a crossroad for this. 100% I would prefer a closure for now but I doubt it will pass.

For the unit tests, how should we approach them? Since parseAddresses() relies on the IMAP extension, should the tests be written to verify headers against fully RFC-5322-compliant cases only when the IMAP extension is available?

Its not exactly like that. parseAddresses does work without IMAP extension. Problem is that is not 822 compliant. Only works with very basic Addresses. I feel that the premises in the OP are exaggeratedly academic.

For Unit Tests, I would do the two versions: with and without imap. (skipping when IMAP is not available). I'm not 100% sure, but I would bet that the CI pipelines are not testing currently all skipped tests. And I'm not sure either if they come with IMAP by default, give it a check. To be sincere, I'm not confident which is the approach. Can you check another example of Unit Test for ​another non required library in similar circumstances?

[SirLouen]

@jdeep additional info

It seems that all tests are simply running in the docker image that we use for wordpress-develop. So basically imap is not there, by default, hence in the CI it won't be running. Any skipped test is running currently. They are planning to introduce an upgraded version for testing with multiple environments including different extensions, but we don't know when this might come.

For this reason, and for now, we need to versions: One version checking for the IMAP extension and markTestSkipped if it doesn't exist. ​Like this
This version can include complex scenarios, like the ones proposed by the OP, with commas and square brackets.

And the other, running very basic tests with very simple addresses (can be multiple addresses, but very simple, like "Name <email(at)example.com>", or just the email).

[jdeep]

@SirLouen Yeah, I was thinking closures might not be the best option since they aren’t commonly used elsewhere in the codebase. At the same time, I didn't want to write redundant address-parsing logic in multiple places, which is why I considered adding a helper function. But, pluggable.php doesn’t have helper functions either, so it didn’t feel like the right place for it.

ideally, I would like in the near future to encapsulate the wp_mail function, so a method with private visibility would be ideal for this case

True. wp_mail() has grown quite complex enough (over 400 LOC) and seems like a good candidate for a dedicated class. But would be a major refactor.

For this reason, and for now, we need to versions: One version checking for the IMAP extension and markTestSkipped if it doesn't exist. Like this
This version can include complex scenarios, like the ones proposed by the OP, with commas and square brackets.
And the other, running very basic tests with very simple addresses (can be multiple addresses, but very simple, like "Name <email(at)example.com>", or just the email).

Okay. Got it. Thanks.

[SirLouen]

@jdeep I feel we are reinventing the wheel here with this closure

You are adapting the array(name,address) format into the "name <address>" format, so after, ​match it back to that format

I think its a great opportunity to do a massive revamp here (​including $to)

So by the time ​all addresses reach this section we already have them sorted out.

There are no filters in between, so there won't be any compat issues.

[jdeep]

@SirLouen I made some refactoring to eliminating closure usage and removing regex parsing.

I think its a great opportunity to do a massive revamp here (including $to)
So by the time all addresses reach this section we already have them sorted out.
There are no filters in between, so there won't be any compat issues.

Yes. This would be a good revamp. The only issue is however, are these two actions:

1. ​wp_mail_succeeded
2. ​wp_mail_failed

These actions pass $mail_data to the action handlers ​which is basically:

<?php
$mail_data = compact( 'to', 'subject', 'message', 'headers', 'attachments' );

Now if we were to change the structure of $to to the return type of parseAddresses(), then it would break existing plugins who use this action.

You can see the draft changes in the ​Pull Request. It simply passes the tests by changing the test assertions itself which is not to be done.

I thought of doing it conditionally but then I would need to write bandaid-fixes later on in the code to maintain backward compatibility which I feel is not good.

I would like to know your approach for this.

[SirLouen]

Yes. This would be a good revamp. The only issue is however, are these two actions:

@jdeep you can simply keep the $to as-is, and then add a $parsed_to to work with.
In fact, I believe that it doesnt matter if you pass an string or an array to parseAddresses
​https://github.com/SirLouen/wordpress-develop/blob/cfcb4e9e168b557ea8867ac2b6729c25906a7f54/src/wp-includes/pluggable.php#L251-L258

It will return the corresponding "parsed_to" with correct values. So you can leave this as-is, and simply create a $parsed_to that you are going to pass to the $address_headers

Also, don't touch the current unit tests, but add new ones with more complex email formats and lists. Ask @tusharbharti, because I believe he was working on something with a lot of emails to test, could be of use.

[tusharbharti]

Hi @jdeep, for list of emails you can use ​https://github.com/WordPress/gutenberg/pull/70709/files#diff-97d7e31909f55f8a55039aba5abc9c00942ee379595ab293be5accdad0886e1d

though some changes are that 1234567890123456789012345678901234567890123456789012345678901234+x@wordpress.org will counted as valid even though it isn't because when we had a discussion regarding email lengths it was agreed upon to not break previous supported email listing that had local part more than 64 chars long.

Ref: ​https://github.com/WordPress/wordpress-develop/pull/5237#discussion_r2256827791

[SirLouen]

First Review + Test

[SirLouen]

@jdeep I have added a patch with a big code review
https://core.trac.wordpress.org/attachment/ticket/62940/62940.diff
And a sample and interesting test, because currently, we are not supporting encoded addresses but with parseAddresses we will.

But we need a couple more tests.

In this patch, I'm also adding support for the From address better not leave them behind. We are not going to be supporting multiple From for many reasons (principally because PHPMailer doesnt not suppor them because dealing with DKIM and PHPMailer is not supported and will not be supported). So Multiple From Addresses is going to be a maybelater for now.

So basically in the remaining tests we should be testing:

1. One From address, Multiple CC, To, BCC, Reply-To, addresses, but with weird forms and charsets for the addresses (like the ones provided in the OP). We will be testing based on a mbstring active (because its going to be always active by default in the testing suite). Forget about the no mbstring environment.

2. The only scenario that is not working is the multiple line header that has to be sorted in #28473. I will cover this in that ticket once this is sorted an merged. So just don't add this. All From/CC/BCC/Reply-To must be single-lines (no \n).

3. You can use a dataProvider. This always looks great. You can use a single test, add all the elements as variables coming from the dataProvider. The dataProvider variables can look like: "Type" and "Address/es".

For example:

"type" => "From",
"addr" => "John Smith <johnny@example.com>"

Then in the test, you can be setting one or another with a case for the test (and can use default values for the other values that are not set). Or maybe you have another idea.

We need to push this fast because the other ticket I mentioned is going to be dependent on this to proceed, and I would like to have them both ready before 6.9.

[jdeep]

Replying to SirLouen:

Thanks for the patch. I am on it.

[SirLouen]

@jdeep I've been thinking and we have to refactor again the code
We cannot parse addresses on the fly
Because the content-type has to be set before we start parsing
If the content type is the last line in the headers, it will be parsed too late leaving the rest of the headers without a content-type (by default its almost always going to be utf-8 but we have to respect the content-type set in the headers

For this reason, we have to do an address parsing after the headers have been set in the switch, not during.

Also, the test you sent me look good. They are simply but escalable. I've been thinking that since you repeat so much the exact same test, maybe we should do them the other way around (testing for all the types the same, and adding new addresses, will simply test for all types). Even you can use the exact same dataProvider for all, including the To (and leave the From dataprovider independent because we can only test with a single address). Remember I'm just giving some ideas that come to my mind, if you have any idea to improve comment it.

[jdeep]

Replying to SirLouen:

@jdeep I've been thinking and we have to refactor again the code
We cannot parse addresses on the fly
Because the content-type has to be set before we start parsing
If the content type is the last line in the headers, it will be parsed too late leaving the rest of the headers without a content-type (by default its almost always going to be utf-8 but we have to respect the content-type set in the headers

For this reason, we have to do an address parsing after the headers have been set in the switch, not during.

I have refactored the code to set the charset from Content-Type before parsing any other headers.

Also, the test you sent me look good. They are simply but escalable.

Okay. I will then add some more testcases.

I've been thinking that since you repeat so much the exact same test, maybe we should do them the other way around (testing for all the types the same, and adding new addresses, will simply test for all types). Even you can use the exact same dataProvider for all, including the To (and leave the From dataprovider independent because we can only test with a single address). Remember I'm just giving some ideas that come to my mind, if you have any idea to improve comment it.

Yup that would work as well. Since the headers of To, Reply-To, CC, BCC are similar and differ only in the header type, we can have a single list of content (address list) for these headers and test them for each header.

[SirLouen]

Replying to jdeep:

Yup that would work as well. Since the headers of To, Reply-To, CC, BCC are similar and differ only in the header type, we can have a single list of content (address list) for these headers and test them for each header.

Looks more solid now.

Ping me when you have updated the tests to the new version

[jdeep]

@SirLouen I have updated the tests.

I'm taking over the review and resolution of PR #9697 because @JDeepD has told me that he is going to be busy for a while.

Here I will be adding the last edits to finally finishing this report.

[SirLouen]

TODO

1. This address cannot be parsed by PHPMailer From: "test<stage" <test@example.com> It breaks with the first angle bracket. ​Reported Upstream

2. Reply-To is unnecessarily inconsistent in PHPMailer. I will leave it away for now, while preserving the same format (as it's not relevant from the wp_mail wrapper). This only affects the test. ​Reported Upstream

It's gonna be hard, because this is blocking two other reports. Problem with all this is that we are going so deep into the mail function, that a ton of problems are arising in an spiral ones blocking each other and then merging changes among all is a party :(