Re: [PR] Harden XML parsing via commons-secure-xml [commons-jelly]

["garydgregory (via GitHub)" <gi...@apache.org>]

garydgregory commented on code in PR #108:
URL: https://github.com/apache/commons-jelly/pull/108#discussion_r3944016540


##########
core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java:
##########
@@ -810,12 +811,17 @@ public SAXParser getParser() {
         // Create and return a new parser
         synchronized (this) {
             try {
-                if (factory == null) {
-                    factory = SAXParserFactory.newInstance();
+                SAXParserFactory parserFactory = factory;
+                if (parserFactory == null) {
+                    // The secure factory's resolver floor would ignore external entities, so the
+                    // documented opt-in keeps using a plain factory; do not cache the per-instance choice.
+                    parserFactory = allowDtdToCallExternalEntities

Review Comment:
   @ppkarwasz 
   Why not always use a secure object and call setFeature to enable external entities?
   



-- 
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: issues-unsubscribe@commons.apache.org

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