Page MenuHomePhabricator

Incorrect email headers using PHP mail in PHP 8.0
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

  • Trigger an email (e.g. a password reset) when using PHP 8.0+

What happens?:

Message is sent with a "From" header like

From: "Wiki name" <address@example.com>
        Return-Path: webmaster@example.com
        Date: Sat, 15 Oct 2022 13:09:48 +0000
        Message-ID: <wiki.634ab11c2f4e03.37398010@example.com>
        X-Mailer: MediaWiki mailer
        List-Unsubscribe: <http://example.com/wiki/Special:Preferences>
        MIME-Version: 1.0
        Content-type: text/plain; charset=UTF-8
        Content-transfer-encoding: 8bit

What should have happened instead?:

Those are all separate headers, rather than being concatenated into the "From" header.

Software version (skip for WMF-hosted wikis like Wikipedia):

Git commit 120ed166aa369420bb6995a230796e2eb926d9ba

Other information (browser name/version, screenshots, etc.):

This is due to this bit of code in UserMailer.php

/**
 * Creates a single string from an associative array
 *
 * @param array $headers Associative Array: keys are header field names,
 *                 values are ... values.
 * @param string $endl The end of line character.  Defaults to "\n"
 *
 * Note RFC2822 says newlines must be CRLF (\r\n)
 * but php mail naively "corrects" it and requires \n for the "correction" to work
 *
 * @return string
 */
private static function arrayToHeaderString( $headers, $endl = PHP_EOL ) {
    $strings = [];
    foreach ( $headers as $name => $value ) {
        // Prevent header injection by stripping newlines from value
        $value = self::sanitizeHeaderValue( $value );
        $strings[] = "$name: $value";
    }
    return implode( $endl, $strings );
}

PHP's behavior has apparently changed in 8.0, it now wants RFC-compliant \r\n newlines.

OTOH, it seems the since 7.2 PHP's mail() function will accept an associative array for the $additional_headers parameter, so it would probably be best to just pass the headers array that way instead of concatenating to a string at all.

P.S. the "Return-Path" header included there is not according to RFC 5321, both because it does not use angle brackets around the address and because "A message-originating SMTP system SHOULD NOT send a message that already contains a Return-path header field."

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Hi @Anomie, how's it going?

I couldn't reproduce this with the ssmtp variant of sendmail. I think the comment on arrayToHeaderString() is incorrect, and the line ending repair actually occurs in the sendmail program, not in PHP. The change in PHP 8.0 is probably just PR #5338 which switches the line endings generated by PHP from \n to \r\n. It then falls to the mailer to misinterpret mixed line endings in the way shown in the task description.

I can change it to use an associative array, and I can confirm that that works in ssmtp, but I'll leave it to you to confirm the fix in whatever sendmail you are using.

Change 844863 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] When calling mail(), use an array for headers

https://gerrit.wikimedia.org/r/844863

Change 844863 merged by jenkins-bot:

[mediawiki/core@master] When calling mail(), use an array for headers

https://gerrit.wikimedia.org/r/844863

Change 844044 had a related patch set uploaded (by Jforrester; author: Tim Starling):

[mediawiki/core@REL1_39] When calling mail(), use an array for headers

https://gerrit.wikimedia.org/r/844044

Change 844045 had a related patch set uploaded (by Jforrester; author: Tim Starling):

[mediawiki/core@REL1_38] When calling mail(), use an array for headers

https://gerrit.wikimedia.org/r/844045

Change 845006 had a related patch set uploaded (by Jforrester; author: Tim Starling):

[mediawiki/core@REL1_35] When calling mail(), use an array for headers

https://gerrit.wikimedia.org/r/845006

Change 844044 merged by jenkins-bot:

[mediawiki/core@REL1_39] When calling mail(), use an array for headers

https://gerrit.wikimedia.org/r/844044

Change 845006 merged by jenkins-bot:

[mediawiki/core@REL1_35] When calling mail(), use an array for headers

https://gerrit.wikimedia.org/r/845006

Change 844045 merged by jenkins-bot:

[mediawiki/core@REL1_38] When calling mail(), use an array for headers

https://gerrit.wikimedia.org/r/844045

Reedy moved this task from Backlog to MediaWiki core on the PHP 8.2 support board.
Reedy moved this task from Backlog to MediaWiki core on the PHP 8.0 support board.
Reedy subscribed.
tstarling claimed this task.