[D7] PhpMail : broken mail headers in PHP 8.0+ because of LF characters

@arx-e: How did you change line 13 of includes/mail.inc?

I did this (which of course could be abbreviated) and it works, but I'm worried it could have some unpredictable side effects.

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

This is on a CentOS server v7.9.2009, so not Windows.

I also made the other changes you listed but they didn't work until I changed line 13.

@rclemings I did the same thing as you as a quick solution with minimal editing.
And it worked for some system messages but not for webform messages for example.

I read in an old issue that LF ("\n) is what should be used in Linux systems and CRLF ("\r\n") was reserved only for windows servers.
I am not sure but maybe this has changed?
The old issue is this one drupal_mail(), CRLF, and Windows →

I suppose if it is safe to have it universally as CRLF, this could be shortened to something like this:
define('MAIL_LINE_ENDINGS', "\r\n");

Right above line 13 in includes/mail.inc it says:

 * $conf['mail_line_endings'] will override this setting.

So I added this to settings.php:

$conf['mail_line_endings'] = "\r\n";

and it had no effect. I had to change line 13 in includes/mail.inc from:

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

to:

define('MAIL_LINE_ENDINGS', "\r\n");

and then outgoing emails were properly formatted (using CentOS 7.9.2009, PHP 8.1, Drupal 7.94, Commerce Email 7.x-2.x-dev, HTML Mail 7.x-2.71, and Mime Mail 7.x-1.2). I checked the password reset email, a Simplenews newsletter, and order notifications from Commerce 1.x and all looked correct.

I'm not smart enough to troubleshoot this further but it looks as if there's something squirrelly about how $conf['mail_line_endings'] works with this combination.

Here's a patch with the fix from #12. Works for me (using CentOS 7.9.2009, PHP 8.1, Drupal 7.94, Commerce Email 7.x-2.x-dev, HTML Mail 7.x-2.71, and Mime Mail 7.x-1.2), but not tested otherwise.

The last submitted patch, 13: d7email_header_fix-3319062-13.patch, failed testing. View results →

Hi,

I'm not sure this is working... I'm still getting the leading spaces..

X-PHP-Script: ipmsusa.org/index.php for 71.58.100.178
 MIME-Version: 1.0
 Content-Type: text/plain; charset=UTF-8; format=flowed; delsp=yes
 Content-Transfer-Encoding: 8Bit
 X-Mailer: Drupal
 Sender: webmaster@ipmsusa.org
 From: "ema13@psu.edu via IPMS/USA Home Page" <webmaster@ipmsusa.org>
 Reply-To: ema13@psu.edu

This is with D7.94, PHP 8.1.16, no special Mailers... and with (and without) the patch from https://www.drupal.org/node/111702 →

Thx,
Eric

The modification as in patch #13 had resolved the problem for me but the issue resurfaced the last couple of weeks or so.

I just now discovered another place where the same modification resolves the issue again:
line 67 in system.mail.inc is now
$mail_headers = join("\n", $mimeheaders);

and it obviously ruins the messages.
This modification solved the problem again:
$mail_headers = join("\r\n", $mimeheaders);

And here is a patch containing both modifications (#13 + #16).

Patch #17 seems to have worked for me.

Thanks,
Eric

The last submitted patch, 17: d7email_header_fix-3319062-17.patch, failed testing. View results →

Manually applied the changes. This works.

I also can confirm that the patch in #17 works, on two D7 sites with PHP 8.2. Thanks for the patches.

Running installations on Debian-based distributions with exim4 or Postfix, the patch from #17 fixes the issue without noticeable side-effects.

Thanks for confimation that the patch is working! However, the patch has a number of failures, so either is it not working correctly in all cases, or the tests needs to be updated. Unfortunatelly it cannot be committed in the current shape until we deal with the failing tests.

As far as I managed to understand the tests fail because of the drupal_html_to_text function that is found in the includes\mail.inc file.
But I also found lots of other places where LF line endings are being used.
And from what is stated here https://www.drupal.org/project/drupal/issues/3270647 🐛 PhpMail : broken mail headers in PHP 8.0+ because of LF characters Fixed the line endings should be CRLF for PHP 8+
So in this patch I have tried to define line endings according to the php version and server software and I replaced all occurrences of LF with a variable $line_endings
My experience with code is relatively limited so I don't really know whether all those changes are needed and/or correct but I have tested these modifications at one Drupal 7.98 site and it seems to be working without any issues.

One more patch keeping the logic that distinguishes between PHP versions and uses CRLF only when PHP is 8 or 8+ or when on a Windows server.
The rest of the changes have been kept to a minimum but the result works for me on a couple PHP8 sites.

Reuploading the patch as the last one had a syntax error.

Tested and verified patch #26.

Thanks for working on this @arx-e!

Only a quick review / questions:

1. Are we sure we need all these changes to the $message['body'] formatting and drupal_html_to_text()?

 function drupal_wrap_mail($text, $indent = '') {
-  // Convert CRLF into LF.
-  $text = str_replace("\r", '', $text);

   if (count($urls)) {
-    $footnotes .= "\n";
+    $footnotes .= MAIL_LINE_ENDINGS;
     for ($i = 0, $max = count($urls); $i < $max; $i++) {
-      $footnotes .= '[' . ($i + 1) . '] ' . $urls[$i] . "\n";
+      $footnotes .= '[' . ($i + 1) . '] ' . $urls[$i] . MAIL_LINE_ENDINGS;

-          $output .= drupal_wrap_mail('', implode('', $indent)) . "\n";
+          $output .= drupal_wrap_mail('', implode('', $indent)) . MAIL_LINE_ENDINGS;

-    $message['body'] = implode("\n\n", $message['body']);
+    $message['body'] = implode($line_endings, $message['body']);

The issue summary does not mention any problems with the mail body and the parent D10 issue have not made any changes to the mail body as well. I am curious if it wouldn't be sufficient to make only this change:

-    // 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);
+    // Use here also the vcariable MAIL_LINE_ENDINGS
+    $mail_headers = join($line_endings, $mimeheaders);

Plus the change to the MAIL_LINE_ENDINGS constant definition, which looks reasonable. If a change to the mail body is needed, then we should explain the reasons for this change.

2. Core is not using PHP_MAJOR_VERSION constant anywhere, let's use the standard comparision used on other places:

PHP_VERSION_ID < 80000;

3. If the updated patch will pass on all PHP versions, we should also backport the new test from D10.

@poker10
No, I was not at all sure about those changes. I was just trying to make the tests work.

So i am uploading a new patch with only:
1. the change in the definition of MAIL_LINE_ENDINGS in includes/mail.inc
2. and the change in modules/system/system.mail.inc

I've manually applied the changes at #29 which seem to fix the issue for me.

I'm keen to see the fix for this issue committed, so I'd be really appreciative if somebody could sort out the issues with the failed tests that seem to be the stumbling block. I'd be happy to help, but as I'm not a programmer I'm not sure how I could actually contribute.

I had the exact same issue and the patch #29 worked fine, solved the issues that I had the the outgoing email format.

Thank you !

The patch in #29 fixed the issue I was having with line endings being stripped in mail headers, preventing html email being sent, in Drupal 7.102
The email headers would show all on one line and the email body shows the plain text html tags.
After the patch, the headers are formatted correctly and the email comes through HTML formatted, with thanks.

#29 looks reasonable but I'd be happier committing this if we could verify that the comment above the changes to the constant is true:

	  * $conf['mail_line_endings'] will override this setting.
	  */

In other words, I'd like to ensure that sites can override these changes if they need to (e.g. because they use a quirky MTA or similar).

An earlier comment in this issue suggested that the $conf / variable does not in fact override the constant.

It'd also be good to see tests passing in an MR with the new d.o testing pipeline.

I converted the patch #29 to the MR and it is failing on PHP8+: https://git.drupalcode.org/project/drupal/-/pipelines/358167

I think the reason is in drupal_html_to_text , which in D7 uses:

$output .= drupal_wrap_mail($chunk, implode('', $indent)) . MAIL_LINE_ENDINGS;

but in D10:

$line_endings = Settings::get('mail_line_endings', PHP_EOL);
// Format it and apply the current indentation.
$output .= static::wrapMail($chunk, implode('', $indent)) . $line_endings;

I compared the tests and they looks the same, so this seems like the only difference. On Linux, PHP 8, in D7, after the patch is applied there will be "\r\n", but on Linux, PHP8, in D10 it is just "\n".

I can update the test with a similar code from D10:

$line_endings = variable_get('mail_line_endings', PHP_EOL);
$output .= drupal_wrap_mail($chunk, implode('', $indent)) . $line_endings;

But the question is, if this is a sign of the broken test, or we are unintentionally changing something else.

It is still failing on PHP8:

I think the patch is not correct.

D10 code:

    $line_endings = Settings::get('mail_line_endings', PHP_EOL);
    // Prepare mail commands.
    $mail_subject = (new UnstructuredHeader('subject', $message['subject']))->getBodyAsString();
    // Note: email 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 $settings['mail_line_endings'] in settings.php.
    $mail_body = preg_replace('@\r?\n@', $line_endings, $message['body']);
    $mail_headers = $headers->toString();

D7 code:

    $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']);
    // Use here also the variable MAIL_LINE_ENDINGS
    $mail_headers = join($line_endings, $mimeheaders);

You can see, that if we change the MAIL_LINE_ENDINGS in D7, it does change also the body line endings, which I suppose is something we do not want to change. These line endings should be only linux/windows dependent. But after the proposed change, on PHP8, also the body has "\r\n" even on linux. That is a cause of the failure.

The correct approach seems to be to only touch headers, as proposed in the issue summary and issue title. So to change this line:

    $mail_headers = join("\n", $mimeheaders);

to be "\n" on PHP<8 and "\r\n" on PHP>=8.

I have updated the MR according to my findings from #36 (the comment I have used in the code is borrowed from Drupal 9.5, see: https://git.drupalcode.org/project/drupal/-/commit/40ad8b5e57e3dc534de77...).

Tests are now green: https://git.drupalcode.org/project/drupal/-/pipelines/358280

But now the change cannot be opted-out by using $conf['mail_line_endings']. Should we introduce another variable? I did not want to use an existing one, because as we found out, mixing line endings and header line endings is not a good option.

Sounds like it might make sense to add another variable; perhaps one we wouldn't add to default.settings.php if it's intended to accommodate sites with edge case mail setups.

Added a new variable mail_headers_line_endings.

Tests are green: https://git.drupalcode.org/project/drupal/-/pipelines/358322

I think it is still worth a manual testing - at least to compare the email headers on linux - PHP 7.4 vs 8.0. Plus this will need a change record, in case we will decide to commit it. Thanks!

I think the new patch looks good.

Having yet another conf / variable is not ideal but it obviously avoids the situation where a change like this breaks sites with a certain mail setup and they're left with no choice but to hack core.

@poker10 and I looked at this to do some manual testing; it's a little tricky because MTAs / SMTP servers etc.. tend to be so accommodating of all sorts of weird stuff in email. For example mailpit (very useful tool included in ddev) has a relevant option - which defaults to false:

--smtp-strict-rfc-headers / MP_SMTP_STRICT_RFC_HEADERS

Force Mailpit to return an SMTP error if message headers contain \n instead to \r\n line breaks. By default Mailpit will silently fix incorrect line breaks generated by some broken sendmail clients (see related Github issue).

https://mailpit.axllent.org/docs/configuration/runtime-options/#smtp-server

However turning that on didn't seem to help us verify that anything was broken without the patch. Almost every way of actually testing mail that we tried (including with e.g. mimemail enabled or not) did not seem to show any problem without the patch on PHP8 (although I think we did reproduce html mail appearing a bit broken).

We eventually managed to observe the specific difference that the patch makes using strace to observe what PHP emits when Drupal's mail system sends a message. (FWIW we used https://www.drupal.org/project/drush_debug_tools → which includes a command to send test email from drush - it's old and dusty but still works).

So we verified that without the patch, on PHP 8.2, just a couple of the mail headers were separated by only \n. For example:

Errors-To: hello@example.com\nSender: hello@example.com\nFrom: hello@example.com

Whereas with the patch applied, this was:

Errors-To: hello@example.com\r\nSender: hello@example.com\r\nFrom: hello@example.com

It looks like - at least in the testing we were doing - those were the only headers being inserted by the code in question. All other headers were not influenced by these changes (and always had the correct line endings).

So I believe that verifies that this patch is having the desired effect.

If it has any unintended consequences with a particular mail set up, the new variable should allow for the change to be overridden without hacking core.

Thanks everyone!

Created a draft CR: https://www.drupal.org/node/3491626 →

After the Drupal 7.103 core update, i'm still experiencing a problem with outgoing e-mails using drupal_mail() function.
This is how a 3 rows html test mail body is shown in all the e-mail clients that the receiver uses (including gmail webmail).

This is a multi-part message in MIME format.

--0239714e6c488a2e4f394263b8f561e40c281d241
Content-Type: multipart/alternative;
 boundary="d736db4fc82e3e0f3789544c5fca10674e03b1370"
Content-Transfer-Encoding: 8bit


--d736db4fc82e3e0f3789544c5fca10674e03b1370
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

Questa è la prima riga.

Questa è la seconda riga.

Questa è la terza riga.


--d736db4fc82e3e0f3789544c5fca10674e03b1370
Content-Type: text/html; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8Bit

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
      </head>
  <body id="mimemail-body" class="dmail-smtp-dmail-invio-diretto-html">
    <div id="center">
      <div id="main">
         <p style="color: #ff5733; font-size: 20px; margin-bottom: 10px;">Questa è la prima riga.</p>
    <p style="color: #33c3ff; font-size: 18px; margin-bottom: 10px;">Questa è la seconda riga.</p>
    <p style="color: #28a745; font-size: 16px; font-style: italic;">Questa è la terza riga.</p></div>
    </div>
  </body>
</html>

--d736db4fc82e3e0f3789544c5fca10674e03b1370--

--0239714e6c488a2e4f394263b8f561e40c281d241--

I'm using PHP 8.3 on Almalinux 8.x.
In my Drupal conf, Mail System is set to use Site-wide default MailSystemInterface class => "MimeMailSystem".
I fixed this problem with the attached patch
Hope it may help others with the same problem

Sorry, I haven't had time to look at the details properly, but do you need to apply a patch?

Can't you fix your problem with the two variables that control the line-endings?

modules/system/system.mail.inc:55:    $line_endings = variable_get('mail_line_endings', MAIL_LINE_ENDINGS);
modules/system/system.mail.inc:65:    $headers_line_endings = variable_get('mail_headers_line_endings', "\n");

As you correctly said, we can adjust $conf['mail_line_endings'] in settings.php, because the $message['body'] uses "$line_endings"

$line_endings = variable_get('mail_line_endings', MAIL_LINE_ENDINGS);
$mail_body = preg_replace('@\r?\n@', $line_endings, $message['body']);

But, i might be mistaken, but i still need to apply the patch, because in the file "/includes/mail.inc"
there is this line (line 10):
define('MAIL_LINE_ENDINGS', isset($_SERVER['WINDIR']) || (isset($_SERVER['SERVER_SOFTWARE']) && strpos($_SERVER['SERVER_SOFTWARE'], 'Win32') !== FALSE) ? "\r\n" : "\n");
that re-defines the MAIL_LINE_ENDINGS constant, setting it to "\n" (i'm on linux - AlmaLinux 8.x)

So, all linux users, must either:

I do not think this is the only one condition which makes this bug present. We have debugged it pretty long and were unable to reproduce the broken emails with the Drupal core itself. So I suppose this is also dependent on mailserver somehow.

The reason why the patch from #45 is not a good idea are explained in #36. This issue was about changing the header line endings. If we have changed also the MAIL_LINE_ENDINGS constant, the mail body line endings would have changed as well, which is something very different (and Drupal 10/11 also does not have this). If there is an issue with mail body line endings, that is probably a separate issue and hopefully can be solved by fine-tuning values of both variables without the need of any custom core changes.

Re:

2- Apply the patch, because in the file "/includes/mail.inc" there is this line (line 10):

I do not think this is correct/needed anymore. The MAIL_LINE_ENDINGS constant is defined on the top of the mail.inc file. Then, when it is used in the mail() function, there is a possibility to override it via mail_line_endings variable:

DefaultMailSystem::mail():

    $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.
    $headers_line_endings = variable_get('mail_headers_line_endings', "\n");

So everything from in the mail body should be changed to specified newlines by this line:

$mail_body = preg_replace('@\r?\n@', $line_endings, $message['body']);

The precedence is by the variable, not by the constant. If this code is not working as expected, that is a separate issue.

Can you please doublecheck again, why do you need to change the constant at all? If you are using another mailer and the seding is not done by core DefaultMailSystem::mail(), then it is indeed a problem of the contrib module.

Thanks!

Drupal 10/11 is using PHP_EOL for the mail body line endings, so it is not using \r\n in linux, see: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

That is another reason against using the \r\n for the mail body line endings in linux in Drupal 7.