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

3 files changed, 63 insertions, 11 deletions

diff --git a/src/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java b/src/java/org/apache/poi/hssf/extractor/OldExcelExtractor.java
index 2cbf089a24..f6a33bf405 100644
--- 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 725ab24888..3eb9b6df6f 100644
--- 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 41d6bc86bc..54107ff341 100644
--- 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"));