CakePHP Mailer does not allow multiple headers of the same name

[burzum]

This is a (multiple allowed):

• bug

• enhancement

• feature-discussion (RFC)

• CakePHP Version: 3.6.x

What you did

->addHeaders(['o:tag' => $newsletter->get('id')])
->addHeaders(['o:tag' => 'swissmailer'])

What happened

Ending up with just o:tag: swissmailer

Duplicate headers are allowed for specific types and third party headers. See https://stackoverflow.com/questions/18039142/email-legitimate-duplicate-email-header-keys-with-mutually-exclusive-values

What you expected to happen

Getting all the headers in the email.

[markstory]

Changing the existing behavior of addHeaders() could break existing applications that rely on the overwrite as a 'feature'. We would be better off adding a new method. For responses we have withHeader() and withAddedHeader(). Does anyone have a suggestion for a new method name on Email?

[ravage84]

How about withAdditionalHeader() (preferred) or withAnotherHeader()?

[garas]

Email is mutable object, and with*() method would suggest instance is immutable and returns another instance like Response.

I would prefer add parameter $replace = true similarly to PHP header()

[markstory]

@garas Modal flags on methods are not an API pattern I'm a huge fan of. It requires memorizing the API or looking at the documentation to understand what the flag does.

[josegonzalez]

Break it for 4.x?

[JoshuaLuckers]

addHeadersAndMergeRecursive()

[dereuromark]

2nd param as BC flag for 3.x sounds reasonable if we can then fix it for 4.x using merge default true. Separate methods could work for both major versions, though.

[markstory]

Adding a new method for the append operation is my preference as it requires the least churn overall.

[ADmad]

One way to solve this without any API changes would be allow providing the value as array:

->addHeaders(['o:tag' => [$newsletter->get('id')]])

The, subsequent call like

->addHeaders(['o:tag' => ['swissmailer'])

would merge the values for that particular header.

addHeaders() method would be updated to use Hash::merge() instead of array_merge() and AbstractTransport::_headersToString() updated to account for values being an array.

[markstory]

@ADmad I like that approach over breaking the existing behavior.