SparkPost / wordpress-sparkpost

WordPress plugin to use SparkPost email
https://wordpress.org/plugins/sparkpost/
20 stars 15 forks source link

Why is Content-Type unsupported? #132

Closed d4mation closed 7 years ago

d4mation commented 7 years ago

Referencing the get_headers() method:

https://github.com/SparkPost/wordpress-sparkpost/blob/eb41d01c5f4b164d7f123ef7fd5e0e7182a8cef7/mailer.http.class.php#L430-L462

If you use wp_mail() and add an attachment using the $attachments parameter to include an attachment that exists on the server it ends up adding Content-Type: multipart/mixed; to the Header and it gets successfully sent by SparkPost with the Attachment in place and the Content-Type header intact. Because wp_mail() added it on its own it works just fine.

However, if you manually set the Content-Type Header as part of the $headers parameter and define a boundary to place within your Message Body to add a base64 encoded Attachment from Memory (Something wp_mail() doesn't support using the $attachments parameter, but it is exactly what it does in reality), then SparkPost strips out the Content-Type Header and sends the message in plaintext.

It seems strange to me since it would appear the HTTP API supports it, but the code is set to remove it if it is noticed.

We're not using a Template for our emails. Just having SparkPost handle delivery to ensure they make it to their destination properly.

Note

I know that we could use the wpsp_body_headers Filter to get around this, but this is unfortunately code that is going to be distributed to be installed on other user's sites. I was testing it and while it works fine in MailCatcher, SparkPost is mangling the email that gets sent out. I don't want to have to include a Filter specific to another plugin within the codebase if I don't have to, so I'm just curious as to why Content-Type needs to be excluded considering it seems to work when using a locally stored Attachment.

d4mation commented 7 years ago

I did just test this after removing Content-Type from the Unsupported Headers array and it does not work. Still seems strange that when wp_mail() sets it it appears to work though 🤔

rajumsys commented 7 years ago

@d4mation thanks for posting in detail. however, i also need a refresher how all things work together. here are a quick things:

rajumsys commented 7 years ago

Can also provide some steps to reproduce the issue you've mentioned including the output you're seeing and expected output.

d4mation commented 7 years ago

@rajumsys I actually just managed to figure this out. But because I don't want to be that guy, I'm going to explain thoroughly what I did.

I was generating my own Multipart Boundaries and setting the Content-Type header manually. Something like this:

$semi_rand = md5( time() );
$mime_boundary = "==Multipart_Boundary_x{$semi_rand}x"; 

$message_multipart = "--{$mime_boundary}\n";

// Wrap the Message in a boundary with its own Headers
$message_multipart .= "Content-Type: text/plain; charset=\"iso-8859-1\"\n";
$message_multipart .= "Content-Transfer-Encoding: 7bit\n\n";
$message_multipart .= $message . "\n\n";

$message_multipart .= "--{$mime_boundary}\n";

// Wrap the Attachment Buffer in a boundary with its own Headers
$message_multipart .= "Content-Type: text/plain\n";
$message_multipart .= " charset=UTF-8\n";
$message_multipart .= " name=\"support_site_info.txt\"\n";
$message_multipart .= "Content-Transfer-Encoding: base64\n";
$message_multipart .= "Content-Disposition: attachment;\n";
$message_multipart .= " filename=\"support_site_info.txt\";\n\n\n";
$message_multipart .= chunk_split( base64_encode( $debug_file ) ) . "\n\n";

$message_multipart .= "--{$mime_boundary}--";

$result = wp_mail(
    'support@website.com',
    $subject,
    $message_multipart,
    array(
        "From: $license_data[customer_name] <$license_data[customer_email]>",
        "Content-Type: multipart/mixed; boundary=\"{$mime_boundary}\"", // Here we define the Boundary within the primary Email Header
    ),
    array(
    )
);

This appeared to work in MailCatcher just fine, but when I went to test it on a remote/real server, the email was straight up plaintext.

Turns out this is because PHPMailer doesn't care if you set all your headers correctly to send an Attachment in this way. If there is nothing set within its own $attachments array, it sets the Content-Type header to Content-Type: text/html; charset="UTF-8" which overrides any other Content-Type you had set in place previously.

I really wanted to avoid having to upload a file to the server just to attach it to an email. Especially since the file is dynamically generated anyhow. The only way to do this with PHPMailer is using the addStringAttachment() method which isn't supported by wp_mail(). However, looking at this WordPress Core Trac Ticket I remembered that the phpmailer_init hook existed. However, in this Ticket they mention there is no way to differentiate between the context in which that Action is fired off.

This got me thinking about Custom Headers. I could easily just pass in a Custom Email Header (Say, the version of my plugin or something) and check for that. If it exists, attach the Debug File. That's what I did and it worked!

Here's the working code in case it helps anyone in the future:

add_action( 'init', 'send_email' );
function send_email() {

    $result = wp_mail(
        'support@website.com',
        $subject,
        $message,
        array(
            "From: $license_data[customer_name] <$license_data[customer_email]>",
            "X-WEBSITE-SUPPORT: " . get_plugin_version(),
        ),
        array(
        )
    );

}

/**
 * Add the Debug File to the Email in a way that PHPMailer can understand
 * 
 * @param       object $phpmailer PHPMailer object passed by reference
 *                                                      
 * @since       {{VERSION}}
 * @return      void
 */
add_action( 'phpmailer_init', 'add_debug_file_to_email' );
function add_debug_file_to_email( &$phpmailer ) {

    foreach ( $phpmailer->getCustomHeaders() as $header ) {

        if ( $header[0] == 'X-WEBSITE-SUPPORT' ) {

            $phpmailer->addStringAttachment( get_debug_file(), 'support_site_info.txt' );
            break;

        }

    }

}

Edit: I had the arguments for addStringAttachment() incorrect.

d4mation commented 7 years ago

Actually, it turns out that while the attachment is added, SparkPost may be clearing it out. It is empty when it makes it to my inbox but MailCatcher shows it has content.

I suppose I'll reopen this.

rajumsys commented 7 years ago

I don't think I was able to follow your previous messages/findings, but at first glance, I see that you've also used phpmailer_init hook ~which is exactly same one SparkPost uses~. So it might be that you and plugin is playing in different world. You can try your manipulation in wpsp_init_mailer hook.

d4mation commented 7 years ago

I've discovered why this is not working. It is specifically a problem with the WordPress SparkPost Plugin.

While it sends the Attachment as a String, it expects ALL Attachments to be a File Path. In get_attachments() it calls another method named read_attachment() which only supports a File Path.

Because my String data is not a file path, it returns an empty String.

https://github.com/SparkPost/wordpress-sparkpost/blob/master/mailer.http.class.php#L222-L225

Is there any chance that this could instead determine whether or not running file_get_contents() is necessary? Running file_exists() on the provided String and if it doesn't exist, just passing the String through as the Data for the get_attachments() method should work fine.

rajumsys commented 7 years ago

@d4mation That's true. SparkPost assumes those to be path. Great finding. I will appreciate if you either send a PR with fix or at least open a new issue explaining just that problem.

d4mation commented 7 years ago

@rajumsys Just tested my fix and opened a PR for it :+1:

I'll close this Issue.