This is an automated email from the ASF dual-hosted git repository. ppkarwasz pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/commons-xml.git
commit 6696dbbb4ef7d1afa7c332e277cc841845a2720f Author: Piotr P. Karwasz <[email protected]> AuthorDate: Thu Apr 23 22:57:41 2026 +0200 feat: allow XmlProvider to return wrappers This is necessary for `TransformerFactory` and `SchemaFactory` implementations that don't pass hardening to generated objects. --- .../apache/commons/xml/factory/XmlFactories.java | 18 ++++------- .../xml/factory/internal/AbstractXmlProvider.java | 18 +++++++++++ .../xml/factory/internal/StockJdkProvider.java | 18 +++++++---- .../commons/xml/factory/spi/XmlProvider.java | 37 +++++++++++++--------- 4 files changed, 58 insertions(+), 33 deletions(-) diff --git a/src/main/java/org/apache/commons/xml/factory/XmlFactories.java b/src/main/java/org/apache/commons/xml/factory/XmlFactories.java index c6a9902..18d63c3 100644 --- a/src/main/java/org/apache/commons/xml/factory/XmlFactories.java +++ b/src/main/java/org/apache/commons/xml/factory/XmlFactories.java @@ -74,8 +74,7 @@ public final class XmlFactories { */ public static DocumentBuilderFactory newDocumentBuilderFactory() { final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - ProviderRegistry.getInstance().providerFor(factory.getClass()).configure(factory); - return factory; + return ProviderRegistry.getInstance().providerFor(factory.getClass()).configure(factory); } /** @@ -96,8 +95,7 @@ public static DocumentBuilderFactory newDocumentBuilderFactory() { */ public static SAXParserFactory newSAXParserFactory() { final SAXParserFactory factory = SAXParserFactory.newInstance(); - ProviderRegistry.getInstance().providerFor(factory.getClass()).configure(factory); - return factory; + return ProviderRegistry.getInstance().providerFor(factory.getClass()).configure(factory); } /** @@ -128,8 +126,7 @@ public static SAXParserFactory newSAXParserFactory() { */ public static SchemaFactory newSchemaFactory() { final SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); - ProviderRegistry.getInstance().providerFor(factory.getClass()).configure(factory); - return factory; + return ProviderRegistry.getInstance().providerFor(factory.getClass()).configure(factory); } /** @@ -157,8 +154,7 @@ public static SchemaFactory newSchemaFactory() { */ public static TransformerFactory newTransformerFactory() { final TransformerFactory factory = TransformerFactory.newInstance(); - ProviderRegistry.getInstance().providerFor(factory.getClass()).configure(factory); - return factory; + return ProviderRegistry.getInstance().providerFor(factory.getClass()).configure(factory); } /** @@ -184,8 +180,7 @@ public static TransformerFactory newTransformerFactory() { */ public static XMLInputFactory newXMLInputFactory() { final XMLInputFactory factory = XMLInputFactory.newInstance(); - ProviderRegistry.getInstance().providerFor(factory.getClass()).configure(factory); - return factory; + return ProviderRegistry.getInstance().providerFor(factory.getClass()).configure(factory); } /** @@ -205,8 +200,7 @@ public static XMLInputFactory newXMLInputFactory() { */ public static XPathFactory newXPathFactory() { final XPathFactory factory = XPathFactory.newInstance(); - ProviderRegistry.getInstance().providerFor(factory.getClass()).configure(factory); - return factory; + return ProviderRegistry.getInstance().providerFor(factory.getClass()).configure(factory); } private XmlFactories() { diff --git a/src/main/java/org/apache/commons/xml/factory/internal/AbstractXmlProvider.java b/src/main/java/org/apache/commons/xml/factory/internal/AbstractXmlProvider.java index 4837b43..9010b51 100644 --- a/src/main/java/org/apache/commons/xml/factory/internal/AbstractXmlProvider.java +++ b/src/main/java/org/apache/commons/xml/factory/internal/AbstractXmlProvider.java @@ -28,6 +28,8 @@ import javax.xml.transform.TransformerConfigurationException; import javax.xml.transform.TransformerFactory; import javax.xml.validation.SchemaFactory; +import javax.xml.validation.Validator; +import javax.xml.validation.ValidatorHandler; import javax.xml.xpath.XPathFactory; import javax.xml.xpath.XPathFactoryConfigurationException; @@ -108,6 +110,22 @@ static void setFeature(final SchemaFactory factory, final String feature, final } } + static void setFeature(final Validator validator, final String feature, final boolean value) { + try { + validator.setFeature(feature, value); + } catch (final SAXNotRecognizedException | SAXNotSupportedException e) { + throw fail(validator, "feature", feature, e); + } + } + + static void setFeature(final ValidatorHandler handler, final String feature, final boolean value) { + try { + handler.setFeature(feature, value); + } catch (final SAXNotRecognizedException | SAXNotSupportedException e) { + throw fail(handler, "feature", feature, e); + } + } + static void setProperty(final XMLInputFactory factory, final String property, final Object value) { try { factory.setProperty(property, value); diff --git a/src/main/java/org/apache/commons/xml/factory/internal/StockJdkProvider.java b/src/main/java/org/apache/commons/xml/factory/internal/StockJdkProvider.java index 2b0b51c..3fd85b6 100644 --- a/src/main/java/org/apache/commons/xml/factory/internal/StockJdkProvider.java +++ b/src/main/java/org/apache/commons/xml/factory/internal/StockJdkProvider.java @@ -49,7 +49,7 @@ public StockJdkProvider() { } @Override - public void configure(final DocumentBuilderFactory factory) { + public DocumentBuilderFactory configure(final DocumentBuilderFactory factory) { setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); setFeature(factory, FEATURE_DISALLOW_DOCTYPE, true); factory.setXIncludeAware(false); @@ -61,10 +61,11 @@ public void configure(final DocumentBuilderFactory factory) { // - ACCESS_EXTERNAL_SCHEMA: protocols allowed for xsi:schemaLocation hints and for xs:import/xs:include/xs:redefine in a validating Schema. setAttribute(factory, XMLConstants.ACCESS_EXTERNAL_DTD, ""); setAttribute(factory, XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + return factory; } @Override - public void configure(final SAXParserFactory factory) { + public SAXParserFactory configure(final SAXParserFactory factory) { setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); setFeature(factory, FEATURE_DISALLOW_DOCTYPE, true); factory.setXIncludeAware(false); @@ -72,16 +73,18 @@ public void configure(final SAXParserFactory factory) { // No ACCESS_EXTERNAL_* here: the JAXP SAXParserFactory API exposes neither setAttribute nor setProperty, so these properties cannot be applied at // factory level. disallow-doctype-decl and validating=false already block external resolution on the normal parse path; callers who modify the posture // should set parser.setProperty(XMLConstants.ACCESS_EXTERNAL_{DTD,SCHEMA}, "") on each SAXParser returned by newSAXParser(). + return factory; } @Override - public void configure(final XMLInputFactory factory) { + public XMLInputFactory configure(final XMLInputFactory factory) { setProperty(factory, XMLInputFactory.SUPPORT_DTD, false); setProperty(factory, XMLInputFactory.IS_VALIDATING, false); + return factory; } @Override - public void configure(final TransformerFactory factory) { + public TransformerFactory configure(final TransformerFactory factory) { setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); // Defence-in-depth: FSP already tightens these to the empty string on the JDK's XMLSecurityManager path. They only fire if the caller turns FSP off // on the returned factory; the default resolver then consults them and refuses the fetch before any I/O. @@ -89,15 +92,17 @@ public void configure(final TransformerFactory factory) { // - ACCESS_EXTERNAL_STYLESHEET: protocols allowed for xsl:import, xsl:include and document() URIs during compile and transform. setAttribute(factory, XMLConstants.ACCESS_EXTERNAL_DTD, ""); setAttribute(factory, XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); + return factory; } @Override - public void configure(final XPathFactory factory) { + public XPathFactory configure(final XPathFactory factory) { setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); + return factory; } @Override - public void configure(final SchemaFactory factory) { + public SchemaFactory configure(final SchemaFactory factory) { setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); // Defence-in-depth: FSP already tightens these to the empty string on the JDK's XMLSecurityManager path. They only fire if the caller turns FSP off // on the returned factory; the default resolver then consults them and refuses the fetch before any I/O. The settings propagate into Validators @@ -106,5 +111,6 @@ public void configure(final SchemaFactory factory) { // - ACCESS_EXTERNAL_SCHEMA: protocols allowed for xs:import, xs:include and xs:redefine schemaLocation URIs during schema compilation. setProperty(factory, XMLConstants.ACCESS_EXTERNAL_DTD, ""); setProperty(factory, XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + return factory; } } diff --git a/src/main/java/org/apache/commons/xml/factory/spi/XmlProvider.java b/src/main/java/org/apache/commons/xml/factory/spi/XmlProvider.java index 06d74d3..187ef09 100644 --- a/src/main/java/org/apache/commons/xml/factory/spi/XmlProvider.java +++ b/src/main/java/org/apache/commons/xml/factory/spi/XmlProvider.java @@ -41,9 +41,10 @@ * <ul> * <li>{@link #supports(Class)} must return {@code true} <em>only</em> for factory classes for which this provider can implement <em>every</em> relevant * {@code configure} method.</li> - * <li>Each {@code configure} method must either complete successfully with the factory fully hardened, or throw. Returning a partially-configured factory is - * a bug; callers rely on the contract that a factory observed after a successful {@code configure} call has every relevant feature locked down. Wrap any - * checked exception (for example {@link javax.xml.parsers.ParserConfigurationException}, {@link javax.xml.stream.XMLStreamException}, + * <li>Each {@code configure} method must return a hardened factory (either the supplied instance hardened in place, or a wrapper that enforces hardening on + * every factory product) or throw. Returning a partially-configured factory is a bug; callers rely on the contract that the returned factory has every + * relevant knob locked down, including on objects it produces. Wrap any checked exception (for example + * {@link javax.xml.parsers.ParserConfigurationException}, {@link javax.xml.stream.XMLStreamException}, * {@link javax.xml.transform.TransformerConfigurationException}) in an unchecked exception whose message names the feature or property that failed.</li> * <li>Providers need not be thread-safe themselves, but they must not hold mutable global state. Each {@code configure} call operates on a caller-supplied * factory.</li> @@ -69,9 +70,10 @@ public interface XmlProvider { * * <p>The default implementation throws {@link UnsupportedOperationException}; providers that match DOM factories must override.</p> * - * @param factory the vanilla factory to harden in place; never {@code null}. + * @param factory the vanilla factory to harden; never {@code null}. + * @return the hardened factory; may be {@code factory} itself or a wrapper that enforces hardening on products it returns. */ - default void configure(final DocumentBuilderFactory factory) { + default DocumentBuilderFactory configure(final DocumentBuilderFactory factory) { throw new UnsupportedOperationException(); } @@ -80,9 +82,10 @@ default void configure(final DocumentBuilderFactory factory) { * * <p>The default implementation throws {@link UnsupportedOperationException}; providers that match SAX factories must override.</p> * - * @param factory the vanilla factory to harden in place; never {@code null}. + * @param factory the vanilla factory to harden; never {@code null}. + * @return the hardened factory; may be {@code factory} itself or a wrapper that enforces hardening on products it returns. */ - default void configure(final SAXParserFactory factory) { + default SAXParserFactory configure(final SAXParserFactory factory) { throw new UnsupportedOperationException(); } @@ -91,9 +94,10 @@ default void configure(final SAXParserFactory factory) { * * <p>The default implementation throws {@link UnsupportedOperationException}; providers that match StAX input factories must override.</p> * - * @param factory the vanilla factory to harden in place; never {@code null}. + * @param factory the vanilla factory to harden; never {@code null}. + * @return the hardened factory; may be {@code factory} itself or a wrapper that enforces hardening on products it returns. */ - default void configure(final XMLInputFactory factory) { + default XMLInputFactory configure(final XMLInputFactory factory) { throw new UnsupportedOperationException(); } @@ -102,9 +106,10 @@ default void configure(final XMLInputFactory factory) { * * <p>The default implementation throws {@link UnsupportedOperationException}; providers that match TrAX factories must override.</p> * - * @param factory the vanilla factory to harden in place; never {@code null}. + * @param factory the vanilla factory to harden; never {@code null}. + * @return the hardened factory; may be {@code factory} itself or a wrapper that enforces hardening on products it returns. */ - default void configure(final TransformerFactory factory) { + default TransformerFactory configure(final TransformerFactory factory) { throw new UnsupportedOperationException(); } @@ -113,9 +118,10 @@ default void configure(final TransformerFactory factory) { * * <p>The default implementation throws {@link UnsupportedOperationException}; providers that match XPath factories must override.</p> * - * @param factory the vanilla factory to harden in place; never {@code null}. + * @param factory the vanilla factory to harden; never {@code null}. + * @return the hardened factory; may be {@code factory} itself or a wrapper that enforces hardening on products it returns. */ - default void configure(final XPathFactory factory) { + default XPathFactory configure(final XPathFactory factory) { throw new UnsupportedOperationException(); } @@ -124,9 +130,10 @@ default void configure(final XPathFactory factory) { * * <p>The default implementation throws {@link UnsupportedOperationException}; providers that match Schema factories must override.</p> * - * @param factory the vanilla factory to harden in place; never {@code null}. + * @param factory the vanilla factory to harden; never {@code null}. + * @return the hardened factory; may be {@code factory} itself or a wrapper that enforces hardening on products it returns. */ - default void configure(final SchemaFactory factory) { + default SchemaFactory configure(final SchemaFactory factory) { throw new UnsupportedOperationException(); } }
