D7 and D8 currently invoke hook_mail_alter() before calling MailInterface::format(). From drupal_mail():
// Invoke hook_mail_alter() to allow all modules to alter the resulting e-mail.
drupal_alter('mail', $message);
// Retrieve the responsible implementation for this message.
$system = drupal_mail_system($module, $key);
// Format the message body.
$message = $system->format($message);
MailInterface::format() is at the least responsible for taking $message['body'], which is an array at this point, and imploding it into a string. See PhpMail::format().
This means that hook_mail_alter() implementations can't be sure of what the "final" state of the message body will be. Spam has been on my mind recently, and I ran into this problem when probing if it would be possible to use hook_mail_alter() to add DKIM headers to a message. Since this requires hashing the message body, and we can't be sure of what the message body is when this hook is invoked, it is not possible. If we can assume that MailInterface::mail() won't alter the message body, then moving the hook invocation after the ::format() call would make adding DKIM headers and similar things possible in hook_mail_alter() implementations.
| Comment | File | Size | Author |
|---|---|---|---|
| #26 | 2173633-26.patch | 1.63 KB | nikhil_110 |
| #25 | 2173633-nr-bot.txt | 144 bytes | needs-review-queue-bot |
| #19 | 2173633-19.patch | 1.1 KB | hardik_patel_12 |
| #5 | 2173633-5.patch | 1.8 KB | rpayanm |
Comments
Comment #1
Garrett Albright commentedForgot to update hook_mail_alter() documentation accordingly.
Comment #2
dawehnerThis sounds like a really good idea to be honest. I wonder whether we have tests for that which we could expand.
Comment #3
Garrett Albright commentedhook_mail_alter() doesn't seem to be covered by tests very much, outside of simpletest.module having an implementation that just sets
$message['send'] = FALSEto test a hook implementation aborting the sending of a message. But then again, it's a pretty straightforward hook, so I'm not sure what else we'd really want to test outside of that.Comment #4
alansaviolobo commentedComment #5
rpayanmComment #6
Garrett Albright commentedPatch applies and achieves the desired result.
Comment #7
alexpottI think we should be testing this and we definitely need a change record. And personally I can see reasons for altering the body and reasons for alter after. So perhaps we need two hooks.
It is possible to send DKIM signed email from Drupal - you just need to send it through a smtp server that adds it.
Also as a feature request this must wait till 8.1
Comment #17
jungleComment #18
jungleComment #19
hardik_patel_12 commentedRe-rolling for 9.1.x , kindly review.
Comment #25
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #26
nikhil_110 commentedAttached patch against Drupal 10.1.x
Comment #27
nikhil_110 commentedComment #28
smustgrave commented@Nikhil_110 thank you for the interest but you have been doing this for a while now. Please read the comments and tags before putting into review.
Issue summary has no proposed solution.
Was previously tagged for tests and change record neither have happened.
Please do some actual checks before putting into review