Fix some cases where file handles are not closed in OldExcelExtractor

Close resources in some more tests to make the file-leak report clean again

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

diff --git a/build.xml b/build.xml
index 3008840fe7ff70918dca87e9a87f74bdd954f11a..cb23812c971adb1f245f342e469533a56ef9aae1 100644 (file)
--- a/build.xml
+++ b/build.xml
@@ -531,7 +531,9 @@ under the License.
             <include name="xmlsec-2.0.1.jar"/>
             <include name="xmlsec-2.0.5.jar"/>
             <include name="bcprov-ext-jdk15on-1.51.jar"/>
+            <include name="bcprov-ext-jdk15on-1.53.jar"/>
             <include name="bcpkix-jdk15on-1.51.jar"/>
+            <include name="bcpkix-jdk15on-1.53.jar"/>
             <include name="slf4j-api-1.7.7.jar"/>
          </fileset>
         </delete>

diff --git a/src/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java b/src/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java
index 2cbf089a243e227b8d4ae0c5369f816dea4c4dad..f6a33bf40543ef5ed5759332864950fdee82a485 100644 (file)
--- a/src/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java
+++ b/src/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java
@@ -44,6 +44,7 @@ import org.apache.poi.poifs.filesystem.DocumentNode;
 import org.apache.poi.poifs.filesystem.NPOIFSFileSystem;
 import org.apache.poi.poifs.filesystem.NotOLE2FileException;
 import org.apache.poi.ss.usermodel.Cell;
+import org.apache.poi.util.IOUtils;
 
 /**
  * A text extractor for old Excel files, which are too old for
@@ -57,6 +58,10 @@ import org.apache.poi.ss.usermodel.Cell;
  */
 public class OldExcelExtractor implements Closeable {
     private RecordInputStream ris;
+
+    // sometimes we hold the stream here and thus need to ensure it is closed at some point
+    private Closeable toClose;
+
     private int biffVersion;
     private int fileType;
 
@@ -69,6 +74,7 @@ public class OldExcelExtractor implements Closeable {
         try {
             poifs = new NPOIFSFileSystem(f);
             open(poifs);
+            toClose = poifs;
             return;
         } catch (OldExcelFormatException e) {
             // will be handled by workaround below
@@ -91,6 +97,11 @@ public class OldExcelExtractor implements Closeable {
             // is thrown while opening
             biffStream.close();
             throw e;
+        } catch (RuntimeException e)  {
+            // ensure that the stream is properly closed here if an Exception
+            // is thrown while opening
+            biffStream.close();
+            throw e;
         }
     }
 
@@ -116,6 +127,7 @@ public class OldExcelExtractor implements Closeable {
             }
         } else {
             ris = new RecordInputStream(bis);
+            toClose = bis;
             prepare();
         }
     }
@@ -279,9 +291,13 @@ public class OldExcelExtractor implements Closeable {
 
     @Override
     public void close() {
-        // not necessary any more ...
+        // some cases require this close here
+        if(toClose != null) {
+            IOUtils.closeQuietly(toClose);
+            toClose = null;
+        }
     }
-    
+
     protected void handleNumericCell(StringBuffer text, double value) {
         // TODO Need to fetch / use format strings
         text.append(value);

diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java
index 725ab24888064569eb4743b41c273ef5c5dfb8f8..3eb9b6df6f85f94b339d0282dbf35304f06bef69 100644 (file)
--- a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java
+++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java
@@ -255,11 +255,27 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
       }
 
       OPCPackage pack = new ZipPackage(file, access);
-      if (pack.partList == null && access != PackageAccess.WRITE) {
-         pack.getParts();
-      }
-      pack.originalPackagePath = file.getAbsolutePath();
-      return pack;
+          try {
+                  if (pack.partList == null && access != PackageAccess.WRITE) {
+                          pack.getParts();
+                  }
+                  pack.originalPackagePath = file.getAbsolutePath();
+                  return pack;
+          } catch (InvalidFormatException e) {
+                  try {
+                          pack.close();
+                  } catch (IOException e1) {
+                          throw new IllegalStateException(e);
+                  }
+                  throw e;
+          } catch (RuntimeException e) {
+                  try {
+                          pack.close();
+                  } catch (IOException e1) {
+                          throw new IllegalStateException(e);
+                  }
+                  throw e;
+          }
    }
 
        /**

diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java
index 41d6bc86bc555954acd97c64ae5f2db205986e00..54107ff341ccaae4d6ad6925bad93ea18dff5cc4 100644 (file)
--- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java
+++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java
@@ -693,7 +693,12 @@ public final class TestPackage {
         
         // OLE2 - Stream
         try {
-            OPCPackage.open(files.openResourceAsStream("SampleSS.xls"));
+                       InputStream stream = files.openResourceAsStream("SampleSS.xls");
+                       try {
+                               OPCPackage.open(stream);
+                       } finally {
+                               stream.close();
+                       }
             fail("Shouldn't be able to open OLE2");
         } catch (OLE2NotOfficeXmlFileException e) {
             assertTrue(e.getMessage().contains("The supplied data appears to be in the OLE2 Format"));
@@ -710,7 +715,12 @@ public final class TestPackage {
         
         // Raw XML - Stream
         try {
-            OPCPackage.open(files.openResourceAsStream("SampleSS.xml"));
+                       InputStream stream = files.openResourceAsStream("SampleSS.xml");
+                       try {
+                               OPCPackage.open(stream);
+                       } finally {
+                               stream.close();
+                       }
             fail("Shouldn't be able to open XML");
         } catch (NotOfficeXmlFileException e) {
             assertTrue(e.getMessage().contains("The supplied data appears to be a raw XML file"));
@@ -727,7 +737,12 @@ public final class TestPackage {
         
         // ODF / ODS - Stream
         try {
-            OPCPackage.open(files.openResourceAsStream("SampleSS.ods"));
+                       InputStream stream = files.openResourceAsStream("SampleSS.ods");
+                       try {
+                               OPCPackage.open(stream);
+                       } finally {
+                               stream.close();
+                       }
             fail("Shouldn't be able to open ODS");
         } catch (ODFNotOfficeXmlFileException e) {
             assertTrue(e.toString().contains("The supplied data appears to be in ODF"));
@@ -744,7 +759,12 @@ public final class TestPackage {
         
         // Plain Text - Stream
         try {
-            OPCPackage.open(files.openResourceAsStream("SampleSS.txt"));
+                       InputStream stream = files.openResourceAsStream("SampleSS.txt");
+                       try {
+                               OPCPackage.open(stream);
+                       } finally {
+                               stream.close();
+                       }
             fail("Shouldn't be able to open Plain Text");
         } catch (NotOfficeXmlFileException e) {
             assertTrue(e.getMessage().contains("No valid entries or contents found"));