From 71f5735238d9af3088682f3eb8321a35b9df2e32 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sat, 12 Mar 2016 11:37:32 +0000 Subject: [PATCH] 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 --- .../filesystem/DocumentFactoryHelper.java | 116 ++++++++++++++++++ .../poi/sl/usermodel/SlideShowFactory.java | 70 +---------- .../java/org/apache/poi/POIXMLDocument.java | 26 +--- .../poi/extractor/ExtractorFactory.java | 11 +- .../poi/ss/usermodel/WorkbookFactory.java | 41 +------ .../org/apache/poi/TestDetectAsOOXML.java | 14 ++- 6 files changed, 138 insertions(+), 140 deletions(-) create mode 100644 src/java/org/apache/poi/poifs/filesystem/DocumentFactoryHelper.java diff --git a/src/java/org/apache/poi/poifs/filesystem/DocumentFactoryHelper.java b/src/java/org/apache/poi/poifs/filesystem/DocumentFactoryHelper.java new file mode 100644 index 0000000000..58658fac71 --- /dev/null +++ b/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] + ); + } + +} diff --git a/src/java/org/apache/poi/sl/usermodel/SlideShowFactory.java b/src/java/org/apache/poi/sl/usermodel/SlideShowFactory.java index 8b7b011e51..db23e1de08 100644 --- a/src/java/org/apache/poi/sl/usermodel/SlideShowFactory.java +++ b/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.FileNotFoundException; -import java.io.IOException; -import java.io.InputStream; -import java.io.PushbackInputStream; +import java.io.*; 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)) { - 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); - } + stream = DocumentFactoryHelper.getDecryptedStream(fs, password); 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] - ); - } - } diff --git a/src/ooxml/java/org/apache/poi/POIXMLDocument.java b/src/ooxml/java/org/apache/poi/POIXMLDocument.java index cbb1d80cc7..04d5c2d621 100644 --- a/src/ooxml/java/org/apache/poi/POIXMLDocument.java +++ b/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 - 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] - ); + return DocumentFactoryHelper.hasOOXMLHeader(inp); } /** diff --git a/src/ooxml/java/org/apache/poi/extractor/ExtractorFactory.java b/src/ooxml/java/org/apache/poi/extractor/ExtractorFactory.java index 68d1ec28a2..3bc5dfaadd 100644 --- a/src/ooxml/java/org/apache/poi/extractor/ExtractorFactory.java +++ b/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.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.poifs.filesystem.*; 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"); 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 7f4d1f7971..621e71d8de 100644 --- a/src/ooxml/java/org/apache/poi/ss/usermodel/WorkbookFactory.java +++ b/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); - 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"); - } + InputStream stream = DocumentFactoryHelper.getDecryptedStream(fs, password); 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"); diff --git a/src/ooxml/testcases/org/apache/poi/TestDetectAsOOXML.java b/src/ooxml/testcases/org/apache/poi/TestDetectAsOOXML.java index ff4f5dccd8..137f631b56 100644 --- a/src/ooxml/testcases/org/apache/poi/TestDetectAsOOXML.java +++ b/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()); } - }