Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: names manually specified for embedded images are overridden and have extension added, breaking cid: references in HTML body #440

Closed
nites67 opened this issue Feb 1, 2023 · 10 comments

Comments

@nites67
Copy link

nites67 commented Feb 1, 2023

Greetings,
Could you please let me know how to use withEmbeddedImages feature while sending emails ?
I tried to use both withAttachments and withEmbeddedImages in the same manner.
Only withAttachments work fine. withEmbeddedImages fail to display the images inline in the email.

Please Support.

@bbottema
Copy link
Owner

bbottema commented Feb 1, 2023

Sure, have a look here.

@nites67
Copy link
Author

nites67 commented Feb 2, 2023

Hi, Thanks for the update. I have seen the link you shared. I think i was not clear in my question.
This is my sample code for embedding inline images. It does not work.

Could you please support ?

List<AttachmentResource> embeddedImagesList = new ArrayList<AttachmentResource>();
AttachmentResource attachResourceImage = null;
for (MultipartFile m: inlineImages) {
    File file = convertMultipartToFile(m);
    attachResourceImage = new AttachmentResource(file.getName(), new FileDataSource(file));
    embeddedImagesList.add(attachResourceImage);
}


Email email = EmailBuilder.startingBlank().from("").toMultiple("").ccMultiple("").bccMultiple("").withSubject("").
                       withHTMLText("").withAttachments("").withEmbeddedImages(embeddedImagesList ).buildEmail();

mailer.sendEmail(email);

@bbottema
Copy link
Owner

bbottema commented Feb 2, 2023

Well, this is a core feature of the library and has been tested in many clients for the past 14 years, so I'm inclined to think this is either an issue with the data you are embedding or an issue with your email client.

However, without concrete data I can't say more, the API usage seems fine. Perhaps you can share the EML text that comes out? For example using EmailConverter.emailToEML() or mailerBuilder.withTransportModeLoggingOnly().

@nites67
Copy link
Author

nites67 commented Feb 2, 2023

Hi, Thanks for the update. As we have a strict security policy, I will be unable to provide EML text. I printed the EML for embeddedImages. Below is the last part of EML which includes data for embeddedImages and the debug logs associated with it .
Could you please let me know if the below logs are valid of inline attachments ?

embeddedImages = [AttachmentResource {
name = 'test'
dataSource.name = abc.png,
dataSource.getContentType = image/png,
description = null,
contentTransferEncoding = null
}]


-------=Part_0_52552.5353666
Content-Type: image/png; filename=test.PNG; name=test.PNG
Content-Transfer-Encoding: base64
Content-Disposition: inline; filename=test.PNG
Content-ID: <test.PNG>

<Here it contains base64 text of the image>

@bbottema
Copy link
Owner

bbottema commented Feb 4, 2023

Yes, this looks good to me. For reference, here's the email from the included demo which I just checked still works as it should:

------=_Part_2_379303133.1675542435528
Content-Type: image/png; filename=thumbsup; name=thumbsup
Content-Transfer-Encoding: base64
Content-Disposition: inline; filename=thumbsup
Content-ID: <thumbsup>

Although, two questions.

  1. The above embeddedImages = [] you indicated is literally the API usage that resulted in that "test.PNG"? That seems a little odd to me.
  2. Can you show me the HTML you use to embed this image? In your case, it should be something like:
    <img width=25 src='cid:test.PNG'>

@nites67
Copy link
Author

nites67 commented Feb 5, 2023

Thanks for your support. The issue has been resolved.

@ztyzbb
Copy link

ztyzbb commented Jun 24, 2023

@bbottema I faced same problem. After read the source code, I realized that if I add image like this

emailBuilder.withEmbeddedImage("test", new FileDataSource(file)); // filename is abc.png

The MimeMessageHelper#possiblyAddExtension will make Content-ID header become <test.png>, so the html should be <img src='cid:test.png'> instead of <img src='cid:test'>, is it intended?
I believe the smiley example in the doc will not work.

@bbottema bbottema reopened this Jun 25, 2023
@bbottema
Copy link
Owner

bbottema commented Oct 9, 2023

This issue escaped my attention, it seems. This is a particularly complicated algorithm, which is tricky to change due to various different use cases that depend on the id's used for attachments. But I'll have a look, because this functionality is extensively tested in junit tests, so it should be easy to either rule it out or reproduce it.

@bbottema
Copy link
Owner

bbottema commented Oct 9, 2023

On the other hand, I can clearly see the Content-ID seems wrong, which is linked to the cid: references. I'll dive into this code once again soon. I think maybe the problem with not having the extension, when it is not referenced by cid:, email clients will assume it should be an attachment. But then if you didn't also provide the right extension with the custom name, email clients such as Outlook will simple use the provided name as attachment file name. For these cases, I make sure the attachments always have the right extension for optimal behaviour in email clients.

(this is all from faint memory, so I might be wrong)

However, this does mess with the Content-ID and causes problems if you do reference it with cid: in your HTML body. Maybe I can make it more intelligent, by first checking if there is a cid: references for a specified attachment name. If not, only then append the correct file extension (or maybe drop it completely, but I have to assume my users provided the name for a reason, on the other hand I do modify it with extension... hmm).

@bbottema bbottema changed the title How to use withEmbeddedImages feature while sending emails ? Bug: names manually specified for embedded images are overridden and have extension added, breaking cid: references in HTML body Oct 9, 2023
@bbottema bbottema added this to the 8.3.1 milestone Oct 9, 2023
bbottema added a commit that referenced this issue Oct 9, 2023
…n and have extension added, breaking cid: references in HTML body - now only fixing file extension in case of attachments, not inline attachments (embedded images)
@bbottema
Copy link
Owner

bbottema commented Oct 9, 2023

I've nailed down the bug and fixed it. It was a pain to deal with the junit tests, due to the fact that during generation of the MimeMessage (EML) the attachment names change. Then reading the resulting mail back and comparing it with the initial email requires some complicated tinkering involving reflection.

Anyway, fix released in 8.3.1. Enjoy.

@bbottema bbottema closed this as completed Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants