diff --git a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipEntrySource.java b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipEntrySource.java index 1d64ffe4e..51ad32ce6 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipEntrySource.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipEntrySource.java @@ -45,4 +45,9 @@ public interface ZipEntrySource { * resources may be freed */ public void close() throws IOException; + + /** + * Has close been called already? + */ + public boolean isClosed(); } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipFileZipEntrySource.java b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipFileZipEntrySource.java index f4117f44b..09317d361 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipFileZipEntrySource.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipFileZipEntrySource.java @@ -39,6 +39,9 @@ public class ZipFileZipEntrySource implements ZipEntrySource { } zipArchive = null; } + public boolean isClosed() { + return (zipArchive == null); + } public Enumeration getEntries() { if (zipArchive == null) diff --git a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java index 36b69ac25..4c2b9df3e 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipInputStreamZipEntrySource.java @@ -76,6 +76,9 @@ public class ZipInputStreamZipEntrySource implements ZipEntrySource { // Free the memory zipEntries = null; } + public boolean isClosed() { + return (zipEntries == null); + } /** * Why oh why oh why are Iterator and Enumeration diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java index 7f174e07a..698f194c4 100644 --- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java +++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java @@ -33,11 +33,14 @@ import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.io.UnsupportedEncodingException; +import org.apache.poi.POIDataSamples; import org.apache.poi.POITextExtractor; import org.apache.poi.POIXMLException; import org.apache.poi.extractor.ExtractorFactory; import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.openxml4j.OpenXML4JTestDataSamples; +import org.apache.poi.openxml4j.exceptions.NotOfficeXmlFileException; +import org.apache.poi.openxml4j.exceptions.ODFNotOfficeXmlFileException; import org.apache.poi.sl.usermodel.SlideShow; import org.apache.poi.sl.usermodel.SlideShowFactory; import org.apache.poi.ss.usermodel.Workbook; @@ -194,6 +197,53 @@ public class TestZipPackage { } catch (Exception e) { } assertTrue("Can't delete tmp file", tmp.delete()); + } + + /** + * If ZipPackage is passed an invalid file, a call to close + * (eg from the OPCPackage open method) should tidy up the + * stream / file the broken file is being read from. + * See bug #60128 for more + */ + @Test + public void testTidyStreamOnInvalidFile() throws Exception { + // Spreadsheet has a good mix of alternate file types + POIDataSamples files = POIDataSamples.getSpreadSheetInstance(); + + File[] notValidF = new File[] { + files.getFile("SampleSS.ods"), files.getFile("SampleSS.txt") + }; + InputStream[] notValidS = new InputStream[] { + files.openResourceAsStream("SampleSS.ods"), files.openResourceAsStream("SampleSS.txt") + }; + for (File notValid : notValidF) { + ZipPackage pkg = new ZipPackage(notValid, PackageAccess.READ); + assertNotNull(pkg.getZipArchive()); + assertFalse(pkg.getZipArchive().isClosed()); + try { + pkg.getParts(); + fail("Shouldn't work"); + } catch (ODFNotOfficeXmlFileException e) { + } catch (NotOfficeXmlFileException ne) {} + pkg.close(); + + assertNotNull(pkg.getZipArchive()); + assertTrue(pkg.getZipArchive().isClosed()); + } + for (InputStream notValid : notValidS) { + ZipPackage pkg = new ZipPackage(notValid, PackageAccess.READ); + assertNotNull(pkg.getZipArchive()); + assertFalse(pkg.getZipArchive().isClosed()); + try { + pkg.getParts(); + fail("Shouldn't work"); + } catch (ODFNotOfficeXmlFileException e) { + } catch (NotOfficeXmlFileException ne) {} + pkg.close(); + + assertNotNull(pkg.getZipArchive()); + assertTrue(pkg.getZipArchive().isClosed()); + } } } diff --git a/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestSecureTempZip.java b/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestSecureTempZip.java index 4d4c5df34..868a38227 100644 --- a/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestSecureTempZip.java +++ b/src/ooxml/testcases/org/apache/poi/poifs/crypt/TestSecureTempZip.java @@ -149,10 +149,12 @@ public class TestSecureTempZip { static class AesZipFileZipEntrySource implements ZipEntrySource { final ZipFile zipFile; final Cipher ci; + boolean closed; AesZipFileZipEntrySource(ZipFile zipFile, Cipher ci) { this.zipFile = zipFile; this.ci = ci; + this.closed = false; } /** @@ -172,6 +174,12 @@ public class TestSecureTempZip { @Override public void close() throws IOException { zipFile.close(); + closed = true; + } + + @Override + public boolean isClosed() { + return closed; } } }