From: Yegor Kozlov <yegor@apache.org>
Date: Tue, 12 Feb 2019 16:55:13 +0000 (+0000)
Subject: Bug 63029: OPCPackage Potentially clobbers files on close()
X-Git-Tag: REL_4_1_0~71
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=caef630c70d343be89805c40f45822b62feb16d7;p=poi.git

Bug 63029: OPCPackage Potentially clobbers files on close()

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1853454 13f79535-47bb-0310-9956-ffa450edef68
---

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 f31808f697..8867e34ae7 100644
--- a/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java
+++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/ZipPackage.java
@@ -144,7 +144,7 @@ public final class ZipPackage extends OPCPackage {
             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 OPCPackage {
 		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()) {
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 3305a7e2dc..4f4e4e5b5e 100644
--- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java
+++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/TestPackage.java
@@ -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")
+
+		}
+
+	}
 }