From 84c8f60992c563ca6489da472ff5438ab5e279a7 Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Wed, 14 Sep 2016 12:35:34 +0000 Subject: [PATCH] Unit test for bug #60128, showing that calling close on a broken package cleans up file or stream git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1760693 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/openxml4j/util/ZipEntrySource.java | 5 ++ .../openxml4j/util/ZipFileZipEntrySource.java | 3 ++ .../util/ZipInputStreamZipEntrySource.java | 3 ++ .../poi/openxml4j/opc/TestZipPackage.java | 52 ++++++++++++++++++- .../poi/poifs/crypt/TestSecureTempZip.java | 8 +++ 5 files changed, 70 insertions(+), 1 deletion(-) 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 1d64ffe4ea..51ad32ce65 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 f4117f44bf..09317d361e 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 36b69ac25b..4c2b9df3e7 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 7f174e07af..698f194c4c 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 4d4c5df345..868a382279 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; } } } -- 2.39.5