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

Add a test for an attachment with UTF-8 symbols #483

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

boris-petrov
Copy link
Contributor

... which fails with:

jakarta.mail.MessagingException: Unable to load BODYSTRUCTURE
	at com.sun.mail.imap.IMAPMessage.loadBODYSTRUCTURE(IMAPMessage.java:1540)
	at com.sun.mail.imap.IMAPMessage.getDataHandler(IMAPMessage.java:784)
	at jakarta.mail.internet.MimeMessage.getContent(MimeMessage.java:1484)
	at com.icegreen.greenmail.test.specificmessages.EncodingTest.testAttachmentWithUTF8NameAndGreenMailApi(EncodingTest.java:231)
        ...

That's because of MimeUtility.encodeText. If I remove that method call, the test passes. Not sure what's going on but I believe this is a bug in Greenmail.

cc @marcelmay

@marcelmay
Copy link
Member

@boris-petrov , thx alot! I will investigate the issue(s) next week

@Mulgish
Copy link

Mulgish commented Oct 27, 2022

Getting the same issue when trying to read emails from Greenmail using Apache Camel.

@marcelmay
Copy link
Member

Thanks alot, @boris-petrov and @Mulgish .

I created a separate issue #498 (and for 1.6.x backport #499 ).

Looks like something breaks (CRLF in header Content-Disposition) when creating/sending the mail,
and I added a workaround for it.

@boris-petrov
Copy link
Contributor Author

@marcelmay thanks for the new release! It fixed the test that I had written here. However, I pushed another commit which just adds a check for the attachment's file name. And it fails. Please check it out. I believe it should be passing?

@marcelmay
Copy link
Member

Thanks again @boris-petrov for another failing example. Will investigate the issue.

@marcelmay
Copy link
Member

Use MimeUtility.decodeText(....getFileName()) when fetching the name, or set mail.mime.decodefilename property to true.

@boris-petrov
Copy link
Contributor Author

@marcelmay MimeUtility.decodeText of course would fix it. As for mail.mime.decodefilename - that's what I have in my own code but something like this fails there. How do I set mail.mime.decodefilename in this test here to prove or disprove that?

@marcelmay
Copy link
Member

@boris-petrov , just added #541 which simplifies adding such custom session properties like 'mail.mime.decodefilename' by default. Will be available with versions 1.6.14 / 2.0.x .

Otherwise you'd have to do something like this:

        Session smtpSession = greenMail.getSmtp().createSession();
        smtpSession.getProperties().setProperty("mail.mime.encodefilename","true");
        MimeMessage mimeMessage = new MimeMessage(smtpSession);
        ...
        Session imapSession = greenMail.getImap().createSession();
        imapSession.getProperties().setProperty("mail.mime.decodefilename","true");
        ... // Receive mail using IMAP session

@marcelmay
Copy link
Member

@boris-petrov , I believe this PR was accidentally re-opened by your force-push (see history above, search for force-pushed) => I would close this PR, as it was merged before.

@boris-petrov
Copy link
Contributor Author

boris-petrov commented Feb 27, 2023

@boris-petrov , I believe this PR was accidentally re-opened by your force-push (see history above, search for force-pushed) => I would close this PR, as it was merged before.

I believe this comment was meant for some other PR? This hasn't been merged.

@boris-petrov , just added #541 which simplifies adding such custom session properties like 'mail.mime.decodefilename' by default. Will be available with versions 1.6.14 / 2.0.x .

That's great! How do I use that in this test? I want to set that option (mail.mime.decodefilename) for the IMAP connection in this test and then it should pass (or fail which would mean there's an issue in Greenmail).

P.S. I rebased on master.

@marcelmay
Copy link
Member

@boris-petrov , you can set the properties for the whole test via

    @Rule
    public GreenMailRule greenMail = new GreenMailRule(new ServerSetup[]{
        ServerSetupTest.SMTP.mailSessionProperty("mail.mime.encodefilename", "true"),
        ServerSetupTest.IMAP.mailSessionProperty("mail.mime.decodefilename", "true")
    });

... or in the test itself via

    public void testAttachmentWithUTF8NameAndGreenMailApi() throws MessagingException, IOException {
        greenMail.setUser("to@localhost", "pwd");
        final Session imapSession =  greenMail.getImap().createSession();
        imapSession.getProperties().setProperty("mail.mime.decodefilename","true"); // For reading
        final IMAPStore store = (IMAPStore) imapSession.getStore();
....
                    GreenMailUtil.sendAttachmentEmail(
                        "to@localhost", "from@localhost", "subject", "body",
                        new byte[]{0, 1, 2}, "image/gif", fileName,
                        "testimage_description",
                        greenMail.getSmtp().getServerSetup()
                            .mailSessionProperty("mail.mime.encodefilename", "true")); // For sending
....

@boris-petrov
Copy link
Contributor Author

@marcelmay thanks for the help and sorry for the delay! I pushed another commit that does what you suggest - it sets mail.mime.decodefilename. So I would expect the value that I read back to be decoded but it isn't - that is, the test still fails. Please take a look when you have the time. Thanks!

@marcelmay
Copy link
Member

@boris-petrov ,

my fault as I assumed it is a session property.

This property is actually a JavaMail system property, not a JavaMail session property:

  System.setProperty("mail.mime.decodefilename", "true");

@boris-petrov
Copy link
Contributor Author

OK, like that? Check the last commit. The test still doesn't work unfortunately. Am I missing something else?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants