Ensure that the Return-Path is set when sending mail on both Windows and non-Windows systems.

• Please set the status of your issues with patches to "patch (code needs review)". This will make it show up in the patch queue, so others can find it and look at it.
• For security reasons, you should use mime_header_encode() on the return path when passing it to mail(). That way, modules don't need to worry about header injection exploits.
• You should use CVS to roll your patches. It is not only much faster, but it's also much easier for us to apply and test your patch locally. You should also roll your patches from the root of a Drupal tree and with the right arguments. Check the handbook for cvs diff instructions.

What happens if you're not using sendmail?

scor, why not just set the 'Return-Path' as part of the initial assignment of the $defaults array?

we only support php 4.3.3 and higher

(personally i think it's silly some created a module for a one line patch)

it's a pain in the ass getting enough reviews to get something committed. but it needs testing because if it gets committed and breaks a bunch of people's sites it's a pain in everyone's butt.

I do not think this patch avoids or gracefully handles this:

The user that the webserver runs as should be added as a trusted user to the sendmail configuration to prevent a 'X-Warning' header from being added to the message when the envelope sender (-f) is set using this method. For sendmail users, this file is /etc/mail/trusted-users.

http://us3.php.net/manual/en/function.mail.php

This patch especially needs testing on a Windows server.

#252502: Sent emails rejected by mail servers when using PHP mail() was a duplicate.

Also bumping release because that "issue" (I would say "inconvenience", because drupal_mail_wrapper() exists just for this purpose) is present in all supported versions.

subscribing

I think this is a pretty important issue which I duplicated. But I also don't know of any portable way to achieve this (except not relying on PHP's mail()). The -f parameter is a sendmail option. Fortunately, all popular mail packages always come with a bin compatible with /usr/bin/sendmail. So -f should be fine across different *nixes.

Windows remains. Anyone on windows?

One key item to note here is the mail header. Unless the user that apache is running as is added to the /etc/mail/trusted-users file Sendmail will append a warning in the email header like this:

X-Authentication-Warning: webserver.hostingcompany.com: user set sender to address@somedomainname.com using -f

This is caught by most/all spam filtering agents (or at least the good ones). Make sure your provider has this configured, I have been with several that did not but after a brief call with thier support most agreed to make the change (one even changed thier server build SOP to add the apache user as a trusted user on every build!)

Subscribing. Updated patch for d5.

Here is the working patch for D7 from the duplicate issue...

@Damien in #13:
No. This is not just an incovenience. We're passing the header wrongly to PHP's mail() function - that's nothing less than a bug in core.

We can't expect admins to install a contrib module, only to achieve Drupal's core functionality works correctly.

Plus: contrib can not even solve it correctly. Return path module (which currently fixes it) is not compatible with other mail backends (see #98412: Incompatible with smtp.module (redeclared function)), because they all use drupal_mail_wrapper(), and it can be used only by one module at the same time.

So please let's go on testing and improving this, so this enerving bug finally gets fixed.

@Pancho: in that case, you will need to do your homework. Will this "-f" flag works with non-sendmail backends? With Postfix? With Exim? With QMail? Under Windows? In safe mode? On shared hosting?

Also note that the "we can't have several mail backend" issue is being solved for D7 in #331180: fix pluggable smtp/mail framework.

Sure. We can do all these tests on different backends, platforms and configurations. And surely I'm ready to take a major part of that work on me.

But since when is it the patch writer's duty to do all the testing work, "my homework" as you call it? I didn't say the patch were rtbc, did I? Sorry, but the way you put it is not okay.

Now, I'm working on another issue atm. But as soon as I'm done with that, I'll come back to this. Then I will check #331180, possibly rework this patch here and write a testing plan.

@22: #9 says this can work with postfix, Exim 4.63 and qmail. It won't work with safe mode (would be best not to try -f in this case). In Windows there is the possibility of ini_set()-ting sendmail_from. I have -f working very nicely on cPanel shared hosting (PHP running as Apache module) - without any X-Warning header. See also http://drupal.org/node/165938#comment-1153548). I think cPanel automatically sorts out the question of "trusted" users.

The X-Warning header may be the killer though - see #12 and #16. Even if that is not a problem, this is all a bit of a mess :P

BTW is there a use case for using returnpath and smtp contrib modules at the same time? SMTP servers seem to set the Return-Path: for you. (OK, returnpath and a mime email backend is a realistic but incompatible combination in D6/D5).

[slightly updated]

The popular PHPMailer library just does the following (simplified):

public function MailSend($header, $body) {
  ...
  $params = sprintf("-oi -f %s", $this->Sender);

  if (strlen(ini_get('safe_mode')) < 1) {
    $old_from = ini_get('sendmail_from');
    ini_set('sendmail_from', $this->Sender);
    @mail($to, $this->EncodeHeader($this->SecureHeader($this->Subject)), $body, $header, $params);
  }
  else {
    @mail($to, $this->EncodeHeader($this->SecureHeader($this->Subject)), $body, $header);
  }
  if (isset($old_from)) {
    ini_set('sendmail_from', $old_from);
  }
}

I.e. the additional flag(s) are set if defined safe_mode is disabled, and 'sendmail_from' is overidden for Windows.

The same is done for the sendmail method.

Yes...

... and if it doesn't work on your particular server (i.e. you get X-Warning headers) then you can try another sending method, e.g. sendmail (as you mention), or smtp.

FWIW I think Joomla bundles PHPMailer, but from a quick glance at the code I couldn't work out which sending method it uses or how it handles this problem.

Subscribe.

I'm confused about these lines in http://drupal.org/files/issues/do-the-return-path-correctly_9_0.patch:

+    // If 'Return-Path' isn't already set in php.ini, we pass it separately
+    // as an additional parameter instead of in the header.
+    // However, if PHP's 'safe_mode' is on, this is not allowed.
+    if (isset($headers['Return-Path']) && !ini_get('safe_mode')) {
+      $return_path_set = strpos(ini_get('sendmail_path'), ' -f');
+      if (!$return_path_set) {
+        $message['Return-Path'] = $headers['Return-Path'];
+        unset($headers['Return-Path']);
+      }
+    }

This does not allow you to override the Return-Path using the mail_alter hook. Was this intentional?

I've adapted the patch for a site I'm maintaining to:

+    if (isset($message['headers']['Return-Path'])) {
+      $return_path_set = strpos(ini_get('sendmail_path'), ' -f');
+      if (!$return_path_set) {
+        $message['Return-Path'] = $message['headers']['Return-Path'];
+        unset($message['headers']['Return-Path']);
+      }
+    }

Should this not be the intention instead of the original code? I'm guessing it was, since it was placed after the drupal_alter() call.

subscribing

subscribing

I agree with @malc0mn in #28. Attached is the modified patch:

Subscribing.

@mrfelton: shouldn't the status be "needs review" right now?

@nvanhove: yes, forgot to update status. thanks.

@nvanhove: yes, forgot to update status. thanks.

Hmmm patch didn't work. Probably because it is so old.
@mrfelton: could you redo the patch?

Recreated #31

I can confirm that this has helped us out on numerous occasions. How about adding it to the core in a new release?

@malc0mn, if you can confirm the patch in #37 works in Drupal7, we could get it in :)

Good point, I'll see what I can do!

I think this is a valid bug report, and from what I can tell (and reading the PHP docs), the fix looks correct.

Just finished setting up the latest dev release as a sandbox on our dev environment to test the patch.

EDIT:

Version used: February 3, 2010 - 14:04, drupal-7.x-dev.tar.gz, 2.19 MB e2145089f78c88d46660d7973906701b

Patching went nicely:

$ patch -p0 < mailreturnpath.patch 
patching file includes/mail.inc
patching file modules/system/system.mail.inc

And for the testing:

1. Logs without patch:

Feb  3 15:02:14 development sendmail[94598]: o13E2EX8094598: from=www, size=953, class=0, nrcpts=1, msgid=<201002031402.o13E2EX8094598@ourserver.be>, relay=www@localhost

Feb  3 15:02:14 development sm-mta[94599]: o13E2E6W094599: from=<www@ourserver.be>, size=1182, class=0, nrcpts=1, msgid=<201002031402.o13E2EX8094598@ourserver.be>, proto=ESMTP, daemon=IPv4, relay=localhost [127.0.0.1]

Feb  3 15:02:14 development sendmail[94598]: o13E2EX8094598: to=my@mail.com, ctladdr=www (80/80), delay=00:00:00, xdelay=00:00:00, mailer=relay, pri=30953, relay=[127.0.0.1] [127.0.0.1], dsn=2.0.0, stat=Sent (o13E2E6W094599 Message accepted for delivery)

Feb  3 15:02:14 development sm-mta[94601]: o13E2E6W094599: to=<my@mail.com>, ctladdr=<www@ourserver.be> (80/80), delay=00:00:00, xdelay=00:00:00, mailer=relay, pri=31182, relay=[X.X.X.X] [X.X.X.X], dsn=2.0.0, stat=Sent (Ok: queued as B82BA33C14)

Bad, baaaad! ;-)

2. Logs with patch:

Feb  3 15:03:55 development sendmail[94618]: o13E3tlb094618: Authentication-Warning: development.int.nascom.be: www set sender to drupal.site@admin.be using -f

Feb  3 15:03:55 development sendmail[94618]: o13E3tlb094618: from=drupal.site@admin.be, size=911, class=0, nrcpts=1, msgid=<201002031403.o13E3tlb094618@ourserver.be>, relay=www@localhost

Feb  3 15:03:55 development sm-mta[94619]: o13E3t52094619: from=<drupal.site@admin.be>, size=1322, class=0, nrcpts=1, msgid=<201002031403.o13E3tlb094618@ourserver.be>, proto=ESMTP, daemon=IPv4, relay=localhost [127.0.0.1]

Feb  3 15:03:57 development sendmail[94618]: o13E3tlb094618: to=my@mail.com, ctladdr=drupal.site@admin.be (80/80), delay=00:00:02, xdelay=00:00:02, mailer=relay, pri=30911, relay=[127.0.0.1] [127.0.0.1], dsn=2.0.0, stat=Sent (o13E3t52094619 Message accepted for delivery)

Feb  3 15:03:57 development sm-mta[94621]: o13E3t52094619: to=<my@mail.com>, delay=00:00:02, xdelay=00:00:00, mailer=relay, pri=31322, relay=[X.X.X.X] [X.X.X.X], dsn=2.0.0, stat=Sent (Ok: queued as 4B32433C14)

Saweeet! ;-)

NOTE: the warning you see in the logs nicely shows the patch is doing what it should. The warning can be avoided by adding www as trusted mail user.

Nice work.. This realy should be into core!

The comments mention safe_mode but the current version of the patch doesn't actually check for safe_mode.

Bad news. If PHP is run in safe mode, this patch will trigger some errors:

    *  Warning: mail(): SAFE MODE Restriction in effect. The fifth parameter is disabled in SAFE MODE in DefaultMailSystem->mail()  (line 76 of /data/drupal7-sandbox/static/drupal-7.x-dev/modules/system/system.mail.inc).
    * Unable to send e-mail. Contact the site administrator if the problem persists.

Somwhere along the way we seem to have dropped this bit of code:

<?php
 && !ini_get('safe_mode')
?>

I have attached a modified patch to correct this, and looking into a 2nd problem that has been introduced by this line:

<?php
$message['Return-Path'] ? '-f ' . $message['Return-Path'] : ''
?>

-f is not allowed in the safe mode.

That seems to be my patch instead of yours?

OK, I think the patch attached here does the trick, please ignore or even better delete the patch in my previous post since I cannot seem to do this myself.

The annoying thing is that when we're in safe mode, the fifth parameter in the PHP mail() function is not allowed. You can pass whatever you want (null, an empty string, ...) PHP will complain about this, hence the somewhat silly approach in the patch to work around this. If someone has a better idea, please let me know!

My fix in attach which only patches the modules/system/system.mail.inc file, not the includes/mail.inc file.
The previous patch swapped the order of some variables in incudes/mail.inc which in my humble opinion, and proven during my testing, makes no difference.

@#48: only slightly modified. I should not have posted it, for which my apologies. If only I could delete it...

I will fix this!

@#51: revised patch in attach (formatted else & comments running to col 80)

Refer to #49 for more information concerning the patch.

I simply took them form the original patch, however I shall remove them.

New patch with fixed formatting attached, as requested.

+++ modules/system/system.mail.inc	(working copy)
@@ -41,23 +41,50 @@
+    if(isset($message['Return-Path']) && !ini_get('safe_mode')) {

Missing space after "if".

+++ modules/system/system.mail.inc	(working copy)
@@ -41,23 +41,50 @@
+      $mail_result =  mail(
...
+      $mail_result =  mail(

Duplicate space here.

+++ modules/system/system.mail.inc	(working copy)
@@ -41,23 +41,50 @@
+        // Note: e-mail uses CRLF for line-endings. PHP's API requires LF
+        // on Unix and CRLF on Windows. Drupal automatically guesses the
+        // line-ending format appropriate for your system. If you need to
+        // override this, adjust $conf['mail_line_endings'] in settings.php.
+        preg_replace('@\r?\n@', $line_endings, $message['body']),
+        // For headers, PHP's API suggests that we use CRLF normally,
+        // but some MTAs incorrectly replace LF with CRLF. See #234403.
+        join("\n", $mimeheaders),
...
+        preg_replace('@\r?\n@', $line_endings, $message['body']),
+        join("\n", $mimeheaders)

Ideally, we should prepare those two arguments before the conditional execution logic, so 1) no duplicate code and 2) no missing docs in the else case.

+++ modules/system/system.mail.inc	(working copy)
@@ -41,23 +41,50 @@
+      // the fifth parameter is not allowed in safe mode no matter what you set
+      // for the fifth parameter (NULL, '', ...), PHP will always 'complain' so
+      // this means bad headers when in safe mode, unless php.ini is set up
+      // correctly

This comment needs some love. :)

"The optional $additional_parameters argument to mail() is not allowed if safe_mode is enabled. Passing any value throws a PHP warning and makes mail() return FALSE."

Powered by Dreditor.

@58: very true concerning the preparation of the arguments... Why didn't I see/do that in the first place :(

New patch in attach solving:

- space issues 'if (' and ' = mail('
- lovely comments
- preparation of mail() arguments

Done...

Thanks - looks good to me.

There is some wrong indentation (single space instead of two)

+++ modules/system/system.mail.inc	(working copy)
@@ -41,23 +41,53 @@
+     }
+     else {

Takes a bit to see that there's one leading space too much here, right ;)

Powered by Dreditor.

@64: Removed those...

@malc0mn don't forget to set the status back to needs review so the testbot will pick it up.

Ah ratz, thanks nvanhove.

Thanks! There we go.

Fantastic!

Shall I do one for the D6 branch as well?

Return-Path.patch queued for re-testing.

@70: that patch would break when running in safe mode... I have a D6 one ready, but want to test it first before submitting. Will be submitting tomorrow morning after testing.

But we'll have to change from 7.x to 6.x in the satus-settings-thingy, no?

@scor: I was aware of that, hence my question for the D6 one. I have it ready and will post tomorrow.

Can I simply change the version of this issue to '6.x-dev' (& status to 'needs review')when posting the D6 patch or do I need to start a new issue?

Thanks scor, will do that!

@74: Scor, D6 patch attached. Made against latest Drupal6 HEAD branch.

Tested in both regular and PHP safe mode.

Committed to CVS HEAD. Moving to D6.

Dang, I usually do that... Apparently forgot it here. Done properly now in attach.

component = mail system + needs review

Anyone had a chance to review this one for D6...? Used it a while now and encountered no probs...

#17 worked for me for D5. However, I also had to add this to my theme in order for the Return-Path to be the right address when a webform is submitted and email is sent:

<?php
function phptemplate_webform_mail_headers($form_values, $node, $sid, $cid) {
  $email_form_value = $node->webform['email_from_address'];
  $headers = array(
    'X-Mailer' => 'Drupal Webform (PHP/'. phpversion() .')',
    'Return-Path' => $form_values['submitted'][$email_form_value],
  );
  return $headers;
}
?>

Sorry if this is a really dumb question...but where is the envelope address configured in the Site Configuration?

I'm assuming the intent here is to make it per-site rather than per-server so that in a multi-site environment the email is fully tailored to each site.

I don't see any way of doing this in the D6 configuration at present and as far as I could tell the patches don't seem to add an extra configuration option. So maybe it's there already and I'm just not looking in the right place?

cheers

There is no configuration option.

We set the Envelope to be as the "From" address:
$headers['From'] = $headers['Sender'] = $headers['Errors-To'] = $headers['Return-Path'] = $default_from;

Configurability could be a nice feature request for Drupal 8. My experience is you want the "From:" address to be customer service and the envelope "From" to be technical staff who can handle bounces etc.

I was rather hoping that it would get dropped into D6. I'm using an edit to php.ini to fix this at present and it's not really the right solution. That's what started me searching the forums in the first place.

I think the configurable nature of this would be critical. It should (a) be configurable, and (b) the two addresses should have independent access control so that the envelope From can only be set by the technical staff.

The current solution would, I suspect, open up the possibility of an incorrect address entry silently killing all email from a site.

cheers

FYI

Out of curiosity - and as part of trying to solve this in my own environment - I looked at the headers from a drupal.org email:

Return-Path: <apache@drupal.org>
Errors-To: info@drupal.org
Sender: info@drupal.org
From: info@drupal.org

cheers

Why does the patch get the return path from the -f argument in sendmail_path instead of from sendmail_from?

(Referring to properly_formatted_mail_return_path_d6_0.patch )

Patch in #79 works well for me in 6.17

Applied #79 on two D6.17-installations. Works. Thanks very much!

This issue is going on for three years, and looks like only half of these notes were addressed, repeated by Steven, Damien and drumm:

The additional_parameters parameter can be used to pass additional flags as command line options to the program configured to be used when sending mail, as defined by the sendmail_path configuration setting. For example, this can be used to set the envelope sender address when using sendmail with the -f sendmail option.

The user that the webserver runs as should be added as a trusted user to the sendmail configuration to prevent a 'X-Warning' header from being added to the message when the envelope sender (-f) is set using this method. For sendmail users, this file is /etc/mail/trusted-users.

From feedback above, looks like other tools are sendmail compatible, so we can reliably pass the -f argument to those. However, two things I've not seen solved:

1. Windows testing was requested multiple times but nobody said they did it.
2. The trusted user issue was not resolved so looks like we are trading one problem for another nasty one.

Who has the answers?

I applied patch #79 correctly but no mails come back. Why?
How can I find out, why this does not work?

Scor told me to push this back to d7.
I got the following error on my hosting site when a user registrates.
I don't know why, I don't understand what's going on.

Warning: mail(): Policy restriction in effect. The fifth parameter is disabled on this system in DefaultMailSystem->mail() (regel 77 van /customers/chirohoepertingen.be/chirohoepertingen.be/httpd.www/modules/system/system.mail.inc).

Host: one.com

I'm open for any kind of experiment...

If that also means that no e-mail was/is sent, then I suppose this issue needs to be critical, as users installing D7 may not be able to send or get any mails.

mail() is one of those PHP functions that gets abused by some hosters. I think we should do the same as for set_time_limit(), and just ignore the warnings triggered by this function using the @ operator. We do have the return value of the function to tell us if the mail was queued or not anyway.

No problem at all with the mails. User gets created, mails arrive. Just a stupid warning that scares people ;).
Can someone write a patch to ignore this?

(so I can fix my website)

Solved my issue, for me this is rtbc, but I'm not a mail code guru.

The caution note for Windows on PHP's mail() doc page can be disregarded for 5.2 as it already handles this automatically (this may not be true for PHP < 5.2):

/* This pattern escapes \n. inside the message body. It prevents
 * premature end of message if \n.\n or \r\n.\r\n is encountered
 * and ensures that \n. sequences are properly displayed in the
 * message body. */
#define PHP_WIN32_MAIL_DOT_PATTERN      "\n."
#define PHP_WIN32_MAIL_DOT_REPLACE      "\n.."

On Windows when there is no Sendmail, the 5th parameter passed to mail() is ignored, so it doesn't use -f as that's a Sendmail (and friends) parameter. PHP will use its own internal SMTP mail implementation which gets "Return-Path:" from the ini variable "sendmail_from" and if that is not set, it gets "Return-Path:" from "From:".

X-Authentication-Warning is a problem for any software that uses Sendmail with the -f parameter when its user is not on the trust list. This includes Wordpress, Web-based mailers such as Squirrelmail and popular forums such as Vbulletin, etc. This header is added by Sendmail itself and it's a server configuration problem. There is nothing Drupal can do to deal with this short of locating and parsing the server's Sendmail configuration files (and having a trusted-user file is optional btw).

Worst case scenario is that the email with the X-Authentication-Warning header set will be rejected by the receiving SMTP server, however, the SMTP server would have to have very strict policies for it to reject it based on that alone. And this header is not exactly uncommon, even for legit emails.

As #95 points out, PHP error notices should be inhibited when calling mail(), since there's still a good probability that mail() succeeded.

In order to support Return-Path on Windows we need to set the sendmail_from ini variable. This is what the PHPMailer class in Wordpress does.

I have added the patch from #79 with wincvs, but how can i set a return_path. Where do i need to config that?

Return-Path.patch queued for re-testing.

Return-Path.patch queued for re-testing.

Bumping and tagging as per standard policy.

Re-rolled #100 against 8.x according to current patch guidelines. No other changes.

#106: DefaultMailSystem-return_path-131737-106.patch queued for re-testing.

#106: DefaultMailSystem-return_path-131737-106.patch queued for re-testing.

Improved patch specifically tests for Windows before modifying the Windows-only setting.

Sorry; inadvertently included another patch.

Here's the correct one. (Wish there was some way to remove an inadvertently-added patch from the testing queue....)

Better title and comments.

Applied patch in #112 to HTML Mail and Mail System:

• HTML Mail 6.x-2.x
• HTML Mail 7.x-2.x
• HTML Mail 8.x-2.x
• Mail System 6.x-2.x

@pillarsdotnet: Did you test this patch in safe_mode? You can't use additional parameters in safe mode, even if it's NULL it will throw a warning and mail() will return FALSE. However, the rest looks good.

Warning: mail() [function.mail]: SAFE MODE Restriction in effect. The fifth parameter is disabled in SAFE MODE in ...

@pillarsdotnet: I see you have fixed this in HTML Mail, a new patch here still would be nice.

Yeah, I found that out and had to fix it on the above projects. Re-rolling...

#116: DefaultMailSystem-return_path-131737-115.patch queued for re-testing.

#116: DefaultMailSystem-return_path-131737-115.patch queued for re-testing.

#116: DefaultMailSystem-return_path-131737-115.patch queued for re-testing.

FWIW, on my host, mail sending fails with no error message. If I remove the space in the '-f ' flag, it works fine: '-f' . $message['Return-Path']

If it doesn't matter otherwise, might be worth removing the space.

@#120: Note that the patch in #116 does remove the space between -f and the return-path address.

On my host (*nix system, safe mode disabled) mail sending fails when passing a fifth parameter to mail(). There should be a configuration option to disable the use of the fifth parameter.

Hitting some strange return-path issues with Windows server. Additionally, this issue could use a little better summary (if possible), as there's a lot of discussion since the OP that takes a bit of time to work through...

I had this problem on one site and it was driving me crazy. The D6 patch posted #76 fixed it. I initially tried putting a ini_set('sendmail_path', '/usr/sbin/sendmail -i -t -fmyemail@returnaddr.com') in the settings.php file, but that didn't work for some reason. Might have something to do with the emails getting sent in a cron run.

We are still waiting for someone to review (and mark as RTBC) the patch in #116.

If nobody is willing to do that, we might as well mark this issue closed (won't fix).

Closing for lack of interest. Please re-open only if you are willing to provide a patch review.

Ok I had a lot of problems with this on my localhost (windows) and on my live sites. (one.com)
The patch works and removes all the warnings.
All mails get delivered.

On localhost there is one warning left: "Error occured, contact administrator if problem persist" (my own translation cause the site is in dutch. Anyway thnx for the patch :D

Note: tested in d7 had to reroll the patch in 116

#116: DefaultMailSystem-return_path-131737-115.patch queued for re-testing.

Here is a D8 reroll if its needed

It seems that this solution is well-tested. See #128, #124, #120, and relevant affected contrib projects in #113.

(Note for reviewers, as I did a double-take on this: The lack of space between the -f flag and the return path is intended. See #120 / #121.)

I can't think of a way to write a test for this bug, since it is tied to specific server/environment configurations.

If this passes testbot, I believe this is RTBC.

The thing is, even if we could write an automated test, it'd be written in a way that only runs when Windows is present so the testbot would happily ignore. We certainly wont be worse off this committed than not.

Edit: The problem with the test is not Windows but sending a mail for real and checking results.

I would invert the logic of the if/then; this code is far more likely to run on non-Windows hosts, so that should be default check.

On it.

Inverted the logic and switched the order of the if/else. Sending for a date with testbot.

Windows machine localhost fails:

"Unable to send e-mail. Contact the site administrator if the problem persists."

Could be a localhost xamp issue.
Other live hosts (not windows) don't report errors anymore.

Ok lets clarify :)

- Before patch:

* cheap shared hosts ==> mail arrives but warnings because of fifth parameter
* Localhost fails: this warning and mail doesn't arrive

Warning: mail() [function.mail]: Failed to connect to mailserver at "localhost" port 25, verify your "SMTP" and "smtp_port" setting in php.ini or use ini_set() in DefaultMailSystem->mail() (line 76 of C:\xampp\htdocs\drupal8\modules\system\system.mail.inc).

Unable to send e-mail. Contact the site administrator if the problem persists.

- After patch

* cheap shared host ==> mail arrives and no warning
* Localhost: only this warning but mail doesn't arrive

Unable to send e-mail. Contact the site administrator if the problem persists.

Ok as it turns out this was a xamp localhost mailing issue. Moving back to rtbc.

Same patch applies to D7.

We're still checking for safe_mode but the comment explaining why has disappeared from the patch (also there's no else { any more so what happens if it's enabled now?).

+++ b/modules/system/system.mail.incundefined
@@ -66,25 +66,30 @@ class DefaultMailSystem implements MailSystemInterface {
     if (isset($message['Return-Path']) && !ini_get('safe_mode')) {
...
-    else {
-      // The optional $additional_parameters argument to mail() is not allowed
-      // if safe_mode is enabled. Passing any value throws a PHP warning and
-      // makes mail() return FALSE.
-      $mail_result = mail(
-        $message['to'],
-        $mail_subject,
-        $mail_body,

Clarifying after following up in IRC: This is the "else" condition and comment catch is referring to. There's no case now for when safe mode is true; both our cases are inside the first if. I'll look back in previous patches for when the change was made.

It looks like the safe mode else case went away as far back as #110. #114 and #122 refer to safe mode. There's also a lot of discussion about it earlier in the issue, but since it goes back to 2007 it's hard to say what's relevant and what's not. Can someone clarify what is supposed to happen when safe mode is enabled? Is it intended that the mail is simply not sent?

Can someone clarify what is supposed to happen when safe mode is enabled? Is it intended that the mail is simply not sent?

No, that was not the intent. Clearly there was a logic error in my patches.

Alright, I'll quickly reroll with the deleted else condition then. Thanks!

Edit: Also, it looks like the windows condition can be the same regardless, so I'll refactor it a little.

fwiw since safe mode is deprecated in PHP 5.3 I'd be fine with throwing warnings when it's enabled in 8.x, but for backport we shouldn't change behaviour.

Oops, lost this one in a crosspost.

Alright, I re-added a case for safe mode on non-Windows systems. I also added a comment to indicate that the lack of space between -f and the optional parameter is intentional.

One thing I noticed is that I can't find an explicit explanation here for the addition of @ to suppress warnings from mail(). The closest I can find is sun's comment in #25. We should probably add a comment justifying why we do this, since it's not exactly a best practice generally.

Edit: Found that explanation in #95.

Wrong diff again... sorry.

With that comment explaining the @.

Please test this patch with:

1. Windows.
2. Non-Windows.
3. Non-Windows with safe mode enabled.

Please test #152 and report which of the three scenarios you tested and what the results were. Thanks!

@aspilicious reported in IRC that he tested the patch on "a crappy host" with safe mode enabled with a limiting PHP configuration as well as on a Windows localhost, and in both cases the mail was sent successfully, without error messages. (Without the patch, he gets errors when attempting to send mail in both cases.)

Tested on ubuntu / php5.3 / apache / postfix.

First test
Return path is set to site_mail with safe_mode disabled. : mail sent ok

Return path is set to www-data@... with safe_mode enabled. : mail sent ok

Second test
Unable to send mail at all - but this may be unrelated. Will reboot and try again.

Ok!

Sorry it's taken so long to get back to this. Patch tested. Screenshots attached.

All good. Double plus good for ubuntu 10.04 / php5.3 / apache / postfix

:)

OMFG YEES!

To clarify, as I understand the error messages seen in #156 are the result of safe mode file system configuration issues unrelated to the mail issue. The emails were sent properly both with and without safe mode. :)

Alright, posting with less New Year's Eve this time.

• aspilicious tested the patch on a windows localhost as well as a "remote host" with a limiting configuration that had caused error messages without the patch. In both cases the message was sent fine without the error.
• KatteKrab tested the patch on a non-Windows install both with and without safe mode enabled.
• Both times, she set me as the recipient for the message, and I received it. (ACROSS THE INTERNET!)
• Without safe mode enabled, there were no error messages, as seen in the top third of the screenshot.
• The bottom two thirds of the screenshot are with safe mode enabled, before and after the message is sent. So, the error message seen in the screenshot was present before sending the message as well as after. This is related to a misconfiguration of the filesystem for safe mode (lots of results on d.o if you search for this message; see for example #979462: using xcache: fopen() [function.fopen]: SAFE MODE Restriction in effect. The script whose uid is X is not allowed to access /tmp and http://drupal.org/node/655992). So it's not related to the patch.

Thanks folks. Committed/pushed to 8.x. Will need a re-roll for 7.x.

Poll test error?? WTF! O.o I'm going to try a re-test.

#161: mail-131737-161.patch queued for re-testing.

Committed and pushed to 7.x. Thanks!

Moving down to D6. Needs a re-roll there.

Note that the D7 commit for this issue seems to have accidentally included the fix for #1353030: Increase length of "alt" and "title" text for images as well.

D6 backport.

In the patch on #168, $message['Return-Path'] never gets set, so the -f parameter never gets added. This is because the part of the code from system.mail.inc that sets $message['Return-Path'] is missing.

Here's an updated patch which fixes this. Copied everything in the mail() function in system.mail.inc into the initial else{} clause of drupal_mail_send. I've tested it on a Linux server with postfix, and it seems to work correctly, where the patch in #168 doesn't.

The patch in #169 doesn't work. Here's an updated patch which I've tested more thoroughly and sends mail correctly on a Linux server with postfix.

#170 works for me on exim.

Hi all,
Total Drupal Noob here and wondered if you could assist..
I got my Template installed but am getting the return path error (i believe)..

Notice: mail(): Policy restriction in effect. The fifth parameter is disabled on this system in DefaultMailSystem->mail() (line 76 of /customers/2/0/a/samanthascakes.co.uk/httpd.www/modules/system/system.mail.inc).
Notice: mail(): Policy restriction in effect. The fifth parameter is disabled on this system in DefaultMailSystem->mail() (line 76 of /customers/2/0/a/samanthascakes.co.uk/httpd.www/modules/system/system.mail.inc).

Is it the "system.mail.inc" file i need to edit?

/**
 * @file
 * Drupal core implementations of MailSystemInterface.
 */

/**
 * The default Drupal mail backend using PHP's mail function.
 */
class DefaultMailSystem implements MailSystemInterface {
  /**
   * Concatenate and wrap the e-mail body for plain-text mails.
   *
   * @param $message
   *   A message array, as described in hook_mail_alter().
   *
   * @return
   *   The formatted $message.
   */
  public function format(array $message) {
    // Join the body array into one string.
    $message['body'] = implode("\n\n", $message['body']);
    // Convert any HTML to plain-text.
    $message['body'] = drupal_html_to_text($message['body']);
    // Wrap the mail body for sending.
    $message['body'] = drupal_wrap_mail($message['body']);
    return $message;
  }

  /**
   * Send an e-mail message, using Drupal variables and default settings.
   *
   * @see http://php.net/manual/en/function.mail.php
   * @see drupal_mail()
   *
   * @param $message
   *   A message array, as described in hook_mail_alter().
   * @return
   *   TRUE if the mail was successfully accepted, otherwise FALSE.
   */
  public function mail(array $message) {
    // If 'Return-Path' isn't already set in php.ini, we pass it separately
    // as an additional parameter instead of in the header.
    // However, if PHP's 'safe_mode' is on, this is not allowed.
    if (isset($message['headers']['Return-Path']) && !ini_get('safe_mode')) {
      $return_path_set = strpos(ini_get('sendmail_path'), ' -f');
      if (!$return_path_set) {
        $message['Return-Path'] = $message['headers']['Return-Path'];
        unset($message['headers']['Return-Path']);
      }
    }
    $mimeheaders = array();
    foreach ($message['headers'] as $name => $value) {
      $mimeheaders[] = $name . ': ' . mime_header_encode($value);
    }
    $line_endings = variable_get('mail_line_endings', MAIL_LINE_ENDINGS);
    // Prepare mail commands.
    $mail_subject = mime_header_encode($message['subject']);
    // Note: e-mail uses CRLF for line-endings. PHP's API requires LF
    // on Unix and CRLF on Windows. Drupal automatically guesses the
    // line-ending format appropriate for your system. If you need to
    // override this, adjust $conf['mail_line_endings'] in settings.php.
    $mail_body = preg_replace('@\r?\n@', $line_endings, $message['body']);
    // For headers, PHP's API suggests that we use CRLF normally,
    // but some MTAs incorrectly replace LF with CRLF. See #234403.
    $mail_headers = join("\n", $mimeheaders);
    if (isset($message['Return-Path']) && !ini_get('safe_mode')) {
      $mail_result = mail(
        $message['to'],
        $mail_subject,
        $mail_body,
        $mail_headers,
        // Pass the Return-Path via sendmail's -f command.
        '-f ' . $message['Return-Path']
      );
    }
    else {
      // The optional $additional_parameters argument to mail() is not allowed
      // if safe_mode is enabled. Passing any value throws a PHP warning and
      // makes mail() return FALSE.
      $mail_result = mail(
        $message['to'],
        $mail_subject,
        $mail_body,
        $mail_headers
      );
    }
    return $mail_result;
  }
}

/**
 * A mail sending implementation that captures sent messages to a variable.
 *
 * This class is for running tests or for development.
 */
class TestingMailSystem extends DefaultMailSystem implements MailSystemInterface {
  /**
   * Accept an e-mail message and store it in a variable.
   *
   * @param $message
   *   An e-mail message.
   */
  public function mail(array $message) {
    $captured_emails = variable_get('drupal_test_email_collector', array());
    $captured_emails[] = $message;
    variable_set('drupal_test_email_collector', $captured_emails);
    return TRUE;
  }
}

The D6 backport in #170 works fine for me on 6.34. Let's get this committed, if possible.