Author: yegor
Date: Tue Feb 12 16:55:13 2019
New Revision: 1853454
URL: http://svn.apache.org/viewvc?rev=1853454&view=rev
Log:
Bug 63029: OPCPackage Potentially clobbers files on close()
Modified:
poi/trunk/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java
poi/trunk/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java
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=1853454&r1=1853453&r2=1853454&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 Tue
Feb 12 16:55:13 2019
@@ -144,7 +144,7 @@ public final class ZipPackage extends OP
if (access == PackageAccess.WRITE) {
throw new InvalidOperationException("Can't open the specified
file: '" + file + "'", e);
}
- if ("java.util.zip.ZipException: archive is not a ZIP
archive".equals(e.getMessage())) {
+ if ("archive is not a ZIP archive".equals(e.getMessage())) {
throw new NotOfficeXmlFileException("archive is not a ZIP
archive", e);
}
LOG.log(POILogger.ERROR, "Error in zip file "+file+" - falling
back to stream processing (i.e. ignoring zip central directory)");
@@ -424,14 +424,18 @@ public final class ZipPackage extends OP
File tempFile = TempFile.createTempFile(tempFileName, ".tmp");
// Save the final package to a temporary file
+ boolean success = false;
try {
save(tempFile);
+ success = true;
} finally {
// Close the current zip file, so we can overwrite it on all
platforms
IOUtils.closeQuietly(this.zipArchive);
try {
- // Copy the new file over the old one
- FileHelper.copyFile(tempFile, targetFile);
+ // Copy the new file over the old one if save()
succeed
+ if(success) {
+ FileHelper.copyFile(tempFile, targetFile);
+ }
} finally {
// Either the save operation succeed or not, we
delete the temporary file
if (!tempFile.delete()) {
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=1853454&r1=1853453&r2=1853454&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
Tue Feb 12 16:55:13 2019
@@ -17,6 +17,8 @@
package org.apache.poi.openxml4j.opc;
+import com.google.common.hash.Hashing;
+import com.google.common.io.Files;
import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
import org.apache.commons.compress.archivers.zip.ZipFile;
@@ -84,6 +86,7 @@ import java.util.List;
import java.util.TreeMap;
import java.util.function.BiConsumer;
import java.util.regex.Pattern;
+import java.util.zip.ZipException;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@@ -1185,4 +1188,39 @@ public final class TestPackage {
.appendValue(expectedMessage);
}
}
+
+ @Test
+ public void testBug63029() throws Exception {
+ File testFile =
OpenXML4JTestDataSamples.getSampleFile("sample.docx");
+ File tmpFile =
OpenXML4JTestDataSamples.getOutputFile("Bug63029.docx");
+ Files.copy(testFile, tmpFile);
+
+ String md5Before = Files.hash(tmpFile,
Hashing.md5()).toString();
+
+ RuntimeException ex = null;
+ try(OPCPackage pkg = OPCPackage.open(tmpFile,
PackageAccess.READ_WRITE))
+ {
+ // add a marshaller that will throw an exception on save
+ pkg.addMarshaller("poi/junit", (part, out) -> {
+ throw new RuntimeException("Bugzilla 63029");
+ });
+
pkg.createPart(PackagingURIHelper.createPartName("/poi/test.xml"), "poi/junit");
+ } catch (RuntimeException e){
+ ex = e;
+ }
+ // verify there was an exception while closing the file
+ assertEquals("Fail to save: an error occurs while saving the
package : Bugzilla 63029", ex.getMessage());
+
+ // assert that md5 after closing is the same, i.e. the source
is left intact
+ String md5After = Files.hash(tmpFile, Hashing.md5()).toString();
+ assertEquals(md5Before, md5After);
+
+ // try to read the source file once again
+ try ( OPCPackage zip = OPCPackage.open(tmpFile,
PackageAccess.READ_WRITE)){
+ // the source is still a valid zip archive.
+ // prior to the fix this used to throw
NotOfficeXmlFileException("archive is not a ZIP archive")
+
+ }
+
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@poi.apache.org
For additional commands, e-mail: commits-h...@poi.apache.org
