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
This commit is contained in:
Yegor Kozlov 2019-02-12 16:55:13 +00:00
parent c438465b08
commit caef630c70
2 changed files with 45 additions and 3 deletions

View File

@ -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()) {

View File

@ -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")
}
}
}