From the patch (to default.settings.php):

/**
 * Line endings for e-mail messages
 *
 * E-mail uses CRLF for line endings.  PHP's mail() API function is
 * documented to require only LF endings in the message body.
 * However, on Windows, mail() behaves differently and it is correct
 * to give if CRLF; some MTAs (such as qmail) will refuse to accept
 * messages from PHP on Windows if you only supply LF.
 *
 * This tries to set the correct line ending for your system.  If it
 * does not work, you can override it.
 */
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

keith.smith’s picture

Status: Active » Needs review

There is a patch, so CNR.

bjaspan: if you have to reroll for some other reason,

+ * to give if CRLF; some MTAs (such as qmail) will refuse to accept

I think "to give if" should be "to get it" maybe?

Robin Monks’s picture

Status: Needs review » Needs work

Patch fails to apply to HEAD.

(Stripping trailing CRs from patch.)
patching file includes/bootstrap.inc
Hunk #1 FAILED at 274.
1 out of 1 hunk FAILED -- saving rejects to file includes/bootstrap.inc.rej
(Stripping trailing CRs from patch.)
patching file includes/mail.inc
Hunk #1 FAILED at 181.
1 out of 1 hunk FAILED -- saving rejects to file includes/mail.inc.rej
(Stripping trailing CRs from patch.)
patching file sites/default/default.settings.php
Hunk #1 succeeded at 175 (offset 70 lines).

Robin

jeffschuler’s picture

Status: Needs work » Needs review
FileSize
3.12 KB

Re-rolled against current D7 HEAD.

Changed "to give if" to "to give it", and changed double-spaces to singles after sentences in comments.

cburschka’s picture

Status: Needs review » Needs work

Drupal already has a mechanism to allow settings.php to override system variables. It would be better to use the $conf array and variable_get() instead of creating a new global.

Edit: As for the auto-detection, this would be best done in mail.inc in a constant.

As in:

define('MAIL_LINE_ENDINGS', isset($_SERVER['WINDIR']) || strpos($_SERVER['SERVER_SOFTWARE'], 'Win32') !== FALSE ? "\r\n" : "\n");

Then while building the mail:

$line_endings = variable_get('mail_line_endings', MAIL_LINE_ENDINGS);

And in default.settings.php, something like:

# Uncomment this if you need to override the line nedings style on your system:
# $conf['mail_line_endings'] = "\r\n";
jeffschuler’s picture

Status: Needs work » Needs review
FileSize
2.43 KB

Thanks Arancaytar, that made sense to me and was helpful.

I've incorporated your suggestions -- suitably, I hope.

dale42’s picture

Tested on a Windows XP system running XAMPP.
XAMPP includes a copy of sendmail.exe which can be optionally used instead of SMTP/smtp_port option.
I tested pre and post patch with both mail configuration options (SMTP and sendmail_path).
Mail was generated by blocking/unblocking an account.

In both mail configurations, mail worked before the patch and continued to work after the patch.

So although the patch isn't required for this configuration, it didn't break anything, either.

Status: Needs review » Needs work

The last submitted patch failed testing.

jeffschuler’s picture

Status: Needs work » Needs review

I just re-tested applying to HEAD and had no problem.

I'm setting back to needs review to ask Testbot for another try.

Status: Needs review » Needs work

The last submitted patch failed testing.

jeffschuler’s picture

Status: Needs work » Needs review
FileSize
2.74 KB

updated for HEAD

bjaspan’s picture

Status: Needs review » Reviewed & tested by the community

RTBC.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Cool. Makes sense to me. Committed to HEAD. Thanks!

moshe weitzman’s picture

Please revert. We don't use our main settings file as a place to do random documentation and overriding. This was mentioned in #4. This should be a regular old pref in a settings page.

moshe weitzman’s picture

Priority: Normal » Critical
Status: Fixed » Active
webchick’s picture

Status: Active » Needs work

I disagree with making this a full-blown setting. But Moshe is right that we don't want to clobber the default.settings.php file with odd-ball settings that only 1/100000000 sites will need to monkey with. It seems like the constant should guess the right thing on 99% of servers.

Incidentally, it's probably about time for one of these http://api.drupal.org/api/globals but for variables for cases exactly like this.

webchick’s picture

Priority: Critical » Normal
Status: Needs work » Fixed

Ok, created #620650: Create a variables.php for documenting hidden variables as a follow-up to cover cases like this.

Also reverted the default.settings.php hunk, and adjusted the comment in mail.sending.inc accordingly.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

marco68’s picture

Hi, I report that in my system (win7x64, IIS, php 5.3.2, Drupal 7.12) :

  • the server variable windir has lower case
  • _SERVER["SERVER_SOFTWARE"] is Microsoft-IIS/6.0

My PHP variables (getted with phpinfo):

_SERVER["windir"] E:\WINDOWS Note the lower case on variable name
_SERVER["SERVER_SOFTWARE"] Microsoft-IIS/6.0

So the proposed patch does't work for me and I cannot send mail without override $conf['mail_line_endings'].
Without the override outgoing messages contains single \n.

Regards
Marco

shakyjake’s picture

Windows Server 2008 R2 / D7.23 / PHP 5.3.24 / IIS 7.5

This isn't working out-of-the-box for me as both isset($_SERVER['WINDIR']) and strpos($_SERVER['SERVER_SOFTWARE'], 'Win32') are returning false

In order to fix it I added the following check to MAIL_LINE_ENDINGS definition-

|| strpos($_SERVER['SERVER_SOFTWARE'], 'IIS/7.5')

Just in case anyone finds themselves in the same situation and comes across this thread.