Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010-2025 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
* Copyright (c) 2010-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
*/
package com.marklogic.client.impl;

Expand Down Expand Up @@ -35,7 +35,6 @@

public class NodeConverter {
static private ObjectMapper mapper;
static private DocumentBuilderFactory documentBuilderFactory;
static private XMLInputFactory xmlInputFactory;

private NodeConverter() {
Expand All @@ -49,16 +48,7 @@ static private ObjectMapper getMapper() {
}
return mapper;
}
static private DocumentBuilderFactory getDocumentBuilderFactory() {
// okay if one thread overwrites another during lazy initialization
if (documentBuilderFactory == null) {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory.setNamespaceAware(true);
factory.setValidating(false);
documentBuilderFactory = factory;
}
return documentBuilderFactory;
}

static private XMLInputFactory getXMLInputFactory() {
// okay if one thread overwrites another during lazy initialization
if (xmlInputFactory == null) {
Expand Down Expand Up @@ -265,7 +255,7 @@ static public Stream<JsonParser> ReaderToJsonParser(Stream<? extends Reader> val

static public Document InputStreamToDocument(InputStream inputStream) {
try {
return (inputStream == null) ? null : getDocumentBuilderFactory().newDocumentBuilder().parse(inputStream);
return (inputStream == null) ? null : XmlFactories.getDocumentBuilderFactory().newDocumentBuilder().parse(inputStream);
} catch(SAXException | IOException | ParserConfigurationException e) {
throw new RuntimeException(e);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,30 +1,31 @@
/*
* Copyright (c) 2010-2025 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
* Copyright (c) 2010-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
*/
package com.marklogic.client.impl;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.stream.FactoryConfigurationError;
import javax.xml.stream.XMLInputFactory;
import javax.xml.stream.XMLOutputFactory;
import javax.xml.transform.TransformerConfigurationException;
import javax.xml.transform.TransformerFactory;
import java.lang.ref.SoftReference;
import java.util.function.Supplier;

public final class XmlFactories {

private static final Logger logger = LoggerFactory.getLogger(XmlFactories.class);

private static final CachedInstancePerThreadSupplier<XMLOutputFactory> cachedOutputFactory =
new CachedInstancePerThreadSupplier<XMLOutputFactory>(new Supplier<XMLOutputFactory>() {
@Override
public XMLOutputFactory get() {
return makeNewOutputFactory();
}
});
new CachedInstancePerThreadSupplier<>(XmlFactories::makeNewOutputFactory);

Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

The new cachedDocumentBuilderFactory field should have a javadoc comment explaining its purpose, similar to how cachedOutputFactory has documentation. This helps maintain consistency in the code and clarifies why DocumentBuilderFactory instances are cached per thread.

Suggested change
/**
* Cached per-thread {@link DocumentBuilderFactory} instances.
* <p>
* Creating {@link DocumentBuilderFactory} instances is relatively expensive and they are
* not guaranteed to be thread safe. To avoid repeated initialization cost while preserving
* thread safety, we cache one instance per thread using {@link CachedInstancePerThreadSupplier}.
* New instances are created via {@link #makeNewDocumentBuilderFactory()} when no cached
* value is available for the current thread.
*/

Copilot uses AI. Check for mistakes.
private static final CachedInstancePerThreadSupplier<DocumentBuilderFactory> cachedDocumentBuilderFactory =
new CachedInstancePerThreadSupplier<>(XmlFactories::makeNewDocumentBuilderFactory);

private XmlFactories() {} // preventing instances of utility class

Expand Down Expand Up @@ -62,21 +63,78 @@ public static TransformerFactory makeNewTransformerFactory() {
try {
factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
} catch (TransformerConfigurationException e) {
logger.warn("Unable to set {} on TransformerFactory; cause: {}", XMLConstants.FEATURE_SECURE_PROCESSING, e.getMessage());
logTransformerWarning(XMLConstants.FEATURE_SECURE_PROCESSING, e.getMessage());
}
try {
factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
} catch (IllegalArgumentException e) {
logger.warn("Unable to set {} on TransformerFactory; cause: {}", XMLConstants.ACCESS_EXTERNAL_DTD, e.getMessage());
logTransformerWarning(XMLConstants.ACCESS_EXTERNAL_DTD, e.getMessage());
}
try {
factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
} catch (IllegalArgumentException e) {
logger.warn("Unable to set {} on TransformerFactory; cause: {}", XMLConstants.ACCESS_EXTERNAL_STYLESHEET, e.getMessage());
logTransformerWarning(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, e.getMessage());
}
return factory;
}

private static void logTransformerWarning(String xmlConstant, String errorMessage) {
logger.warn("Unable to set {} on TransformerFactory; cause: {}", xmlConstant, errorMessage);
}

private static DocumentBuilderFactory makeNewDocumentBuilderFactory() {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
// Default to best practices for conservative security including recommendations per
// https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.md
try {
factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
} catch (ParserConfigurationException e) {
logger.warn("Unable to set FEATURE_SECURE_PROCESSING on DocumentBuilderFactory; cause: {}", e.getMessage());
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

The warning message format is inconsistent with other warning messages in the same method. Lines 97, 102, 107, and 112 use feature name descriptions (e.g., 'disallow-doctype-decl'), while line 92 uses the constant name. For consistency, consider using the constant name format like 'Unable to set {} on DocumentBuilderFactory; cause: {}' with XMLConstants.FEATURE_SECURE_PROCESSING as a parameter, similar to how the logTransformerWarning method works.

Suggested change
logger.warn("Unable to set FEATURE_SECURE_PROCESSING on DocumentBuilderFactory; cause: {}", e.getMessage());
logger.warn("Unable to set {} on DocumentBuilderFactory; cause: {}", XMLConstants.FEATURE_SECURE_PROCESSING, e.getMessage());

Copilot uses AI. Check for mistakes.
}
try {
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
} catch (ParserConfigurationException e) {
logger.warn("Unable to set disallow-doctype-decl on DocumentBuilderFactory; cause: {}", e.getMessage());
}
try {
factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
} catch (ParserConfigurationException e) {
logger.warn("Unable to set external-general-entities on DocumentBuilderFactory; cause: {}", e.getMessage());
}
try {
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
} catch (ParserConfigurationException e) {
logger.warn("Unable to set external-parameter-entities on DocumentBuilderFactory; cause: {}", e.getMessage());
}
try {
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
} catch (ParserConfigurationException e) {
logger.warn("Unable to set load-external-dtd on DocumentBuilderFactory; cause: {}", e.getMessage());
}
factory.setXIncludeAware(false);
factory.setExpandEntityReferences(false);
factory.setNamespaceAware(true);
factory.setValidating(false);

return factory;
}

/**
* Returns a shared {@link DocumentBuilderFactory} configured with secure defaults.
* <p>
* Creating XML factories is potentially a pretty expensive operation. Using a shared instance helps to amortize
* this initialization cost via reuse.
*
* @return a securely configured {@link DocumentBuilderFactory}
*
* @since 8.1.0
*
* @see #makeNewDocumentBuilderFactory() if you really (really?) need a non-shared instance
*/
public static DocumentBuilderFactory getDocumentBuilderFactory() {
return cachedDocumentBuilderFactory.get();
}

/**
* Returns a shared {@link XMLOutputFactory}. This factory will have its
* {@link XMLOutputFactory#IS_REPAIRING_NAMESPACES} property set to {@code true}.
Expand All @@ -88,31 +146,12 @@ public static TransformerFactory makeNewTransformerFactory() {
*
* @throws FactoryConfigurationError see {@link XMLOutputFactory#newInstance()}
*
* @see #makeNewOutputFactory() if you really (really?) need an non-shared instance
* @see #makeNewOutputFactory() if you really (really?) need a non-shared instance
*/
public static XMLOutputFactory getOutputFactory() {
return cachedOutputFactory.get();
}

/**
* Represents a supplier of results.
*
* <p>There is no requirement that a new or distinct result be returned each
* time the supplier is invoked.
*
* @param <T> the type of results supplied by this supplier
*/
// TODO replace with java.util.function.Supplier<T> after Java 8 migration
interface Supplier<T> {

/**
* Gets a result.
*
* @return a result
*/
T get();
}

/**
* A supplier that caches results per thread.
* <p>
Expand All @@ -129,7 +168,7 @@ interface Supplier<T> {
*/
private static class CachedInstancePerThreadSupplier<T> implements Supplier<T> {

private final ThreadLocal<SoftReference<T>> cachedInstances = new ThreadLocal<SoftReference<T>>();
private final ThreadLocal<SoftReference<T>> cachedInstances = new ThreadLocal<>();

/**
* The underlying supplier, invoked to originally retrieve the per-thread result
Expand Down Expand Up @@ -167,7 +206,7 @@ public T get() {
}

// ... and retain it for later re-use
cachedInstances.set(new SoftReference<T>(cachedInstance));
cachedInstances.set(new SoftReference<>(cachedInstance));
}

return cachedInstance;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,41 +1,26 @@
/*
* Copyright (c) 2010-2025 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
* Copyright (c) 2010-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
*/
package com.marklogic.client.io;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.charset.StandardCharsets;

import javax.xml.XMLConstants;
import javax.xml.namespace.QName;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.xpath.XPath;
import javax.xml.xpath.XPathConstants;
import javax.xml.xpath.XPathExpression;
import javax.xml.xpath.XPathExpressionException;
import javax.xml.xpath.XPathFactory;

import com.marklogic.client.MarkLogicIOException;
import com.marklogic.client.MarkLogicInternalException;
import com.marklogic.client.impl.XmlFactories;
import com.marklogic.client.io.marker.*;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.w3c.dom.DOMException;
import org.w3c.dom.Document;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import org.w3c.dom.ls.DOMImplementationLS;
import org.w3c.dom.ls.LSException;
import org.w3c.dom.ls.LSInput;
import org.w3c.dom.ls.LSOutput;
import org.w3c.dom.ls.LSParser;
import org.w3c.dom.ls.LSResourceResolver;
import org.w3c.dom.ls.*;

import com.marklogic.client.MarkLogicIOException;
import com.marklogic.client.MarkLogicInternalException;
import javax.xml.namespace.QName;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.xpath.*;
import java.io.*;
import java.nio.charset.StandardCharsets;

/**
* A DOM Handle represents XML content as a DOM document for reading or writing.
Expand Down Expand Up @@ -199,7 +184,7 @@ public String toString() {
*/
public DocumentBuilderFactory getFactory() throws ParserConfigurationException {
if (factory == null)
factory = makeDocumentBuilderFactory();
factory = XmlFactories.getDocumentBuilderFactory();
return factory;
}
/**
Expand All @@ -209,32 +194,6 @@ public DocumentBuilderFactory getFactory() throws ParserConfigurationException {
public void setFactory(DocumentBuilderFactory factory) {
this.factory = factory;
}
protected DocumentBuilderFactory makeDocumentBuilderFactory() throws ParserConfigurationException {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
// default to best practices for conservative security including recommendations per
// https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.md
try {
factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
} catch (ParserConfigurationException e) {}
try {
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
} catch (ParserConfigurationException e) {}
try {
factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
} catch (ParserConfigurationException e) {}
try {
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
} catch (ParserConfigurationException e) {}
try {
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
} catch (ParserConfigurationException e) {}
factory.setXIncludeAware(false);
factory.setExpandEntityReferences(false);
factory.setNamespaceAware(true);
factory.setValidating(false);

return factory;
}

/**
* Get the processor used to evaluate XPath expressions.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010-2025 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
* Copyright (c) 2010-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
*/
package com.marklogic.client.io;

Expand Down Expand Up @@ -577,10 +577,7 @@ protected void receiveContent(InputStream content) {

Document document = null;
if (content != null) {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory.setNamespaceAware(true);
factory.setValidating(false);
DocumentBuilder builder = factory.newDocumentBuilder();
DocumentBuilder builder = XmlFactories.getDocumentBuilderFactory().newDocumentBuilder();
document = builder.parse(new InputSource(new InputStreamReader(content, StandardCharsets.UTF_8)));
content.close();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
/*
* Copyright (c) 2010-2025 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
* Copyright (c) 2010-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved.
*/
package com.marklogic.client.test;

import com.marklogic.client.impl.XmlFactories;
import com.marklogic.client.io.*;
import com.marklogic.client.test.util.Referred;
import com.marklogic.client.test.util.Refers;
import jakarta.xml.bind.JAXBContext;
import jakarta.xml.bind.JAXBException;
import org.custommonkey.xmlunit.SimpleNamespaceContext;
import org.custommonkey.xmlunit.XMLUnit;
import org.custommonkey.xmlunit.XpathEngine;
Expand All @@ -17,16 +20,14 @@
import org.w3c.dom.Element;
import org.xml.sax.SAXException;

import jakarta.xml.bind.JAXBContext;
import jakarta.xml.bind.JAXBException;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;

import static org.custommonkey.xmlunit.XMLAssert.assertXMLEqual;
import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class BufferableHandleTest {
static private XpathEngine xpather;
Expand Down Expand Up @@ -61,11 +62,7 @@ public void testReadWrite()
throws JAXBException, ParserConfigurationException, SAXException, IOException, XpathException
{

DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory.setNamespaceAware(true);
factory.setValidating(false);

Document domDocument = factory.newDocumentBuilder().newDocument();
Document domDocument = XmlFactories.getDocumentBuilderFactory().newDocumentBuilder().newDocument();
Element root = domDocument.createElement("root");
root.setAttribute("xml:lang", "en");
root.setAttribute("foo", "bar");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.marklogic.client.DatabaseClientBuilder;
import com.marklogic.client.DatabaseClientFactory;
import com.marklogic.client.extra.okhttpclient.OkHttpClientConfigurator;
import com.marklogic.client.impl.XmlFactories;
import com.marklogic.client.impl.okhttp.RetryIOExceptionInterceptor;
import com.marklogic.client.io.DocumentMetadataHandle;
import com.marklogic.mgmt.ManageClient;
Expand Down Expand Up @@ -228,10 +229,7 @@ public static String testDocumentToString(Document document) {

public static Document testStringToDocument(String document) {
try {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory.setNamespaceAware(true);
factory.setValidating(false);
return factory.newDocumentBuilder().parse(
return XmlFactories.getDocumentBuilderFactory().newDocumentBuilder().parse(
new InputSource(new StringReader(document)));
} catch (SAXException e) {
throw new RuntimeException(e);
Expand Down
Loading