WordPress.org

Make WordPress Core

Opened 3 months ago

Last modified 7 weeks ago

#43542 new defect (bug)

Duplicate MIME-Version header

Reported by: junktrunk Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.9.4
Component: Mail Keywords: needs-patch good-first-bug reporter-feedback 2nd-opinion
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 2 months ago.
.diff (867 bytes) - added by bennemann 7 weeks ago.
Patch for preventing to set multiple headers
class-phpmailer.php (143.3 KB) - added by aryamaaru 4 weeks ago.
43542.2.diff (3.2 KB) - added by lbenicio 13 days ago.

Download all attachments as: .zip

Change History (9)

#1 @junktrunk
3 months ago

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

#2 @danieltj
3 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
2 months ago

#3 @studyboi
2 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
7 weeks ago

Patch for preventing to set multiple headers

#4 @bennemann
7 weeks ago

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

#5 @SergeyBiryukov
7 weeks ago

  • Description modified (diff)

@lbenicio
13 days ago

Note: See TracTickets for help on using tickets.