Contrib.social

Invoke hook_mail_alter after MailInterface::format instead of before

Open on Drupal.org →
Created on 14 January 2014, about 11 years ago
Updated 12 April 2023, almost 2 years ago

Problem/Motivation

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.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

✨ Feature request
Status

Needs work

Version

10.1 ✨

Component
Mail  →

Last updated 19 days ago

No maintainer
Created by

🇺🇸United States Garrett Albright

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Needs change record

    A change record needs to be drafted before an issue is committed. Note: Change records used to be called change notifications.

  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment almost 2 years ago →
    needs-review-queue-bot

    The 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 almost 2 years ago →
    🇮🇳India nikhil_110

    Attached patch against Drupal 10.1.x

  • Status changed to Needs review almost 2 years ago
  • Comment almost 2 years ago →
    🇮🇳India nikhil_110
  • Status changed to Needs work almost 2 years ago
  • Comment almost 2 years ago →
    🇺🇸United States smustgrave

    @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

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024