Skip to content

Consolidate more XML parsing, FileLike usages #7054

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
XingY merged 3 commits into from
Sep 20, 2025
Merged

Consolidate more XML parsing, FileLike usages #7054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4b4dd39
Arbitrary file access during archive extraction ("Zip Slip")
XingY Sep 18, 2025
db79419
XML parsing
XingY Sep 18, 2025
56a66dc
code review
XingY Sep 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions api/src/org/labkey/api/util/FileUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ public static boolean deleteDir(Path dir, Logger log)
}


public static void deleteDir(@NotNull Path dir) throws IOException
public static boolean deleteDir(@NotNull Path dir) throws IOException
{
if (Files.exists(dir))
{
Expand All @@ -276,17 +276,21 @@ public static void deleteDir(@NotNull Path dir) throws IOException
// TODO: On Windows, collect is yielding AccessDenied Exception, so only do this for cloud
try (Stream<Path> paths = Files.walk(dir))
{
boolean success = true;
for (Path path : paths.sorted(Comparator.reverseOrder()).toList())
{
Files.deleteIfExists(path);
success = Files.deleteIfExists(path) && success;
}
return success;
}
}
else
{
deleteDir(dir.toFile()); // Note: we maintain existing behavior from before Path work, which is to ignore any error
return deleteDir(dir.toFile()); // Note: we maintain existing behavior from before Path work, which is to ignore any error
}
}

return true;
}


Expand Down
21 changes: 0 additions & 21 deletions api/src/org/labkey/api/util/JunitUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,12 @@
import org.labkey.api.data.ContainerManager;
import org.labkey.api.module.Module;
import org.labkey.api.settings.AppProps;
import org.w3c.dom.Document;
import org.w3c.dom.Node;
import org.xml.sax.InputSource;

import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import java.io.File;
import java.io.IOException;
import java.io.StringReader;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.LinkedList;
Expand All @@ -64,21 +58,6 @@ public static String getAttributeValue(Node elem, String attr)
return node.getNodeValue();
}


public static Document tidyAsDocument(String html) throws Exception
{
ArrayList<String> errors = new ArrayList<>();
String tidy = JSoupUtil.convertHtmlToXml(html, errors);

DocumentBuilderFactory fact = DocumentBuilderFactory.newInstance();
fact.setValidating(false);
fact.setCoalescing(true);
fact.setIgnoringComments(true);
fact.setNamespaceAware(false);
DocumentBuilder builder = fact.newDocumentBuilder();
return builder.parse(new InputSource(new StringReader(tidy)));
}

/**
* Returns the container called "_junit" to be used by test cases.
*/
Expand Down
3 changes: 3 additions & 0 deletions api/src/org/labkey/api/util/XmlBeansUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.labkey.api.settings.LookAndFeelProperties;
import org.xml.sax.SAXException;

import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.parsers.SAXParserFactory;
Expand Down Expand Up @@ -145,6 +146,8 @@ public static void addComment(XmlTokenSource doc, String comment)
DOCUMENT_BUILDER_FACTORY.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
DOCUMENT_BUILDER_FACTORY.setFeature("http://xml.org/sax/features/external-general-entities", false);
DOCUMENT_BUILDER_FACTORY.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
DOCUMENT_BUILDER_FACTORY.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
DOCUMENT_BUILDER_FACTORY.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
DOCUMENT_BUILDER_FACTORY.setXIncludeAware(false);
DOCUMENT_BUILDER_FACTORY.setExpandEntityReferences(false);
}
Expand Down
6 changes: 2 additions & 4 deletions pipeline/src/org/labkey/pipeline/api/ParamParserImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.apache.logging.log4j.Logger;
import org.labkey.api.pipeline.ParamParser;
import org.labkey.api.util.StringUtilsLabKey;
import org.labkey.api.util.XmlBeansUtil;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
Expand All @@ -27,7 +28,6 @@
import org.xml.sax.SAXParseException;

import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.transform.OutputKeys;
import javax.xml.transform.Transformer;
import javax.xml.transform.TransformerException;
Expand Down Expand Up @@ -129,9 +129,7 @@ public void parse(InputStream inputStream)
{
try
{
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setValidating(false);
Copy link
Contributor

@labkey-jeckels labkey-jeckels Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we're setting this on the centralized factory. Probably OK to not have this scenario disable it, but calling it out in case you hadn't already considered it too.

Copy link
Contributor Author

@XingY XingY Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've checked and false seems the default setting.

DocumentBuilder db = dbf.newDocumentBuilder();
DocumentBuilder db = XmlBeansUtil.DOCUMENT_BUILDER_FACTORY.newDocumentBuilder();

InputSource source = new InputSource(new InputStreamReader(inputStream));
_doc = db.parse(source);
Expand Down
6 changes: 2 additions & 4 deletions query/src/org/labkey/query/ModuleQueryMetadataDef.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.labkey.api.util.DOMUtil;
import org.labkey.api.util.PageFlowUtil;
import org.labkey.api.util.Path;
import org.labkey.api.util.XmlBeansUtil;
import org.labkey.api.util.logging.LogHelper;
import org.labkey.query.persist.QueryDef;
import org.labkey.query.persist.QueryManager;
Expand All @@ -30,7 +31,6 @@
import org.xml.sax.SAXException;

import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.transform.TransformerException;
import java.io.IOException;
Expand Down Expand Up @@ -112,9 +112,7 @@ else if (rootElementName.equalsIgnoreCase("query"))

protected Document parseFile(Resource r) throws ParserConfigurationException, IOException, SAXException
{
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setValidating(false);
DocumentBuilder db = dbf.newDocumentBuilder();
DocumentBuilder db = XmlBeansUtil.DOCUMENT_BUILDER_FACTORY.newDocumentBuilder();

return db.parse(r.getInputStream());
}
Expand Down
10 changes: 3 additions & 7 deletions query/src/org/labkey/query/olap/rolap/RolapReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@
import org.labkey.api.module.Module;
import org.labkey.api.module.ModuleLoader;
import org.labkey.api.util.ConfigurationException;
import org.labkey.api.util.XmlBeansUtil;
import org.w3c.dom.Document;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;

import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -97,20 +97,16 @@ public Map<String,String> getAnnotations()

private void loadDocument(File file) throws SAXException, ParserConfigurationException, IOException
{
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setValidating(false);
DocumentBuilder db = dbf.newDocumentBuilder();
DocumentBuilder db = XmlBeansUtil.DOCUMENT_BUILDER_FACTORY.newDocumentBuilder();
_document = db.parse(file);
}


private void loadDocument(Reader reader) throws IOException
{
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setValidating(false);
try
{
DocumentBuilder db = dbf.newDocumentBuilder();
DocumentBuilder db = XmlBeansUtil.DOCUMENT_BUILDER_FACTORY.newDocumentBuilder();
_document = db.parse(new InputSource(reader));
}
catch (SAXException|ParserConfigurationException x)
Expand Down
4 changes: 2 additions & 2 deletions search/src/org/labkey/search/model/LuceneSearchServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@
import org.labkey.api.util.StringUtilsLabKey;
import org.labkey.api.util.TestContext;
import org.labkey.api.util.URLHelper;
import org.labkey.api.util.XmlBeansUtil;
import org.labkey.api.util.logging.LogHelper;
import org.labkey.api.view.ActionURL;
import org.labkey.api.view.UnauthorizedException;
Expand All @@ -120,7 +121,6 @@
import org.xml.sax.ContentHandler;
import org.xml.sax.SAXException;

import javax.xml.parsers.DocumentBuilderFactory;
import java.io.BufferedInputStream;
import java.io.Closeable;
import java.io.File;
Expand Down Expand Up @@ -221,7 +221,7 @@ public LuceneSearchServiceImpl()
try
{
InputStream is = getClass().getResourceAsStream("tikaConfig.xml");
org.w3c.dom.Document doc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(is);
org.w3c.dom.Document doc = XmlBeansUtil.DOCUMENT_BUILDER_FACTORY.newDocumentBuilder().parse(is);
config = new TikaConfig(doc, new ServiceLoader(Thread.currentThread().getContextClassLoader(), LoadErrorHandler.IGNORE, new ProblemHandler(_log), true));
}
catch (Exception e)
Expand Down