bbottema / simple-java-mail

Simple API, Complex Emails (Jakarta Mail smtp wrapper)
http://www.simplejavamail.org
Apache License 2.0
1.23k stars 269 forks source link

Content-ID for embedded image has unexpected extension at the end #332

Closed dadouam closed 2 years ago

dadouam commented 3 years ago

Hello, I'm trying to use the library to achieve a rather simple workflow:

The lib is working pretty well for that purpose and provides a much better experience than the JavaMail API. However, I'm facing an issue with some embedded images with e-mails sent from the Apple Mail App (don't know if it happens specifically with this client).

The original email has its Content-ID set like this:

--Apple-Mail=_99049156-A46B-4137-9A9A-DFF8F09A8EA7
Content-Transfer-Encoding: 7bit
Content-Type: text/html;
    charset=us-ascii

<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Hello<div class=""><img apple-inline="yes" id="71BD1091-7D82-4679-9C8F-7A0EEF348399" width="788" height="634" src="cid:E977B3EA-BC23-486B-A803-C23C9A849C51@home" class=""></div></body></html>
--Apple-Mail=_99049156-A46B-4137-9A9A-DFF8F09A8EA7
Content-Transfer-Encoding: base64
Content-Disposition: inline;
    filename*=utf-8''Capture%20d%E2%80%99e%CC%81cran%202021%2D07%2D05%20a%CC%80%2009.57.31.png
Content-Type: image/png;
    x-unix-mode=0644;
    name="=?utf-8?B?Q2FwdHVyZSBk4oCZZcyBY3JhbiAyMDIxLTA3LTA1IGHMgCAwOS41Ny4zMS5w?=
 =?utf-8?B?bmc=?="
Content-Id: <E977B3EA-BC23-486B-A803-C23C9A849C51@home>

There is no .png at the end of Content-ID and the src:cid img property is properly set.

Once I receive such an email and try to send it back, no matter if

The end result is a cid value of E977B3EA-BC23-486B-A803-C23C9A849C51@home.png and it breaks the embedded image because there is a mismatch between this cid and the src:cid.

I looked at the code and I'm pretty sure this behavior comes from MimeMessageHelper.determineResourceName(), perhaps as a side-effect of https://github.com/bbottema/simple-java-mail/issues/307 ?

Here is the original resource, from copying(): the "name" property is the correct CID, and dataSource.getName() returns the original file name (which is the one Gmail, Apple Mail... uses as display name for the embedded image "attachment" rather than the cid).

originalEmbeddedResource

Here is the resource before entering into determineResourceName:

beforeDetermineResourceName

Here is the end result: .png was unexpectedly appended to resourceName, and while I don't think it's as big a problem, the fileName ignores the value of dateSource.getName() and is always equal to resourceName, which I'm not sure why as having a different cid name/file name could be important in some cases?

afterDetermineResourceName

I'll look for some workaround around the fileName because I think it's what triggers this addition of the extension at the end.

But I think it would be nice to allow some customization of how AttachmentResource is transformed into a body part, or at least provide a way to take name and dataSource.getName() "as is" and trust the developer that the values are already correct?

Thanks!

bbottema commented 3 years ago

Thank you for your thorough research, I'll have a closer look soon. This resource naming algorithm keeps giving me headaches, though. It's not as straightforward as you might think.

bbottema commented 3 years ago

Since this is one of the trickier parts of the library, it would be of great help if you could provide an example email along with a bare example of your code using it to create a new email. Either here, or if it cannot be published I could treat it discretely only on my own laptop...

dadouam commented 3 years ago

Hi, thank you for your reply! Here is a basic .eml sample that allows me to reproduce the issue. sample.eml.zip

Here is the associated test case, assuming sample.eml is on the test resources classpath. I believe the modifiedEmail step is not needed here but it's here to show what kind of things I'd do with the originalEmail.

  @Test
  void testExtension() throws IOException {
    String eml = IOUtils.toString(getClass().getResourceAsStream("/sample.eml"), StandardCharsets.UTF_8);
    assertThat(eml)
        .contains("Content-Id: <DB294AA3-160F-4825-923A-B16C8B674543@home>")
        .contains("filename=filename.png");

    Email originalEmail = EmailConverter.emlToEmail(eml);
    AttachmentResource originalResource = originalEmail.getEmbeddedImages().get(0);
    assertThat(originalResource.getName()).isEqualTo("DB294AA3-160F-4825-923A-B16C8B674543@home");
    assertThat(originalResource.getDataSource().getName()).isEqualTo("filename.png");

    Email modifiedEmail = EmailBuilder.copying(originalEmail)
        .clearRecipients()
        .to("other@company.com")
        .buildEmail();
    AttachmentResource resource = modifiedEmail.getEmbeddedImages().get(0);
    assertThat(resource.getName()).isEqualTo("DB294AA3-160F-4825-923A-B16C8B674543@home");
    assertThat(resource.getDataSource().getName()).isEqualTo("filename.png");

    String modifiedEml = EmailConverter.emailToEML(modifiedEmail);
    assertThat(modifiedEml)
        .contains("cid:DB294AA3-160F-4825-923A-B16C8B674543@home\"")
        .contains("Content-ID: <DB294AA3-160F-4825-923A-B16C8B674543@home.png>")
        .contains("filename=\"DB294AA3-160F-4825-923A-B16C8B674543@home.png\"");
  }

Btw, I did manage to workaround this issue using EmailPopulatingBuilder withEmbeddedImage(@NotNull String name, @NotNull byte[] data, @NotNull String mimetype); with byte[] data created from the original resource - so that the filename extension doesn't interfere with the name used as CID.

xalloa commented 3 years ago

Hello,

First of all I must thank you for this great work!!

I also have the same issue (along with not recognizing mime type for PNG files) and I worked around it by "replacing" the FileDataSource.

I digged a little bit to the issue and saw that the FileDataSource class returns as name the file name and this name is used as a CID instead of the name given in the withEmbeddedImage call.

So I replaced the FileDataSource class with the following class that keeps the name separately and returns this one. So when I create an instance of this class I take case to pass the name that I give to withEmbeddedImage call and it works so far:

package some.package;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import javax.activation.FileDataSource;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;

public class ExtendedFileDataSource extends FileDataSource {
    private static final String DEFAULT_MIME_TYPE = "application/octet-stream";

    private final File file;
    private final String name;

    public ExtendedFileDataSource(File file, String name) {
        super(file);
        this.file = file;
        if (name == null) {
            this.name = file.getName();
        } else {
            this.name = name;
        }
    }

    public ExtendedFileDataSource(String name) {
        this(new File(name), name);
    }

    @Override
    public String getContentType() {
        final String superContentType = super.getContentType();
        if (superContentType.equals(DEFAULT_MIME_TYPE)) {
            try {
                return Files.probeContentType(file.toPath());
            } catch (IOException e) {
                return DEFAULT_MIME_TYPE;
            }
        } else {
            return superContentType;
        }
    }

    @Override
    public String getName() {
        return name;
    }
}
bbottema commented 2 years ago

This is a combination of regression bugs from #219, #249, #310. This was solved in #351 and released in 6.7.0. Give it a try!