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

Thanks for this. This issue is probably the root cause of a few issues that I just related.

I'm writing in this thread for now as it is the closest to what I can find that matches my problem, but maybe someone can direct me to post elsewhere?

Clean Drupal install plus htmlmail, mailsystem, mimemail (for testing). Sending mail works fine up until PHP version 7.4.16. PHP version 7.4.17 doesn't exist, next is PHP 7.4.18. From this version until the current version (7.4.29 at time of this post), mail sending does not work from Drupal. Again, this is Windows Server / FastCGI. This appears to work fine on ubuntu/linux with the latest version of PHP.

PHP 7.4.18 has a fix: https://bugs.php.net/bug.php?id=80710 that deals with a CRLF header injection vulnerability. I'm seeing this thread seems somewhat related. I'm wondering if anyone else is successfully running Drupal 7 on Windows Server / latest PHP and successfully sending mail?

I the problem might be to do with mime_header_encode() function, I see a comment "Using \n as the chunk separator may cause problems on some systems and may have to be changed to \r\n or \r."

This took quite a while to track down, so I'm hoping I can save someone else the pain and maybe even find the culprit.

Removing as I've further tracked this down to the mimemail module, as disabling that module permits Drupal to successfully send a password reset email (for testing purposes), thus core appears to be working fine.

@RogerRogers: You've got it backwards - Mime Mail other modules such as Weform have this problem on some sites because of core Drupal, not the other way around.

@TR, I'm not sure which is root cause of the problem. Whatever the case, when I disable mimemail sending works again. I posted a more thorough explanation here: https://www.drupal.org/project/mimemail/issues/3283706.

@TR, I stand corrected. Mimemail is not the only problem, and I'm therefore guessing you might be correct that this is a broader issue. I commented out a few lines in mimemail and was able to get basic emails going, e.g. password resets, but sending an htmlmail test fails.

I'm speechless - after a full day of pulling my hair out - to find it's an ancient problem. I was cleaning up a site design for a client and needed html and couldn't get it working correctly. I ran numerous tests and was nearly given up when I stumbled on this notation. Sure enough, degrading my environment to 7.4 cleared the problem.
Using webforms with MimeMail just gave a text email no matter what I did.
There is a comment in the mimemail project about a modification to a file as a possible workaround, however it didn't help me.
see https://www.drupal.org/project/mimemail/issues/3261886 #2.
I see this marked as closed but I'm unclear about how to solve this, or workaround it, patch it,etc...

Running PHP 8.1.7 d9.4.3 (except now I'm on php 7.4xx)

edit -
Digging further I found this:

    $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']);
    // For headers, PHP's API suggests that we use CRLF normally,
    // but some MTAs incorrectly replace LF with CRLF. See #234403.
    $mail_headers = str_replace("\r\n", "\n", $headers->toString());
    $mail_subject = str_replace("\r\n", "\n", $mail_subject);

]

as well - in a dialog by much smarter people - there was a reference to a potential method to give a temporary solution like giving a php.ini value (slightly different than above).

I tracked down the text I wanted to share : from https://bugs.php.net/bug.php?id=81158
Alternately, you could allow systems to specify the line ending to use in php.ini so that they could configure it together with sendmail. Then just do something like `if (ini_get('sendmail.fix-line-endings') ) $email = str_replace(["\r\n", "\r", "\n"], ini_get('sendmail.line-ending'), $email)` to fix the line endings. Most frameworks use some kind of solution like this, controlling the behavior with configuration.

Edit - tried every iteration I could think of, couldn't get it to work setting the Mail line ending. I could affect it, but it didn't fix regardless of value I used (\r\n, \n) :

/* JAS 7/26 trying to override mailline endings */
$settings['mail_line_endings'] = "\r\n";
/* $conf['mail_line_endings'] = '\r\n';  taken from an outlook 2012 problem solution */

so my choice is html in emails with the slower 7.4 php or 8.0 with plaintext.

Hope this helps someone

Was able to get it to work just by adding a space to a core file.

D 9.4.5
PHP 8.1.8
WebForm 6.1.3
Was getting the same issue. PHP 7.4 worked fine sending an HTML email from WebForm. Switched to PHP 8.1 and the HTML was no longer formatting correctly in the received email. Checked the headers for the issue that people say prevented it (a space being added to the beginning of content-type, etc.), and sure enough, those spaces were there. They can be removed in the core file:

/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php

Line 104, add a space AFTER the \r\n and it works!
Before:
$mail_headers = str_replace("\r\n", "\n", $headers->toString());
After:
$mail_headers = str_replace("\r\n ", "\n", $headers->toString());

Excellent! confirm #11 worked for me!

so I was trying to set a settings.php value before and could tickle it but it never worked:
$settings['mail_line_endings'] = "\r\n ";

would it be possible to figure out a way to do something with a settings.php variable ala {$settings['mail_headers'] = "\r\n ";} or something?
Since a) modding core can be perilous b) each upgrade I'll have to hand edit this... It'd be nice to have a semi-permanent workaround!!!
I'm just happy to get off 7.4 php!

WebForm needs to be updated to work with 8.1, as after I implemented this core file change, then went to EDIT a content page, I received deprecation errors:

Deprecated function: Calling static trait method Drupal\webform\Element\WebformEntityTrait::setOptions is deprecated, it should only be called on a class using the trait in Drupal\webform\Plugin\WebformElement\WebformEntitySelect->setOptions() (line 395 of modules/webform/src/Plugin/WebformElement/WebformEntityReferenceTrait.php).

Commented out that line (395) and the error went away, not sure what damage that does, though.

If #11 is in fact the solution - any chance that someone can create a proper patch? I'd be happy to test further as well from there.

I noticed the issue once I moved to PHP 8.1 as well. I'm seeing mail header issues when using MimeMail at this point (haven't tested further yet with other modules such as Swiftmailer).

** @slangston - in regards to the Webform specific issue and PHP 8.1, have you applied the Webform PHP 8.1 patches here:
https://www.drupal.org/project/webform/issues/3283614

No, #11 is not the solution.

As @tr states, "#11 is not the solution" - I think that was a bit short.

#11 did in fact work - however- there is no doubt this is bigger issue - since moving to 8/8.1 on different platforms gives different results. There will need to be either an override option for those whose environments fail or more logic in the replacement to determine when/if it's needed.
BUT - I now have TWO sites that are fixed by this single character edit. What I've done is set the file to read only on the servers and lock the file after I edit it. when the recent 9.4.5 came out I broke myself inadvertently. This is also a horrible idea, I know, since should the phpmailer.php plugin file get tweaked, I will break again.

I can share that after some additional testing that the Symfony Mailer module works well with PHP 8+. I'm not sure if it makes use of Drupal\Core\Mail\Plugin\Mail\PhpMail or not though. The headers certainly look fine and work perfectly. Also, it might be worth mentioning that the module does not use the Mail System module like Mime Mail does.

** For now, I will have to proceed with using Symfony Mailer when bumping up sites to PHP 8+. Some of my projects rely on Mime Mail as it offers better integration with other modules (such as Queue Mail, Views Send) since it has simply been around longer.

After many tests outside Drupal on the same server, PHP8 do not tolerate the simple "\n" contrary to PHP7 to add multiple headers with mail() function. It is therefore absolutely necessary to transmit "\r\n" as the header separator, otherwise a space is added to disable the header.

I found the following solution : Replace Into Drupal file /core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php :
$mail_headers = str_replace("\r\n", "\n", $headers->toString());
by
$mail_headers = str_replace("\r\r\n", "\r\n", $headers->toString());

Here's #11 in patch form.

@albert, looks like it is #20 not #11.

@albert - thanks for the patch. Seems to work nicely on PHP 8.1 site.

Still not fixed with Drupal 9.4.6

@slangston How would it be fixed while this issue is open, in need of review, and not yet committed to Drupal Core?

Patch #21 seems to work for 9.4.1 on PHP 8.1.

Appreciate the patch - I've tested it now on 2 sites, worked nicely! (PHP 8.1.6 and PHP 8.1.11 both with D9.4.7)

Have also tested this on 3 of my sites. Fixed the issue perfectly.

Changing the status based on the comments above.

+++ b/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php
@@ -101,7 +101,7 @@ public function mail(array $message) {
     $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 = str_replace("\r\n", "\n", $headers->toString());
+    $mail_headers = str_replace("\r\r\n", "\r\n", $headers->toString());

I don't think we'll have headers with "\r\r\n" so shouldn't this entire str_replace() call be removed rather than changed to something that will likely do nothing?

Also if we're removing it, we should also remove the comment just above.

No tests with this but I can' think of a test that would actually be useful here.

@catch made changes as per the comment #31

Fix is straightforward and has had multiple reviews.

+++ b/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php
@@ -99,9 +99,7 @@ public function mail(array $message) {
-    // For headers, PHP's API suggests that we use CRLF normally,
-    // but some MTAs incorrectly replace LF with CRLF. See #234403.
-    $mail_headers = str_replace("\r\n", "\n", $headers->toString());
+    $mail_headers = $headers->toString();
     $mail_subject = str_replace("\r\n", "\n", $mail_subject);

Don't we need to do the same thing for $mail_subject - also I think this is testable.

Plus should we be keeping behaviour the same on PHP 7 given that the behaviour is changed in PHP 8 only?

We could also choose to add test coverage of this method here. We can add a mail function in the Drupal\Core\Mail\Plugin\Mail namespace in order to mock the mail function.

I have uploaded the patch accordingly to address #34.
Please review it

The patch #35 is not correct.

+++ b/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php
@@ -99,10 +99,8 @@ public function mail(array $message) {
-    $mail_subject = str_replace("\r\n", "\n", $mail_subject);
+    $mail_subject = $mail_subject->toString();

$mail_subject is already a string, not an object.

The patch also contains unrelated test class ExceptionLoggingSubscriberTest.

#11 worked for me

I think it should just be this, right?

+++ b/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php
@@ -99,9 +99,7 @@ public function mail(array $message) {
-    // For headers, PHP's API suggests that we use CRLF normally,
-    // but some MTAs incorrectly replace LF with CRLF. See #234403.
-    $mail_headers = str_replace("\r\n", "\n", $headers->toString());
-    $mail_subject = str_replace("\r\n", "\n", $mail_subject);
+    $mail_headers = $headers->toString();

     $request = \Drupal::request();

Any chance this will be in the next update any time soon? We're running several sites on PHP 7.4 and cannot update to 8.1 because of this issue.

bugfix in previous patch

The patch in #40 seems to be almost the same as the one in #32 and does not address all points raised by @alexpott from #34. Therefore I think it should stay as Needs work.

The patch in #40 seems to be almost the same as the one in #32 and does not address all points raised by @alexpott from #34. Therefore I think it should stay as Needs work.

Yes. Exactly what I was just about to say.

Same error on my website, after update PHP to 8.1 and Drupal to 9.4.9 the 'reply' function didn't work anymore.
I apply'd the patch #40 and this works.
Tnx!

Error: Call to undefined function Drupal\Core\Mail\Plugin\Mail\mail() in Drupal\Core\Mail\Plugin\Mail\PhpMail->mail() (line 120 of core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php).

I applied the solution in #40, no change, the error persists.

PHP 8.1
Drupal 9.5.3

I assume this issue is still present in Drupal 10 (since it's very similar to Drupal 9.5), but I haven't actually tested Drupal 10 yet. Has anyone confirmed yet whether this is still an issue?

I believe so... My drupal 10 webform handler is set to use html, but in my emails I see all the html tags.

Patch in #40 works for me using mime_mail but with htmlmail it's the same as without the patch. Not clear why.

Drupal core 9.5.5
Mime Mail 8.x-1.0-alpha5
HTML Mail 8.x-3.0-alpha3

Here's another patch, which addresses the concern of breaking the current behaviour for PHP < 8 raised by @alexpott in #34. Since Drupal 10 doesn't support PHP versions lower than 8, the check can be removed again in the 10.x branch.

I've tested the patch on a Drupal 9.5.8 site with PHP 8.1 on Debian and it works: the webform module is able to send HTML mails again.

It was also brought up that this needs a unit test, but I could use some help here, as I'm not really a PHP developer anymore these days. When I look at \Drupal\Tests\Core\Mail\Plugin\Mail\PhpMailTest.php, I see how the doMail() method was mocked and I could probably verify the arguments passed to the mock somehow, but since the code now checks the constant PHP_VERSION to determine how to behave, this makes unit testing hard or impossible. Is there some way of injecting the PHP version as a dependency of the PhpMail class? It would seem weird to me write a unit test that makes different assertions based on which PHP version the test runs on, right? If anyone could give me some pointers here, I'd be happy to give it another try. (For Drupal 10, a test would be more straighforward.)

Patch in #40 works for me using mime_mail but with htmlmail it's the same as without the patch. Not clear why.

As a co-maintainer of HTML Mail, I don't think this observation should be a reason to block or postpone the current issue. The HTML Mail module will not be supported going forward because it relies on a very old and unsupported PECL package. HTML Mail is being abandoned in favor of Mime Mail, and Mime Mail *relies* on this current issue being fixed in core.

Thanks all for the work on this so far, here's a first go at a test.

And again, slightly more verbose :p

Thanks!

Just wanted to add:

• The test doesn't verify the Reply-To because the behavior varies based on sendmail_path which is a PHP_INI_SYSTEM config, and I'm not sure the best way to test that, and I think it's out of scope. (Whereas just adding checks for the other static values seems fine imho.)
• I don't know for sure that the subject is correct, I just took what's currently being output. I did double-check that the CRLF + space combo makes sense, and it looks like it might be legit (not enough of an expert to say for sure!):
  

  If it is desirable to encode more text than will fit in an 'encoded-word' of 75 characters, multiple 'encoded-word's (separated by CRLF SPACE) may be used.

  http://www.faqs.org/rfcs/rfc2047.html

• Is it worth a follow-up issue to expand the tests further for the shell escaping and OS/PHP ini dependent behavior? (Not sure how to tackle the sendmail_path stuff.)

Thanks!

Not sure the expanding test coverage is needed as this fix is only needed for 9.5.

I don't see how this is only needed for 9.5.x:

    // 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']);
    // For headers, PHP's API suggests that we use CRLF normally,
    // but some MTAs incorrectly replace LF with CRLF. See #234403.
    $mail_headers = str_replace("\r\n", "\n", $headers->toString());
    $mail_subject = str_replace("\r\n", "\n", $mail_subject);

Doesn't some of this code need to be removed now we only support PHP 8+? i.e. what #32 does?

We should get a correct patch into 10.x first.

I don't see how this is only needed for 9.5.x:

It's not. The fix in the patch is also very relevant for Drupal 10+. The backwards compatibility for PHP < 8 is only relevant for Drupal 9.5 (which is still supported, I think), though. Since Drupal 10 doesnt support PHP 7. That concerns these lines:

    if (version_compare(PHP_VERSION, '8.0.0') < 0) {
      // For headers, PHP's API suggests that we use CRLF normally,
      // but some MTAs incorrectly replace LF with CRLF. See #234403.
      // PHP 8+ requires headers to be separated by CRLF,
      // so we'll replace CRLF by LF only when using PHP < 8. See:
      // - https://bugs.php.net/bug.php?id=81158
      // - https://github.com/php/php-src/commit/6983ae751cd301886c966b84367fc7aaa1273b2d#diff-c6922cd89f6f75912eb377833ca1eddb7dd41de088be821024b8a0e340fed3df
      $mail_headers = str_replace("\r\n", "\n", $mail_headers);
      $mail_subject = str_replace("\r\n", "\n", $mail_subject);
    }

In Drupal 10 and onwards, this particular block can be removed, but the rest of the patch is still needed.

Could you also please elaborate why this issue has been reverted to 'Needs work', since there is a working patch with test coverage (#52, 3270647-52-d9.patch)? You seem to suggest the patch is incorrect, but I don't see why.

@P44T it's needs work because issues need to go into Drupal 10 before they can go into Drupal 9, i.e. we should commit a version like "in Drupal 10 and onwards, this particular block can be removed, but the rest of the patch is still needed." which I don't think is on this issue yet and certainly not with a recent test run.

@catch, thank you for clarifying. I've added new patch files:

• 3270647-59-d10.patch: based against 10.0.x, including the block for Drupal 9.5.
• 3270647-59-d10-only.patch: minimal patch based against 10.0.x, without backward compatiblity for Drupal 9.5.
• 3270647-59-d9.patch: based against 9.5.x.

Please let me know if there's anything else I can do!

Patches have been updated to address a coding standards issue.

Got my wires crossed but just to reiterate/confirm we want

D9 = 3270647-52-d9.patch from #52
D10/11.x = 3270647-60-d10-only.patch from #60

Committed/pushed the patches from #60 and 59 to 11.x/10.1.x/10.0.x/9.5.x. Did my best with issue credits but a lot of people on this issue so apologies for any ommisions. Thanks!

Unfortunate this was closed as what ended up in 9.5 does not work. The solution from #20 does work for us.

Hello,
has it not been fixed on v9.5.10 - Issue #3270647 ???