Add javadoc and useful messages from checkstyle

checkstyle.xml

@@ -28,15 +28,32 @@
         <property name="id" value="transformerFactoryNewInstance"/>
         <property name="matchAcrossLines" value="true"/>
         <property name="format" value="TransformerFactory\.newInstance\("/>
+        <property name="message"
+                  value="Usage of TransformerFactory.newInstance() is only allowed in XMLUtil.newXXEProtectedTransformerFactory(). If the location of this call in XMLUtil has changed, please modify the expected line number in org.hl7.fhir.utilities/checkstyle_suppressions.xml"
+        />
     </module>
     <module name="RegexpMultiline">
         <property name="id" value="documentBuilderFactoryNewInstance"/>
         <property name="matchAcrossLines" value="true"/>
         <property name="format" value="DocumentBuilderFactory\.newInstance\("/>
+        <property name="message"
+                  value="Usage of DocumentBuilderFactory.newInstance() is only allowed in XMLUtil.newXXEProtectedDocumentBuilderFactory(). If the location of this call in XMLUtil has changed, please modify the expected line number in org.hl7.fhir.utilities/checkstyle_suppressions.xml"
+        />
     </module>
     <module name="RegexpMultiline">
         <property name="id" value="saxParserFactoryNewInstance"/>
         <property name="matchAcrossLines" value="true"/>
         <property name="format" value="SAXParserFactory\.newInstance\("/>
+        <property name="message"
+                  value="Usage of SAXParserFactory.newInstance() is only allowed in XMLUtil.newXXEProtectedSaxParserFactory(). If the location of this call in XMLUtil has changed, please modify the expected line number in org.hl7.fhir.utilities/checkstyle_suppressions.xml"
+        />
+    </module>
+    <module name="RegexpMultiline">
+        <property name="id" value="getXMLReader"/>
+        <property name="matchAcrossLines" value="true"/>
+        <property name="format" value="\.getXMLReader\("/>
+        <property name="message"
+                  value="Usage of SAXParserFactory.getXMLReader() is only allowed in XMLUtil.getXXEProtectedXMLReader(...). If the location of this call in XMLUtil has changed, please modify the expected line number in org.hl7.fhir.utilities/checkstyle_suppressions.xml"
+        />
     </module>
 </module>

org.hl7.fhir.utilities/checkstyle_suppressions.xml

@@ -4,7 +4,8 @@
         "https://checkstyle.org/dtds/suppressions_1_2.dtd">
 
 <suppressions>
-    <suppress id="transformerFactoryNewInstance" files="/src/main/java/org/hl7/fhir/utilities/xml/XMLUtil.java" lines="508"/>
-    <suppress id="documentBuilderFactoryNewInstance" files="/src/main/java/org/hl7/fhir/utilities/xml/XMLUtil.java" lines="515"/>
-    <suppress id="saxParserFactoryNewInstance" files="/src/main/java/org/hl7/fhir/utilities/xml/XMLUtil.java" lines="522"/>
+    <suppress id="transformerFactoryNewInstance" files="/src/main/java/org/hl7/fhir/utilities/xml/XMLUtil.java" lines="516"/>
+    <suppress id="documentBuilderFactoryNewInstance" files="/src/main/java/org/hl7/fhir/utilities/xml/XMLUtil.java" lines="532"/>
+    <suppress id="saxParserFactoryNewInstance" files="/src/main/java/org/hl7/fhir/utilities/xml/XMLUtil.java" lines="551"/>
+    <suppress id="getXMLReader" files="/src/main/java/org/hl7/fhir/utilities/xml/XMLUtil.java" lines="569"/>
 </suppressions>

org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/xml/XMLUtil.java

@@ -504,6 +504,14 @@ public class XMLUtil {
     return e == null ? null : e.getAttribute(aname);
   }
 
+  /**
+   * This method is used to create a new TransformerFactory instance with external processing features configured
+   * securely.
+   * <p/>
+   * <b>IMPORTANT</b> This method should be the only place where TransformerFactory is instantiated in this project.
+   *
+   * @return A TransformerFactory instance external processing features configured securely.
+   */
   public static TransformerFactory newXXEProtectedTransformerFactory() {
     final TransformerFactory transformerFactory = TransformerFactory.newInstance();
     transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
@@ -511,6 +519,15 @@ public class XMLUtil {
     return transformerFactory;
   }
 
+  /**
+   * This method is used to create a new DocumentBuilderFactory instance with external processing features configured
+   * securely.
+   * <p/>
+   * <b>IMPORTANT</b> This method should be the only place where DocumentBuilderFactory is instantiated in this project.
+   *
+   * @return A DocumentBuilderFactory instance external processing features configured securely.
+   * @throws ParserConfigurationException If a DocumentBuilder cannot be configured with the requested features.
+   */
   public static DocumentBuilderFactory newXXEProtectedDocumentBuilderFactory() throws ParserConfigurationException {
     final DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
     documentBuilderFactory.setFeature(APACHE_XML_FEATURES_DISALLOW_DOCTYPE_DECL, true);
@@ -518,6 +535,18 @@ public class XMLUtil {
     return documentBuilderFactory;
   }
 
+  /**
+   * This method is used to create a new SAXParserFactory instance with external processing features configured
+   * securely.
+   * <p/>
+   * <b>IMPORTANT</b> This method should be the only place where SAXParserFactory is instantiated in this project.
+   *
+   * @return A SAXParserFactory instance external processing features configured securely.
+   * @throws SAXNotSupportedException When the underlying XMLReader recognizes the property name but doesn't support
+   * the property.
+   * @throws SAXNotRecognizedException When the underlying XMLReader does not recognize the property name.
+   * @throws ParserConfigurationException If a SAXParser cannot be configured with the requested features.
+   */
   public static SAXParserFactory newXXEProtectedSaxParserFactory() throws SAXNotSupportedException, SAXNotRecognizedException, ParserConfigurationException {
     final SAXParserFactory spf = SAXParserFactory.newInstance();
     spf.setFeature(SAX_FEATURES_EXTERNAL_GENERAL_ENTITIES, false);
@@ -526,6 +555,15 @@ public class XMLUtil {
     return spf;
   }
 
+  /**
+   * This method is used to create a new XMLReader instance from a passed SAXParserFactory with external processing
+   * features configured securely.
+   *
+   * @param spf The SAXParserFactory to create the XMLReader from.
+   * @return A XMLReader instance external processing features configured securely.
+   * @throws ParserConfigurationException If a SAXParser cannot be configured with the requested features.
+   * @throws SAXException If any SAX exceptions occur during the creation of the XMLReader.
+   */
   public static XMLReader getXXEProtectedXMLReader(SAXParserFactory spf) throws ParserConfigurationException, SAXException {
     final SAXParser saxParser = spf.newSAXParser();
     final XMLReader xmlReader = saxParser.getXMLReader();

org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/xml/XMLUtilTests.java

@@ -76,4 +76,12 @@ public class XMLUtilTests {
     SAXParseException e = assertThrows(SAXParseException.class, () -> xmlReader.parse(new StreamSource(templateFile).getSystemId()));
     assertThat(e.getMessage()).contains("DOCTYPE is disallowed");
   }
+
+  @Test
+  public void testXMLReaderThrowsExceptionForExternalEntity() throws ParserConfigurationException, IOException, SAXException, TransformerException {
+    SAXParserFactory spf = SAXParserFactory.newInstance();
+
+    IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> XMLUtil.getXXEProtectedXMLReader(spf));
+    assertThat(e.getMessage()).contains("SAXParserFactory has insecure feature setting");
+  }
 }