duplicate image items in attachment, solved

DTB - January 13, 2009 - 23:03
Project:Mime Mail
Version:6.x-1.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:needs work
Issue tags:duplicate image attach
Description

Hi,
If use one image more than once, mime module process all image, and attach to mail.
I modify the function _mimemail_file and solve this problem, but need more testing.
The modified function:

function _mimemail_file($url = NULL, $name = '', $type = '', $disposition = 'related') {
  static $files = array();
  static $filenames = array(); // this modifycation, store all filenames with full path
  if ($url) {
    $url = _mimemail_url($url, 'TRUE');

    // If the $url is absolute, we're done here.
    if (strpos($url, '://') || preg_match('!mailto:!', $url)) {
      return $url;
    }
    else {
    // The $url is a relative file path, continue processing.
      $file = $url;
    }
  }

  if ($file && file_exists($file)) {
    if (isset($filenames[$file])) return 'cid:'. $filenames[$file]; // this modifycation, if key exists, return old value

    $content_id = md5($file) .'@'. $_SERVER['HTTP_HOST'];

    if (!$name) $name = substr($file, strrpos($file, '/') + 1);

    $new_file = array(
      'name' => $name,
      'file' => $file,
      'Content-ID' => $content_id,
      'Content-Disposition' => $disposition,
    );
    $new_file['Content-Type'] = _mimemail_mimetype($file, $type);

    $files[] = $new_file;
    $filenames[$file] = $content_id; // this modifycation, store filename to key, content_id to value
    return 'cid:'. $content_id;
  }

  $ret = $files;
  $files = array();
  return $ret;
}

#1

jerdavis - February 22, 2009 - 01:17
Status:active» needs work

Thanks for submitting code, but please read http://drupal.org/patch/create and provide a patch, it's much much easier for us to work with and saves everyone time.

Thanks!

Jer

#2

folkertdv - February 23, 2009 - 12:27

Came across this bug months ago as well and fixed it but somehow never submitted a patch.
The suggested patch looks clean and I've created a patch file for submission. Great work DTB :)

AttachmentSize
mimemail-358439-2.patch 959 bytes

#3

jerdavis - February 23, 2009 - 16:12
Status:needs work» fixed

Committed!

This should show up in the next development snapshot. Thanks for creating the patch folkertdv and thanks for submitting the code DTB.

Jer

#4

System Message - March 9, 2009 - 16:20
Status:fixed» closed

Automatically closed -- issue fixed for 2 weeks with no activity.

#5

folkertdv - April 2, 2009 - 08:02
Status:closed» needs review

Reopening this issue because the patch above introduces new problems (an additional patch file is provided).

Problem:
If the mimemail function is called, certain files are embedded and their cid's are stored in the filenames array (to prevent duplicates).
Subsequent calls of the mimemail function will not embed files that have been embedded in previous mails.

Cause:
The static filenames array isn't emptied at the end of the _mimemail_file function like the static files array.

Adding the single line in the attached patch file has fixed the problem for me.

Cheers!

AttachmentSize
mimemail-358439-5.patch 254 bytes

#6

folkertdv - April 15, 2009 - 08:15

Bumping this issue before it closes, this (minor) patch still needs reviewing :)

#7

Eikaa - April 21, 2009 - 07:10

@folkertdv patch in #5 works

thanks a lot!

i was wondering why only the latest subscriber got the attachments...

#8

makr - July 18, 2009 - 14:08

Thanks, this also resolves issue http://drupal.org/node/453334, where only the first recipient receives the email-newsletter containing pictures (simplenews + mimemail).

#9

askibinski - August 3, 2009 - 12:19

Has this already been commited to the latest dev?

#10

miro_dietiker - September 16, 2009 - 17:03

subscribing & reviewing

#11

miro_dietiker - September 18, 2009 - 10:40
Status:needs review» reviewed & tested by the community

Works just perfect. Please commit.

#12

zoo33 - October 30, 2009 - 11:49
Status:reviewed & tested by the community» needs work

The fix suggested in #5 would just undo the static nature of the $filenames variable, wouldn't it? In that case, a better solution would be to just remove the "static" keyword. Or rather, revert the original patch.

By the way, the $files variable has the same problem, it shouldn't be static if that means it has to be reset each time the function executes. Static variables are for data that should be preserved between function calls.

#13

VDMi.Frans - November 8, 2009 - 21:02

subscribe

 
 

Drupal is a registered trademark of Dries Buytaert.