The user_mail($mail, $subject, $message, $header = NULL) function has this code:

  $default_from = variable_get('site_mail', ini_get('sendmail_from'));
  if ($default_from) {
    $defaults['From'] = $defaults['Reply-To'] = $defaults['Sender'] = $defaults['Return-Path'] = $defaults['Errors-To'] = $default_from;
  }
  $mimeheaders = array();
  foreach ($defaults as $name => $value) {
    $mimeheaders[] = $name .': '. mime_header_encode($value);
  }
  $headers = join("\n", $mimeheaders);
  if (!is_null($header)) {
    $headers .="\n".$header;
  }

The effect of this is that if header information is already passed to the function, as it is for example by contact.module and many contrib modules, the function appends conflicting headers establishing the default "From" email, even though such headers may already exist. This leads to unpredictable results, where the 'from' address in an email is determined based on how a particular email client handles to conflicting headers. This is not a problem for Drupal 5, however, because drupal_mail uses a separate $from argument.

This code, by the way, was added as a result of this issue: http://drupal.org/node/133789 -- I'm not too sure what the intention is with that change, but if someone can explain it, I'd be happy to submit a patch that doesn't interfere with that task. Maybe we could just check to see if 'From:' is present in $header, and not add the default from headers if so?

CommentFileSizeAuthor
#2 user_headers.patch1.89 KBmcarbone
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mcarbone’s picture

On second thought, maybe this code should just be entirely removed. Since 4.7 leaves the headers up to the developer anyway, why try to prevent one small possible problem out of many? contact.module passes valid headers, and there's no reason to assume that contrib modules don't also.

mcarbone’s picture

Status: Active » Needs review
FileSize
1.89 KB

Well, here's a patch that removes all the recently added default header code. As I said, I don't see why that code should've been added to Drupal 4-7, where headers are already entrusted to the developer to form correctly.

drumm’s picture

Status: Needs review » Needs work

- Any issues with contact module do need to be fixed. Some specific detail on this would help us understand the problem.
- These headers are useful and we should keep them for at least the default mail handler. They can be omitted for mails passed to 3rd party mail handlers.

mcarbone’s picture

1) I don't see it as an issue with the contact module, which passes in perfectly correct headers.

2) This code was added recently, when a similar change was made to Drupal 5 and later. It makes sense for those versions, but it doesn't work for the API as defined for 4.7. user_mail expects fully constructed headers, and has always done so until this code was added a few months ago. Perhaps it can be changed to only add the headers if the $header variable is NULL, but even that contradicts the API as previously designed (i.e., the headers are defined by the function call only).

killes@www.drop.org’s picture

Status: Needs work » Fixed

I reverted the original patch.

Anonymous’s picture

Status: Fixed » Closed (fixed)