| Project: | Mass Contact |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | wims |
| Status: | closed (fixed) |
Issue Summary
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
| Attachment | Size |
|---|---|
| 511cfad8189d.diff | 9.18 KB |
Comments
#1
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
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
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
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
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
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
#7
It's probably because the .diff was rolled against an old version of the development release of this module. I have an old, old dev version of the .module I can post, but I'll see if I can get around to rolling a new diff over the next couple of days.
#8
What's the current status on this? Has this fix now been included in the dev release? Any chance of getting it included in the stable release?
#9
i applied the patch manually.. and its rombuse all the module..
#10
It took me a bit of time, but patching manually (and a bit of logic) ended up in a full working module. Just the thing i needed.
Thanks :-)
#11
Could someone upload a working version (or diff)? I may give the patch another go, but I had no luck doing it by hand before.
Benji
#12
I've just hacked this in by hand - works for me now.
#13
I tested iridium's version and it works perfectly now. Now I can send HTML content with attachment.
Thanks.
#14
Hi, can someone please tell how to use this patch?
#15
What a pitty that this patch is not integrated in the last dev version.
The #12 by iridium77 patch is on the v 1.9.2.10 version of the mass_contact.module
The last dev verrsion is v 1.9.2.16 so just spot the differences.
Is it planed to integrate these bugfixes in the last version?
#16
I finally found a copy of this module that was patched. This is an old, old version of the module, but it should work, or prove educational to anyone trying to re-implement this against head.
#17
Is there any chance of getting it committed? I am willing to test if you need it. Just let me know!
#18
I've committed the proposed changes to the 6.x-1.x-dev version here: http://drupal.org/cvs?commit=361692
Please test it and let me know if it works for you. It may be a while before the download gets updated.
I don't like this solution, but I'm going to leave it until I figure out something better.
One option is to use the SMTP Authentication module instead of the normal drupal_mail, which is what I was using, and why I didn't see this behavior.
Also, for future reference, you're more likely to get patches submitted if you create them against the current -dev release and do it the correct way: http://drupal.org/patch/create
#19
thanks, I will try it out shortly!
#20
It worked perfectly. The attachment was not corrupted and there were no errors. Thank you very much!
#21
I hope this is the correct place to insert a "finding" on this matter. I was struggling to get Mass_contact module to work. Then when reading this particular ticket, I tried using the Mass_contact in Plain Text mode. It worked GREAT!
So, suddenly the problem I thought was a "false SPAM" was in fact a problem with MIME headers...
I only know enough to be dangerous. So, I'm leaving this thought and finding here for the "Big Guns" to review..
After TON's of debugging, I found that the drupal_mail() call was the problem. Specifically the Header's. The messages where getting sent, but the headers were not good and thus the mail messages where getting dumped at the server. This explains why the on screen messaging that says your email was sent is being displayed. They are being sent, it's just not good stuff..
When I make the following change within the drupal_mail() function found in the includes/mail.inc file... HTML mail and plain text mail worked GREAT. So did Attachments..
Specifically I commented OUT the 'Content-Type' entry that was there, and used one of the one's that have been successful when making non-Drupal sites.
// Build the default headers$headers = array(
'MIME-Version' => '1.0',
// 'Content-Type' => 'text/plain; charset=UTF-8; format=flowed; delsp=yes',
'Content-Type' => 'text/html;',
'Content-Transfer-Encoding' => '8Bit',
'X-Mailer' => 'Drupal'
);
Everything works now... Somebody who knows more .. might be able to explain why..
#22
@ScottBaetz: Which version are you using? It sounds to me like your problem should not have existed after I submitted the changes recommended in this issue.
#23
@oadaeh: Thanks for the direct response.. I'm using 6.x-1.0.
#24
Turns out this wasn't the solution AT ALL. Nope. The module in fact had a misplaced set of IF statements that once corrected worked BEAUTIFULLY..
Here's my reference
http://www.webcheatsheet.com/PHP/send_email_text_html_attachment.php#html
I wanted to validate that MIME format - as I mentioned - I'm not a MIME expert. This page does a fine job. With this information in hand, I quickly realized that the
function mass_contact_mail_page_validate($form, &$form_state) {was building the HTML without attachment wrong. Turns out that there is a set of IF statements for Attachments that need a bit of clean up. Here's the Original code..
// If there is an attachment, add it.
if ($_FILES['files']['size']['attachment'] > 0) {
// There is currently no Mime Mail module for D6.
// Update: now that there is a D6 version of Mime Mail, I'll have to fix
// this, but not right now.
/*
if (module_exists('mimemail') && variable_get('mimemail_alter', 0) == 1) {
$message_attachment = array(
array(
'filepath' => $_FILES['files']['tmp_name']['attachment'],
'filename' => $_FILES['files']['name']['attachment'],
'filemime' => $_FILES['files']['type']['attachment'],
)
);
}
else {
*/
if ($form_state['values']['html'])
$message .= "\n--". $boundary_html ."--\n";
$message_attachment = "\n--". $boundary_attachment ."\n";
$message_attachment .= 'Content-Type: '. $_FILES['files']['type']['attachment'] .'; name="'. basename($_FILES['files']['name']['attachment']) .'"'."\n";
$message_attachment .= "Content-Transfer-Encoding: base64\n";
$message_attachment .= 'Content-Disposition: attachment; filename="'. basename($_FILES['files']['name']['attachment']) .'"'."\n\n";
$message_attachment .= chunk_split(base64_encode(file_get_contents($_FILES['files']['tmp_name']['attachment'])));
$message_attachment .= "\n--". $boundary_attachment ."--\n";
$message .= $message_attachment;
// }
}
Here's the updated code..
if ($form_state['values']['html']) { $message .= "\n--". $boundary_html ."--\n"; } // This ALWAYS IS CHECKED NOW!!
// This is ALWAYS CHECKED NOW TOO!
if ($_FILES['files']['size']['attachment'] > 0) {
$message_attachment = "\n--". $boundary_attachment ."\n";
$message_attachment .= 'Content-Type: '. $_FILES['files']['type']['attachment'] .'; name="'. basename($_FILES['files']['name']['attachment']) .'"'."\n";
$message_attachment .= "Content-Transfer-Encoding: base64\n";
$message_attachment .= 'Content-Disposition: attachment; filename="'. basename($_FILES['files']['name']['attachment']) .'"'."\n\n";
$message_attachment .= chunk_split(base64_encode(file_get_contents($_FILES['files']['tmp_name']['attachment'])));
$message_attachment .= "\n--". $boundary_attachment ."--\n";
$message .= $message_attachment;
// }
}
I've reordered the IF statements so that the HTML is ALWAYS seen..
Hope this helps someone..
#25
@ScottBaetz: The code you have listed in your version of the original code is not correct. If you download a copy again and check that point in the file, you will find that there is an opening and closing brace around the
$message .= "\n--". $boundary_html ."--\n";line. The code, as you have it posted above, will not even work, because there is a functional error with that if statement.In every copy of the code you can download, that area looks like this:
if ($form_state['values']['html']) {$message .= "\n--". $boundary_html ."--\n";
}
Also, moving the
if ($_FILES['files']['size']['attachment'] > 0) {line down is totally unnecessary.#26
Automatically closed -- issue fixed for 2 weeks with no activity.