From a874e223af546e5d32d2bc9aae4520a9a97c2def Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Mon, 4 May 2015 09:15:48 +0000 Subject: [PATCH] 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 --- .../poifs/filesystem/NPOIFSFileSystem.java | 9 ++++- .../poi/poifs/filesystem/POIFSFileSystem.java | 35 +++++++++++-------- src/java/org/apache/poi/util/IOUtils.java | 31 ++++++++++++++++ .../poi/ss/usermodel/WorkbookFactory.java | 10 +++++- .../apache/poi/ss/TestWorkbookFactory.java | 25 +++++++++++++ 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 { *

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(); + } } -- 2.39.5