Use centralized XML parser config

ms2/src/org/systemsbiology/jrap/MSXMLParser.java

@@ -40,8 +40,8 @@
 
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
+import org.labkey.api.util.XmlBeansUtil;
 import org.xml.sax.*;
-import org.xml.sax.helpers.XMLReaderFactory;
 
 import java.io.FileInputStream;
 import java.io.File;
@@ -136,35 +136,22 @@ public MSXMLParser(File file) {
         this.file = file;
 
         // The defaults for the parser
-        boolean namespaces = DEFAULT_NAMESPACES;
         boolean namespacePrefixes = DEFAULT_NAMESPACE_PREFIXES;
-        boolean validation = DEFAULT_VALIDATION;
-        boolean schemaValidation = DEFAULT_SCHEMA_VALIDATION;
-        boolean schemaFullChecking = DEFAULT_SCHEMA_FULL_CHECKING;
-        boolean dynamicValidation = DEFAULT_DYNAMIC_VALIDATION;
 
         // Create a new index handler
         indexHandler = new SAX2IndexHandler();
 
-        // Create parser
+        // Create parser using the centrally-configured, XXE-safe factory
         try
         {
-            parser = XMLReaderFactory.createXMLReader(DEFAULT_PARSER_NAME);
+            parser = XmlBeansUtil.SAX_PARSER_FACTORY_ALLOWING_DOCTYPE.newSAXParser().getXMLReader();
         } catch (Exception e)
         {
-            System.err.println("error: Unable to instantiate parser ("
-                    + DEFAULT_PARSER_NAME + ")");
+            System.err.println("error: Unable to instantiate parser");
         }
 
-        // Set parser features
-        try
-        {
-            parser.setFeature(NAMESPACES_FEATURE_ID, namespaces);
-        } catch (SAXException e)
-        {
-            System.err.println("warning: Parser does not support feature ("
-                    + NAMESPACES_FEATURE_ID + ")");
-        }
+        // Set parser features. Namespaces and validation are configured by the factory; only the
+        // remaining, non-security-relevant features need to be set here.
         try
         {
             parser.setFeature(NAMESPACE_PREFIXES_FEATURE_ID, namespacePrefixes);
@@ -174,42 +161,6 @@ public MSXMLParser(File file) {
                     + NAMESPACE_PREFIXES_FEATURE_ID + ")");
         }
         try
-        {
-            parser.setFeature(VALIDATION_FEATURE_ID, validation);
-        } catch (SAXException e)
-        {
-            System.err.println("warning: Parser does not support feature ("
-                    + VALIDATION_FEATURE_ID + ")");
-        }
-        try
-        {
-            parser.setFeature(SCHEMA_VALIDATION_FEATURE_ID, schemaValidation);
-        } catch (SAXNotRecognizedException | SAXNotSupportedException e)
-        {
-            System.err.println("warning: Parser does not support feature ("
-                    + SCHEMA_VALIDATION_FEATURE_ID + ")");
-
-        }
-        try
-        {
-            parser.setFeature(SCHEMA_FULL_CHECKING_FEATURE_ID,
-                    schemaFullChecking);
-        } catch (SAXNotRecognizedException | SAXNotSupportedException e)
-        {
-            System.err.println("warning: Parser does not support feature ("
-                    + SCHEMA_FULL_CHECKING_FEATURE_ID + ")");
-
-        }
-        try
-        {
-            parser.setFeature(DYNAMIC_VALIDATION_FEATURE_ID, dynamicValidation);
-        } catch (SAXNotRecognizedException | SAXNotSupportedException e)
-        {
-            System.err.println("warning: Parser does not support feature ("
-                    + DYNAMIC_VALIDATION_FEATURE_ID + ")");
-
-        }
-        try
         {
             parser.setFeature(CONT_AFTER_FATAL_ERROR_ID, false);
         } catch (SAXNotRecognizedException | SAXNotSupportedException e)

protein/api-src/org/labkey/api/protein/annotation/XMLProteinHandler.java

@@ -19,12 +19,13 @@
 import org.labkey.api.protein.uniprot.ParseActions;
 import org.labkey.api.protein.uniprot.ParseContext;
 import org.labkey.api.protein.uniprot.ParserTree;
+import org.labkey.api.util.XmlBeansUtil;
 import org.xml.sax.Attributes;
 import org.xml.sax.SAXException;
 import org.xml.sax.XMLReader;
 import org.xml.sax.helpers.DefaultHandler;
-import org.xml.sax.helpers.XMLReaderFactory;
 
+import javax.xml.parsers.ParserConfigurationException;
 import java.io.File;
 import java.io.IOException;
 import java.sql.Connection;
@@ -111,17 +112,21 @@ public XMLProteinHandler(Connection conn, XMLProteinLoader loader) throws SAXExc
                 )
         );
 
-        // create parser
-        setParser(XMLReaderFactory.createXMLReader(DEFAULT_PARSER_NAME));
+        // create parser using the centrally-configured, XXE-safe factory
+        try
+        {
+            setParser(XmlBeansUtil.SAX_PARSER_FACTORY.newSAXParser().getXMLReader());
+        }
+        catch (ParserConfigurationException e)
+        {
+            throw new SAXException(e);
+        }
 
         try
         {
-            getParser().setFeature(NAMESPACES_FEATURE_ID, DEFAULT_NAMESPACES);
+            // Namespaces and validation are configured by the factory; only the remaining,
+            // non-security-relevant features need to be set here.
             getParser().setFeature(NAMESPACE_PREFIXES_FEATURE_ID, DEFAULT_NAMESPACE_PREFIXES);
-            getParser().setFeature(VALIDATION_FEATURE_ID, DEFAULT_VALIDATION);
-            getParser().setFeature(SCHEMA_VALIDATION_FEATURE_ID, DEFAULT_SCHEMA_VALIDATION);
-            getParser().setFeature(SCHEMA_FULL_CHECKING_FEATURE_ID, DEFAULT_SCHEMA_FULL_CHECKING);
-            getParser().setFeature(DYNAMIC_VALIDATION_FEATURE_ID, DEFAULT_DYNAMIC_VALIDATION);
         }
         catch (Exception e)
         {

protein/src/org/labkey/protein/ProteinController.java

@@ -1101,7 +1101,7 @@ public void setNameType(String nameType)
         }
     }
 
-    @RequiresPermission(AdminPermission.class)
+    @RequiresPermission(AdminOperationsPermission.class)
     public static class SetBestNameAction extends FormHandlerAction<SetBestNameForm>
     {
         @Override
@@ -1424,11 +1424,6 @@ public void testActionPermissions()
 
             ProteinController controller = new ProteinController();
 
-            // @RequiresPermission(AdminPermission.class)
-            assertForAdminPermission(user,
-                    new SetBestNameAction()
-            );
-
             // @RequiresSiteAdmin
             assertForRequiresSiteAdmin(user,
                 controller.new LoadGoAction(),
@@ -1445,7 +1440,8 @@ controller.new ShowAnnotInsertDetailsAction()
             // @AdminConsoleAction
             // @RequiresPermission(AdminOperationsPermission.class)
             assertForAdminOperationsPermission(ContainerManager.getRoot(), user,
-                    controller.new ShowProteinAdminAction()
+                    controller.new ShowProteinAdminAction(),
+                    new SetBestNameAction()
             );
         }
     }