From 5581372ec76c4b5f3dea02bb614d43019b87e07f Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Fri, 12 Jun 2026 17:59:57 -0700 Subject: [PATCH 1/2] Use centralized XML parser config --- .../org/systemsbiology/jrap/MSXMLParser.java | 61 ++----------------- .../protein/annotation/XMLProteinHandler.java | 21 ++++--- .../org/labkey/protein/ProteinController.java | 2 +- 3 files changed, 20 insertions(+), 64 deletions(-) diff --git a/ms2/src/org/systemsbiology/jrap/MSXMLParser.java b/ms2/src/org/systemsbiology/jrap/MSXMLParser.java index afdbcd57be..ba2c4e0494 100644 --- a/ms2/src/org/systemsbiology/jrap/MSXMLParser.java +++ b/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.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) diff --git a/protein/api-src/org/labkey/api/protein/annotation/XMLProteinHandler.java b/protein/api-src/org/labkey/api/protein/annotation/XMLProteinHandler.java index 83b8898547..9fd53b4881 100644 --- a/protein/api-src/org/labkey/api/protein/annotation/XMLProteinHandler.java +++ b/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) { diff --git a/protein/src/org/labkey/protein/ProteinController.java b/protein/src/org/labkey/protein/ProteinController.java index 323d00f720..f5717a0b73 100644 --- a/protein/src/org/labkey/protein/ProteinController.java +++ b/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 { @Override From 148668ec86bc63699ddd78428647c6ed84d29fdc Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Fri, 12 Jun 2026 18:15:10 -0700 Subject: [PATCH 2/2] Self-review --- ms2/src/org/systemsbiology/jrap/MSXMLParser.java | 2 +- protein/src/org/labkey/protein/ProteinController.java | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/ms2/src/org/systemsbiology/jrap/MSXMLParser.java b/ms2/src/org/systemsbiology/jrap/MSXMLParser.java index ba2c4e0494..a4a83ecfcc 100644 --- a/ms2/src/org/systemsbiology/jrap/MSXMLParser.java +++ b/ms2/src/org/systemsbiology/jrap/MSXMLParser.java @@ -144,7 +144,7 @@ public MSXMLParser(File file) { // Create parser using the centrally-configured, XXE-safe factory try { - parser = XmlBeansUtil.SAX_PARSER_FACTORY.newSAXParser().getXMLReader(); + parser = XmlBeansUtil.SAX_PARSER_FACTORY_ALLOWING_DOCTYPE.newSAXParser().getXMLReader(); } catch (Exception e) { System.err.println("error: Unable to instantiate parser"); diff --git a/protein/src/org/labkey/protein/ProteinController.java b/protein/src/org/labkey/protein/ProteinController.java index f5717a0b73..c1a7c9af65 100644 --- a/protein/src/org/labkey/protein/ProteinController.java +++ b/protein/src/org/labkey/protein/ProteinController.java @@ -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() ); } }