DocumentBuilderFactory is safe to be a constant if we don't change it.

Signed-off-by: David Matějček <david.matejcek@omnifish.ee>
jakartaee · Sep 26, 2022 · 2ccb17a · 2ccb17a
1 parent 8cf656a
commit 2ccb17a
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 52 deletions.
diff --git a/impl/src/main/java/org/apache/taglibs/standard/tag/common/xml/DocumentBuilderProvider.java b/impl/src/main/java/org/apache/taglibs/standard/tag/common/xml/DocumentBuilderProvider.java
@@ -16,15 +16,35 @@
 
 package org.apache.taglibs.standard.tag.common.xml;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
 
 /**
+ * Provides preconfigured {@link DocumentBuilder} instances.
+ *
  * @author David Matejcek
  */
 public class DocumentBuilderProvider {
 
+    private static final DocumentBuilderFactory DBF;
+    private static final DocumentBuilderFactory DBF_SECURE;
+    static {
+        DBF = DocumentBuilderFactory.newInstance();
+        DBF.setNamespaceAware(true);
+        DBF.setValidating(false);
+
+        DBF_SECURE = DocumentBuilderFactory.newInstance();
+        DBF_SECURE.setNamespaceAware(true);
+        DBF_SECURE.setValidating(false);
+        try {
+            DBF_SECURE.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        } catch (ParserConfigurationException e) {
+            throw new Error("Parser does not support secure processing");
+        }
+    }
+
     /**
      * Creates a namespace-aware {@link DocumentBuilder} with disabled validation.
      * <p>
@@ -33,15 +53,29 @@ public class DocumentBuilderProvider {
      *
      * @return new {@link DocumentBuilder} instance.
      */
-    static DocumentBuilder createDocumentBuilder() {
+    public static DocumentBuilder createDocumentBuilder() {
+        try {
+            return DBF.newDocumentBuilder();
+        } catch (ParserConfigurationException e) {
+            throw new Error("Could not initialize the DocumentBuilder!", e);
+        }
+    }
+
+
+    /**
+     * Creates a namespace-aware {@link DocumentBuilder} with disabled validation and enabled
+     * {@link XMLConstants#FEATURE_SECURE_PROCESSING}.
+     * <p>
+     * Note that {@link DocumentBuilder} instances are not thread-safe and their implementation can
+     * be chosen as described in {@link DocumentBuilderFactory} documentation.
+     *
+     * @return new {@link DocumentBuilder} instance.
+     */
+    public static DocumentBuilder createSecureDocumentBuilder() {
         try {
-            DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
-            dbf.setNamespaceAware(true);
-            dbf.setValidating(false);
-            return dbf.newDocumentBuilder();
+            return DBF_SECURE.newDocumentBuilder();
         } catch (ParserConfigurationException e) {
-            // this should never happen with a well-behaving JAXP implementation.
-            throw new Error(e);
+            throw new Error("Could not initialize the DocumentBuilder!", e);
         }
     }
 }
diff --git a/impl/src/main/java/org/apache/taglibs/standard/tag/common/xml/ParseSupport.java b/impl/src/main/java/org/apache/taglibs/standard/tag/common/xml/ParseSupport.java
@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2022 Contributors to the Eclipse Foundation
  * Copyright (c) 1997-2020 Oracle and/or its affiliates. All rights reserved.
  * Copyright 2004 The Apache Software Foundation
  *
@@ -17,21 +18,20 @@
 
 package org.apache.taglibs.standard.tag.common.xml;
 
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.jsp.JspException;
+import jakarta.servlet.jsp.JspTagException;
+import jakarta.servlet.jsp.PageContext;
+import jakarta.servlet.jsp.tagext.BodyTagSupport;
+
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Reader;
 import java.io.StringReader;
 
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.jsp.JspException;
-import jakarta.servlet.jsp.JspTagException;
-import jakarta.servlet.jsp.PageContext;
-import jakarta.servlet.jsp.tagext.BodyTagSupport;
 import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
-import javax.xml.parsers.DocumentBuilderFactory;
-import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.transform.TransformerConfigurationException;
 import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.dom.DOMResult;
@@ -72,7 +72,6 @@ public abstract class ParseSupport extends BodyTagSupport {
     private int scopeDom;			   // processed 'scopeDom' attr
 
     // state in support of XML parsing...
-    private DocumentBuilderFactory dbf;
     private DocumentBuilder db;
     private TransformerFactory tf;
     private TransformerHandler th;
@@ -91,7 +90,6 @@ private void init() {
 	xml = null;
 	systemId = null;
 	filter = null;
-	dbf = null;
 	db = null;
 	tf = null;
 	th = null;
@@ -107,18 +105,7 @@ private void init() {
     public int doEndTag() throws JspException {
       try {
 
-	// set up our DocumentBuilder
-        if (dbf == null) {
-            dbf = DocumentBuilderFactory.newInstance();
-            dbf.setNamespaceAware(true);
-            dbf.setValidating(false);
-            try {
-                dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
-            } catch (ParserConfigurationException e) {
-                throw new AssertionError("Parser does not support secure processing");
-            }
-        }
-        db = dbf.newDocumentBuilder();
+    db = DocumentBuilderProvider.createSecureDocumentBuilder();
 
 	// if we've gotten a filter, set up a transformer to support it
 	if (filter != null) {
@@ -167,8 +154,6 @@ else if (xmlText instanceof Reader)
 	throw new JspException(ex);
       } catch (IOException ex) {
 	throw new JspException(ex);
-      } catch (ParserConfigurationException ex) {
-	throw new JspException(ex);
       } catch (TransformerConfigurationException ex) {
 	throw new JspException(ex);
       }

diff --git a/impl/src/main/java/org/apache/taglibs/standard/tag/common/xml/TransformSupport.java b/impl/src/main/java/org/apache/taglibs/standard/tag/common/xml/TransformSupport.java
@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2022 Contributors to the Eclipse Foundation
  * Copyright (c) 1997-2020 Oracle and/or its affiliates. All rights reserved.
  * Copyright 2004 The Apache Software Foundation
  *
@@ -17,21 +18,21 @@
 
 package org.apache.taglibs.standard.tag.common.xml;
 
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.jsp.JspException;
+import jakarta.servlet.jsp.JspTagException;
+import jakarta.servlet.jsp.PageContext;
+import jakarta.servlet.jsp.tagext.BodyTagSupport;
+
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Reader;
 import java.io.StringReader;
 import java.io.Writer;
 import java.util.List;
 
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.jsp.JspException;
-import jakarta.servlet.jsp.JspTagException;
-import jakarta.servlet.jsp.PageContext;
-import jakarta.servlet.jsp.tagext.BodyTagSupport;
 import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
-import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.transform.Result;
 import javax.xml.transform.Source;
@@ -81,7 +82,6 @@ public abstract class TransformSupport extends BodyTagSupport {
     private Transformer t;			   // actual Transformer
     private TransformerFactory tf;		   // reusable factory
     private DocumentBuilder db;			   // reusable factory
-    private DocumentBuilderFactory dbf;		   // reusable factory
 
 
     //*********************************************************************
@@ -115,21 +115,9 @@ public int doStartTag() throws JspException {
 
 	//************************************
 	// Initialize
-
-	// set up our DocumentBuilderFactory if necessary
-	if (dbf == null) {
-	    dbf = DocumentBuilderFactory.newInstance();
-            dbf.setNamespaceAware(true);
-            dbf.setValidating(false);
-            try {
-                dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
-            } catch (ParserConfigurationException e) {
-                throw new AssertionError("Parser does not support secure processing");
-            }
-	}
-        if (db == null)
-	    db = dbf.newDocumentBuilder();
-
+        if (db == null) {
+            db = DocumentBuilderProvider.createSecureDocumentBuilder();
+        }
 	// set up the TransformerFactory if necessary
         if (tf == null) {
             tf = TransformerFactory.newInstance();