Fixed mailing issues with multipart/mixed (i.e. HTML e-mails with attachments)

hotspoons - May 27, 2009 - 03:39
Project:Mass Contact
Version:6.x-1.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:needs review
Description

When I first tried this module out about a month ago, I used the stable version of 6.x, and the only thing it would ever send out correctly was plain text e-mails. I updated it to the 6.x-1.x-dev version, and it would send out HTML e-mails sometimes, but would never send out e-mails with attachments.

I took it upon myself to figure it out, and it ends up being two issues; one was the module's fault, not putting an boundary end-marker after the HTML portion of an e-mail message containing both HTML/plain text (i.e. multipart/alternative) *and* an attachment. The second (larger) issue was the drupal_mail() function calling drupal_wrap_mail(), which is great for plain text, but not so great for anything else, and it essentially mangled the e-mail messages to incomprehensible nothing.

I fixed both issues, mainly by adding a conditional to add the end marker right before the attachment, if the e-mail contains HTML, as well as duplicating the drupal_mail() function locally (calling it _mass_contact_mail()), removing the drupal_wrap_mail() call. I also learned a whole hell of a lot more about how (multipart) e-mail works than I did earlier today.

I attached a diff - let me know if it works. Thanks!

-Rich

AttachmentSize
511cfad8189d.diff9.18 KB

#1

DanielJohnston - May 31, 2009 - 00:07

Interested in this one. If I can get Mass Contact working *at all*, then I'll install the patch and give it a bash. Oh how I wish Simplenews by role was available in 6.x

#2

DanielJohnston - May 31, 2009 - 00:42

Shazam! It works like a dream, thanks. I no longer get weird unidentified message parts in received emails. Can we get this committed? I haven't marked as 'tested & reviewed' as I don't think just me counts as 'the community', but surely this should be tested and ok'ed sharpish, as the module's basically useless for HTML mail otherwise?

#3

hotspoons - May 31, 2009 - 22:39

I'm glad it works! I agree on committing it since the module is more or less useless for D6 in its current state. One suggestion for the maintainer may be to open an upstream bug report for the drupal_mail() function to detect non-plain-text e-mails, and in that case, do not call the drupal_wrap_mail() function; that would eliminate duplicating the drupal_mail function locally and make this fix only require a two lines of code (the change @@ -1269,16 +1380,18 @@ function mass_contact_mail_page_submit ). The upstream fix would be something like this:

Current:

<?php
 
if (function_exists($function = $module .'_mail')) {
   
$function($key, $message, $params);
  }

 
// Invoke hook_mail_alter() to allow all modules to alter the resulting e-mail.
 
drupal_alter('mail', $message);

 
// Concatenate and wrap the e-mail body.
 
$message['body'] = is_array($message['body']) ? drupal_wrap_mail(implode("\n\n", $message['body'])) : drupal_wrap_mail($message['body']);
?>

Proposed change to drupal_mail():

<?php
 
if (function_exists($function = $module .'_mail')) {
   
$function($key, $message, $params);
  }

 
// Invoke hook_mail_alter() to allow all modules to alter the resulting e-mail.
 
drupal_alter('mail', $message);

 
// Test to see if hook_mail modified the content type for the header,
  // and if not, concatenate and wrap the e-mail body.
 
if($message['headers']['Content-Type'] == $headers['Content-Type'])
   
$message['body'] = is_array($message['body']) ? drupal_wrap_mail(implode("\n\n", $message['body'])) : drupal_wrap_mail($message['body']);
?>

Or similar (possibly look for "text/plain" contained in the header, though it would add a minor performance penalty). However, for the time being, my fix should work just fine. Thanks!

#4

kate - November 3, 2009 - 15:44

I am using Drupal 6 and just tried to get the dev version of the mass_contact module to work and was experiencing the same problems as mentioned above. So I tried the patch posted here, but I get three out five chunks failed. I am pretty new to this stuff but I would love to get this working. Any ideas on how I can it to work?

Thanks,
Kate

#5

univate - November 9, 2009 - 03:27
Status:patch (to be ported)» needs review

I am needing similar functionality so I will test this out.

Also wondering whats the reason for the addition of all the blank lines in this?

#6

ligerbots - November 29, 2009 - 15:40

Could someone upload the fixed .module file? For some reason, the diff file didn't work. I tried to fix it by hand, but must have missed something.
Benji

 
 

Drupal is a registered trademark of Dries Buytaert.