You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by ki...@apache.org on 2017/09/05 21:30:29 UTC

svn commit: r1807418 - in /poi/trunk/src/ooxml: java/org/apache/poi/POIXMLTypeLoader.java testcases/org/apache/poi/TestPOIXMLDocument.java

Author: kiwiwings
Date: Tue Sep  5 21:30:29 2017
New Revision: 1807418

URL: http://svn.apache.org/viewvc?rev=1807418&view=rev
Log:
#61478 - POI OOXML-Schema lookup uses wrong classloader

Modified:
    poi/trunk/src/ooxml/java/org/apache/poi/POIXMLTypeLoader.java
    poi/trunk/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java

Modified: poi/trunk/src/ooxml/java/org/apache/poi/POIXMLTypeLoader.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/apache/poi/POIXMLTypeLoader.java?rev=1807418&r1=1807417&r2=1807418&view=diff
==============================================================================
--- poi/trunk/src/ooxml/java/org/apache/poi/POIXMLTypeLoader.java (original)
+++ poi/trunk/src/ooxml/java/org/apache/poi/POIXMLTypeLoader.java Tue Sep  5 21:30:29 2017
@@ -33,6 +33,7 @@ import javax.xml.stream.XMLStreamReader;
 
 import org.apache.poi.openxml4j.opc.PackageNamespaces;
 import org.apache.poi.util.DocumentHelper;
+import org.apache.poi.util.Removal;
 import org.apache.xmlbeans.SchemaType;
 import org.apache.xmlbeans.SchemaTypeLoader;
 import org.apache.xmlbeans.XmlBeans;
@@ -49,7 +50,7 @@ import org.xml.sax.SAXException;
 @SuppressWarnings("deprecation")
 public class POIXMLTypeLoader {
 
-    private static ThreadLocal<ClassLoader> classLoader = new ThreadLocal<ClassLoader>();
+    private static ThreadLocal<SchemaTypeLoader> typeLoader = new ThreadLocal<SchemaTypeLoader>();
 
     // TODO: Do these have a good home like o.a.p.openxml4j.opc.PackageNamespaces and PackageRelationshipTypes?
     // These constants should be common to all of POI and easy to use by other applications such as Tika
@@ -109,20 +110,26 @@ public class POIXMLTypeLoader {
      * when the user code is finalized.
      * 
      * @param cl the classloader to be used when XmlBeans classes and definitions are looked up
+     * @deprecated in POI 3.17 - setting a classloader from the outside is now obsolete,
+     *  the classloader of the SchemaType will be used
      */
+    @Deprecated
+    @Removal(version="4.0")
     public static void setClassLoader(ClassLoader cl) {
-        classLoader.set(cl);
     }
     
-    private static SchemaTypeLoader getTypeLoader() {
-        ClassLoader cl = classLoader.get();
-        return (cl == null)
-            ? XmlBeans.getContextTypeLoader()
-            : XmlBeans.typeLoaderForClassLoader(cl);
+    private static SchemaTypeLoader getTypeLoader(SchemaType type) {
+        SchemaTypeLoader tl = typeLoader.get();
+        if (tl == null) {
+            ClassLoader cl = type.getClass().getClassLoader();
+            tl = XmlBeans.typeLoaderForClassLoader(cl);
+            typeLoader.set(tl);
+        }
+        return tl;
     }
     
     public static XmlObject newInstance(SchemaType type, XmlOptions options) {
-        return getTypeLoader().newInstance(type, getXmlOptions(options));
+        return getTypeLoader(type).newInstance(type, getXmlOptions(options));
     }
 
     public static XmlObject parse(String xmlText, SchemaType type, XmlOptions options) throws XmlException {
@@ -154,34 +161,34 @@ public class POIXMLTypeLoader {
     public static XmlObject parse(InputStream jiois, SchemaType type, XmlOptions options) throws XmlException, IOException {
         try {
             Document doc = DocumentHelper.readDocument(jiois);
-            return getTypeLoader().parse(doc.getDocumentElement(), type, getXmlOptions(options));
+            return getTypeLoader(type).parse(doc.getDocumentElement(), type, getXmlOptions(options));
         } catch (SAXException e) {
             throw new IOException("Unable to parse xml bean", e);
         }
     }
 
     public static XmlObject parse(XMLStreamReader xsr, SchemaType type, XmlOptions options) throws XmlException {
-        return getTypeLoader().parse(xsr, type, getXmlOptions(options));
+        return getTypeLoader(type).parse(xsr, type, getXmlOptions(options));
     }
 
     public static XmlObject parse(Reader jior, SchemaType type, XmlOptions options) throws XmlException, IOException {
         try {
             Document doc = DocumentHelper.readDocument(new InputSource(jior));
-            return getTypeLoader().parse(doc.getDocumentElement(), type, getXmlOptions(options));
+            return getTypeLoader(type).parse(doc.getDocumentElement(), type, getXmlOptions(options));
         } catch (SAXException e) {
             throw new XmlException("Unable to parse xml bean", e);
         }
     }
 
     public static XmlObject parse(Node node, SchemaType type, XmlOptions options) throws XmlException {
-        return getTypeLoader().parse(node, type, getXmlOptions(options));
+        return getTypeLoader(type).parse(node, type, getXmlOptions(options));
     }
 
     public static XmlObject parse(XMLInputStream xis, SchemaType type, XmlOptions options) throws XmlException, XMLStreamException {
-        return getTypeLoader().parse(xis, type, getXmlOptions(options));
+        return getTypeLoader(type).parse(xis, type, getXmlOptions(options));
     }
     
     public static XMLInputStream newValidatingXMLInputStream ( XMLInputStream xis, SchemaType type, XmlOptions options ) throws XmlException, XMLStreamException {
-        return getTypeLoader().newValidatingXMLInputStream(xis, type, getXmlOptions(options));
+        return getTypeLoader(type).newValidatingXMLInputStream(xis, type, getXmlOptions(options));
     }
 }

Modified: poi/trunk/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java?rev=1807418&r1=1807417&r2=1807418&view=diff
==============================================================================
--- poi/trunk/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java (original)
+++ poi/trunk/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java Tue Sep  5 21:30:29 2017
@@ -28,6 +28,7 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.Thread.UncaughtExceptionHandler;
 import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -40,6 +41,7 @@ import org.apache.poi.openxml4j.exceptio
 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.IOUtils;
 import org.apache.poi.util.NullOutputStream;
 import org.apache.poi.util.PackageHelper;
 import org.apache.poi.util.TempFile;
@@ -321,38 +323,61 @@ public final class TestPOIXMLDocument {
         }
     }
     
-    @Test(expected=IllegalStateException.class)
-    public void testOSGIClassLoadingAsIs() throws IOException {
+    @Test
+    public void testOSGIClassLoading() {
+        // the schema type loader is cached per thread in POIXMLTypeLoader.
+        // So create a new Thread and change the context class loader (which would normally be used)
+        // to not contain the OOXML classes
+        Runnable run = new Runnable() {
+            public void run() {
+                InputStream is = POIDataSamples.getSlideShowInstance().openResourceAsStream("table_test.pptx");
+                XMLSlideShow ppt = null;
+                try {
+                    ppt = new XMLSlideShow(is);
+                    ppt.getSlides().get(0).getShapes();
+                } catch (IOException e) {
+                    fail("failed to load XMLSlideShow");
+                } finally {
+                    IOUtils.closeQuietly(ppt);
+                    IOUtils.closeQuietly(is);
+                }
+            }
+        };
+
         Thread thread = Thread.currentThread();
         ClassLoader cl = thread.getContextClassLoader();
-        InputStream is = POIDataSamples.getSlideShowInstance().openResourceAsStream("table_test.pptx");
-        try {
-            thread.setContextClassLoader(cl.getParent());
-            XMLSlideShow ppt = new XMLSlideShow(is);
-            ppt.getSlides().get(0).getShapes();
-            ppt.close();
-        } finally {
-            thread.setContextClassLoader(cl);
-            is.close();
+        UncaughtHandler uh = new UncaughtHandler();
+        
+        // check schema type loading and check if we could run in an OOM
+        Thread ta[] = new Thread[30];
+        for (int j=0; j<10; j++) {
+            for (int i=0; i<ta.length; i++) {
+                ta[i] = new Thread(run);
+                ta[i].setContextClassLoader(cl.getParent());
+                ta[i].setUncaughtExceptionHandler(uh);
+                ta[i].start();
+            }
+            for (int i=0; i<ta.length; i++) {
+                try {
+                    ta[i].join();
+                } catch (InterruptedException e) {
+                    fail("failed to join thread");
+                }
+            }
         }
+        assertFalse(uh.hasException());
     }
 
-
-    @Test
-    public void testOSGIClassLoadingFixed() throws IOException {
-        Thread thread = Thread.currentThread();
-        ClassLoader cl = thread.getContextClassLoader();
-        InputStream is = POIDataSamples.getSlideShowInstance().openResourceAsStream("table_test.pptx");
-        try {
-            thread.setContextClassLoader(cl.getParent());
-            POIXMLTypeLoader.setClassLoader(cl);
-            XMLSlideShow ppt = new XMLSlideShow(is);
-            ppt.getSlides().get(0).getShapes();
-            ppt.close();
-        } finally {
-            thread.setContextClassLoader(cl);
-            POIXMLTypeLoader.setClassLoader(null);
-            is.close();
+    private static class UncaughtHandler implements UncaughtExceptionHandler {
+        Throwable e;
+        
+        public synchronized void uncaughtException(Thread t, Throwable e) {
+            this.e = e;
+            
+        }
+        
+        public synchronized boolean hasException() {
+            return e != null;
         }
     }
 



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org