Improved parsing of OOXML documents, see Bugzilla 47668

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

6 files changed, 80 insertions, 39 deletions

diff --git a/src/ooxml/java/org/apache/poi/POIXMLDocument.java b/src/ooxml/java/org/apache/poi/POIXMLDocument.java
index 1f0d907080..a4da9c77b2 100644
--- a/src/ooxml/java/org/apache/poi/POIXMLDocument.java
+++ b/src/ooxml/java/org/apache/poi/POIXMLDocument.java
@@ -17,7 +17,7 @@
 package org.apache.poi;
 
 import java.io.*;
-import java.util.List;
+import java.util.*;
 
 import org.apache.poi.poifs.common.POIFSConstants;
 import org.apache.poi.util.IOUtils;
@@ -191,21 +191,33 @@ public abstract class POIXMLDocument extends POIXMLDocumentPart{
         return pkg;
     }
 
+    protected final void load(POIXMLFactory factory) throws IOException {
+    	Map<PackageRelationship, POIXMLDocumentPart> context = new HashMap<PackageRelationship, POIXMLDocumentPart>();
+        try {
+            read(factory, context);
+        } catch (OpenXML4JException e){
+            throw new POIXMLException(e);
+        }
+    	onDocumentRead();
+    	context.clear();
+    }
+
     /**
      * Write out this document to an Outputstream.
      *
-     * @param stream - the java OutputStream you wish to write the XLS to
+     * @param stream - the java OutputStream you wish to write the file to
      *
      * @exception IOException if anything can't be written.
      */
     public final void write(OutputStream stream) throws IOException {
         //force all children to commit their changes into the underlying OOXML Package
-        onSave();
+        Set<PackageRelationship> context = new HashSet<PackageRelationship>();
+        onSave(context);
+        context.clear();
 
         //save extended and custom properties
         getProperties().commit();
 
         getPackage().save(stream);
     }
-
 }
diff --git a/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java b/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java
index 76671d1e10..487bf533b8 100755
--- a/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java
+++ b/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java
@@ -17,8 +17,7 @@
 package org.apache.poi;
 
 import java.io.IOException;
-import java.util.LinkedList;
-import java.util.List;
+import java.util.*;
 import java.net.URI;
 
 import org.apache.xmlbeans.XmlOptions;
@@ -75,11 +74,11 @@ public class POIXMLDocumentPart {
 
     /**
      * Creates an POIXMLDocumentPart representing the given package part and relationship.
-     * Called by {@link #read(POIXMLFactory)} when reading in an exisiting file.
+     * Called by {@link #read(POIXMLFactory, java.util.Map)} when reading in an exisiting file.
      *
      * @param part - The package part that holds xml data represenring this sheet.
      * @param rel - the relationship of the given package part
-     * @see #read(POIXMLFactory)
+     * @see #read(POIXMLFactory, java.util.Map) 
      */
     public POIXMLDocumentPart(PackagePart part, PackageRelationship rel){
         this.relations = new LinkedList<POIXMLDocumentPart>();
@@ -172,11 +171,14 @@ public class POIXMLDocumentPart {
      * Save changes in the underlying OOXML package.
      * Recursively fires {@link #commit()} for each package part
      */
-    protected final void onSave() throws IOException{
-        commit();
-        for(POIXMLDocumentPart p : relations){
-            p.onSave();
-        }
+    protected final void onSave(Set<PackageRelationship> alreadySaved) throws IOException{
+    	commit();
+    	alreadySaved.add(this.getPackageRelationship());
+    	for(POIXMLDocumentPart p : relations){
+    		if (!alreadySaved.contains(p.getPackageRelationship())) {
+    			p.onSave(alreadySaved);
+    		}
+    	}
     }
 
     /**
@@ -228,10 +230,10 @@ public class POIXMLDocumentPart {
      *
      * @param factory   the factory object that creates POIXMLFactory instances
      */
-    protected final void read(POIXMLFactory factory) throws OpenXML4JException {
-        PackageRelationshipCollection rels = packagePart.getRelationships();
-        for (PackageRelationship rel : rels) {
-            if(rel.getTargetMode() == TargetMode.INTERNAL){
+    protected void read(POIXMLFactory factory, Map<PackageRelationship, POIXMLDocumentPart> context) throws OpenXML4JException {
+    	PackageRelationshipCollection rels = packagePart.getRelationships();
+    	for (PackageRelationship rel : rels) {
+    		if(rel.getTargetMode() == TargetMode.INTERNAL){
                 URI uri = rel.getTargetURI();
 
                 PackagePart p;
@@ -249,16 +251,22 @@ public class POIXMLDocumentPart {
                     }
                 }
 
-                POIXMLDocumentPart childPart = factory.createDocumentPart(rel, p);
-                childPart.parent = this;
-                addRelation(childPart);
-
-                if(p != null && p.hasRelationships()) childPart.read(factory);
-            }
-        }
+                if (!context.containsKey(rel)) {
+    				POIXMLDocumentPart childPart = factory.createDocumentPart(rel, p);
+    				childPart.parent = this;
+    				addRelation(childPart);
+                    if(p != null){
+                        context.put(rel, childPart);
+                        if(p.hasRelationships()) childPart.read(factory, context);
+                    }
+    			}
+    			else {
+    				addRelation(context.get(rel));
+    			}
+    		}
+    	}
     }
 
-
     /**
      * Fired when a new package part is created
      */
diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackageRelationship.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackageRelationship.java
index ab15fdf687..03bd296294 100755
--- a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackageRelationship.java
+++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackageRelationship.java
@@ -129,8 +129,10 @@ public final class PackageRelationship {
 
 	@Override
 	public int hashCode() {
-		return this.id.hashCode() + this.relationshipType.hashCode()
-				+ this.source.hashCode() + this.targetMode.hashCode()
+		return this.id.hashCode()
+                + this.relationshipType.hashCode()
+				+ (this.source == null ? 0 : this.source.hashCode())
+                + this.targetMode.hashCode()
 				+ this.targetUri.hashCode();
 	}
 
diff --git a/src/ooxml/java/org/apache/poi/util/PackageHelper.java b/src/ooxml/java/org/apache/poi/util/PackageHelper.java
index 186d85acc3..953a4ab866 100755
--- a/src/ooxml/java/org/apache/poi/util/PackageHelper.java
+++ b/src/ooxml/java/org/apache/poi/util/PackageHelper.java
@@ -19,7 +19,9 @@ package org.apache.poi.util;
 import org.apache.poi.openxml4j.opc.*;
 import org.apache.poi.openxml4j.opc.OPCPackage;
 import org.apache.poi.openxml4j.exceptions.OpenXML4JException;
+import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
 import org.apache.poi.util.IOUtils;
+import org.apache.poi.POIXMLException;
 
 import java.io.*;
 import java.net.URI;
@@ -41,6 +43,18 @@ public class PackageHelper {
         return clone(pkg, createTempFile());
     }
 
+    public static OPCPackage open(InputStream is) throws IOException {
+        File file = TempFile.createTempFile("poi-ooxml-", ".tmp");
+        FileOutputStream out = new FileOutputStream(file);
+        IOUtils.copy(is, out);
+        out.close();
+        try {
+            return OPCPackage.open(file.getAbsolutePath());
+        } catch (InvalidFormatException e){
+            throw new POIXMLException(e);
+        }
+    }
+
     /**
      * Clone the specified package.
      *
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java
index 15ddd8f127..1daf33d52a 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java
@@ -149,12 +149,14 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable<X
         super(ensureWriteAccess(pkg));
 
         //build a tree of POIXMLDocumentParts, this workbook being the root
-        try {
-            read(XSSFFactory.getInstance());
-        } catch (OpenXML4JException e){
-            throw new POIXMLException(e);
-        }
-        onDocumentRead();
+        load(XSSFFactory.getInstance());
+    }
+
+    public XSSFWorkbook(InputStream is) throws IOException {
+        super(PackageHelper.open(is));
+
+        //build a tree of POIXMLDocumentParts, this workbook being the root
+        load(XSSFFactory.getInstance());
     }
 
     /**
diff --git a/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java b/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java
index 8f354c3f50..9081909b3f 100644
--- a/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java
+++ b/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java
@@ -18,6 +18,7 @@ package org.apache.poi.xwpf.usermodel;
 
 import java.io.IOException;
 import java.io.OutputStream;
+import java.io.InputStream;
 import java.util.*;
 
 import org.apache.poi.POIXMLDocument;
@@ -63,12 +64,14 @@ public class XWPFDocument extends POIXMLDocument {
         super(ensureWriteAccess(pkg));
 
         //build a tree of POIXMLDocumentParts, this document being the root
-        try {
-            read(XWPFFactory.getInstance());
-        } catch (OpenXML4JException e){
-            throw new POIXMLException(e);
-        }
-        onDocumentRead();
+        load(XWPFFactory.getInstance());
+    }
+    
+    public XWPFDocument(InputStream is) throws IOException {
+        super(PackageHelper.open(is));
+
+        //build a tree of POIXMLDocumentParts, this workbook being the root
+        load(XWPFFactory.getInstance());
     }
 
     public XWPFDocument(){