bbottema/simple-java-mail

Allow zero data attachments so Outlook message conversions don't crash and burn

LaurentFl opened this issue · 4 comments

I get an error when converting an Outlook msg received from a sender who sends email with an attachment which has no data:
java.lang.IllegalArgumentException: data is required at org.simplejavamail.internal.util.Preconditions.verifyNonnull(Preconditions.java:41) at org.simplejavamail.internal.util.Preconditions.checkNonEmptyArgument(Preconditions.java:30) at org.simplejavamail.email.internal.EmailPopulatingBuilderImpl.withAttachment(EmailPopulatingBuilderImpl.java:1698) at org.simplejavamail.internal.outlooksupport.converter.OutlookEmailConverter.buildEmailFromOutlookMessage(OutlookEmailConverter.java:117) at org.simplejavamail.internal.outlooksupport.converter.OutlookEmailConverter.outlookMsgToEmailBuilder(OutlookEmailConverter.java:59) at org.simplejavamail.converter.EmailConverter.outlookMsgToEmailBuilder(EmailConverter.java:194) at org.simplejavamail.converter.EmailConverter.outlookMsgToEmailBuilder(EmailConverter.java:178)

Maybe it will be better if those attachments are ignored instead of throwing an Exception and block all the email conversion.
To fix this, we can check if attachment.getData() != null before calling builder.withAttachment() in OutlookEmailConverter.buildEmailFromOutlookMessage(OutlookEmailConverter.java:117)?
Or check if ((OutlookFileAttachment) attachment).getData() != null in OutlookMessage.fetchTrueAttachments() before adding it to fileAttachments ArrayList?

Maybe we need to keep this behavior to be RFC compliant?

Would it be possible for you to provide a sample .msg and void main for reproducing the bug? I think the code has changed a little so the line numbers you report don't match up anymore (and I would like to inspect the code a bit deeper).

Here are 2 .msg as example.
TestNoData.zip

And simply one of those lines should reproduce the bug:
EmailConverter.outlookMsgToEML(new File("TestNoData.msg"))
or
EmailConverter.outlookMsgToEmailBuilder(new File("TestNoData.msg"))

(I'm using v6.5.4)

Thanks I'll look into it asap

Fix released in 6.7.2. I decided to allow for these attachments since

  • A). the metadata might still be useful and
  • B). you need that data if you want to contact the sender about missing data.