Bug 65854: Use revert() instead of close() when OPCPackage is opened read-only

Also add tests, however it is not possible to verify that no
logging takes place, so you have to use debugging to verify it.

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

diff --git a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/OPCPackage.java b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/OPCPackage.java
index ac9568afba89373300dc8b36b8b5d711f1f4f4f1..1035459f821a7add0b034c472bd1f8d0d0316d5b 100644 (file)
--- a/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/OPCPackage.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/openxml4j/opc/OPCPackage.java
@@ -211,7 +211,9 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
            // pack.originalPackagePath = file.getAbsolutePath();
            return pack;
        } catch (InvalidFormatException | RuntimeException e) {
-           IOUtils.closeQuietly(pack);
+           // use revert() to free resources when the packgae is opened read-only
+           pack.revert();
+
            throw e;
        }
    }
@@ -272,14 +274,14 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
     */
    public static OPCPackage open(File file, PackageAccess access)
          throws InvalidFormatException {
-      if (file == null) {
-          throw new IllegalArgumentException("'file' must be given");
-      }
-      if (file.exists() && file.isDirectory()) {
-          throw new IllegalArgumentException("file must not be a directory");
-      }
-
-      OPCPackage pack = new ZipPackage(file, access);
+       if (file == null) {
+           throw new IllegalArgumentException("'file' must be given");
+       }
+       if (file.exists() && file.isDirectory()) {
+           throw new IllegalArgumentException("file must not be a directory");
+       }
+
+       OPCPackage pack = new ZipPackage(file, access);
        try {
            if (pack.partList == null && access != PackageAccess.WRITE) {
                pack.getParts();
@@ -287,7 +289,11 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
            pack.originalPackagePath = file.getAbsolutePath();
            return pack;
        } catch (InvalidFormatException | RuntimeException e) {
-           IOUtils.closeQuietly(pack);
+           if (access == PackageAccess.READ) {
+               pack.revert();
+           } else {
+               IOUtils.closeQuietly(pack);
+           }
            throw e;
        }
    }

diff --git a/poi-ooxml/src/test/java/org/apache/poi/openxml4j/opc/compliance/TestOPCCompliancePackageModel.java b/poi-ooxml/src/test/java/org/apache/poi/openxml4j/opc/compliance/TestOPCCompliancePackageModel.java
index 6e03152cb7b409035fd0e48906713dd5487952c3..48d01fa57589e93135685c0c15f5b1f9753f6137 100644 (file)
--- a/poi-ooxml/src/test/java/org/apache/poi/openxml4j/opc/compliance/TestOPCCompliancePackageModel.java
+++ b/poi-ooxml/src/test/java/org/apache/poi/openxml4j/opc/compliance/TestOPCCompliancePackageModel.java
@@ -30,10 +30,12 @@ import org.apache.poi.openxml4j.exceptions.InvalidOperationException;
 import org.apache.poi.openxml4j.exceptions.PartAlreadyExistsException;
 import org.apache.poi.openxml4j.opc.ContentTypes;
 import org.apache.poi.openxml4j.opc.OPCPackage;
+import org.apache.poi.openxml4j.opc.PackageAccess;
 import org.apache.poi.openxml4j.opc.PackagePartName;
 import org.apache.poi.openxml4j.opc.PackageRelationshipTypes;
 import org.apache.poi.openxml4j.opc.PackagingURIHelper;
 import org.apache.poi.openxml4j.opc.TargetMode;
+import org.apache.poi.poifs.crypt.temp.AesZipFileZipEntrySource;
 import org.apache.poi.util.TempFile;
 import org.junit.jupiter.api.Test;
 
@@ -88,6 +90,26 @@ class TestOPCCompliancePackageModel {
         );
     }
 
+    @Test
+    void testInvalidformatExceptionZipSource() throws IOException {
+        try (AesZipFileZipEntrySource source = AesZipFileZipEntrySource.createZipEntrySource(
+                POIDataSamples.getOpenXML4JInstance().openResourceAsStream("OPCCompliance_DerivedPartNameFAIL.docx"))) {
+            assertThrows(InvalidFormatException.class, () ->
+                            OPCPackage.open(source),
+                    "Should fail for invalid file"
+            );
+        }
+    }
+
+    @Test
+    void testInvalidformatExceptionFile() {
+        assertThrows(InvalidFormatException.class,
+                () -> OPCPackage.open(POIDataSamples.getOpenXML4JInstance().
+                                getFile("OPCCompliance_DerivedPartNameFAIL.docx"), PackageAccess.READ),
+                "Should fail for invalid file"
+        );
+    }
+
     /**
      * Rule M1.12 : Packages shall not contain equivalent part names and package
      * implementers shall neither create nor recognize packages with equivalent