You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@opennlp.apache.org by "rzo1 (via GitHub)" <gi...@apache.org> on 2026/06/03 05:58:13 UTC

Re: [PR] OPENNLP-1835 - Tolerate unsupported XML parser security options (opennlp)

rzo1 commented on code in PR #1066:
URL: https://github.com/apache/opennlp/pull/1066#discussion_r3346245971


##########
opennlp-tools/src/main/java/opennlp/tools/util/XmlUtil.java:
##########
@@ -85,17 +87,74 @@ public static SAXParser createSaxParser() {
       logger.warn("Failed to enable XMLConstants.FEATURE_SECURE_PROCESSING, it's unsupported on" +
           " this platform.", e);
     }
+    setFeatureIfSupported(spf, "http://apache.org/xml/features/disallow-doctype-decl", true);
+    setFeatureIfSupported(spf, "http://xml.org/sax/features/external-general-entities", false);
+    setFeatureIfSupported(spf, "http://xml.org/sax/features/external-parameter-entities", false);
+    setFeatureIfSupported(spf, "http://apache.org/xml/features/nonvalidating/load-external-dtd",
+        false);
     try {
-      spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
-      spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
-      spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
-      spf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
       final SAXParser parser = spf.newSAXParser();
-      parser.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");
-      parser.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
+      setPropertyIfSupported(parser, XMLConstants.ACCESS_EXTERNAL_DTD, "");
+      setPropertyIfSupported(parser, XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
       return parser;
     } catch (ParserConfigurationException | SAXException e) {
       throw new IllegalStateException(e);
     }
   }
+
+  private static void setFeatureIfSupported(DocumentBuilderFactory factory, String name,
+                                            boolean value) {
+    try {
+      factory.setFeature(name, value);
+    } catch (ParserConfigurationException e) {
+      logger.warn("Failed to set XML parser feature " + name + ", it's unsupported on " +

Review Comment:
   Can we use log substitution for the `name` via  `{}` please.



##########
opennlp-tools/src/main/java/opennlp/tools/util/XmlUtil.java:
##########
@@ -85,17 +87,74 @@ public static SAXParser createSaxParser() {
       logger.warn("Failed to enable XMLConstants.FEATURE_SECURE_PROCESSING, it's unsupported on" +
           " this platform.", e);
     }
+    setFeatureIfSupported(spf, "http://apache.org/xml/features/disallow-doctype-decl", true);
+    setFeatureIfSupported(spf, "http://xml.org/sax/features/external-general-entities", false);
+    setFeatureIfSupported(spf, "http://xml.org/sax/features/external-parameter-entities", false);
+    setFeatureIfSupported(spf, "http://apache.org/xml/features/nonvalidating/load-external-dtd",
+        false);
     try {
-      spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
-      spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
-      spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
-      spf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
       final SAXParser parser = spf.newSAXParser();
-      parser.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");
-      parser.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
+      setPropertyIfSupported(parser, XMLConstants.ACCESS_EXTERNAL_DTD, "");
+      setPropertyIfSupported(parser, XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
       return parser;
     } catch (ParserConfigurationException | SAXException e) {
       throw new IllegalStateException(e);
     }
   }
+
+  private static void setFeatureIfSupported(DocumentBuilderFactory factory, String name,
+                                            boolean value) {
+    try {
+      factory.setFeature(name, value);
+    } catch (ParserConfigurationException e) {
+      logger.warn("Failed to set XML parser feature " + name + ", it's unsupported on " +
+          "this platform.", e);
+    }
+  }
+
+  private static void setAttributeIfSupported(DocumentBuilderFactory factory, String name,
+                                             Object value) {
+    try {
+      factory.setAttribute(name, value);
+    } catch (IllegalArgumentException e) {
+      logger.warn("Failed to set XML parser attribute " + name + ", it's unsupported on " +

Review Comment:
   Can we use log substitution for the `name` via  `{}` please.



##########
opennlp-tools/src/main/java/opennlp/tools/util/XmlUtil.java:
##########
@@ -85,17 +87,74 @@ public static SAXParser createSaxParser() {
       logger.warn("Failed to enable XMLConstants.FEATURE_SECURE_PROCESSING, it's unsupported on" +
           " this platform.", e);
     }
+    setFeatureIfSupported(spf, "http://apache.org/xml/features/disallow-doctype-decl", true);
+    setFeatureIfSupported(spf, "http://xml.org/sax/features/external-general-entities", false);
+    setFeatureIfSupported(spf, "http://xml.org/sax/features/external-parameter-entities", false);
+    setFeatureIfSupported(spf, "http://apache.org/xml/features/nonvalidating/load-external-dtd",
+        false);
     try {
-      spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
-      spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
-      spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
-      spf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
       final SAXParser parser = spf.newSAXParser();
-      parser.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");
-      parser.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
+      setPropertyIfSupported(parser, XMLConstants.ACCESS_EXTERNAL_DTD, "");
+      setPropertyIfSupported(parser, XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
       return parser;
     } catch (ParserConfigurationException | SAXException e) {
       throw new IllegalStateException(e);
     }
   }
+
+  private static void setFeatureIfSupported(DocumentBuilderFactory factory, String name,
+                                            boolean value) {
+    try {
+      factory.setFeature(name, value);
+    } catch (ParserConfigurationException e) {
+      logger.warn("Failed to set XML parser feature " + name + ", it's unsupported on " +
+          "this platform.", e);
+    }
+  }
+
+  private static void setAttributeIfSupported(DocumentBuilderFactory factory, String name,
+                                             Object value) {
+    try {
+      factory.setAttribute(name, value);
+    } catch (IllegalArgumentException e) {
+      logger.warn("Failed to set XML parser attribute " + name + ", it's unsupported on " +
+          "this platform.", e);
+    }
+  }
+
+  private static void setXIncludeAwareIfSupported(DocumentBuilderFactory factory, boolean state) {
+    try {
+      factory.setXIncludeAware(state);
+    } catch (UnsupportedOperationException e) {
+      logger.warn("Failed to set XML parser XInclude awareness, it's unsupported on " +
+          "this platform.", e);
+    }
+  }
+
+  private static void setPropertyIfSupported(SAXParser parser, String name, Object value) {
+    try {
+      parser.setProperty(name, value);
+    } catch (SAXNotRecognizedException | SAXNotSupportedException e) {
+      logger.warn("Failed to set XML parser property " + name + ", it's unsupported on " +

Review Comment:
   Can we use log substitution for the `name` via  `{}` please.



##########
opennlp-tools/src/main/java/opennlp/tools/util/XmlUtil.java:
##########
@@ -85,17 +87,74 @@ public static SAXParser createSaxParser() {
       logger.warn("Failed to enable XMLConstants.FEATURE_SECURE_PROCESSING, it's unsupported on" +
           " this platform.", e);
     }
+    setFeatureIfSupported(spf, "http://apache.org/xml/features/disallow-doctype-decl", true);
+    setFeatureIfSupported(spf, "http://xml.org/sax/features/external-general-entities", false);
+    setFeatureIfSupported(spf, "http://xml.org/sax/features/external-parameter-entities", false);
+    setFeatureIfSupported(spf, "http://apache.org/xml/features/nonvalidating/load-external-dtd",
+        false);
     try {
-      spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
-      spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
-      spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
-      spf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
       final SAXParser parser = spf.newSAXParser();
-      parser.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");
-      parser.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
+      setPropertyIfSupported(parser, XMLConstants.ACCESS_EXTERNAL_DTD, "");
+      setPropertyIfSupported(parser, XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
       return parser;
     } catch (ParserConfigurationException | SAXException e) {
       throw new IllegalStateException(e);
     }
   }
+
+  private static void setFeatureIfSupported(DocumentBuilderFactory factory, String name,
+                                            boolean value) {
+    try {
+      factory.setFeature(name, value);
+    } catch (ParserConfigurationException e) {
+      logger.warn("Failed to set XML parser feature " + name + ", it's unsupported on " +
+          "this platform.", e);
+    }
+  }
+
+  private static void setAttributeIfSupported(DocumentBuilderFactory factory, String name,
+                                             Object value) {
+    try {
+      factory.setAttribute(name, value);
+    } catch (IllegalArgumentException e) {
+      logger.warn("Failed to set XML parser attribute " + name + ", it's unsupported on " +
+          "this platform.", e);
+    }
+  }
+
+  private static void setXIncludeAwareIfSupported(DocumentBuilderFactory factory, boolean state) {
+    try {
+      factory.setXIncludeAware(state);
+    } catch (UnsupportedOperationException e) {
+      logger.warn("Failed to set XML parser XInclude awareness, it's unsupported on " +
+          "this platform.", e);
+    }
+  }
+
+  private static void setPropertyIfSupported(SAXParser parser, String name, Object value) {
+    try {
+      parser.setProperty(name, value);
+    } catch (SAXNotRecognizedException | SAXNotSupportedException e) {
+      logger.warn("Failed to set XML parser property " + name + ", it's unsupported on " +
+          "this platform.", e);
+    }
+  }
+
+  private static void setFeatureIfSupported(SAXParserFactory factory, String name, boolean value) {
+    try {
+      factory.setFeature(name, value);
+    } catch (ParserConfigurationException | SAXException e) {
+      logger.warn("Failed to set XML parser feature " + name + ", it's unsupported on " +

Review Comment:
   Can we use log substitution for the `name` via  `{}` please.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org