Merge pull request #42 from ctabin/fix-inputstream-close

Fixes non-closed InputStream

src/main/java/ch/astorm/jotlmsg/OutlookMessage.java

@@ -422,13 +422,14 @@ public MimeMessage toMimeMessage(Session session) throws IOException, MessagingE
         multipart.addBodyPart(body);
 
         for(OutlookMessageAttachment attachment : getAttachments()) {
-            InputStream inputStream = attachment.getNewInputStream();
-            if(inputStream!=null) { 
-                MimeBodyPart part = new MimeBodyPart();
-                part.setDataHandler(new DataHandler(new ByteArrayDataSource(inputStream, attachment.getMimeType())));
-                part.setFileName(attachment.getName());
-                multipart.addBodyPart(part);
-            }
+            String name = attachment.getName();
+            String mimeType = attachment.getMimeType();
+            byte[] data = readAttachement(attachment);
+
+            MimeBodyPart part = new MimeBodyPart();
+            part.setDataHandler(new DataHandler(new ByteArrayDataSource(data, mimeType)));
+            part.setFileName(name);
+            multipart.addBodyPart(part);
         }
 
         message.setContent(multipart);
@@ -561,20 +562,18 @@ public void writeTo(OutputStream outputStream) throws IOException {
         int attachmentCounter = 0;
         for(OutlookMessageAttachment attachment : attachments) {
             if(attachmentCounter>=2048) { throw new RuntimeException("too many attachments (max=2048)"); } //limitation, see page 15, point 2.2.2
-
-            InputStream is = attachment.getNewInputStream();
+
             String name = attachment.getName();
-            String mimeName = attachment.getMimeType();
-            byte[] data = IOUtils.toByteArray(is);
-            is.close();
+            String mimeType = attachment.getMimeType();
+            byte[] data = readAttachement(attachment);
 
             StoragePropertiesChunk attachStorage = new StoragePropertiesChunk();
             attachStorage.setProperty(new PropertyValue(MAPIProperty.OBJECT_TYPE, FLAG_READABLE | FLAG_WRITEABLE, ByteBuffer.allocate(4).putInt(7).array())); //MAPI_ATTACH
             if(name!=null) { 
                 attachStorage.setProperty(new PropertyValue(MAPIProperty.ATTACH_FILENAME, FLAG_READABLE | FLAG_WRITEABLE, StringUtil.getToUnicodeLE(name))); 
                 attachStorage.setProperty(new PropertyValue(MAPIProperty.ATTACH_LONG_FILENAME, FLAG_READABLE | FLAG_WRITEABLE, StringUtil.getToUnicodeLE(name))); 
             }
-            if(mimeName!=null) { attachStorage.setProperty(new PropertyValue(MAPIProperty.ATTACH_MIME_TAG, FLAG_READABLE | FLAG_WRITEABLE, StringUtil.getToUnicodeLE(name))); }
+            if(mimeType!=null) { attachStorage.setProperty(new PropertyValue(MAPIProperty.ATTACH_MIME_TAG, FLAG_READABLE | FLAG_WRITEABLE, StringUtil.getToUnicodeLE(name))); }
             attachStorage.setProperty(new PropertyValue(MAPIProperty.ATTACH_NUM, FLAG_READABLE | FLAG_WRITEABLE, ByteBuffer.allocate(4).putInt(attachmentCounter).array()));
             attachStorage.setProperty(new PropertyValue(MAPIProperty.ATTACH_METHOD, FLAG_READABLE | FLAG_WRITEABLE, ByteBuffer.allocate(4).putInt(1).array())); //ATTACH_BY_VALUE
             attachStorage.setProperty(new PropertyValue(MAPIProperty.ATTACH_DATA, FLAG_READABLE | FLAG_WRITEABLE, data));
@@ -591,6 +590,13 @@ public void writeTo(OutputStream outputStream) throws IOException {
         fs.close();
     }
 
+    private byte[] readAttachement(OutlookMessageAttachment attachment) throws IOException {
+        try(InputStream is = attachment.getNewInputStream()) {
+            if(is==null) { throw new IllegalStateException("null inputstream for attachement "+attachment.getName()+" ("+attachment.getMimeType()+")"); }
+            return IOUtils.toByteArray(is);
+        }
+    }
+
     //extracted from org.apache.poi.hsmf.datatypes.TimePropertyValue
     private final long OFFSET = 1000L * 60L * 60L * 24L * (365L * 369L + 89L);
     private byte[] dateToBytes(Date date) {

src/main/java/ch/astorm/jotlmsg/OutlookMessageAttachment.java

@@ -86,7 +86,7 @@ public MemoryInputStreamCreator(InputStream source) {
         /**
          * Reads a new {@code InputStream} with the content of the source.
          * The first time this method is called, the source {@code InputStream} will be fully
-         * read and stored in-memory. Then, a new {@code ByteArrayInputStream} is
+         * read, closed and stored in-memory. Then, a new {@code ByteArrayInputStream} is
          * returned.
          * 
          * @param omt The attachment.
@@ -96,7 +96,8 @@ public MemoryInputStreamCreator(InputStream source) {
         @Override
         public InputStream newInputStream(OutlookMessageAttachment omt) throws IOException {
             if(content==null) { 
-                content = IOUtils.toByteArray(source); 
+                content = IOUtils.toByteArray(source);
+                source.close();
                 source = null;
             }
             return new ByteArrayInputStream(content);

src/test/java/ch/astorm/jotlmsg/OutlookMessageMIMETest.java

@@ -3,6 +3,9 @@
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import javax.mail.Message;
@@ -13,6 +16,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import org.junit.jupiter.api.Test;
 
 public class OutlookMessageMIMETest {
@@ -113,4 +117,76 @@ public void testSimpleDoubleGeneration() throws Exception {
         String text2 = IOUtils.toString(multipart2.getBodyPart(1).getInputStream(), StandardCharsets.UTF_8);
         assertEquals("Hello, World!", text2);
     }
+
+    @Test
+    public void testInvalidAttachment() throws Exception {
+        OutlookMessage message = new OutlookMessage();
+        message.setSubject("This is a message");
+        message.setFrom("[email protected]");
+        message.setReplyTo(Arrays.asList("[email protected]", "[email protected]"));
+        message.setPlainTextBody("Hello,\n\nThis is a simple message.\n\n.Bye.");
+        message.addRecipient(OutlookMessageRecipient.Type.TO, "[email protected]", "Cédric");
+        message.addAttachment("message.txt", "text/plain", a -> null);
+
+        File temporaryFile = new File("tmp");
+        try { message.writeTo(temporaryFile); assertTrue(false); }
+        catch(IllegalStateException ise) { }
+        temporaryFile.delete();
+
+        try { message.toMimeMessage(); assertTrue(false); }
+        catch(IllegalStateException ise) { }
+    }
+
+    @Test
+    public void testClosedStream() throws Exception {
+        OutlookMessage message = new OutlookMessage();
+        message.setSubject("This is a message");
+        message.setFrom("[email protected]");
+        message.setReplyTo(Arrays.asList("[email protected]", "[email protected]"));
+        message.setPlainTextBody("Hello,\n\nThis is a simple message.\n\n.Bye.");
+        message.addRecipient(OutlookMessageRecipient.Type.TO, "[email protected]", "Cédric");
+
+        CheckableInputStream cis = new CheckableInputStream();
+        message.addAttachment("message.txt", "text/plain", a -> cis);
+
+        message.toMimeMessage();
+        assertTrue(cis.closed);
+
+        try { message.toMimeMessage(); assertTrue(false); }
+        catch(IllegalStateException ise) { }
+    }
+
+    @Test
+    public void testRepeteableStream() throws Exception {
+        OutlookMessage message = new OutlookMessage();
+        message.setSubject("This is a message");
+        message.setFrom("[email protected]");
+        message.setReplyTo(Arrays.asList("[email protected]", "[email protected]"));
+        message.setPlainTextBody("Hello,\n\nThis is a simple message.\n\n.Bye.");
+        message.addRecipient(OutlookMessageRecipient.Type.TO, "[email protected]", "Cédric");
+
+        CheckableInputStream cis = new CheckableInputStream();
+        message.addAttachment("message.txt", "text/plain", cis);
+
+        message.toMimeMessage();
+        assertTrue(cis.closed);
+
+        message.toMimeMessage();
+    }
+
+    private static class CheckableInputStream extends InputStream {
+        private boolean closed = false;
+
+        @Override
+        public int read() throws IOException {
+            if(closed) { throw new IllegalStateException("stream is closed"); }
+            return -1;
+        }
+
+        @Override
+        public void close() throws IOException {
+            closed = true;
+            super.close();
+        }
+    }
 }