After a long debugging journey, I found an issue with embedded images.
Swiftmailer is embedding all images starting with "image:" into the mail body. This feature was not working on my environment.

If found out, that the following line was the issue:

$image->cid = rand(0, 9999999999);

In my environment this line always returnes null because 9999999999 was higher than mt_getrandmax().

I replaced 

$image->cid = rand(0, 9999999999);

with

$image->cid = rand(0, mt_getrandmax());

to make it work again. Find attached the patch.

CommentFileSizeAuthor
#8 swiftmailer-2965564-8-image-cid.patch2.13 KBgeek-merlin
#4 rand-2965564-4.patch697 bytescrasx
#2 screenshot-error.png25.31 KBangela_g
swiftmailer-image-embed-random-generator.patch762 bytesayalon

Comments

ayalon created an issue. See original summary.

angela_g’s picture

angela_g commented
StatusFileSize
new screenshot-error.png25.31 KB

Faced the same problem on local environment, the provided solution worked for me too.

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Title: Inline Image broken / random number generator » Fix the rand call for 32 bit systems
Status: Active » Needs work

Thanks for this. According to the PHP docs, this can be simplified to rand() without parameters.

crasx’s picture

crasx commented
Status: Needs work » Needs review
StatusFileSize
new rand-2965564-4.patch697 bytes

Agreed, here is a patch for rand with no parameters.

adamps’s picture

adamps commented
Title: Fix the rand call for 32 bit systems » Embedded images don't work on 32-bit systems
Version: 8.x-1.x-dev » 8.x-2.x-dev
Priority: Major » Normal

Docs for rand() says

Note: On some platforms (such as Windows), getrandmax() is only 32767. If you require a range larger than 32767, specifying min and max will allow you to create a range larger than this, or consider using mt_rand() instead.

We don't want duplicate. Maybe even better to use a string? This website shows that possibility. And Random::name() has a $unique parameter.

Please can you patch for the 2.x branch? Hopefully the tests will work then.

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Status: Needs review » Needs work

> We don't want duplicate. Maybe even better to use a string?

OMG yes you are right. So rand here is not optimal in the first place.
Why not simply use "cid:image-$i"? It just has to be unique in the mail and also is better for debugging.
If not this, Random::name(8,TRUE) will also be better.

adamps’s picture

adamps commented

Why not simply use "cid:image-$i"? It just has to be unique in the mail and also is better for debugging.

+1 from me for that.

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Status: Needs work » Needs review
StatusFileSize
new swiftmailer-2965564-8-image-cid.patch2.13 KB

Well, not quite. Looked deeper into this and the string is a magic token that gets replaced later and should be unique.

Patch flying in with a stab at this that uses your suggestion of Random::name().

adamps’s picture

adamps commented

Thanks, looks good.

// 'cid:[randomname]' and keep track of the this. It will be replaced by

Typo "the this"

  • geek-merlin committed ed332df on 8.x-2.x 
    Issue #2965564 by ayalon, crasx, geek-merlin, AdamPS: Embedded images...
geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Status: Needs review » Fixed

Thanks! Fixed that typo on commit.

Status: Fixed » Closed (fixed)

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