From d9cfcbdd189ef2d3a2796b648ddb5b84619912b9 Mon Sep 17 00:00:00 2001
From: Javen O'Neal <onealj@apache.org>
Date: Sat, 10 Sep 2016 19:33:32 +0000
Subject: [PATCH] bug 60102: throw IOException when writing a closed document

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1760206 13f79535-47bb-0310-9956-ffa450edef68
---
 .../java/org/apache/poi/POIXMLDocument.java   | 10 +++--
 .../org/apache/poi/TestPOIXMLDocument.java    | 37 ++++++++++++++++++-
 2 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/src/ooxml/java/org/apache/poi/POIXMLDocument.java b/src/ooxml/java/org/apache/poi/POIXMLDocument.java
index 4ec3d442ed..93afa6163f 100644
--- a/src/ooxml/java/org/apache/poi/POIXMLDocument.java
+++ b/src/ooxml/java/org/apache/poi/POIXMLDocument.java
@@ -40,6 +40,7 @@ import org.apache.xmlbeans.impl.common.SystemCache;
 /**
  * This holds the common functionality for all POI OOXML Document classes.
  */
+// TODO: implements AutoCloseable in Java 7+ when POI drops support for Java 6.
 public abstract class POIXMLDocument extends POIXMLDocumentPart implements Closeable {
     public static final String DOCUMENT_CREATOR = "Apache POI";
 
@@ -230,6 +231,11 @@ public abstract class POIXMLDocument extends POIXMLDocumentPart implements Close
      */
     @SuppressWarnings("resource")
     public final void write(OutputStream stream) throws IOException {
+        OPCPackage p = getPackage();
+        if(p == null) {
+            throw new IOException("Cannot write data, document seems to have been closed already");
+        }
+        
         //force all children to commit their changes into the underlying OOXML Package
         // TODO Shouldn't they be committing to the new one instead?
         Set<PackagePart> context = new HashSet<PackagePart>();
@@ -239,10 +245,6 @@ public abstract class POIXMLDocument extends POIXMLDocumentPart implements Close
         //save extended and custom properties
         getProperties().commit();
 
-        OPCPackage p = getPackage();
-        if(p == null) {
-            throw new IOException("Cannot write data, document seems to have been closed already");
-        }
         p.save(stream);
     }
 }
diff --git a/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java b/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java
index 9d25d8e054..16b28a4507 100644
--- a/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java
+++ b/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java
@@ -22,6 +22,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.fail;
 
 import java.io.File;
 import java.io.FileOutputStream;
@@ -33,9 +34,11 @@ import java.util.HashSet;
 import java.util.List;
 
 import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
+import org.apache.poi.openxml4j.exceptions.OpenXML4JRuntimeException;
 import org.apache.poi.openxml4j.opc.OPCPackage;
 import org.apache.poi.openxml4j.opc.PackagePart;
 import org.apache.poi.openxml4j.opc.PackageRelationshipTypes;
+import org.apache.poi.util.NullOutputStream;
 import org.apache.poi.util.PackageHelper;
 import org.apache.poi.util.TempFile;
 import org.junit.Test;
@@ -120,12 +123,43 @@ public final class TestPOIXMLDocument {
         FileOutputStream out = new FileOutputStream(tmp);
         doc.write(out);
         out.close();
+        
+        // Should not be able to write to an output stream that has been closed
+        try {
+            doc.write(out);
+            fail("Should not be able to write to an output stream that has been closed.");
+        } catch (final OpenXML4JRuntimeException e) {
+            // FIXME: A better exception class (IOException?) and message should be raised
+            // indicating that the document could not be written because the output stream is closed.
+            // see {@link org.apache.poi.openxml4j.opc.ZipPackage#saveImpl(java.io.OutputStream)}
+            if (e.getMessage().matches("Fail to save: an error occurs while saving the package : The part .+ fail to be saved in the stream with marshaller .+")) {
+                // expected
+            } else {
+                throw e;
+            }
+        }
+
+        // Should not be able to write a document that has been closed
         doc.close();
+        try {
+            doc.write(new NullOutputStream());
+            fail("Should not be able to write a document that has been closed.");
+        } catch (final IOException e) {
+            if (e.getMessage().equals("Cannot write data, document seems to have been closed already")) {
+                // expected
+            } else {
+                throw e;
+            }
+        }
+        
+        // Should be able to close a document multiple times, though subsequent closes will have no effect.
+        doc.close();
+
 
         @SuppressWarnings("resource")
         OPCPackage pkg2 = OPCPackage.open(tmp.getAbsolutePath());
+        doc = new OPCParser(pkg1);
         try {
-            doc = new OPCParser(pkg1);
             doc.parse(new TestFactory());
             context = new HashMap<String,POIXMLDocumentPart>();
             traverse(doc, context);
@@ -150,6 +184,7 @@ public final class TestPOIXMLDocument {
             }
         } finally {
             doc.close();
+            pkg1.close();
             pkg2.close();
         }
     }
-- 
2.39.5