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?

From the PHP Mail() function page:
- The additional_parameters parameter was added in PHP 4.0.5
- The additional_parameters parameter is disabled in safe_mode and the mail() function will expose a warning message and return FALSE when used (PHP 4.2.3)

That means they could be some issue on some configurations older than PHP 4.0.5.
There is a module here : http://drupal.org/project/returnpath
Don't you think it would be good to have this basic feature in the core instead of having to use an extra module?

re The additional_parameters parameter is disabled in safe_mode and the mail() function will expose a warning message and return FALSE when used (PHP 4.2.3)
-> I added the safe_mode compatibility to the patch.

What happens if you're not using sendmail?

Steven, I got it working fine on 3 different servers: postfix, Exim 4.63 and qmail (all with safe_mode OFF).

before: Return-Path: <httpd@web9.hostingcompany.com>
after: Return-Path: <myemail@mysite.com>

is there any chance to get this committed before next month?

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.

#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.

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?

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.

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.

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

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.

Subscribe.

subscribing

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

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

The last submitted patch, 131737-set-the-return-path-correctly-in-sendmail.patch, failed testing.

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?

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

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:

And for the testing:

1. Logs without patch:

Bad, baaaad! ;-)

2. Logs with patch:

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.

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

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.

} else { should be in two lines. also, comments can run up to the 80th characters, so we could save some space here.

usually comments are not deleted from the issue queues.

@#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.

@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...

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

@64: Removed those...

Ah ratz, thanks nvanhove.

Fantastic!

Shall I do one for the D6 branch as well?

@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!

Committed to CVS HEAD. Moving to D6.

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

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

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

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.

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:

cheers

Patch in #79 works well for me in 6.17

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?

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...

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.

@aspilicious: want to try this patch? implements damz's idea.

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):

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.

Return-Path.patch queued for re-testing.

Bumping and tagging as per standard policy.

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

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

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....)

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: I see you have fixed this in HTML Mail, a new patch here still would be nice.

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

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

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

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...

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).

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

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.

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.

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 as it turns out this was a xamp localhost mailing issue. Moving back to rtbc.

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?).

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?

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.

Oops, lost this one in a crosspost.

Wrong diff again... sorry.

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

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.

OMFG YEES!

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.

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

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.