aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java17
-rw-r--r--src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java116
-rw-r--r--src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java6
-rw-r--r--src/ooxml/testcases/org/apache/poi/extractor/TestExtractorFactory.java6
-rw-r--r--src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java18
-rw-r--r--src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java6
-rw-r--r--test-data/openxml4j/invalid.xlsxbin0 -> 22 bytes
7 files changed, 129 insertions, 40 deletions
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 e029d7dece..b7720ee42a 100644
--- a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java
+++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java
@@ -248,9 +248,10 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
* @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 implements RelationshipSource, Closeable {
}
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;
diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java
index fe4d6f1ed8..8f7efa02c5 100644
--- a/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java
+++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java
@@ -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 OPCPackage {
*/
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 OPCPackage {
* 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 OPCPackage {
* 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 OPCPackage {
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 OPCPackage {
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++;
diff --git a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
index 13369a5ed1..9000656e57 100644
--- a/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
+++ b/src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java
@@ -134,15 +134,15 @@ public class ZipSecureFile extends ZipFile {
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);
}
diff --git a/src/ooxml/testcases/org/apache/poi/extractor/TestExtractorFactory.java b/src/ooxml/testcases/org/apache/poi/extractor/TestExtractorFactory.java
index 0aa022332e..9d206f7197 100644
--- a/src/ooxml/testcases/org/apache/poi/extractor/TestExtractorFactory.java
+++ b/src/ooxml/testcases/org/apache/poi/extractor/TestExtractorFactory.java
@@ -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",
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 d84ecab81b..5f83bc52d0 100644
--- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java
+++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java
@@ -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();
+ }
+ }
+ }
}
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 698f194c4c..0989d10cb2 100644
--- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java
+++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestZipPackage.java
@@ -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());
}
diff --git a/test-data/openxml4j/invalid.xlsx b/test-data/openxml4j/invalid.xlsx
new file mode 100644
index 0000000000..15cb0ecb3e
--- /dev/null
+++ b/test-data/openxml4j/invalid.xlsx
Binary files differ