Issue #6474: disable external dtd load by default

config/pmd.xml

@@ -96,13 +96,13 @@
   </rule>
   <rule ref="category/java/codestyle.xml/ClassNamingConventions">
     <properties>
-     <!-- Definitions and XmlLoader.FeaturesForVerySecureJavaInstallations aren't utility classes.
+     <!-- Definitions and XmlLoader.LoadExternalDtdFeatureProvider aren't utility classes.
           JavadocTokenTypes and TokenTypes aren't utility classes.
             They are token definition classes. Also, they are part of the API. -->
      <property name="violationSuppressXPath"
                value="//ClassOrInterfaceDeclaration[@Image='Definitions'
                    or @Image='JavadocTokenTypes' or @Image='TokenTypes'
-                   or @Image='FeaturesForVerySecureJavaInstallations']"/>
+                   or @Image='LoadExternalDtdFeatureProvider']"/>
     </properties>
   </rule>
   <rule ref="category/java/codestyle.xml/ConfusingTernary">

pom.xml

@@ -2678,12 +2678,12 @@
               </targetTests>
               <excludedMethods>
                 <!--cause of https://github.com/checkstyle/checkstyle/issues/3605-->
-                <param>addFeaturesForVerySecureJavaInstallations</param>
+                <param>setFeaturesBySystemProperty</param>
               </excludedMethods>
               <avoidCallsTo>
                 <!--cause of https://github.com/checkstyle/checkstyle/issues/3605-->
                 <avoidCallsTo>
-                  com.puppycrawl.tools.checkstyle.XmlLoader$FeaturesForVerySecureJavaInstallations
+                  com.puppycrawl.tools.checkstyle.XmlLoader$LoadExternalDtdFeatureProvider
                 </avoidCallsTo>
               </avoidCallsTo>
               <coverageThreshold>100</coverageThreshold>

src/main/java/com/puppycrawl/tools/checkstyle/XmlLoader.java

@@ -65,7 +65,7 @@ protected XmlLoader(Map<String, String> publicIdToResourceNameMap)
             throws SAXException, ParserConfigurationException {
         this.publicIdToResourceNameMap = new HashMap<>(publicIdToResourceNameMap);
         final SAXParserFactory factory = SAXParserFactory.newInstance();
-        FeaturesForVerySecureJavaInstallations.addFeaturesForVerySecureJavaInstallations(factory);
+        LoadExternalDtdFeatureProvider.setFeaturesBySystemProperty(factory);
         factory.setValidating(true);
         parser = factory.newSAXParser().getXMLReader();
         parser.setContentHandler(this);
@@ -113,7 +113,10 @@ public void error(SAXParseException exception) throws SAXException {
      * Used for setting specific for secure java installations features to SAXParserFactory.
      * Pulled out as a separate class in order to suppress Pitest mutations.
      */
-    public static final class FeaturesForVerySecureJavaInstallations {
+    public static final class LoadExternalDtdFeatureProvider {
+
+        /** System property name to enable external DTD load. */
+        public static final String ENABLE_EXTERNAL_DTD_LOAD = "checkstyle.enableExternalDtdLoad";
 
         /** Feature that enables loading external DTD when loading XML files. */
         private static final String LOAD_EXTERNAL_DTD =
@@ -123,20 +126,26 @@ public static final class FeaturesForVerySecureJavaInstallations {
                 "http://xml.org/sax/features/external-general-entities";
 
         /** Stop instances being created. **/
-        private FeaturesForVerySecureJavaInstallations() {
+        private LoadExternalDtdFeatureProvider() {
         }
 
         /**
          * Configures SAXParserFactory with features required
-         * for execution on very secured environments.
+         * to use external DTD file loading, this is not activated by default to no allow
+         * usage of schema files that checkstyle do not know
+         * it is even security problem to allow files from outside.
          * @param factory factory to be configured with special features
          * @throws SAXException if an error occurs
          * @throws ParserConfigurationException if an error occurs
          */
-        public static void addFeaturesForVerySecureJavaInstallations(SAXParserFactory factory)
+        public static void setFeaturesBySystemProperty(SAXParserFactory factory)
                 throws SAXException, ParserConfigurationException {
-            factory.setFeature(LOAD_EXTERNAL_DTD, true);
-            factory.setFeature(EXTERNAL_GENERAL_ENTITIES, true);
+
+            final boolean enableExternalDtdLoad = Boolean.valueOf(
+                System.getProperty(ENABLE_EXTERNAL_DTD_LOAD, "false"));
+
+            factory.setFeature(LOAD_EXTERNAL_DTD, enableExternalDtdLoad);
+            factory.setFeature(EXTERNAL_GENERAL_ENTITIES, enableExternalDtdLoad);
         }
 
     }

src/test/java/com/puppycrawl/tools/checkstyle/ConfigurationLoaderTest.java

@@ -406,6 +406,9 @@ public void testExternalEntity() throws Exception {
         final Properties props = new Properties();
         props.setProperty("checkstyle.basedir", "basedir");
 
+        System.setProperty(
+                XmlLoader.LoadExternalDtdFeatureProvider.ENABLE_EXTERNAL_DTD_LOAD, "true");
+
         final DefaultConfiguration config =
             (DefaultConfiguration) loadConfiguration(
                 "InputConfigurationLoaderExternalEntity.xml", props);
@@ -421,6 +424,9 @@ public void testExternalEntitySubdirectory() throws Exception {
         final Properties props = new Properties();
         props.setProperty("checkstyle.basedir", "basedir");
 
+        System.setProperty(
+                XmlLoader.LoadExternalDtdFeatureProvider.ENABLE_EXTERNAL_DTD_LOAD, "true");
+
         final DefaultConfiguration config =
             (DefaultConfiguration) loadConfiguration(
                 "subdir/InputConfigurationLoaderExternalEntitySubDir.xml", props);
@@ -436,6 +442,9 @@ public void testExternalEntityFromUri() throws Exception {
         final Properties props = new Properties();
         props.setProperty("checkstyle.basedir", "basedir");
 
+        System.setProperty(
+                XmlLoader.LoadExternalDtdFeatureProvider.ENABLE_EXTERNAL_DTD_LOAD, "true");
+
         final File file = new File(
                 getPath("subdir/InputConfigurationLoaderExternalEntitySubDir.xml"));
         final DefaultConfiguration config =

src/test/java/com/puppycrawl/tools/checkstyle/XmlLoaderTest.java

@@ -47,7 +47,7 @@ public void testParserConfiguredSuccessfully() throws Exception {
     @Test
     public void testIsProperUtilsClass() throws ReflectiveOperationException {
         assertTrue("Constructor is not private", isUtilsClassHasPrivateConstructor(
-                XmlLoader.FeaturesForVerySecureJavaInstallations.class, true));
+                XmlLoader.LoadExternalDtdFeatureProvider.class, true));
     }
 
     @Test

src/xdocs/config_reporting.xml

@@ -69,5 +69,16 @@
         to an empty string.
       </p>
     </section>
+
+    <section name="Enable External DTD load">
+      <p>
+        The property <code>checkstyle.enableExternalDtdLoad</code>
+        defines ability use custom DTD files inconfig and load them from some location.
+        The property type
+        is <a href="property_types.html#boolean">boolean</a> and defaults
+        to <code>false</code>.
+      </p>
+    </section>
+
   </body>
 </document>