I am trying to send an e-mail with embedded images. In order to embed images, the "src" has to be a relative path. Also, there is an option in Mimemail module "Link images only" and I have it turned off, as default.

I've included two types of images: one non-styled (just the original image) and one styled (generated automatically by Drupal in "styles" directory).

In the node body, the non-styled image has the "src" value as:

src="/sites/files/image/newsletter-header_1.png"

and the styled image has the "src" value:

src="/sites/files/styles/newsletter/public/images/img1_1.jpg?itok=L7G7Chx8"

The itok is added since 7.20

As the result, when the mail is received, the first image is displayed at once (without clicking on "show images" in the email client) and the second image is not embedded at all (after clicking on "show images", the email html code shows img with malformed "src" as described below).

I noticed that the problem might be around line 178 in mimemail.inc.

In line 

else {
      $url = _mimemail_url($url, 'TRUE');
      // The $url is absolute, we're done here.
      $scheme = file_uri_scheme($url);
      if ($scheme == 'http' || $scheme == 'https' || preg_match('!mailto:!', $url)) {
        return $url;
      }
      // The $url is a non-local URI that needs to be converted to a URL.
      else {
        $file = (drupal_realpath($url)) ? drupal_realpath($url) : file_create_url($url);

First, the $url gets the leading slash stripped and becomes "sites/files/styles/newsletter/public/images/img1_1.jpg?itok=L7G7Chx8".

Then, the file_create_url("sites/files/styles/newsletter/public/images/img1_1.jpg?itok=L7G7Chx8") returns this:
http://example.com/sites/files/styles/newsletter/public/images/img1_1.jpg%3Fitok%3DL7G7Chx8

And then, there is a check whether this is an existing file, which fails. When typing the addres with html entities in the browser address there is a blank screen with just one message: "Error generating image".

I've suspended the itok generation for now (by using $conf['image_allow_insecure_derivatives'] = TRUE;) and the mail is ok. But it's a security issue.

[Edit] I am not sure whether it is mimemail problem or other module problem.

CommentFileSizeAuthor
#23 fix-itok-images-2145659-20.patch1.74 KBanybody
#20 fix-itok-images-2145659-20.patch827 bytesanybody
#9 mimemail-2145659-9.patch1.17 KBsgabe
#8 mimemail_embedding-2145659.patch814 bytesfatherguddha

Comments

raincloud’s picture

raincloud commented
Issue summary: View changes
raincloud’s picture

raincloud commented
Issue summary: View changes
raincloud’s picture

raincloud commented
Title: Styled images with relative paths are not embedded (itoc?) » Styled images with relative paths are not embedded (itok?)
Issue summary: View changes
fatherguddha’s picture

fatherguddha commented

The problem appears to be that drupal_realpath($url) won't handle the new security key (?itok=L7G7Chx8).

Stripping off the parameter appears to solve the problem. I think this is safe when images are embedded (as opposed to linked).

I've replaced the following code which appears just a few lines above the code you referenced:

    $image = preg_match('!\.(png|gif|jpg|jpeg)$!i', $url);
    $linkonly = variable_get('mimemail_linkonly', 0);

with:

    $linkonly = variable_get('mimemail_linkonly', 0);    
    if (!$linkonly) {
      $url = preg_replace('/\\?itok=.*$/', '', $url); //Remove security key 
    }
    
    $image = preg_match('!\.(png|gif|jpg|jpeg)$!i', $url);
sgabe’s picture

sgabe commented
Status: Active » Closed (duplicate)

This should be already fixed by #2152705: Images with 'itok' token not showing up. I am marking this as a duplicate.

fatherguddha’s picture

fatherguddha commented

Hmm...I was still experiencing this problem with the 7.x-1.x-dev build on January 19th, which did include the patch on issue 2152705.

sgabe’s picture

sgabe commented
Status: Closed (duplicate) » Active

On second fought, I see this is about embedding images, the other issue is about linking images, so this is a different issue after all. My bad, sorry.

fatherguddha’s picture

fatherguddha commented
StatusFileSize
new mimemail_embedding-2145659.patch814 bytes

I've created a patch for the Feb 3rd 7.x-1.x-dev version. Hopefully the solution really is as straightforward as it seems.

sgabe’s picture

sgabe commented
Status: Active » Needs review
StatusFileSize
new mimemail-2145659-9.patch1.17 KB

I think we can handle this in _mimemail_url() when returning an URL to embed a file. See the attached patch.

Status: Needs review » Needs work

The last submitted patch, 9: mimemail-2145659-9.patch, failed testing.

sgabe’s picture

sgabe commented
Status: Needs work » Needs review

Test does not run online, but still needs review.

The last submitted patch, 8: mimemail_embedding-2145659.patch, failed testing.

fatherguddha’s picture

fatherguddha commented

Patch at #9 works for me.

sgabe’s picture

sgabe commented
Title: Styled images with relative paths are not embedded (itok?) » Images with 'itok' token are not embedded
Status: Needs review » Fixed

Committed, thanks!

fatherguddha’s picture

fatherguddha commented

Thanks for your work on this - appreciate it :)

Status: Fixed » Closed (fixed)

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

matysek145’s picture

matysek145 commented
Version: 7.x-1.0-beta1 » 7.x-1.0-beta3
Status: Closed (fixed) » Needs work

Hi!
So, maybe I don't get something, but how is removing itok from the url supposed to make my images embed correctly?

I use Mime Mail in Simplenews with Entity Reference and this is my case:
1) Create some nodes with images
2) Create Simplenews newsletter with previous nodes referenced. Images are shown with some image style.
3) So far so good. New styled images are generated and i can see them in sent (test) newsletter
4) But now i upload new image in one of the nodes. Newsletter is already generated, so i don't even visit its node page, so new image has no chance to generate its style.
5) Send newsletter again - there is no itok, so if image doesn't already exist, is not going to be created, and finally is not displayed in newsletter

When i commented this line
$url = preg_replace('/\\?itok=.*$/', '', $url);
everything started to work :)

So I'm pretty sure, we need this itok. Or, if there is something obvious i miss, please, tell me :)
Cheers!

sgabe’s picture

sgabe commented
Status: Needs work » Closed (fixed)

@matysek145: This issue is about existing images. What you are looking for is in #1309248: Force generation of Image Styles.

killes@www.drop.org’s picture

killes@www.drop.org commented
Status: Closed (fixed) » Needs work

I think this patch is not sufficient.

The check in line 170 of mimemail.inc still fails:

$image = preg_match('!\.(png|gif|jpg|jpeg)$!i', $url);

If I strip the itok parameter before calling this, it works in my case.

This is for images that have been embedded into the mail text.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs work » Needs review
StatusFileSize
new fix-itok-images-2145659-20.patch827 bytes

I can confirm the problem still exists!

To fix this I'd suggest to simply move the preg_replace two lines lower into the $to_link!
I think that's correct and what we need. Let's see what testbot says. Patch attached!

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Priority: Normal » Major

Setting up the priority because the image embedding functionality is broken for all images with an itok!

Status: Needs review » Needs work

The last submitted patch, 20: fix-itok-images-2145659-20.patch, failed testing.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs work » Needs review
StatusFileSize
new fix-itok-images-2145659-20.patch1.74 KB

Netbeans on windows makes me crazy :P

Status: Needs review » Needs work

The last submitted patch, 23: fix-itok-images-2145659-20.patch, failed testing.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Well now I think the logic of the tests themselves should be checked. From my point of view they seem to test the broken functionality. Is there a maintainer who can have a look?

adamps’s picture

adamps commented
Status: Needs work » Closed (fixed)

@Anybody @matysek145 I think the brief comment in #18 is important to understand. To expand, as I understand it:

  • This issue is about embedding images.
  • Embedding images is only going to work if the file already exists.
  • There is another issue #1309248 to force the images to exist that needs port to D7.
  • Embedding existing images was broken by the itok and this has now been fixed.

Now I guess that you two are interested in a different scenario - when the option "Link images only" is selected. The commit for this issue seems to have made things worse for linked images. Previously, if the image didn't exist, it was generated on the fly. Without the itok this no longer works.

@sgabe the maintainer has already closed this issue twice because he believes the original issue is solved. Seeing as this is a different bug, I have created a new issue: #2552613

@Anybody, the patch #25 seems wrong to me. Surely "Remove security token from URL" needs to be done if not $to_link. I think this patch breaks the tests because it breaks the fix - maybe you could check by testing your fix in the scenario described in this issue? Maybe you have not actually selected "Link images only", but are happy to accept linked images rather than embedded ones? If so I suggest you turn this option on.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Thank you, @AdamPS.

I'm currently very busy and can not have a look in the next weeks. So I'd hope for feedback from other issue contributors here first.
I'll reply ASAP.

adamps’s picture

adamps commented

@Anybody Thanks for the update. I think it would be best if you put any further discussion in the new linked issue.