If an empty stream or file is given to WorkbookFactory.create, give a more informative exception - EmptyFileException

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1677562 13f79535-47bb-0310-9956-ffa450edef68

5 files changed, 93 insertions, 17 deletions

diff --git a/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java b/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java
index f21f6a39a0..3aa0707192 100644
--- a/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java
+++ b/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java
@@ -36,6 +36,7 @@ import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 
+import org.apache.poi.EmptyFileException;
 import org.apache.poi.poifs.common.POIFSBigBlockSize;
 import org.apache.poi.poifs.common.POIFSConstants;
 import org.apache.poi.poifs.dev.POIFSViewable;
@@ -210,6 +211,9 @@ public class NPOIFSFileSystem extends BlockStore
        try {
           // Initialize the datasource
           if (srcFile != null) {
+              if (srcFile.length() == 0)
+                  throw new EmptyFileException();
+              
               FileBackedDataSource d = new FileBackedDataSource(srcFile, readOnly);
               channel = d.getChannel();
               _data = d;
@@ -236,7 +240,10 @@ public class NPOIFSFileSystem extends BlockStore
           // TODO Decide if we can handle these better whilst
           //  still sticking to the iterator contract
           if(closeChannelOnError) {
-             channel.close();
+              if (channel != null) {
+                  channel.close();
+                  channel = null;
+              }
           }
           throw e;
        }
diff --git a/src/java/org/apache/poi/poifs/filesystem/POIFSFileSystem.java b/src/java/org/apache/poi/poifs/filesystem/POIFSFileSystem.java
index 7169d91f9e..f1d73d8989 100644
--- a/src/java/org/apache/poi/poifs/filesystem/POIFSFileSystem.java
+++ b/src/java/org/apache/poi/poifs/filesystem/POIFSFileSystem.java
@@ -25,7 +25,6 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.io.PushbackInputStream;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Iterator;
@@ -37,7 +36,17 @@ import org.apache.poi.poifs.dev.POIFSViewable;
 import org.apache.poi.poifs.property.DirectoryProperty;
 import org.apache.poi.poifs.property.Property;
 import org.apache.poi.poifs.property.PropertyTable;
-import org.apache.poi.poifs.storage.*;
+import org.apache.poi.poifs.storage.BATBlock;
+import org.apache.poi.poifs.storage.BlockAllocationTableReader;
+import org.apache.poi.poifs.storage.BlockAllocationTableWriter;
+import org.apache.poi.poifs.storage.BlockList;
+import org.apache.poi.poifs.storage.BlockWritable;
+import org.apache.poi.poifs.storage.HeaderBlock;
+import org.apache.poi.poifs.storage.HeaderBlockConstants;
+import org.apache.poi.poifs.storage.HeaderBlockWriter;
+import org.apache.poi.poifs.storage.RawDataBlockList;
+import org.apache.poi.poifs.storage.SmallBlockTableReader;
+import org.apache.poi.poifs.storage.SmallBlockTableWriter;
 import org.apache.poi.util.CloseIgnoringInputStream;
 import org.apache.poi.util.IOUtils;
 import org.apache.poi.util.LongField;
@@ -201,19 +210,15 @@ public class POIFSFileSystem
      */
     public static boolean hasPOIFSHeader(InputStream inp) throws IOException {
         // We want to peek at the first 8 bytes
-        inp.mark(8);
-
-        byte[] header = new byte[8];
-        IOUtils.readFully(inp, header);
-        LongField signature = new LongField(HeaderBlockConstants._signature_offset, header);
-
-        // Wind back those 8 bytes
-        if(inp instanceof PushbackInputStream) {
-            PushbackInputStream pin = (PushbackInputStream)inp;
-            pin.unread(header);
-        } else {
-            inp.reset();
-        }
+        byte[] header = IOUtils.peekFirst8Bytes(inp);
+        return hasPOIFSHeader(header);
+    }
+    /**
+     * Checks if the supplied first 8 bytes of a stream / file
+     *  has a POIFS (OLE2) header.
+     */
+    public static boolean hasPOIFSHeader(byte[] header8Bytes) {
+        LongField signature = new LongField(HeaderBlockConstants._signature_offset, header8Bytes);
 
         // Did it match the signature?
         return (signature.get() == HeaderBlockConstants._signature);
diff --git a/src/java/org/apache/poi/util/IOUtils.java b/src/java/org/apache/poi/util/IOUtils.java
index 4f18214c46..e0f2bd16c1 100644
--- a/src/java/org/apache/poi/util/IOUtils.java
+++ b/src/java/org/apache/poi/util/IOUtils.java
@@ -22,11 +22,14 @@ import java.io.Closeable;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.io.PushbackInputStream;
 import java.nio.ByteBuffer;
 import java.nio.channels.ReadableByteChannel;
 import java.util.zip.CRC32;
 import java.util.zip.Checksum;
 
+import org.apache.poi.EmptyFileException;
+
 public final class IOUtils {
 
     private static final POILogger logger = POILogFactory
@@ -35,6 +38,34 @@ public final class IOUtils {
 	private IOUtils() {
 		// no instances of this class
 	}
+	
+	/**
+	 * Peeks at the first 8 bytes of the stream. Returns those bytes, but
+	 *  with the stream unaffected. Requires a stream that supports mark/reset,
+	 *  or a PushbackInputStream. If the stream has >0 but <8 bytes, 
+	 *  remaining bytes will be zero.
+	 * @throws EmptyFileException if the stream is empty
+	 */
+	public static byte[] peekFirst8Bytes(InputStream stream) throws IOException, EmptyFileException {
+        // We want to peek at the first 8 bytes
+	    stream.mark(8);
+
+        byte[] header = new byte[8];
+        int read = IOUtils.readFully(stream, header);
+        
+        if (read < 1)
+            throw new EmptyFileException();
+
+        // Wind back those 8 bytes
+        if(stream instanceof PushbackInputStream) {
+            PushbackInputStream pin = (PushbackInputStream)stream;
+            pin.unread(header);
+        } else {
+            stream.reset();
+        }
+        
+        return header;
+	}
 
 	/**
 	 * Reads all the data from the input stream, and returns the bytes read.
diff --git a/src/ooxml/java/org/apache/poi/ss/usermodel/WorkbookFactory.java b/src/ooxml/java/org/apache/poi/ss/usermodel/WorkbookFactory.java
index bdf37eafca..80d9b76d4f 100644
--- a/src/ooxml/java/org/apache/poi/ss/usermodel/WorkbookFactory.java
+++ b/src/ooxml/java/org/apache/poi/ss/usermodel/WorkbookFactory.java
@@ -23,6 +23,7 @@ import java.io.InputStream;
 import java.io.PushbackInputStream;
 import java.security.GeneralSecurityException;
 
+import org.apache.poi.EmptyFileException;
 import org.apache.poi.EncryptedDocumentException;
 import org.apache.poi.POIXMLDocument;
 import org.apache.poi.hssf.record.crypto.Biff8EncryptionKey;
@@ -35,6 +36,7 @@ import org.apache.poi.poifs.filesystem.DirectoryNode;
 import org.apache.poi.poifs.filesystem.NPOIFSFileSystem;
 import org.apache.poi.poifs.filesystem.OfficeXmlFileException;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
+import org.apache.poi.util.IOUtils;
 import org.apache.poi.xssf.usermodel.XSSFWorkbook;
 
 /**
@@ -153,14 +155,19 @@ public class WorkbookFactory {
      *  from an InputStream requires more memory than loading
      *  from a File, so prefer {@link #create(File)} where possible.
      * @throws EncryptedDocumentException If the wrong password is given for a protected file
+     * @throws EmptyFileException If an empty stream is given
      */
     public static Workbook create(InputStream inp, String password) throws IOException, InvalidFormatException, EncryptedDocumentException {
         // If clearly doesn't do mark/reset, wrap up
         if (! inp.markSupported()) {
             inp = new PushbackInputStream(inp, 8);
         }
+        
+        // Ensure that there is at least some data there
+        byte[] header8 = IOUtils.peekFirst8Bytes(inp);
 
-        if (POIFSFileSystem.hasPOIFSHeader(inp)) {
+        // Try to create
+        if (POIFSFileSystem.hasPOIFSHeader(header8)) {
             NPOIFSFileSystem fs = new NPOIFSFileSystem(inp);
             return create(fs, password);
         }
@@ -187,6 +194,7 @@ public class WorkbookFactory {
      * <p>Note that in order to properly release resources the 
      *  Workbook should be closed after use.
      * @throws EncryptedDocumentException If the wrong password is given for a protected file
+     * @throws EmptyFileException If an empty stream is given
      */
     public static Workbook create(File file, String password) throws IOException, InvalidFormatException, EncryptedDocumentException {
         if (! file.exists()) {
diff --git a/src/ooxml/testcases/org/apache/poi/ss/TestWorkbookFactory.java b/src/ooxml/testcases/org/apache/poi/ss/TestWorkbookFactory.java
index 582c8c3646..27d6d9ec1a 100644
--- a/src/ooxml/testcases/org/apache/poi/ss/TestWorkbookFactory.java
+++ b/src/ooxml/testcases/org/apache/poi/ss/TestWorkbookFactory.java
@@ -17,14 +17,18 @@
 
 package org.apache.poi.ss;
 
+import java.io.ByteArrayInputStream;
+import java.io.File;
 import java.io.InputStream;
 
+import org.apache.poi.EmptyFileException;
 import org.apache.poi.EncryptedDocumentException;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.poi.ss.usermodel.Workbook;
 import org.apache.poi.ss.usermodel.WorkbookFactory;
+import org.apache.poi.util.TempFile;
 import org.apache.poi.xssf.usermodel.XSSFWorkbook;
 import org.apache.poi.openxml4j.opc.OPCPackage;
 
@@ -254,4 +258,25 @@ public final class TestWorkbookFactory extends TestCase {
             fail("Shouldn't be able to open with the wrong password");
         } catch (EncryptedDocumentException e) {}
     }
+    
+    /**
+     * Check that a helpful exception is given on an empty file / stream
+     */
+    public void testEmptyFile() throws Exception {
+        InputStream emptyStream = new ByteArrayInputStream(new byte[0]);
+        File emptyFile = TempFile.createTempFile("empty", ".poi");
+        
+        try {
+            WorkbookFactory.create(emptyStream);
+            fail("Shouldn't be able to create for an empty stream");
+        } catch (EmptyFileException e) {
+        }
+        
+        try {
+            WorkbookFactory.create(emptyFile);
+            fail("Shouldn't be able to create for an empty file");
+        } catch (EmptyFileException e) {
+        }
+        emptyFile.delete();
+    }
 }