Author: onealj
Date: Wed Sep 14 12:57:39 2016
New Revision: 1760702
URL: http://svn.apache.org/viewvc?rev=1760702&view=rev
Log:
bug 60128: close open file descriptors when exceptions are thrown from
OPCPackage.open
Added:
poi/trunk/test-data/openxml4j/invalid.xlsx (with props)
Modified:
poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java
poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java
poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
poi/trunk/src/ooxml/testcases/org/apache/poi/extractor/TestExtractorFactory.java
poi/trunk/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java
poi/trunk/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java
Modified: poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java?rev=1760702&r1=1760701&r2=1760702&view=diff
==============================================================================
--- poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java
(original)
+++ poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java Wed
Sep 14 12:57:39 2016
@@ -248,9 +248,10 @@ public abstract class OPCPackage impleme
* @throws InvalidFormatException
* If the specified file doesn't exist, and a parsing error
* occur.
+ * @throws InvalidOperationException
*/
public static OPCPackage open(String path, PackageAccess access)
- throws InvalidFormatException {
+ throws InvalidFormatException,
InvalidOperationException {
if (path == null || "".equals(path.trim())) {
throw new IllegalArgumentException("'path' must be
given");
}
@@ -261,8 +262,20 @@ public abstract class OPCPackage impleme
}
OPCPackage pack = new ZipPackage(path, access);
+ boolean success = false;
if (pack.partList == null && access != PackageAccess.WRITE) {
- pack.getParts();
+ try {
+ pack.getParts();
+ success = true;
+ } finally {
+ if (! success) {
+ try {
+ pack.close();
+ } catch (final IOException e) {
+ throw new
InvalidOperationException("Could not close OPCPackage while cleaning up", e);
+ }
+ }
+ }
}
pack.originalPackagePath = new File(path).getAbsolutePath();
return pack;
Modified: poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java?rev=1760702&r1=1760701&r2=1760702&view=diff
==============================================================================
--- poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java
(original)
+++ poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java Wed
Sep 14 12:57:39 2016
@@ -19,6 +19,7 @@ package org.apache.poi.openxml4j.opc;
import java.io.File;
import java.io.FileInputStream;
+import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
@@ -88,9 +89,18 @@ public final class ZipPackage extends OP
*/
ZipPackage(InputStream in, PackageAccess access) throws IOException {
super(access);
- @SuppressWarnings("resource")
ThresholdInputStream zis = ZipHelper.openZipStream(in);
- this.zipArchive = new ZipInputStreamZipEntrySource(zis);
+ try {
+ this.zipArchive = new ZipInputStreamZipEntrySource(zis);
+ } catch (final IOException e) {
+ try {
+ zis.close();
+ } catch (final IOException e2) {
+ e2.addSuppressed(e);
+ throw new IOException("Failed to close zip input stream while
cleaning up", e2);
+ }
+ throw new IOException("Failed to read zip entry source", e);
+ }
}
/**
@@ -100,8 +110,9 @@ public final class ZipPackage extends OP
* The path of the file to open or create.
* @param access
* The package access mode.
+ * @throws InvalidOperationException
*/
- ZipPackage(String path, PackageAccess access) {
+ ZipPackage(String path, PackageAccess access) throws
InvalidOperationException {
this(new File(path), access);
}
@@ -112,9 +123,9 @@ public final class ZipPackage extends OP
* The file to open or create.
* @param access
* The package access mode.
+ * @throws InvalidOperationException
*/
- @SuppressWarnings("resource")
- ZipPackage(File file, PackageAccess access) {
+ ZipPackage(File file, PackageAccess access) throws
InvalidOperationException {
super(access);
ZipEntrySource ze;
@@ -127,35 +138,71 @@ public final class ZipPackage extends OP
throw new InvalidOperationException("Can't open the specified
file: '" + file + "'", e);
}
logger.log(POILogger.ERROR, "Error in zip file "+file+" - falling
back to stream processing (i.e. ignoring zip central directory)");
- // some zips can't be opened via ZipFile in JDK6, as the central
directory
- // contains either non-latin entries or the compression type can't
be handled
- // the workaround is to iterate over the stream and not the
directory
- FileInputStream fis = null;
- ThresholdInputStream zis = null;
+ ze = openZipEntrySourceStream(file);
+ }
+ this.zipArchive = ze;
+ }
+
+ private static ZipEntrySource openZipEntrySourceStream(File file) throws
InvalidOperationException {
+ final FileInputStream fis;
+ // Acquire a resource that is needed to read the next level of
openZipEntrySourceStream
+ try {
+ // open the file input stream
+ fis = new FileInputStream(file);
+ } catch (final FileNotFoundException e) {
+ // If the source cannot be acquired, abort (no resources to free
at this level)
+ throw new InvalidOperationException("Can't open the specified file
input stream from file: '" + file + "'", e);
+ }
+
+ // If an error occurs while reading the next level of
openZipEntrySourceStream, free the acquired resource
+ try {
+ // read from the file input stream
+ return openZipEntrySourceStream(fis);
+ } catch (final Exception e) {
try {
- fis = new FileInputStream(file);
- zis = ZipHelper.openZipStream(fis);
- ze = new ZipInputStreamZipEntrySource(zis);
- } catch (IOException e2) {
- if (zis != null) {
- try {
- zis.close();
- } catch (IOException e3) {
- throw new InvalidOperationException("Can't open the
specified file: '" + file + "'"+
- " and couldn't close the file input stream",
e);
- }
- } else if (fis != null) {
- try {
- fis.close();
- } catch (IOException e3) {
- throw new InvalidOperationException("Can't open the
specified file: '" + file + "'"+
- " and couldn't close the file input stream",
e);
- }
- }
- throw new InvalidOperationException("Can't open the specified
file: '" + file + "'", e);
+ // abort: close the file input stream
+ fis.close();
+ } catch (final IOException e2) {
+ throw new InvalidOperationException("Could not close the
specified file input stream from file: '" + file + "'", e2);
}
+ throw new InvalidOperationException("Failed to read the file input
stream from file: '" + file + "'", e);
+ }
+ }
+
+ private static ZipEntrySource openZipEntrySourceStream(FileInputStream
fis) throws InvalidOperationException {
+ final ThresholdInputStream zis;
+ // Acquire a resource that is needed to read the next level of
openZipEntrySourceStream
+ try {
+ // open the zip input stream
+ zis = ZipHelper.openZipStream(fis);
+ } catch (final IOException e) {
+ // If the source cannot be acquired, abort (no resources to free
at this level)
+ throw new InvalidOperationException("Could not open the file input
stream", e);
+ }
+
+ // If an error occurs while reading the next level of
openZipEntrySourceStream, free the acquired resource
+ try {
+ // read from the zip input stream
+ return openZipEntrySourceStream(zis);
+ } catch (final Exception e) {
+ try {
+ // abort: close the zip input stream
+ zis.close();
+ } catch (final IOException e2) {
+ throw new InvalidOperationException("Failed to read the zip
entry source stream and could not close the zip input stream", e2);
+ }
+ throw new InvalidOperationException("Failed to read the zip entry
source stream");
+ }
+ }
+
+ private static ZipEntrySource
openZipEntrySourceStream(ThresholdInputStream zis) throws
InvalidOperationException {
+ // Acquire the final level resource. If this is acquired successfully,
the zip package was read successfully from the input stream
+ try {
+ // open the zip entry source stream
+ return new ZipInputStreamZipEntrySource(zis);
+ } catch (IOException e) {
+ throw new InvalidOperationException("Could not open the specified
zip entry source stream", e);
}
- this.zipArchive = ze;
}
/**
@@ -220,11 +267,12 @@ public final class ZipPackage extends OP
boolean hasSettingsXML = false;
entries = this.zipArchive.getEntries();
while (entries.hasMoreElements()) {
- ZipEntry entry = entries.nextElement();
- if (entry.getName().equals("mimetype")) {
+ final ZipEntry entry = entries.nextElement();
+ final String name = entry.getName();
+ if ("mimetype".equals(name)) {
hasMimetype = true;
}
- if (entry.getName().equals("settings.xml")) {
+ if ("settings.xml".equals(name)) {
hasSettingsXML = true;
}
numEntries++;
Modified:
poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java?rev=1760702&r1=1760701&r2=1760702&view=diff
==============================================================================
--- poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
(original)
+++ poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
Wed Sep 14 12:57:39 2016
@@ -134,15 +134,15 @@ public class ZipSecureFile extends ZipFi
return MAX_TEXT_SIZE;
}
- public ZipSecureFile(File file, int mode) throws IOException {
+ public ZipSecureFile(File file, int mode) throws ZipException, IOException
{
super(file, mode);
}
- public ZipSecureFile(File file) throws IOException {
+ public ZipSecureFile(File file) throws ZipException, IOException {
super(file);
}
- public ZipSecureFile(String name) throws IOException {
+ public ZipSecureFile(String name) throws ZipException, IOException {
super(name);
}
Modified:
poi/trunk/src/ooxml/testcases/org/apache/poi/extractor/TestExtractorFactory.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/extractor/TestExtractorFactory.java?rev=1760702&r1=1760701&r2=1760702&view=diff
==============================================================================
---
poi/trunk/src/ooxml/testcases/org/apache/poi/extractor/TestExtractorFactory.java
(original)
+++
poi/trunk/src/ooxml/testcases/org/apache/poi/extractor/TestExtractorFactory.java
Wed Sep 14 12:57:39 2016
@@ -682,9 +682,12 @@ public class TestExtractorFactory {
// Text
try {
ExtractorFactory.createExtractor(OPCPackage.open(txt.toString()));
- fail();
+ fail("TestExtractorFactory.testPackage() failed on " +
txt.toString());
} catch(UnsupportedFileFormatException e) {
// Good
+ } catch (Exception e) {
+ System.out.println("TestExtractorFactory.testPackage() failed on "
+ txt.toString());
+ throw e;
}
}
@@ -942,6 +945,7 @@ public class TestExtractorFactory {
"openxml4j/OPCCompliance_CoreProperties_OnlyOneCorePropertiesPartFAIL.docx",
"openxml4j/OPCCompliance_CoreProperties_UnauthorizedXMLLangAttributeFAIL.docx",
"openxml4j/OPCCompliance_DerivedPartNameFAIL.docx",
+ "openxml4j/invalid.xlsx",
"spreadsheet/54764-2.xlsx", // see TestXSSFBugs.bug54764()
"spreadsheet/54764.xlsx", // see TestXSSFBugs.bug54764()
"spreadsheet/Simple.xlsb",
Modified:
poi/trunk/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java?rev=1760702&r1=1760701&r2=1760702&view=diff
==============================================================================
--- poi/trunk/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java
(original)
+++ poi/trunk/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java
Wed Sep 14 12:57:39 2016
@@ -943,4 +943,22 @@ public final class TestPackage {
ZipSecureFile.setMaxTextSize(before);
}
}
+
+ // bug 60128
+ @Test
+ public void testCorruptFile() throws IOException {
+ OPCPackage pkg = null;
+ File file = OpenXML4JTestDataSamples.getSampleFile("invalid.xlsx");
+ try {
+ pkg = OPCPackage.open(file, PackageAccess.READ);
+ } catch (Exception e) {
+ System.out.println(e.getClass().getName());
+ System.out.println(e.getMessage());
+ e.printStackTrace();
+ } finally {
+ if (pkg != null) {
+ pkg.close();
+ }
+ }
+ }
}
Modified:
poi/trunk/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java
URL:
http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java?rev=1760702&r1=1760701&r2=1760702&view=diff
==============================================================================
---
poi/trunk/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java
(original)
+++
poi/trunk/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java
Wed Sep 14 12:57:39 2016
@@ -184,6 +184,7 @@ public class TestZipPackage {
public void testClosingStreamOnException() throws IOException {
InputStream is =
OpenXML4JTestDataSamples.openSampleStream("dcterms_bug_56479.zip");
File tmp = File.createTempFile("poi-test-truncated-zip", "");
+ // create a corrupted zip file by truncating a valid zip file to the
first 100 bytes
OutputStream os = new FileOutputStream(tmp);
for (int i = 0; i < 100; i++) {
os.write(is.read());
@@ -192,10 +193,15 @@ public class TestZipPackage {
os.close();
is.close();
+ // feed the corrupted zip file to OPCPackage
try {
OPCPackage.open(tmp, PackageAccess.READ);
} catch (Exception e) {
+ // expected: the zip file is invalid
+ // this test does not care if open() throws an exception or not.
}
+ // If the stream is not closed on exception, it will keep a file
descriptor to tmp,
+ // and requests to the OS to delete the file will fail.
assertTrue("Can't delete tmp file", tmp.delete());
}
Added: poi/trunk/test-data/openxml4j/invalid.xlsx
URL:
http://svn.apache.org/viewvc/poi/trunk/test-data/openxml4j/invalid.xlsx?rev=1760702&view=auto
==============================================================================
Binary file - no diff available.
Propchange: poi/trunk/test-data/openxml4j/invalid.xlsx
------------------------------------------------------------------------------
--- svn:mime-type (added)
+++ svn:mime-type Wed Sep 14 12:57:39 2016
@@ -0,0 +1 @@
+application/vnd.openxmlformats-officedocument.spreadsheetml.sheet
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
