Ensure origin headers of mails sent using the mailer.transport service comply to RFC5322

Postponed on 📌 [PP-1] Add symfony mailer transports to DIC Postponed

Great, this seems like a very thorough and precise treatment of these two headers - although I didn't check the standards myself to confirm correctness.

Message::getPreparedHeaders() has code that overlaps/conflicts with this issue (I'm looking at v6.4.17). It will set from based on sender so potentially we don't need setDefaultFrom(). And it will set sender based on from, which will undo the work of removeRedundantSender().

I'm adjusted the terminology in the title/IS slightly. Emails will pass through the transport layer, but they aren't sent directly to it as there are other layers on top (including delivery and likely another Drupal-specific one that we create).

I see this as a part of ✨ Create a new mailer service to wrap symfony mailer Active - it's one of the things that should be done as part of building every email. There is similar, but simpler code in MailManager. We could agree to create some Drupal-specific email events, in which case this should be adjusted to use them. So I feel that "postponed" is the correct status for a couple of weeks more.

@znerol How does that all sound to you?

Message::getPreparedHeaders() has code that overlaps/conflicts with this issue (I'm looking at v6.4.17). It will set from based on sender so potentially we don't need setDefaultFrom().

Interesting idea. I tested that. The very first test fails at line 55 (last assertion in the following snipped).

    $originalEmail = $sentMessage->getOriginalMessage();
    assert($originalEmail instanceof Email);
    $actualFrom = $originalEmail->getFrom();
    $this->assertEquals([$expectedAddress], $actualFrom);

The test expects that $originalEmail->getFrom() contains the site address. This isn't the case, because Message::getPreparedHeaders() does only derive, but not save the from header.

However, imagine a message subscriber which logs messages which failed. This might want to log the from header as well.

And it will set sender based on from, which will undo the work of removeRedundantSender().

That is not true. removeRedundantSender() only removes sender if there are multiple from addresses. Message::getPreparedHeaders() only adds sender when there is none and when there are multiple from addresses.

Regarding the process, nothing blocks this. Even if the implementation should change at some point, its good to have the tests in place. And I guess it will take some time and energy to review them as well.