[bug-67579] add new XSSFWorkbook constructor

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

7 files changed, 157 insertions, 24 deletions

diff --git a/poi-ooxml/src/main/java/org/apache/poi/ooxml/util/PackageHelper.java b/poi-ooxml/src/main/java/org/apache/poi/ooxml/util/PackageHelper.java
index aa175bde29..c70a866c12 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/ooxml/util/PackageHelper.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/ooxml/util/PackageHelper.java
@@ -43,20 +43,25 @@ import org.apache.poi.util.Removal;
  */
 public final class PackageHelper {
 
-    public static OPCPackage open(InputStream is) throws IOException {
-        return open(is, false);
+    /**
+     * @param stream The InputStream to read from - which is closed when it is read
+     * @return OPCPackage
+     * @throws IOException If reading data from the stream fails
+     */
+    public static OPCPackage open(InputStream stream) throws IOException {
+        return open(stream, false);
     }
 
     /**
      * @param stream The InputStream to read from
-     * @param closeStream whether to close the stream (default is false)
-     * @since POI 5.2.0
+     * @param closeStream whether to close the stream
      * @return OPCPackage
      * @throws IOException If reading data from the stream fails
+     * @since POI 5.2.0
      */
     public static OPCPackage open(InputStream stream, boolean closeStream) throws IOException {
         try {
-            return OPCPackage.open(stream);
+            return OPCPackage.open(stream, closeStream);
         } catch (InvalidFormatException e){
             throw new POIXMLException(e);
         } finally {
diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/OPCPackage.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/OPCPackage.java
index 2c79feea4e..ddb306e69d 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/OPCPackage.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/OPCPackage.java
@@ -329,6 +329,38 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
     }
 
     /**
+     * Open a package.
+     *
+     * Note - uses quite a bit more memory than {@link #open(String)}, which
+     * doesn't need to hold the whole zip file in memory, and can take advantage
+     * of native methods
+     *
+     * @param in
+     *            The InputStream to read the package from.
+     * @param closeStream
+     *            Whether to close the input stream.
+     * @return A PackageBase object
+     *
+     * @throws InvalidFormatException
+     *              Throws if the specified file exist and is not valid.
+     * @throws IOException If reading the stream fails
+     * @since POI 5.2.5
+     */
+    public static OPCPackage open(InputStream in, boolean closeStream) throws InvalidFormatException,
+            IOException {
+        OPCPackage pack = new ZipPackage(in, PackageAccess.READ_WRITE, closeStream);
+        try {
+            if (pack.partList == null) {
+                pack.getParts();
+            }
+        } catch (InvalidFormatException | RuntimeException e) {
+            IOUtils.closeQuietly(pack);
+            throw e;
+        }
+        return pack;
+    }
+
+    /**
      * Opens a package if it exists, else it creates one.
      *
      * @param file
diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java
index 2eb59e32f9..bd613c10a8 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/ZipPackage.java
@@ -115,7 +115,7 @@ public final class ZipPackage extends OPCPackage {
 
     /**
      * Constructor. Opens a Zip based Open XML document from
-     *  an InputStream.
+     *  an InputStream. The InputStream is closed.
      *
      * @param in
      *            Zip input stream to load.
@@ -135,6 +135,30 @@ public final class ZipPackage extends OPCPackage {
     }
 
     /**
+     * Constructor. Opens a Zip based Open XML document from
+     *  an InputStream. The InputStream is closed.
+     *
+     * @param in
+     *            Zip input stream to load.
+     * @param access
+     *            The package access mode.
+     * @param closeStream
+     *            Whether to close the input stream.
+     * @throws IllegalArgumentException
+     *             If the specified input stream is not an instance of
+     *             ZipInputStream.
+     * @throws IOException
+     *            if input stream cannot be opened, read, or closed
+     * @since POI 5.2.5
+     */
+    ZipPackage(InputStream in, PackageAccess access, boolean closeStream) throws IOException {
+        super(access);
+        try (ZipArchiveThresholdInputStream zis = ZipHelper.openZipStream(in, closeStream)) {
+            this.zipArchive = new ZipInputStreamZipEntrySource(zis);
+        }
+    }
+
+    /**
      * Constructor. Opens a Zip based Open XML document from a file.
      *
      * @param path
diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/internal/NoCloseInputStream.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/internal/NoCloseInputStream.java
new file mode 100644
index 0000000000..b27685aab2
--- /dev/null
+++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/internal/NoCloseInputStream.java
@@ -0,0 +1,33 @@
+/* ====================================================================
+   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.openxml4j.opc.internal;
+
+import org.apache.poi.util.Internal;
+
+import java.io.FilterInputStream;
+import java.io.InputStream;
+
+@Internal
+final class NoCloseInputStream extends FilterInputStream {
+    NoCloseInputStream(InputStream stream) {
+        super(stream);
+    }
+
+    @Override
+    public void close() {}
+}
diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/internal/ZipHelper.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/internal/ZipHelper.java
index 26f3cffae1..f9b1524da9 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/internal/ZipHelper.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/internal/ZipHelper.java
@@ -163,20 +163,33 @@ public final class ZipHelper {
     }
 
     /**
-     * Opens the specified stream as a secure zip
+     * Opens the specified stream as a secure zip. Closes the Input Stream.
      *
-     * @param stream
-     *            The stream to open.
+     * @param stream The stream to open.
      * @return The zip stream freshly open.
      */
     @SuppressWarnings("resource")
     public static ZipArchiveThresholdInputStream openZipStream(InputStream stream) throws IOException {
+        return openZipStream(stream, true);
+    }
+
+    /**
+     * Opens the specified stream as a secure zip. Closes the Input Stream.
+     *
+     * @param stream The stream to open.
+     * @param closeStream whether to close the stream
+     * @return The zip stream freshly open.
+     */
+    @SuppressWarnings("resource")
+    public static ZipArchiveThresholdInputStream openZipStream(
+            final InputStream stream, final boolean closeStream) throws IOException {
         // Peek at the first few bytes to sanity check
         InputStream checkedStream = FileMagic.prepareToCheckMagic(stream);
         verifyZipHeader(checkedStream);
-        
+
+        final InputStream processStream = closeStream ? checkedStream : new NoCloseInputStream(checkedStream);
         // Open as a proper zip stream
-        return new ZipArchiveThresholdInputStream(new ZipArchiveInputStream(checkedStream));
+        return new ZipArchiveThresholdInputStream(new ZipArchiveInputStream(processStream));
     }
 
     /**
diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java
index e1005ee1a4..dbf5ab59b9 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java
@@ -277,17 +277,40 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Date1904Su
      *       pkg.close(); // gracefully closes the underlying zip file
      *   }</pre>
      *
+     * @param stream The InputStream, which is closed when it is read.
      * @throws IOException If reading data from the stream fails
      * @throws POIXMLException a RuntimeException that can be caused by invalid OOXML data
      * @throws IllegalStateException a number of other runtime exceptions can be thrown, especially if there are problems with the
      * input format
      */
-    public XSSFWorkbook(InputStream is) throws IOException {
-        this(is, false);
+    public XSSFWorkbook(InputStream stream) throws IOException {
+        this(stream, true);
     }
 
-    private XSSFWorkbook(InputStream is, boolean closeStream) throws IOException {
-        this(PackageHelper.open(is, closeStream));
+    /**
+     * Constructs a XSSFWorkbook object, by buffering the whole stream into memory
+     *  and then opening an {@link OPCPackage} object for it.
+     *
+     * <p>Using an {@link InputStream} requires more memory than using a File, so
+     *  if a {@link File} is available then you should instead do something like
+     *   <pre>{@code
+     *       OPCPackage pkg = OPCPackage.open(path);
+     *       XSSFWorkbook wb = new XSSFWorkbook(pkg);
+     *       // work with the wb object
+     *       ......
+     *       pkg.close(); // gracefully closes the underlying zip file
+     *   }</pre>
+     *
+     * @param stream The InputStream.
+     * @param closeStream Whether to close the stream.
+     * @throws IOException If reading data from the stream fails
+     * @throws POIXMLException a RuntimeException that can be caused by invalid OOXML data
+     * @throws IllegalStateException a number of other runtime exceptions can be thrown, especially if there are problems with the
+     * input format
+     * @since POI 5.2.5
+     */
+    public XSSFWorkbook(InputStream stream, boolean closeStream) throws IOException {
+        this(PackageHelper.open(stream, closeStream));
     }
 
     /**
diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java
index f5eb9f18aa..7d6d7b668b 100644
--- a/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java
+++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java
@@ -1460,13 +1460,17 @@ public final class TestXSSFWorkbook extends BaseTestXWorkbook {
         }
     }
 
-    static class NoCloseInputStream extends FilterInputStream {
-        NoCloseInputStream(InputStream stream) {
-            super(stream);
+    @Test
+    void testWorkbookCloseCanBeStoppedFromClosingInputStream() throws Exception {
+        try (WrappedStream stream = new WrappedStream(
+                HSSFTestDataSamples.openSampleFileStream("github-321.xlsx"))) {
+            // uses new constructor, available since POI 5.2.5
+            try (XSSFWorkbook wb = new XSSFWorkbook(stream, false)) {
+                XSSFSheet xssfSheet = wb.getSheetAt(0);
+                assertNotNull(xssfSheet);
+            }
+            assertFalse(stream.isClosed(), "stream should not be closed by XSSFWorkbook");
         }
-
-        @Override
-        public void close() {}
     }
 
     @Test
@@ -1494,9 +1498,8 @@ public final class TestXSSFWorkbook extends BaseTestXWorkbook {
             try (ZipArchiveInputStream zis = new ZipArchiveInputStream(Files.newInputStream(tempFile.toPath()))) {
                 ZipArchiveEntry entry;
                 while ((entry = zis.getNextZipEntry()) != null) {
-                    // NoCloseInputStream is needed to stop XSSFWorkbook closing the underlying InputStream
-                    // this might not sound great but POI has worked like this for years and we can't just change it
-                    XSSFWorkbook wb = new XSSFWorkbook(new NoCloseInputStream(zis));
+                    // Since POI 5.2.5, you can stop XSSFWorkbook closing the InputStream by using this new constructor
+                    XSSFWorkbook wb = new XSSFWorkbook(zis, false);
                     assertNotNull(wb);
                     count++;
                 }