Sonar Fixes + Refactor scattered XML initializations to XMLHelper

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

2 files changed, 272 insertions, 40 deletions

diff --git a/src/java/org/apache/poi/util/StaxHelper.java b/src/java/org/apache/poi/util/StaxHelper.java
index 1ce05b9eff..e612d92e7f 100644
--- a/src/java/org/apache/poi/util/StaxHelper.java
+++ b/src/java/org/apache/poi/util/StaxHelper.java
@@ -17,8 +17,6 @@
 
 package org.apache.poi.util;
 
-import java.util.function.Consumer;
-
 import javax.xml.stream.XMLEventFactory;
 import javax.xml.stream.XMLInputFactory;
 import javax.xml.stream.XMLOutputFactory;
@@ -26,50 +24,33 @@ import javax.xml.stream.XMLOutputFactory;
 
 /**
  * Provides handy methods for working with StAX parsers and readers
+ *
+ * @deprecated use {@link XMLHelper}
  */
+@Deprecated
+@Removal(version = "5.0.0")
 public final class StaxHelper {
-    private static final POILogger logger = POILogFactory.getLogger(StaxHelper.class);
-
     private StaxHelper() {
     }
 
     /**
      * Creates a new StAX XMLInputFactory, with sensible defaults
      */
-    @SuppressWarnings({"squid:S2755"})
     public static XMLInputFactory newXMLInputFactory() {
-        XMLInputFactory factory = XMLInputFactory.newInstance();
-        trySet(XMLInputFactory.IS_NAMESPACE_AWARE, (n) -> factory.setProperty(n, true));
-        trySet(XMLInputFactory.IS_VALIDATING, (n) -> factory.setProperty(n, false));
-        trySet(XMLInputFactory.SUPPORT_DTD, (n) -> factory.setProperty(n, false));
-        trySet(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, (n) -> factory.setProperty(n, false));
-        return factory;
+        return XMLHelper.newXMLInputFactory();
     }
 
     /**
      * Creates a new StAX XMLOutputFactory, with sensible defaults
      */
     public static XMLOutputFactory newXMLOutputFactory() {
-        XMLOutputFactory factory = XMLOutputFactory.newInstance();
-        trySet(XMLOutputFactory.IS_REPAIRING_NAMESPACES, (n) -> factory.setProperty(n, true));
-        return factory;
+        return XMLHelper.newXMLOutputFactory();
     }
 
     /**
      * Creates a new StAX XMLEventFactory, with sensible defaults
      */
     public static XMLEventFactory newXMLEventFactory() {
-        // this method seems safer on Android than getFactory()
-        return XMLEventFactory.newInstance();
-    }
-
-    private static void trySet(String name, Consumer<String> securityFeature) {
-        try {
-            securityFeature.accept(name);
-        } catch (Exception e) {
-            logger.log(POILogger.WARN, "StAX Property unsupported", name, e);
-        } catch (AbstractMethodError ame) {
-            logger.log(POILogger.WARN, "Cannot set StAX property because outdated StAX parser in classpath", name, ame);
-        }
+        return XMLHelper.newXMLEventFactory();
     }
 }
diff --git a/src/java/org/apache/poi/util/XMLHelper.java b/src/java/org/apache/poi/util/XMLHelper.java
index 9c29546eba..fd02caca2f 100644
--- a/src/java/org/apache/poi/util/XMLHelper.java
+++ b/src/java/org/apache/poi/util/XMLHelper.java
@@ -17,19 +17,71 @@
 
 package org.apache.poi.util;
 
+import java.io.StringReader;
+import java.lang.reflect.Method;
+import java.util.concurrent.TimeUnit;
+
 import javax.xml.XMLConstants;
+import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+import javax.xml.stream.XMLEventFactory;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLOutputFactory;
+import javax.xml.transform.OutputKeys;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerConfigurationException;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.validation.SchemaFactory;
+
+import org.xml.sax.ErrorHandler;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.SAXParseException;
+import org.xml.sax.XMLReader;
 
 /**
  * Helper methods for working with javax.xml classes.
  */
+@Internal
 public final class XMLHelper {
+    static final String FEATURE_LOAD_DTD_GRAMMAR = "http://apache.org/xml/features/nonvalidating/load-dtd-grammar";
+    static final String FEATURE_LOAD_EXTERNAL_DTD = "http://apache.org/xml/features/nonvalidating/load-external-dtd";
+    static final String FEATURE_DISALLOW_DOCTYPE_DECL = "http://apache.org/xml/features/disallow-doctype-decl";
+    static final String FEATURE_PARAMETER_ENTITIES = "http://xml.org/sax/features/external-parameter-entities";
+    static final String FEATURE_EXTERNAL_ENTITIES = "http://xml.org/sax/features/external-general-entities";
+    static final String PROPERTY_ENTITY_EXPANSION_LIMIT = "http://www.oracle.com/xml/jaxp/properties/entityExpansionLimit";
+    static final String PROPERTY_SECURITY_MANAGER = "http://apache.org/xml/properties/security-manager";
+    static final String METHOD_ENTITY_EXPANSION_XERCES = "setEntityExpansionLimit";
+
+    static final String[] SECURITY_MANAGERS = {
+            //"com.sun.org.apache.xerces.internal.util.SecurityManager",
+            "org.apache.xerces.util.SecurityManager"
+    };
+
+
     private static POILogger logger = POILogFactory.getLogger(XMLHelper.class);
+    private static long lastLog;
+
+    // DocumentBuilderFactory.newDocumentBuilder is thread-safe
+    // see https://stackoverflow.com/questions/12455602/is-documentbuilder-thread-safe
+    private static final DocumentBuilderFactory documentBuilderFactory = getDocumentBuilderFactory();
+
+    private static final SAXParserFactory saxFactory = getSaxParserFactory();
 
     @FunctionalInterface
     private interface SecurityFeature {
-        void accept(String name) throws ParserConfigurationException;
+        void accept(String name, boolean value) throws ParserConfigurationException, SAXException, TransformerException;
+    }
+
+    @FunctionalInterface
+    private interface SecurityProperty {
+        void accept(String name, Object value) throws SAXException;
+    }
+
+    private XMLHelper() {
     }
 
     /**
@@ -40,26 +92,225 @@ public final class XMLHelper {
     @SuppressWarnings({"squid:S2755"})
     public static DocumentBuilderFactory getDocumentBuilderFactory() {
         DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+        factory.setNamespaceAware(true);
+        // this doesn't appear to work, and we still need to limit
+        // entity expansions to 1 in trySetXercesSecurityManager
         factory.setExpandEntityReferences(false);
-        trySet(XMLConstants.FEATURE_SECURE_PROCESSING, (n) -> factory.setFeature(n, true));
-        trySet(XMLConstants.ACCESS_EXTERNAL_SCHEMA, (n) -> factory.setAttribute(n, ""));
-        trySet(XMLConstants.ACCESS_EXTERNAL_DTD, (n) -> factory.setAttribute(n, ""));
-        trySet("http://xml.org/sax/features/external-general-entities", (n) -> factory.setFeature(n, false));
-        trySet("http://xml.org/sax/features/external-parameter-entities", (n) -> factory.setFeature(n, false));
-        trySet("http://apache.org/xml/features/nonvalidating/load-external-dtd", (n) -> factory.setFeature(n, false));
-        trySet("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", (n) -> factory.setFeature(n, false));
-        trySet("http://apache.org/xml/features/disallow-doctype-decl", (n) -> factory.setFeature(n, true));
-        trySet("XIncludeAware", (n) -> factory.setXIncludeAware(false));
+        factory.setValidating(false);
+        trySet(factory::setFeature, XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        trySet(factory::setAttribute, XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
+        trySet(factory::setAttribute, XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        trySet(factory::setFeature, FEATURE_EXTERNAL_ENTITIES, false);
+        trySet(factory::setFeature, FEATURE_PARAMETER_ENTITIES, false);
+        trySet(factory::setFeature, FEATURE_LOAD_EXTERNAL_DTD, false);
+        trySet(factory::setFeature, FEATURE_LOAD_DTD_GRAMMAR, false);
+        trySet(factory::setFeature, FEATURE_DISALLOW_DOCTYPE_DECL, true);
+        trySet((n, b) -> factory.setXIncludeAware(b), "XIncludeAware", false);
+
+        Object manager = getXercesSecurityManager();
+        if (manager == null || !trySet(factory::setAttribute, PROPERTY_SECURITY_MANAGER, manager)) {
+            // separate old version of Xerces not found => use the builtin way of setting the property
+            // Note: when entity_expansion_limit==0, there is no limit!
+            trySet(factory::setAttribute, PROPERTY_ENTITY_EXPANSION_LIMIT, 1);
+        }
+
+        return factory;
+    }
+
+    /**
+     * Creates a new document builder, with sensible defaults
+     *
+     * @throws IllegalStateException If creating the DocumentBuilder fails, e.g.
+     *                               due to {@link ParserConfigurationException}.
+     */
+    public static DocumentBuilder newDocumentBuilder() {
+        try {
+            DocumentBuilder documentBuilder = documentBuilderFactory.newDocumentBuilder();
+            documentBuilder.setEntityResolver(XMLHelper::ignoreEntity);
+            documentBuilder.setErrorHandler(new DocHelperErrorHandler());
+            return documentBuilder;
+        } catch (ParserConfigurationException e) {
+            throw new IllegalStateException("cannot create a DocumentBuilder", e);
+        }
+    }
+
+    public static SAXParserFactory getSaxParserFactory() {
+        try {
+            SAXParserFactory factory = SAXParserFactory.newInstance();
+            factory.setValidating(false);
+            factory.setNamespaceAware(true);
+            trySet(factory::setFeature, XMLConstants.FEATURE_SECURE_PROCESSING, true);
+            trySet(factory::setFeature, FEATURE_LOAD_DTD_GRAMMAR, false);
+            trySet(factory::setFeature, FEATURE_LOAD_EXTERNAL_DTD, false);
+            return factory;
+        } catch (RuntimeException | Error re) { // NOSONAR
+            // this also catches NoClassDefFoundError, which may be due to a local class path issue
+            // This may occur if the code is run inside a web container or a restricted JVM
+            // See bug 61170: https://bz.apache.org/bugzilla/show_bug.cgi?id=61170
+            logThrowable(re, "Failed to create SAXParserFactory", "-");
+            throw re;
+        } catch (Exception e) {
+            logThrowable(e, "Failed to create SAXParserFactory", "-");
+            throw new RuntimeException("Failed to create SAXParserFactory", e);
+        }
+    }
+
+    /**
+     * Creates a new SAX XMLReader, with sensible defaults
+     */
+    public static XMLReader newXMLReader() throws SAXException, ParserConfigurationException {
+        XMLReader xmlReader = saxFactory.newSAXParser().getXMLReader();
+        xmlReader.setEntityResolver(XMLHelper::ignoreEntity);
+        trySet(xmlReader::setFeature, XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        Object manager = getXercesSecurityManager();
+        if (manager == null || !trySet(xmlReader::setProperty, PROPERTY_SECURITY_MANAGER, manager)) {
+            // separate old version of Xerces not found => use the builtin way of setting the property
+            trySet(xmlReader::setProperty, PROPERTY_ENTITY_EXPANSION_LIMIT, 1);
+        }
+        return xmlReader;
+    }
+
+    /**
+     * Creates a new StAX XMLInputFactory, with sensible defaults
+     */
+    @SuppressWarnings({"squid:S2755"})
+    public static XMLInputFactory newXMLInputFactory() {
+        XMLInputFactory factory = XMLInputFactory.newInstance();
+        trySet(factory::setProperty, XMLInputFactory.IS_NAMESPACE_AWARE, true);
+        trySet(factory::setProperty, XMLInputFactory.IS_VALIDATING, false);
+        trySet(factory::setProperty, XMLInputFactory.SUPPORT_DTD, false);
+        trySet(factory::setProperty, XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+        return factory;
+    }
+
+    /**
+     * Creates a new StAX XMLOutputFactory, with sensible defaults
+     */
+    public static XMLOutputFactory newXMLOutputFactory() {
+        XMLOutputFactory factory = XMLOutputFactory.newInstance();
+        trySet(factory::setProperty, XMLOutputFactory.IS_REPAIRING_NAMESPACES, true);
         return factory;
     }
 
-    private static void trySet(String name, SecurityFeature feature) {
+    /**
+     * Creates a new StAX XMLEventFactory, with sensible defaults
+     */
+    public static XMLEventFactory newXMLEventFactory() {
+        // this method seems safer on Android than getFactory()
+        return XMLEventFactory.newInstance();
+    }
+
+    public static TransformerFactory getTransformerFactory() {
+        TransformerFactory factory = TransformerFactory.newInstance();
+        trySet(factory::setFeature, XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        return factory;
+    }
+
+    public static Transformer newTransformer() throws TransformerConfigurationException {
+        Transformer serializer = getTransformerFactory().newTransformer();
+        // TODO set encoding from a command argument
+        serializer.setOutputProperty(OutputKeys.ENCODING, "UTF-8");
+        serializer.setOutputProperty(OutputKeys.INDENT, "no");
+        serializer.setOutputProperty(OutputKeys.METHOD, "xml");
+        return serializer;
+    }
+
+    public static SchemaFactory getSchemaFactory() {
+        SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
+        trySet(factory::setFeature, XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        trySet(factory::setProperty, XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        trySet(factory::setProperty, XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
+        return factory;
+    }
+
+
+    private static Object getXercesSecurityManager() {
+        // Try built-in JVM one first, standalone if not
+        for (String securityManagerClassName : SECURITY_MANAGERS) {
+            try {
+                Object mgr = Class.forName(securityManagerClassName).newInstance();
+                Method setLimit = mgr.getClass().getMethod(METHOD_ENTITY_EXPANSION_XERCES, Integer.TYPE);
+                setLimit.invoke(mgr, 1);
+                // Stop once one can be setup without error
+                return mgr;
+            } catch (ClassNotFoundException ignored) {
+                // continue without log, this is expected in some setups
+            } catch (Throwable e) {     // NOSONAR - also catch things like NoClassDefError here
+                logThrowable(e, "SAX Feature unsupported", securityManagerClassName);
+            }
+        }
+
+        return null;
+    }
+
+    @SuppressWarnings("UnusedReturnValue")
+    private static boolean trySet(SecurityFeature feature, String name, boolean value) {
         try {
-            feature.accept(name);
+            feature.accept(name, value);
+            return true;
         } catch (Exception e) {
-            logger.log(POILogger.WARN, "SAX Feature unsupported", name, e);
+            logThrowable(e, "SAX Feature unsupported", name);
         } catch (AbstractMethodError ame) {
-            logger.log(POILogger.WARN, "Cannot set SAX feature because outdated XML parser in classpath", name, ame);
+            logThrowable(ame, "Cannot set SAX feature because outdated XML parser in classpath", name);
         }
+        return false;
+    }
+
+    private static boolean trySet(SecurityProperty property, String name, Object value) {
+        try {
+            property.accept(name, value);
+            return true;
+        } catch (Exception e) {
+            logThrowable(e, "SAX Feature unsupported", name);
+        } catch (AbstractMethodError ame) {
+            logThrowable(ame, "Cannot set SAX feature because outdated XML parser in classpath", name);
+        }
+        return false;
+    }
+
+    private static void logThrowable(Throwable t, String message, String name) {
+        if (System.currentTimeMillis() > lastLog + TimeUnit.MINUTES.toMillis(5)) {
+            logger.log(POILogger.WARN, message + " [log suppressed for 5 minutes]", name, t);
+            lastLog = System.currentTimeMillis();
+        }
+    }
+
+    private static class DocHelperErrorHandler implements ErrorHandler {
+
+        public void warning(SAXParseException exception) {
+            printError(POILogger.WARN, exception);
+        }
+
+        public void error(SAXParseException exception) {
+            printError(POILogger.ERROR, exception);
+        }
+
+        public void fatalError(SAXParseException exception) throws SAXException {
+            printError(POILogger.FATAL, exception);
+            throw exception;
+        }
+
+        /**
+         * Prints the error message.
+         */
+        private void printError(int type, SAXParseException ex) {
+            String systemId = ex.getSystemId();
+            if (systemId != null) {
+                int index = systemId.lastIndexOf('/');
+                if (index != -1) {
+                    systemId = systemId.substring(index + 1);
+                }
+            }
+            String message = (systemId == null ? "" : systemId) +
+                    ':' + ex.getLineNumber() +
+                    ':' + ex.getColumnNumber() +
+                    ':' + ex.getMessage();
+
+            logger.log(type, message, ex);
+        }
+    }
+
+    private static InputSource ignoreEntity(String publicId, String systemId) {
+        return new InputSource(new StringReader(""));
     }
 }