Refactor some common code from the various Document-Factories into a helper class

Fix a potential file-handle-leak for password protected workbooks or slideshows

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

src/java/org/apache/poi/poifs/filesystem/DocumentFactoryHelper.java

@@ -0,0 +1,116 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.poifs.filesystem;
+
+import org.apache.poi.EncryptedDocumentException;
+import org.apache.poi.poifs.common.POIFSConstants;
+import org.apache.poi.poifs.crypt.Decryptor;
+import org.apache.poi.poifs.crypt.EncryptionInfo;
+import org.apache.poi.util.IOUtils;
+
+import java.io.FilterInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PushbackInputStream;
+import java.security.GeneralSecurityException;
+
+/**
+ * A small base class for the various factories, e.g. WorkbookFactory,
+ * SlideShowFactory to combine common code here.
+ */
+public class DocumentFactoryHelper {
+    /**
+     * Wrap the OLE2 data in the NPOIFSFileSystem into a decrypted stream by using
+     * the given password.
+     *
+     * @param fs The OLE2 stream for the document
+     * @param password The password, null if the default password should be used
+     * @return A stream for reading the decrypted data
+     * @throws IOException If an error occurs while decrypting or if the password does not match
+     */
+    public static InputStream getDecryptedStream(final NPOIFSFileSystem fs, String password)
+            throws IOException {
+        EncryptionInfo info = new EncryptionInfo(fs);
+        Decryptor d = Decryptor.getInstance(info);
+
+        try {
+            boolean passwordCorrect = false;
+            if (password != null && d.verifyPassword(password)) {
+                passwordCorrect = true;
+            }
+            if (!passwordCorrect && d.verifyPassword(Decryptor.DEFAULT_PASSWORD)) {
+                passwordCorrect = true;
+            }
+
+            if (passwordCorrect) {
+                // wrap the stream in a FilterInputStream to close the NPOIFSFileSystem
+                // as well when the resulting OPCPackage is closed
+                return new FilterInputStream(d.getDataStream(fs.getRoot())) {
+                    @Override
+                    public void close() throws IOException {
+                        fs.close();
+
+                        super.close();
+                    }
+                };
+            } else {
+                if (password != null)
+                    throw new EncryptedDocumentException("Password incorrect");
+                else
+                    throw new EncryptedDocumentException("The supplied spreadsheet is protected, but no password was supplied");
+            }
+        } catch (GeneralSecurityException e) {
+            throw new IOException(e);
+        }
+    }
+
+    /**
+     * Checks that the supplied InputStream (which MUST
+     *  support mark and reset, or be a PushbackInputStream)
+     *  has a OOXML (zip) header at the start of it.
+     * If your InputStream does not support mark / reset,
+     *  then wrap it in a PushBackInputStream, then be
+     *  sure to always use that, and not the original!
+     * @param inp An InputStream which supports either mark/reset, or is a PushbackInputStream
+     */
+    public static boolean hasOOXMLHeader(InputStream inp) throws IOException {
+        // We want to peek at the first 4 bytes
+        inp.mark(4);
+
+        byte[] header = new byte[4];
+        int bytesRead = IOUtils.readFully(inp, header);
+
+        // Wind back those 4 bytes
+        if(inp instanceof PushbackInputStream) {
+            PushbackInputStream pin = (PushbackInputStream)inp;
+            pin.unread(header, 0, bytesRead);
+        } else {
+            inp.reset();
+        }
+
+        // Did it match the ooxml zip signature?
+        return (
+                bytesRead == 4 &&
+                        header[0] == POIFSConstants.OOXML_FILE_HEADER[0] &&
+                        header[1] == POIFSConstants.OOXML_FILE_HEADER[1] &&
+                        header[2] == POIFSConstants.OOXML_FILE_HEADER[2] &&
+                        header[3] == POIFSConstants.OOXML_FILE_HEADER[3]
+        );
+    }
+
+}

src/java/org/apache/poi/sl/usermodel/SlideShowFactory.java

@@ -16,29 +16,21 @@
 ==================================================================== */
 package org.apache.poi.sl.usermodel;
 
-import java.io.File;
+import java.io.*;
-import java.io.FileNotFoundException;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.PushbackInputStream;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
-import java.security.GeneralSecurityException;
 
 import org.apache.poi.EncryptedDocumentException;
 import org.apache.poi.OldFileFormatException;
+import org.apache.poi.poifs.filesystem.DocumentFactoryHelper;
 import org.apache.poi.hssf.record.crypto.Biff8EncryptionKey;
 import org.apache.poi.poifs.crypt.Decryptor;
-import org.apache.poi.poifs.crypt.EncryptionInfo;
 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.util.IOUtils;
 
 public class SlideShowFactory {
-    /** The first 4 bytes of an OOXML file, used in detection */
-    private static final byte[] OOXML_FILE_HEADER = { 0x50, 0x4b, 0x03, 0x04 };
-    
     /**
      * Creates a SlideShow from the given NPOIFSFileSystem.
      *
@@ -63,37 +55,16 @@ public class SlideShowFactory {
      *
      * @throws IOException if an error occurs while reading the data
      */
-    public static SlideShow<?,?> create(NPOIFSFileSystem fs, String password) throws IOException {
+    public static SlideShow<?,?> create(final NPOIFSFileSystem fs, String password) throws IOException {
         DirectoryNode root = fs.getRoot();
 
         // Encrypted OOXML files go inside OLE2 containers, is this one?
         if (root.hasEntry(Decryptor.DEFAULT_POIFS_ENTRY)) {
-            EncryptionInfo info = new EncryptionInfo(fs);
-            Decryptor d = Decryptor.getInstance(info);
-
-            boolean passwordCorrect = false;
             InputStream stream = null;
             try {
-                if (password != null && d.verifyPassword(password)) {
+                stream = DocumentFactoryHelper.getDecryptedStream(fs, password);
-                    passwordCorrect = true;
-                }
-                if (!passwordCorrect && d.verifyPassword(Decryptor.DEFAULT_PASSWORD)) {
-                    passwordCorrect = true;
-                }
-                if (passwordCorrect) {
-                    stream = d.getDataStream(root);
-                }
-
-                if (!passwordCorrect) {
-                    String err = (password != null)
-                        ? "Password incorrect"
-                        : "The supplied spreadsheet is protected, but no password was supplied";
-                    throw new EncryptedDocumentException(err);
-                }
 
                 return createXSLFSlideShow(stream);
-            } catch (GeneralSecurityException e) {
-                throw new IOException(e);
             } finally {
                 if (stream != null) stream.close();
             }
@@ -171,7 +142,7 @@ public class SlideShowFactory {
             NPOIFSFileSystem fs = new NPOIFSFileSystem(inp);
             return create(fs, password);
         }
-        if (hasOOXMLHeader(inp)) {
+        if (DocumentFactoryHelper.hasOOXMLHeader(inp)) {
             return createXSLFSlideShow(inp);
         }
         throw new IllegalArgumentException("Your InputStream was neither an OLE2 stream, nor an OOXML stream");
@@ -291,35 +262,4 @@ public class SlideShowFactory {
             throw new IOException(e);
         }
     }
-
-    /**
-     * This copied over from ooxml, because we can't rely on these classes in the main package
-     * 
-     * @see org.apache.poi.POIXMLDocument#hasOOXMLHeader(InputStream)
-     */
-    protected static boolean hasOOXMLHeader(InputStream inp) throws IOException {
-        // We want to peek at the first 4 bytes
-        inp.mark(4);
-
-        byte[] header = new byte[4];
-        int bytesRead = IOUtils.readFully(inp, header);
-
-        // Wind back those 4 bytes
-        if(inp instanceof PushbackInputStream) {
-            PushbackInputStream pin = (PushbackInputStream)inp;
-            pin.unread(header, 0, bytesRead);
-        } else {
-            inp.reset();
-        }
-
-        // Did it match the ooxml zip signature?
-        return (
-            bytesRead == 4 &&
-            header[0] == OOXML_FILE_HEADER[0] &&
-            header[1] == OOXML_FILE_HEADER[1] &&
-            header[2] == OOXML_FILE_HEADER[2] &&
-            header[3] == OOXML_FILE_HEADER[3]
-        );
-    }
-
 }

src/ooxml/java/org/apache/poi/POIXMLDocument.java

@@ -36,6 +36,7 @@ import org.apache.poi.openxml4j.opc.PackagePart;
 import org.apache.poi.openxml4j.opc.PackageRelationship;
 import org.apache.poi.openxml4j.opc.PackageRelationshipCollection;
 import org.apache.poi.poifs.common.POIFSConstants;
+import org.apache.poi.poifs.filesystem.DocumentFactoryHelper;
 import org.apache.poi.util.IOUtils;
 import org.apache.xmlbeans.impl.common.SystemCache;
 
@@ -122,30 +123,11 @@ public abstract class POIXMLDocument extends POIXMLDocumentPart implements Close
      *  then wrap it in a PushBackInputStream, then be
      *  sure to always use that, and not the original!
      * @param inp An InputStream which supports either mark/reset, or is a PushbackInputStream
+     *
+     * @deprecated use the method from DocumentFactoryHelper, deprecated as of 3.15-beta1, therefore eligible for removal in 3.17
      */
     public static boolean hasOOXMLHeader(InputStream inp) throws IOException {
-        // We want to peek at the first 4 bytes
+        return DocumentFactoryHelper.hasOOXMLHeader(inp);
-        inp.mark(4);
-
-        byte[] header = new byte[4];
-        int bytesRead = IOUtils.readFully(inp, header);
-
-        // Wind back those 4 bytes
-        if(inp instanceof PushbackInputStream) {
-            PushbackInputStream pin = (PushbackInputStream)inp;
-            pin.unread(header, 0, bytesRead);
-        } else {
-            inp.reset();
-        }
-
-        // Did it match the ooxml zip signature?
-        return (
-                bytesRead == 4 &&
-                header[0] == POIFSConstants.OOXML_FILE_HEADER[0] &&
-                header[1] == POIFSConstants.OOXML_FILE_HEADER[1] &&
-                header[2] == POIFSConstants.OOXML_FILE_HEADER[2] &&
-                header[3] == POIFSConstants.OOXML_FILE_HEADER[3]
-        );
     }
 
     /**

src/ooxml/java/org/apache/poi/extractor/ExtractorFactory.java

@@ -51,14 +51,7 @@ import org.apache.poi.openxml4j.opc.PackageAccess;
 import org.apache.poi.openxml4j.opc.PackagePart;
 import org.apache.poi.openxml4j.opc.PackageRelationshipCollection;
 import org.apache.poi.openxml4j.opc.PackageRelationshipTypes;
-import org.apache.poi.poifs.filesystem.DirectoryEntry;
+import org.apache.poi.poifs.filesystem.*;
-import org.apache.poi.poifs.filesystem.DirectoryNode;
-import org.apache.poi.poifs.filesystem.Entry;
-import org.apache.poi.poifs.filesystem.NPOIFSFileSystem;
-import org.apache.poi.poifs.filesystem.NotOLE2FileException;
-import org.apache.poi.poifs.filesystem.OPOIFSFileSystem;
-import org.apache.poi.poifs.filesystem.OfficeXmlFileException;
-import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.poi.xdgf.extractor.XDGFVisioExtractor;
 import org.apache.poi.xslf.extractor.XSLFPowerPointExtractor;
 import org.apache.poi.xslf.usermodel.XSLFRelation;
@@ -190,7 +183,7 @@ public class ExtractorFactory {
 		if(NPOIFSFileSystem.hasPOIFSHeader(inp)) {
 			return createExtractor(new NPOIFSFileSystem(inp));
 		}
-		if(POIXMLDocument.hasOOXMLHeader(inp)) {
+		if(DocumentFactoryHelper.hasOOXMLHeader(inp)) {
 			return createExtractor(OPCPackage.open(inp));
 		}
 		throw new IllegalArgumentException("Your InputStream was neither an OLE2 stream, nor an OOXML stream");

src/ooxml/java/org/apache/poi/ss/usermodel/WorkbookFactory.java

@@ -17,18 +17,16 @@
 package org.apache.poi.ss.usermodel;
 
 import java.io.*;
-import java.security.GeneralSecurityException;
 
 import org.apache.poi.EmptyFileException;
 import org.apache.poi.EncryptedDocumentException;
-import org.apache.poi.POIXMLDocument;
+import org.apache.poi.poifs.filesystem.DocumentFactoryHelper;
 import org.apache.poi.hssf.record.crypto.Biff8EncryptionKey;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
 import org.apache.poi.openxml4j.opc.OPCPackage;
 import org.apache.poi.openxml4j.opc.PackageAccess;
 import org.apache.poi.poifs.crypt.Decryptor;
-import org.apache.poi.poifs.crypt.EncryptionInfo;
 import org.apache.poi.poifs.filesystem.DirectoryNode;
 import org.apache.poi.poifs.filesystem.NPOIFSFileSystem;
 import org.apache.poi.poifs.filesystem.OfficeXmlFileException;
@@ -82,40 +80,7 @@ public class WorkbookFactory {
 
         // Encrypted OOXML files go inside OLE2 containers, is this one?
         if (root.hasEntry(Decryptor.DEFAULT_POIFS_ENTRY)) {
-            EncryptionInfo info = new EncryptionInfo(fs);
+            InputStream stream = DocumentFactoryHelper.getDecryptedStream(fs, password);
-            Decryptor d = Decryptor.getInstance(info);
-
-            boolean passwordCorrect = false;
-            InputStream stream = null;
-            try {
-                if (password != null && d.verifyPassword(password)) {
-                    passwordCorrect = true;
-                }
-                if (!passwordCorrect && d.verifyPassword(Decryptor.DEFAULT_PASSWORD)) {
-                    passwordCorrect = true;
-                }
-                if (passwordCorrect) {
-                    // wrap the stream in a FilterInputStream to close the NPOIFSFileSystem
-                    // as well when the resulting OPCPackage is closed
-                    stream = new FilterInputStream(d.getDataStream(root)) {
-                        @Override
-                        public void close() throws IOException {
-                            fs.close();
-
-                            super.close();
-                        }
-                    };
-                }
-            } catch (GeneralSecurityException e) {
-                throw new IOException(e);
-            }
-
-            if (! passwordCorrect) {
-                if (password != null)
-                    throw new EncryptedDocumentException("Password incorrect");
-                else
-                    throw new EncryptedDocumentException("The supplied spreadsheet is protected, but no password was supplied");
-            }
 
             OPCPackage pkg = OPCPackage.open(stream);
             return create(pkg);
@@ -212,7 +177,7 @@ public class WorkbookFactory {
             NPOIFSFileSystem fs = new NPOIFSFileSystem(inp);
             return create(fs, password);
         }
-        if (POIXMLDocument.hasOOXMLHeader(inp)) {
+        if (DocumentFactoryHelper.hasOOXMLHeader(inp)) {
             return new XSSFWorkbook(OPCPackage.open(inp));
         }
         throw new InvalidFormatException("Your InputStream was neither an OLE2 stream, nor an OOXML stream");

src/ooxml/testcases/org/apache/poi/TestDetectAsOOXML.java

@@ -28,6 +28,7 @@ import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.openxml4j.opc.OPCPackage;
+import org.apache.poi.poifs.filesystem.DocumentFactoryHelper;
 
 /**
  * Class to test that HXF correctly detects OOXML
@@ -47,21 +48,21 @@ public class TestDetectAsOOXML extends TestCase
 		in = new PushbackInputStream(
 				HSSFTestDataSamples.openSampleFileStream("SampleSS.xlsx"), 10
 		);
-		assertTrue(POIXMLDocument.hasOOXMLHeader(in));
+		assertTrue(DocumentFactoryHelper.hasOOXMLHeader(in));
 		in.close();
 		
 		// xls file isn't
 		in = new PushbackInputStream(
 				HSSFTestDataSamples.openSampleFileStream("SampleSS.xls"), 10
 		);
-		assertFalse(POIXMLDocument.hasOOXMLHeader(in));
+		assertFalse(DocumentFactoryHelper.hasOOXMLHeader(in));
 		in.close();
 		
 		// text file isn't
 		in = new PushbackInputStream(
 				HSSFTestDataSamples.openSampleFileStream("SampleSS.txt"), 10
 		);
-		assertFalse(POIXMLDocument.hasOOXMLHeader(in));
+		assertFalse(DocumentFactoryHelper.hasOOXMLHeader(in));
 		in.close();
 	}
     
@@ -73,13 +74,14 @@ public class TestDetectAsOOXML extends TestCase
         
         // detect header
         InputStream in = new PushbackInputStream(testInput, 10);
-        assertFalse(POIXMLDocument.hasOOXMLHeader(in));
+        assertFalse(DocumentFactoryHelper.hasOOXMLHeader(in));
+		//noinspection deprecation
+		assertFalse(POIXMLDocument.hasOOXMLHeader(in));
         
         // check if InputStream is still intact
         byte[] test = new byte[3];
-        in.read(test);
+        assertEquals(3, in.read(test));
         assertTrue(Arrays.equals(testData, test));
         assertEquals(-1, in.read());
 	}
-
 }