svn commit: r1853454 - in /poi/trunk/src/ooxml: java/org/apache/poi/openxml4j/opc/ZipPackage.java testcases/org/apache/poi/openxml4j/opc/TestPackage.java

[ye...@apache.org]

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-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org