From 86d91cceef3c183d5f7858e2884059f79b2e07a9 Mon Sep 17 00:00:00 2001 From: Andreas Beeker Date: Tue, 5 Sep 2017 21:30:29 +0000 Subject: [PATCH] #61478 - POI OOXML-Schema lookup uses wrong classloader git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1807418 13f79535-47bb-0310-9956-ffa450edef68 --- .../java/org/apache/poi/POIXMLTypeLoader.java | 35 ++++---- .../org/apache/poi/TestPOIXMLDocument.java | 79 ++++++++++++------- 2 files changed, 73 insertions(+), 41 deletions(-) diff --git a/src/ooxml/java/org/apache/poi/POIXMLTypeLoader.java b/src/ooxml/java/org/apache/poi/POIXMLTypeLoader.java index 911eef63d8..7452da8f71 100644 --- a/src/ooxml/java/org/apache/poi/POIXMLTypeLoader.java +++ b/src/ooxml/java/org/apache/poi/POIXMLTypeLoader.java @@ -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 = new ThreadLocal(); + private static ThreadLocal typeLoader = new ThreadLocal(); // 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)); } } diff --git a/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java b/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java index 432e9f0936..a636e20fb8 100644 --- a/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java +++ b/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java @@ -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.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.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