WordPress.org

Make WordPress Core

Opened 18 months ago

Closed 41 hours ago

Last modified 40 hours ago

#43542 closed defect (bug) (fixed)

Duplicate MIME-Version header

Reported by: junktrunk Owned by: SergeyBiryukov
Milestone: 5.3 Priority: normal
Severity: normal Version: 4.9.4
Component: Mail Keywords: good-first-bug 2nd-opinion has-patch
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Issue: Duplicate "MIME-Version" header information can be sent via pluggable.php and class-phpmailer.php via plugins, causing the mail to fail to be delivered when using a service like AWS SMTP that rejects this duplicate header element.

I see that this issue has been reported in the past but closed as an invalid bug (e.g. #22492 from 5 years ago). But I argue that that may not be the correct conclusion.

Wordpress plugin developer guidance on the use of wp_mail() doesn't prohibit including MIME-Version in $headers , and does otherwise offer to "build the proper Mime header automagically". I have read an argument online that it may be proper for the plugin to build out the full header including the mandatory MIME-Version in the event the administrator has configured their site to use a different email service besides WP/PHP, though I haven't substantiated that fully.

Personally I think this should be a configurable option within any plugin, defaulted to off, if included at all. But in googling this issue, I see that this problem occurs in a large number of plugins, and for years, going unfixed, resulting in probably millions of unsent messages. For example, the following bug report for Email Subscription (the plugin I am working with that caused me to re-re-discover this problem) is from 2014, where it's still a problem today: https://github.com/Nyholm/Wordpress-Email-Subscription/issues/10

Regardless of the best component where this element should be added, the default installation of WP should have checks to avoid duplication that causes failures, especially given there is an easy fix. In pluggable.php, in the loop processing each of the $headers the plugin supplied, we can check to see whether any of those will be duplicated by class-phpmailer.php, omitting them if so:

        // Set custom headers
        if ( !empty( $headers ) ) {
                foreach ( (array) $headers as $name => $content ) {
                        // Only add custom header if not one that will be added automatically
                        // by wp-includes/class-phpmailer.php
                        if (('MIME-Version' !== $name) && ('X-Mailer' !== $name)) 
                                $phpmailer->addCustomHeader( sprintf( '%1$s: %2$s', $name, $content ) );
                }

(In my code snippet above I included X-Mailer too because that's also added automatically by class-phpmailer.php and doesn't make sense to duplicate, though it is not causing the same mail delivery failure when duplicated.)

Attachments (4)

43542.diff (729 bytes) - added by lbenicio 18 months ago.
.diff (867 bytes) - added by bennemann 17 months ago.
Patch for preventing to set multiple headers
class-phpmailer.php (143.3 KB) - added by aryamaaru 16 months ago.
43542.2.diff (3.2 KB) - added by lbenicio 15 months ago.

Download all attachments as: .zip

Change History (15)

#1 @junktrunk
18 months ago

I'm also happy to commit the fix if you agree this should be fixed.

#2 @danieltj
18 months ago

  • Keywords needs-patch good-first-bug reporter-feedback added

Welcome to Trac @junktrunk.

The best thing to do is to create a patch if you think you have a possible fix for this and upload to the ticket as either a .patch or .diff file (diffs and preferred). Then if someone has time they can test the patch for you and provide feedback on it before possible inclusion within core.

The old ticket references BuddyPress, are you running into this issue because of that plugin or is this directly related to core? If it is related BuddyPress, it'll be worth reaching out to the developers for that plugin rather than here. If it's not, carry on as you were! :)

Related to #22492.

@lbenicio
18 months ago

#3 @studyboi
17 months ago

  • Keywords 2nd-opinion added

To solve the problem of having the header be duplicated by having this function called twice, I added a fail-safe to prevent it. It checks whether a header has been created yet before executing the code to add one.

// Set custom headers
        if ( !empty( $headers ) ) {
                foreach ( (array) $headers as $name => $content ) {
                        // Only add custom header if not one that will be added automatically
                        // by wp-includes/class-phpmailer.php
                        if (('MIME-Version' !== $name) && ('X-Mailer' !== $name)){ 
                                if (get_header($name) == ""){
                                        $phpmailer->addCustomHeader( sprintf( '%1$s: %2$s', $name, $content ) );
                                }
                        }
                }

@bennemann
17 months ago

Patch for preventing to set multiple headers

#4 @bennemann
17 months ago

I just put the solution from @studyboi into a .diff

#5 @SergeyBiryukov
17 months ago

  • Description modified (diff)

@lbenicio
15 months ago

#6 @noisysocks
15 months ago

  • Keywords has-patch added; needs-patch removed

#7 @SergeyBiryukov
5 months ago

  • Keywords reporter-feedback removed
  • Milestone changed from Awaiting Review to 5.3
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

This ticket was mentioned in Slack in #core by sergey. View the logs.


5 weeks ago

#9 @SergeyBiryukov
41 hours ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 46115:

Mail: Avoid setting duplicate MIME-Version and X-Mailer headers in wp_mail(), they are added automatically by PHPMailer.

Props lbenicio, junktrunk, danieltj, studyboi, bennemann.
Fixes #43542.

#10 @SergeyBiryukov
41 hours ago

In 46116:

Coding Standards: Fix WPCS issue in [46115].

See #43542.

#11 @SergeyBiryukov
40 hours ago

In 46118:

Mail: Add a unit test to make sure wp_mail() does not duplicate the MIME-Version header added automatically by PHPMailer.

See #43542.

Note: See TracTickets for help on using tickets.