wp_mail header ignores multiple headers with same name

[pentatonicfunk]

Context:
​https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/pluggable.php#L344
Snippet wp_mail

default:
    // Add it to our grand headers array.
    $headers[ trim( $name ) ] = trim( $content );
    break;

Passing headers this way:

$headers[] = 'x-my-things: thing1';
$headers[] = 'x-my-things: thing2';

Will resulting headers only generated one ( the last one )

$headers['x-my-things'] = 'thing2';

Expectation, it should retain the headers index, should behave similarly with phpmaailer::addCustomHeader, headers indexed by numeric, and each item contains name and value.

Example use case Mailgun x-mailgun-tag, ​https://documentation.mailgun.com/en/latest/user_manual.html?highlight=x-mailgun-tag

[SirLouen]

Reproduction Report

Description

✅ This report validates whether the issue can be reproduced.

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.29
• Server: nginx/1.29.0
• Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.29)
• Browser: Chrome 138.0.0.0
• OS: Windows 10/11
• Theme: Twenty Twenty-Five 1.2
• MU Plugins: None activated
• Plugins:
  • Micro Email Testing 1.0.0
  • Test Reports 1.2.0

Steps

1. Instructions as explained in OP, using two headers with same key, different value
2. 🐞 Only second value will prevail

Actual Results

• ✅ Error condition occurs (reproduced).

Additional Notes

• I can still see your point of why someone might need to add multiple identical header tags (updating Mailgun ​docs here)

• In fact, this is not an enhancement, but a regression that was introduced in [5639]. Historically, headers added to AddCustomHeader in the default case. But in this commit, headers were firstly trimmed to their keys.

Problem is that the fix will be a little less direct as it should because ​some lines after there is a $phpmailer->clearCustomHeaders

So the solution as suggested could go, to iterate adding another array with id => value to the current created default array, and then ​releasing those headers with an extra iterator.

Having this in mind, it doesn't look too difficult, so I will add good-first-bug in case someone wants to take this.

[rishabhwp]

I was able to reproduce the issue and would love to work on fixing it.

[SirLouen]

Replying to rishabhwp:

I was able to reproduce the issue and would love to work on fixing it.

Sure, go for it, I will be reviewing it when you send it.

[SirLouen]

Hey @rishabhwp sorry for the late reply
Thanks for submitting a patch.

It looks good, but it feels a little redundant despite I can clearly see how this could be potentially causing some backwards compatibility issues.

For example here

// Handle multipart boundary if present.
		if ( ! empty( $headers ) ) {
			if ( false !== stripos( $content_type, 'multipart' ) && ! empty( $boundary ) ) {
				$phpmailer->addCustomHeader( sprintf( 'Content-Type: %s; boundary="%s"', $content_type, $boundary ) );
			}
		}

$headers is not being used at all, its just a check to see if we have to build manually the boundary or not (which in fact is completely broken right now, because PHPMailer doesn't support this format, so basically those 3 lines are completely useless.

Decision Required

The problem appears here:
​https://github.com/SirLouen/wordpress-develop/blob/965ee6152caa8a3e2bea2bf18e405220e45c53df/src/wp-includes/pluggable.php#L296-L297

The original headers should be correctly formatted to accept pretty much any scenario. But there, they are flattened to form a simple 2 dimensional associative array.

So basically the key here is to create some Unit Tests, and check if we can simply upgrade $headers to a array of associative array structure and use it the way you are using it, without breaking the system, or keep the header structure flat, but in the original form.

The only risky point I see for BC issues is in wp_mail_succeeded action, since most users extending this hook, might be taking the simple string array, instead of a new improved version with an associative array.

The pre_wp_mail, wp_mail filters hooks are taking the headers as they are originally. But the wp_mail_succeeded is taking the wrong watered down version

But this version is intrinsically buggy (as the bug here itself). We could flatten the associative array (as its being done right now) but with the original format, or we could simply break the backwards compatibility in this action with the array of associative arrays. In both cases, if extenders are consuming this $headers like a simple associative array, they are going to find their functions broken, so I would prefer the later option which makes all the sense to me because users are consuming wrongly designed headers.

Once we sort out this, we can continue with the patch.

[SirLouen]

#30128 supersedes this report.

So I can conclude that we can easily hit two birds with one stone here and close this in favor of the other one. It will be way worse if we want to separate these two tickets because the logic is very closely related to each other, so we will end in unnecessary merging conflicts that I would prefer to avoid.