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 a9202c985079aac0e12f1a888a5a22267f4f4301 Author: Piotr P. Karwasz <[email protected]> AuthorDate: Mon Apr 27 19:53:32 2026 +0200 Remove the `XmlProvider` SPI (#7) JAXP implementations are a closed world of dying specimens, so a service-loader extension point cannot be exercised in practice and only adds an indirection between `XmlFactories` and the four bundled hardening recipes. This change folds dispatch into `XmlFactories` itself, where a switch on the concrete factory class name routes each call to the matching recipe directly. The four per-implementation classes survive as the visible unit of hardening logic but no longer share a common abstract base, and the shared setter helpers move to a small utility class. Prose mentioning `ServiceLoader` is dropped from the README and site documentation. The build artefact shrinks from 38 KiB to 36 KiB. Assisted-By: Claude Opus 4.7 (1M context) <[email protected]> --- README.md | 18 +-- .../commons/xml/factory/CompositeProvider.java | 147 -------------------- .../commons/xml/factory/HardeningException.java | 4 +- .../xml/factory/HardeningSAXParserFactory.java | 15 +- .../{AbstractXmlProvider.java => JaxpSetters.java} | 31 ++--- .../org/apache/commons/xml/factory/Limits.java | 4 +- .../apache/commons/xml/factory/SaxonProvider.java | 27 ++-- .../commons/xml/factory/StockJdkProvider.java | 49 +++---- .../commons/xml/factory/WoodstoxProvider.java | 21 +-- .../apache/commons/xml/factory/XercesProvider.java | 37 ++--- .../apache/commons/xml/factory/XmlFactories.java | 121 +++++++++++++--- .../apache/commons/xml/factory/XmlProvider.java | 153 --------------------- src/site/markdown/index.md | 18 +-- .../commons/xml/factory/CompositeProviderTest.java | 145 ------------------- .../xml/factory/SaxonXPathExternalCallsTest.java | 2 +- .../factory/UnsupportedXmlImplementationTest.java | 11 +- 16 files changed, 187 insertions(+), 616 deletions(-) diff --git a/README.md b/README.md index e169e91..3fd7b92 100644 --- a/README.md +++ b/README.md @@ -40,10 +40,10 @@ such as standalone Xerces, Woodstox, or Saxon's TrAX, need further configuration library author has no control over which implementation is on the classpath at runtime, so the effective security posture of their code depends on a deployment decision made elsewhere. -This library provides that baseline. Each `XmlFactories` call returns a fresh factory hardened by a provider-specific -SPI, so the returned object behaves the same way security-wise regardless of which JAXP implementation resolved. -Security becomes a property of the call, not of the classpath, and there is one place to update when a new hardening -setting becomes available or a default changes. +This library provides that baseline. Each `XmlFactories` call returns a fresh factory hardened by an +implementation-specific recipe, so the returned object behaves the same way security-wise regardless of which JAXP +implementation resolved. Security becomes a property of the call, not of the classpath, and there is one place to +update when a new hardening setting becomes available or a default changes. ## Usage @@ -64,13 +64,9 @@ stylesheet) is blocked, and DOCTYPE input is rejected wherever the underlying im ### Supported implementations Out of the box the library recognises the stock JDK JAXP implementations, Apache Xerces 2.x, Woodstox, and Saxon-HE. If -a factory resolves to an implementation not covered by any bundled or registered provider, every `XmlFactories` method -throws `IllegalStateException` with a message naming the unsupported class. - -Support for additional implementations can be plugged in by publishing an -`org.apache.commons.xml.factory.spi.XmlProvider` through the standard Java `ServiceLoader` mechanism. Bundled providers -always take precedence, so a third-party provider cannot hijack hardening for a factory class this library already -supports. +a factory resolves to an implementation not covered by any bundled hardening recipe, every `XmlFactories` method throws +`IllegalStateException` with a message naming the unsupported class. Adding support for a new JAXP implementation +requires a code change to this library. **DOM parsing** via `DocumentBuilderFactory`: diff --git a/src/main/java/org/apache/commons/xml/factory/CompositeProvider.java b/src/main/java/org/apache/commons/xml/factory/CompositeProvider.java deleted file mode 100644 index 8eeef6f..0000000 --- a/src/main/java/org/apache/commons/xml/factory/CompositeProvider.java +++ /dev/null @@ -1,147 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.commons.xml.factory; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.ServiceLoader; - -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.SAXParserFactory; -import javax.xml.stream.XMLInputFactory; -import javax.xml.transform.TransformerFactory; -import javax.xml.validation.SchemaFactory; -import javax.xml.xpath.XPathFactory; - -import org.xml.sax.XMLReader; - -/** - * Internal {@link XmlProvider} that routes each {@code configure} call to the bundled or {@link ServiceLoader}-discovered provider responsible for the concrete - * JAXP factory class. - * - * <h2>Dispatch order</h2> - * - * <p>Providers are consulted in a single fixed list, built once when this class is loaded:</p> - * - * <ol> - * <li>Bundled providers in declared order: {@link StockJdkProvider}, {@link XercesProvider}, {@link WoodstoxProvider}, {@link SaxonProvider}.</li> - * <li>Third-party providers discovered via {@link ServiceLoader}, in the order {@link ServiceLoader} yields them.</li> - * </ol> - * - * <p>Bundled providers come first so that a third-party provider on the classpath cannot intercept hardening of a factory class this library already - * supports.</p> - * - * <p>If no provider matches, {@link HardeningException} is thrown.</p> - * - * <p>Although this class implements {@link XmlProvider}, its {@link #supports(Class)} method returns {@code false}: callers ask the composite to - * {@code configure}, never to advertise support. The interface implementation lets {@link org.apache.commons.xml.factory.XmlFactories} treat hardening - * uniformly without an extra abstraction.</p> - */ -final class CompositeProvider implements XmlProvider { - - private static final CompositeProvider INSTANCE = new CompositeProvider(defaultProviders()); - - private final List<XmlProvider> providers; - - /** - * Constructs a composite over the supplied provider list. - * - * <p>Package-private for test use; production code should call {@link #getInstance()}.</p> - */ - CompositeProvider(final List<XmlProvider> providers) { - this.providers = Collections.unmodifiableList(new ArrayList<>(providers)); - } - - /** - * Gets the process-wide composite instance. - * - * @return the singleton. - */ - public static CompositeProvider getInstance() { - return INSTANCE; - } - - /** - * Stub implementation: always returns {@code false}. - * - * <p>The {@link XmlProvider} contract requires this method, but the class is internal and nothing in the library invokes it.</p> - * - * @param factoryClass ignored. - * @return {@code false}. - */ - @Override - public boolean supports(final Class<?> factoryClass) { - return false; - } - - @Override - public DocumentBuilderFactory configure(final DocumentBuilderFactory factory) { - return providerFor(factory.getClass()).configure(factory); - } - - @Override - public SAXParserFactory configure(final SAXParserFactory factory) { - return providerFor(factory.getClass()).configure(factory); - } - - @Override - public XMLReader configure(final XMLReader reader) { - return providerFor(reader.getClass()).configure(reader); - } - - @Override - public XMLInputFactory configure(final XMLInputFactory factory) { - return providerFor(factory.getClass()).configure(factory); - } - - @Override - public TransformerFactory configure(final TransformerFactory factory) { - return providerFor(factory.getClass()).configure(factory); - } - - @Override - public XPathFactory configure(final XPathFactory factory) { - return providerFor(factory.getClass()).configure(factory); - } - - @Override - public SchemaFactory configure(final SchemaFactory factory) { - return providerFor(factory.getClass()).configure(factory); - } - - private XmlProvider providerFor(final Class<?> factoryClass) { - for (final XmlProvider provider : providers) { - if (provider.supports(factoryClass)) { - return provider; - } - } - throw new HardeningException(String.format( - "No XmlProvider supports JAXP factory class %s. Add a supported JAXP implementation to the classpath, " - + "or register a custom XmlProvider via ServiceLoader.", - factoryClass.getName())); - } - - private static List<XmlProvider> defaultProviders() { - final List<XmlProvider> all = new ArrayList<>(); - all.add(new StockJdkProvider()); - all.add(new XercesProvider()); - all.add(new WoodstoxProvider()); - all.add(new SaxonProvider()); - return all; - } -} diff --git a/src/main/java/org/apache/commons/xml/factory/HardeningException.java b/src/main/java/org/apache/commons/xml/factory/HardeningException.java index 30aa9f9..ff6a9b8 100644 --- a/src/main/java/org/apache/commons/xml/factory/HardeningException.java +++ b/src/main/java/org/apache/commons/xml/factory/HardeningException.java @@ -21,8 +21,8 @@ * * <p>Two failure modes share this type:</p> * <ul> - * <li>No registered {@link XmlProvider XmlProvider} recognises the concrete factory class.</li> - * <li>A recognised provider tried to apply a hardening setting and the implementation rejected it.</li> + * <li>No bundled hardening recipe matches the concrete factory class.</li> + * <li>A recipe tried to apply a hardening setting and the implementation rejected it.</li> * </ul> * * <p>The message names the unsupported factory class or the specific feature, attribute or property that failed; the cause, when present, is the original diff --git a/src/main/java/org/apache/commons/xml/factory/HardeningSAXParserFactory.java b/src/main/java/org/apache/commons/xml/factory/HardeningSAXParserFactory.java index 58779ab..efd974f 100644 --- a/src/main/java/org/apache/commons/xml/factory/HardeningSAXParserFactory.java +++ b/src/main/java/org/apache/commons/xml/factory/HardeningSAXParserFactory.java @@ -16,31 +16,34 @@ */ package org.apache.commons.xml.factory; +import java.util.function.UnaryOperator; + import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; import org.xml.sax.SAXException; +import org.xml.sax.XMLReader; /** - * Universal SAX wrapper that defers per-parser hardening to an {@link XmlProvider}'s {@link XmlProvider#configure(org.xml.sax.XMLReader)} method. + * Universal SAX wrapper that defers per-parser hardening to a supplied {@link XMLReader} hardener. * * <p>{@link SAXParserFactory} has no property API, only a feature API. Therefore, complex configuration must be performed on each new - * {@link org.xml.sax.XMLReader} parser.</p> + * {@link XMLReader} parser.</p> */ final class HardeningSAXParserFactory extends DelegatingSAXParserFactory { - private final XmlProvider provider; + private final UnaryOperator<XMLReader> hardener; - HardeningSAXParserFactory(final SAXParserFactory delegate, final XmlProvider provider) { + HardeningSAXParserFactory(final SAXParserFactory delegate, final UnaryOperator<XMLReader> hardener) { super(delegate); - this.provider = provider; + this.hardener = hardener; } @Override public SAXParser newSAXParser() throws ParserConfigurationException, SAXException { final SAXParser parser = super.newSAXParser(); - provider.configure(parser.getXMLReader()); + hardener.apply(parser.getXMLReader()); return parser; } } diff --git a/src/main/java/org/apache/commons/xml/factory/AbstractXmlProvider.java b/src/main/java/org/apache/commons/xml/factory/JaxpSetters.java similarity index 84% rename from src/main/java/org/apache/commons/xml/factory/AbstractXmlProvider.java rename to src/main/java/org/apache/commons/xml/factory/JaxpSetters.java index 47e564a..8b52ba1 100644 --- a/src/main/java/org/apache/commons/xml/factory/AbstractXmlProvider.java +++ b/src/main/java/org/apache/commons/xml/factory/JaxpSetters.java @@ -16,11 +16,6 @@ */ package org.apache.commons.xml.factory; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; -import java.util.Set; - import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; @@ -34,12 +29,12 @@ import org.xml.sax.XMLReader; /** - * Common scaffolding for {@link XmlProvider} implementations bundled with this library. + * Setter helpers shared by the bundled hardening providers. * - * <p>Subclasses pass the fully qualified class names of the factory implementations they handle to {@link #AbstractXmlProvider(String...)}. The inherited - * {@link #supports(Class)} then returns {@code true} for exactly those classes, matching on {@link Class#getName()}.</p> + * <p>Each overload wraps a single JAXP setter (feature, attribute or property) in a try/catch that translates any thrown exception into a + * {@link HardeningException} whose message names the offending feature, attribute or property and the concrete factory class.</p> */ -abstract class AbstractXmlProvider implements XmlProvider { +final class JaxpSetters { /** Action that may throw any exception; used to share a single try/catch around every JAXP setter. */ @FunctionalInterface @@ -91,6 +86,10 @@ static void setFeature(final ValidatorHandler handler, final String feature, fin apply(handler, "feature", feature, () -> handler.setFeature(feature, value)); } + static void setFeature(final XMLReader reader, final String feature, final boolean value) { + apply(reader, "feature", feature, () -> reader.setFeature(feature, value)); + } + static void setProperty(final XMLInputFactory factory, final String property, final Object value) { apply(factory, "property", property, () -> factory.setProperty(property, value)); } @@ -103,10 +102,6 @@ static void setProperty(final XMLReader reader, final String property, final Obj apply(reader, "property", property, () -> reader.setProperty(property, value)); } - static void setFeature(final XMLReader reader, final String feature, final boolean value) { - apply(reader, "feature", feature, () -> reader.setFeature(feature, value)); - } - static void setProperty(final SchemaFactory factory, final String property, final Object value) { apply(factory, "property", property, () -> factory.setProperty(property, value)); } @@ -119,14 +114,6 @@ static void setProperty(final ValidatorHandler handler, final String property, f apply(handler, "property", property, () -> handler.setProperty(property, value)); } - private final Set<String> supported; - - protected AbstractXmlProvider(final String... supportedClassNames) { - this.supported = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(supportedClassNames))); - } - - @Override - public final boolean supports(final Class<?> factoryClass) { - return factoryClass != null && supported.contains(factoryClass.getName()); + private JaxpSetters() { } } diff --git a/src/main/java/org/apache/commons/xml/factory/Limits.java b/src/main/java/org/apache/commons/xml/factory/Limits.java index e059b71..ab16351 100644 --- a/src/main/java/org/apache/commons/xml/factory/Limits.java +++ b/src/main/java/org/apache/commons/xml/factory/Limits.java @@ -16,8 +16,8 @@ */ package org.apache.commons.xml.factory; -import static org.apache.commons.xml.factory.AbstractXmlProvider.setAttribute; -import static org.apache.commons.xml.factory.AbstractXmlProvider.setProperty; +import static org.apache.commons.xml.factory.JaxpSetters.setAttribute; +import static org.apache.commons.xml.factory.JaxpSetters.setProperty; import java.util.Collections; import java.util.LinkedHashMap; diff --git a/src/main/java/org/apache/commons/xml/factory/SaxonProvider.java b/src/main/java/org/apache/commons/xml/factory/SaxonProvider.java index ec10bef..da14957 100644 --- a/src/main/java/org/apache/commons/xml/factory/SaxonProvider.java +++ b/src/main/java/org/apache/commons/xml/factory/SaxonProvider.java @@ -27,13 +27,11 @@ import org.xml.sax.XMLReader; /** - * {@link XmlProvider} for Saxon-HE ({@code net.sf.saxon:Saxon-HE}). + * Hardening recipes for Saxon-HE ({@code net.sf.saxon:Saxon-HE}). * * <p>Saxon supplies {@link TransformerFactory} and {@link XPathFactory} implementations; it does not ship a DOM, SAX, StAX or Schema factory of its own.</p> - * - * <p>Must be declared {@code public} so {@link java.util.ServiceLoader} can load it from {@code META-INF/services/}.</p> */ -final class SaxonProvider extends AbstractXmlProvider { +final class SaxonProvider { /** * A Saxon {@link Configuration} that locks down every channel through which Saxon would otherwise reach external resources. @@ -42,7 +40,7 @@ final class SaxonProvider extends AbstractXmlProvider { * * <ol> * <li><b>SAX layer.</b> {@link #makeParser} hands every {@link XMLReader} Saxon would otherwise use through - * {@link CompositeProvider#configure(XMLReader)}, which routes it to the matching bundled {@link XmlProvider}. DOCTYPE, external entities and XInclude + * {@link XmlFactories#harden(XMLReader)}, which routes it to the matching bundled hardening recipe. DOCTYPE, external entities and XInclude * are refused at parse time.</li> * <li><b>URI-resolution layer.</b> {@link Feature#ALLOWED_PROTOCOLS} is set to the empty string. This blocks XSLT inclusions {@code xsl:include}, * {@code xsl:import}, {@code xsl:source-document}, and the XPath/XSLT functions {@code fn:doc}, {@code fn:document}, {@code fn:unparsed-text}, @@ -68,7 +66,7 @@ private HardenedConfiguration() { @Override public XMLReader makeParser(final String className) throws TransformerFactoryConfigurationError { try { - return CompositeProvider.getInstance().configure(super.makeParser(className)); + return XmlFactories.harden(super.makeParser(className)); } catch (final HardeningException e) { throw new TransformerFactoryConfigurationError(e); } @@ -88,16 +86,7 @@ private static XPathFactory configure(final XPathFactory factory) { } } - /** Default constructor; invoked by {@link java.util.ServiceLoader} and the registry. */ - public SaxonProvider() { - super("net.sf.saxon.TransformerFactoryImpl", - "com.saxonica.config.ProfessionalTransformerFactory", - "com.saxonica.config.EnterpriseTransformerFactory", - "net.sf.saxon.xpath.XPathFactoryImpl"); - } - - @Override - public TransformerFactory configure(final TransformerFactory factory) { + static TransformerFactory configure(final TransformerFactory factory) { try { return SaxonProviderConfigurer.configure(factory); } catch (LinkageError e) { @@ -106,8 +95,7 @@ public TransformerFactory configure(final TransformerFactory factory) { } } - @Override - public XPathFactory configure(final XPathFactory factory) { + static XPathFactory configure(final XPathFactory factory) { try { return SaxonProviderConfigurer.configure(factory); } catch (LinkageError e) { @@ -115,4 +103,7 @@ public XPathFactory configure(final XPathFactory factory) { throw new IllegalStateException(e); } } + + private SaxonProvider() { + } } diff --git a/src/main/java/org/apache/commons/xml/factory/StockJdkProvider.java b/src/main/java/org/apache/commons/xml/factory/StockJdkProvider.java index d5c129a..6d70f5d 100644 --- a/src/main/java/org/apache/commons/xml/factory/StockJdkProvider.java +++ b/src/main/java/org/apache/commons/xml/factory/StockJdkProvider.java @@ -16,6 +16,10 @@ */ package org.apache.commons.xml.factory; +import static org.apache.commons.xml.factory.JaxpSetters.setAttribute; +import static org.apache.commons.xml.factory.JaxpSetters.setFeature; +import static org.apache.commons.xml.factory.JaxpSetters.setProperty; + import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.SAXParserFactory; @@ -27,7 +31,7 @@ import org.xml.sax.XMLReader; /** - * {@link XmlProvider} for the stock JDK's JAXP implementation. + * Hardening recipes for the stock JDK's JAXP implementation. * * <p>This is the internal fork of Apache Xerces, Xalan and friends shipped inside {@code com.sun.org.apache.*} and {@code com.sun.xml.internal.*} packages.</p> * @@ -46,10 +50,8 @@ * * <p>SAX hardening lives in {@link #configure(XMLReader)}: {@link SAXParserFactory} has no property API, so the {@link HardeningSAXParserFactory} wrapper * funnels each produced parser's {@link XMLReader} through that method.</p> - * - * <p>Must be declared {@code public} so {@link java.util.ServiceLoader} can load it from {@code META-INF/services/}.</p> */ -final class StockJdkProvider extends AbstractXmlProvider { +final class StockJdkProvider { /** * {@code jdk.xml.overrideDefaultParser}: pin to the JDK's bundled SAX parser; defense-in-depth against a sysprop swap to a third-party parser. @@ -61,21 +63,7 @@ final class StockJdkProvider extends AbstractXmlProvider { */ private static final String XERCES_LOAD_EXTERNAL_DTD = "http://apache.org/xml/features/nonvalidating/load-external-dtd";; - /** - * Default constructor; invoked by {@link java.util.ServiceLoader} and the registry. - */ - public StockJdkProvider() { - super("com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl", - "com.sun.org.apache.xerces.internal.jaxp.SAXParserFactoryImpl", - "com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl$JAXPSAXParser", - "com.sun.xml.internal.stream.XMLInputFactoryImpl", - "com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl", - "com.sun.org.apache.xpath.internal.jaxp.XPathFactoryImpl", - "com.sun.org.apache.xerces.internal.jaxp.validation.XMLSchemaFactory"); - } - - @Override - public DocumentBuilderFactory configure(final DocumentBuilderFactory factory) { + static DocumentBuilderFactory configure(final DocumentBuilderFactory factory) { // Required: enables the JDK XMLSecurityManager limits. setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); // Let DOCTYPE-only documents parse silently without SSRF: skip the external DTD subset on non-validating parsers. @@ -88,16 +76,14 @@ public DocumentBuilderFactory configure(final DocumentBuilderFactory factory) { return factory; } - @Override - public SAXParserFactory configure(final SAXParserFactory factory) { + static SAXParserFactory configure(final SAXParserFactory factory) { // Required: enables the JDK XMLSecurityManager limits. setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); // The remaining hardening (limits, ACCESS_EXTERNAL_*) lives in the XMLReader configure() because SAXParserFactory has no property API. - return new HardeningSAXParserFactory(factory, this); + return new HardeningSAXParserFactory(factory, StockJdkProvider::configure); } - @Override - public XMLReader configure(final XMLReader reader) { + static XMLReader configure(final XMLReader reader) { // Required: enables the JDK XMLSecurityManager limits on a raw reader (e.g. one Saxon picked). setFeature(reader, XMLConstants.FEATURE_SECURE_PROCESSING, true); // Let DOCTYPE-only documents parse silently without SSRF: skip the external DTD subset on non-validating parsers. @@ -110,8 +96,7 @@ public XMLReader configure(final XMLReader reader) { return reader; } - @Override - public XMLInputFactory configure(final XMLInputFactory factory) { + static XMLInputFactory configure(final XMLInputFactory factory) { // Required: XMLInputFactory rejects FSP, so the limits below are the only way to enable JDK XMLSecurityManager caps on the StAX path. Limits.applyToJdkStax(factory); // Required: XMLInputFactory has no ACCESS_EXTERNAL_* either; an explicit deny-all resolver is the only way to block external entity fetching. @@ -119,8 +104,7 @@ public XMLInputFactory configure(final XMLInputFactory factory) { return factory; } - @Override - public TransformerFactory configure(final TransformerFactory factory) { + static TransformerFactory configure(final TransformerFactory factory) { // Defense-in-depth: pin to the JDK's bundled SAX parser; see FEATURE_OVERRIDE_DEFAULT_PARSER. setFeature(factory, FEATURE_OVERRIDE_DEFAULT_PARSER, false); // Required: enables the JDK XMLSecurityManager limits used by the internal SAX parser. @@ -133,8 +117,7 @@ public TransformerFactory configure(final TransformerFactory factory) { return factory; } - @Override - public XPathFactory configure(final XPathFactory factory) { + static XPathFactory configure(final XPathFactory factory) { // Defense-in-depth: pin to the JDK's bundled SAX parser; see FEATURE_OVERRIDE_DEFAULT_PARSER. setFeature(factory, FEATURE_OVERRIDE_DEFAULT_PARSER, false); // Required: enables JDK XPath limits; XPathFactory has no property API for finer control. @@ -142,8 +125,7 @@ public XPathFactory configure(final XPathFactory factory) { return factory; } - @Override - public SchemaFactory configure(final SchemaFactory factory) { + static SchemaFactory configure(final SchemaFactory factory) { // Defense-in-depth: pin to the JDK's bundled SAX parser; see FEATURE_OVERRIDE_DEFAULT_PARSER. setFeature(factory, FEATURE_OVERRIDE_DEFAULT_PARSER, false); // Required: enables the JDK XMLSecurityManager limits; propagates to Validator and ValidatorHandler. @@ -155,4 +137,7 @@ public SchemaFactory configure(final SchemaFactory factory) { setProperty(factory, XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); return factory; } + + private StockJdkProvider() { + } } diff --git a/src/main/java/org/apache/commons/xml/factory/WoodstoxProvider.java b/src/main/java/org/apache/commons/xml/factory/WoodstoxProvider.java index 4b45128..7e8f843 100644 --- a/src/main/java/org/apache/commons/xml/factory/WoodstoxProvider.java +++ b/src/main/java/org/apache/commons/xml/factory/WoodstoxProvider.java @@ -19,9 +19,9 @@ import javax.xml.stream.XMLInputFactory; /** - * {@link XmlProvider} for the FasterXML Woodstox StAX implementation ({@code com.ctc.wstx:woodstox-core}). + * Hardening recipe for the FasterXML Woodstox StAX implementation ({@code com.ctc.wstx:woodstox-core}). * - * <p>Woodstox is a StAX-only library, so this provider implements only {@link #configure(XMLInputFactory)}.</p> + * <p>Woodstox is a StAX-only library, so this class only handles {@link XMLInputFactory}.</p> * * <p>Hardening recipe used below:</p> * <ul> @@ -32,24 +32,17 @@ * default and resolves external entities/DTDs through whatever {@code XMLResolver} the factory carries; without an explicit deny-all resolver, those * lookups would proceed.</li> * </ul> - * - * <p>Must be declared {@code public} so {@link java.util.ServiceLoader} can load it from {@code META-INF/services/}.</p> */ -final class WoodstoxProvider extends AbstractXmlProvider { - - /** - * Default constructor; invoked by {@link java.util.ServiceLoader} and the registry. - */ - public WoodstoxProvider() { - super("com.ctc.wstx.stax.WstxInputFactory"); - } +final class WoodstoxProvider { - @Override - public XMLInputFactory configure(final XMLInputFactory factory) { + static XMLInputFactory configure(final XMLInputFactory factory) { // Defense-in-depth: align Woodstox's built-in caps with the JDK 25 secure values; Woodstox's own defaults are functional but looser. Limits.applyToWoodstox(factory); // Required: Woodstox resolves external entities and DTDs by default; an explicit deny-all resolver is the only way to block the lookups. factory.setXMLResolver(DenyAllResolver.XML); return factory; } + + private WoodstoxProvider() { + } } diff --git a/src/main/java/org/apache/commons/xml/factory/XercesProvider.java b/src/main/java/org/apache/commons/xml/factory/XercesProvider.java index 0df23ff..3c58fa4 100644 --- a/src/main/java/org/apache/commons/xml/factory/XercesProvider.java +++ b/src/main/java/org/apache/commons/xml/factory/XercesProvider.java @@ -16,6 +16,8 @@ */ package org.apache.commons.xml.factory; +import static org.apache.commons.xml.factory.JaxpSetters.setFeature; + import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; @@ -36,10 +38,10 @@ import org.xml.sax.XMLReader; /** - * {@link XmlProvider} for the external Apache Xerces distribution (the {@code xerces:xercesImpl} artifact). + * Hardening recipes for the external Apache Xerces distribution (the {@code xerces:xercesImpl} artifact). * * <p>Factory classes live in the {@code org.apache.xerces.*} package. External Xerces does not ship a {@code TransformerFactory}, {@code XMLInputFactory} or - * {@code XPathFactory}, so this provider only overrides DOM, SAX and Schema {@code configure} methods.</p> + * {@code XPathFactory}, so this class only handles DOM, SAX and Schema factories.</p> * * <p>Hardening recipe applied to every factory below uses the same building blocks:</p> * <ul> @@ -60,10 +62,8 @@ * </ol> * </li> * </ul> - * - * <p>Must be declared {@code public} so {@link java.util.ServiceLoader} can load it from {@code META-INF/services/}.</p> */ -public final class XercesProvider extends AbstractXmlProvider { +final class XercesProvider { /** * Hardened Xerces {@link DocumentBuilderFactory} wrapper. @@ -173,18 +173,7 @@ private static Object newSecurityManager() { } } - /** - * Default constructor; invoked by {@link java.util.ServiceLoader} and the registry. - */ - public XercesProvider() { - super("org.apache.xerces.jaxp.DocumentBuilderFactoryImpl", - "org.apache.xerces.jaxp.SAXParserFactoryImpl", - "org.apache.xerces.jaxp.SAXParserImpl$JAXPSAXParser", - "org.apache.xerces.jaxp.validation.XMLSchemaFactory"); - } - - @Override - public DocumentBuilderFactory configure(final DocumentBuilderFactory factory) { + static DocumentBuilderFactory configure(final DocumentBuilderFactory factory) { // Required: enables Xerces' built-in SecurityManager (which is what carries the limits). setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); // Let DOCTYPE-only documents parse silently without SSRF: skip the external DTD subset on non-validating parsers. @@ -197,16 +186,14 @@ public DocumentBuilderFactory configure(final DocumentBuilderFactory factory) { return new HardeningDocumentBuilderFactory(factory, DenyAllResolver.ENTITY2); } - @Override - public SAXParserFactory configure(final SAXParserFactory factory) { + static SAXParserFactory configure(final SAXParserFactory factory) { // Required: enables Xerces' built-in SecurityManager (which is what carries the limits). setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); // The remaining hardening (limits, entity resolver) lives in the XMLReader configure() because SAXParserFactory has no property API. - return new HardeningSAXParserFactory(factory, this); + return new HardeningSAXParserFactory(factory, XercesProvider::configure); } - @Override - public XMLReader configure(final XMLReader reader) { + static XMLReader configure(final XMLReader reader) { // Required: enables the JDK XMLSecurityManager limits on a raw reader (e.g. one Saxon picked). setFeature(reader, XMLConstants.FEATURE_SECURE_PROCESSING, true); // Let DOCTYPE-only documents parse silently without SSRF: skip the external DTD subset on non-validating parsers. @@ -222,8 +209,7 @@ public XMLReader configure(final XMLReader reader) { return reader; } - @Override - public SchemaFactory configure(final SchemaFactory factory) { + static SchemaFactory configure(final SchemaFactory factory) { // Required: enables Xerces' built-in SecurityManager (which is what carries the limits). setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); try { @@ -237,4 +223,7 @@ public SchemaFactory configure(final SchemaFactory factory) { // Required: Xerces' Schema does not propagate the SchemaFactory's resolver or security manager to Validator/ValidatorHandler; the wrapper re-installs. return new HardeningSchemaFactory(factory); } + + private XercesProvider() { + } } 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 911b1d8..0519a27 100644 --- a/src/main/java/org/apache/commons/xml/factory/XmlFactories.java +++ b/src/main/java/org/apache/commons/xml/factory/XmlFactories.java @@ -65,16 +65,91 @@ */ public final class XmlFactories { + static DocumentBuilderFactory dispatch(final DocumentBuilderFactory factory) { + switch (factory.getClass().getName()) { + case "com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl": + return StockJdkProvider.configure(factory); + case "org.apache.xerces.jaxp.DocumentBuilderFactoryImpl": + return XercesProvider.configure(factory); + default: + throw noProvider(factory); + } + } + + private static SAXParserFactory dispatch(final SAXParserFactory factory) { + switch (factory.getClass().getName()) { + case "com.sun.org.apache.xerces.internal.jaxp.SAXParserFactoryImpl": + return StockJdkProvider.configure(factory); + case "org.apache.xerces.jaxp.SAXParserFactoryImpl": + return XercesProvider.configure(factory); + default: + throw noProvider(factory); + } + } + + private static XMLInputFactory dispatch(final XMLInputFactory factory) { + switch (factory.getClass().getName()) { + case "com.sun.xml.internal.stream.XMLInputFactoryImpl": + return StockJdkProvider.configure(factory); + case "com.ctc.wstx.stax.WstxInputFactory": + return WoodstoxProvider.configure(factory); + default: + throw noProvider(factory); + } + } + + private static TransformerFactory dispatch(final TransformerFactory factory) { + switch (factory.getClass().getName()) { + case "com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl": + return StockJdkProvider.configure(factory); + case "net.sf.saxon.TransformerFactoryImpl": + case "com.saxonica.config.ProfessionalTransformerFactory": + case "com.saxonica.config.EnterpriseTransformerFactory": + return SaxonProvider.configure(factory); + default: + throw noProvider(factory); + } + } + + private static XPathFactory dispatch(final XPathFactory factory) { + switch (factory.getClass().getName()) { + case "com.sun.org.apache.xpath.internal.jaxp.XPathFactoryImpl": + return StockJdkProvider.configure(factory); + case "net.sf.saxon.xpath.XPathFactoryImpl": + return SaxonProvider.configure(factory); + default: + throw noProvider(factory); + } + } + + private static SchemaFactory dispatch(final SchemaFactory factory) { + switch (factory.getClass().getName()) { + case "com.sun.org.apache.xerces.internal.jaxp.validation.XMLSchemaFactory": + return StockJdkProvider.configure(factory); + case "org.apache.xerces.jaxp.validation.XMLSchemaFactory": + return XercesProvider.configure(factory); + default: + throw noProvider(factory); + } + } + /** * Hardens an existing {@link XMLReader}. * * @param reader the reader to harden; never {@code null}. * @return a hardened reader. - * @throws IllegalStateException if no registered {@link XmlProvider} recognises the reader's concrete class, or if the recognised provider cannot apply the - * hardening settings to it. + * @throws IllegalStateException if the reader's concrete class is not recognised by any bundled hardening recipe, or if the matching recipe cannot apply + * its settings to it. */ public static XMLReader harden(final XMLReader reader) { - return CompositeProvider.getInstance().configure(reader); + switch (reader.getClass().getName()) { + case "com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl$JAXPSAXParser": + return StockJdkProvider.configure(reader); + case "org.apache.xerces.jaxp.SAXParserImpl$JAXPSAXParser": + return XercesProvider.configure(reader); + default: + throw noProvider(reader); + } } /** @@ -85,11 +160,11 @@ public static XMLReader harden(final XMLReader reader) { * encounters an {@code xi:include} element fails.</p> * * @return a hardened factory. - * @throws IllegalStateException if no registered {@link XmlProvider} recognises the underlying JAXP implementation, or if the recognised provider cannot - * apply the hardening settings to it. + * @throws IllegalStateException if the underlying JAXP implementation is not recognised by any bundled hardening recipe, or if the matching recipe cannot + * apply its settings to it. */ public static DocumentBuilderFactory newDocumentBuilderFactory() { - return CompositeProvider.getInstance().configure(DocumentBuilderFactory.newInstance()); + return dispatch(DocumentBuilderFactory.newInstance()); } /** @@ -100,11 +175,11 @@ public static DocumentBuilderFactory newDocumentBuilderFactory() { * an {@code xi:include} element fails.</p> * * @return a hardened factory. - * @throws IllegalStateException if no registered {@link XmlProvider} recognises the underlying JAXP implementation, or if the recognised provider cannot - * apply the hardening settings to it. + * @throws IllegalStateException if the underlying JAXP implementation is not recognised by any bundled hardening recipe, or if the matching recipe cannot + * apply its settings to it. */ public static SAXParserFactory newSAXParserFactory() { - return CompositeProvider.getInstance().configure(SAXParserFactory.newInstance()); + return dispatch(SAXParserFactory.newInstance()); } /** @@ -121,11 +196,11 @@ public static SAXParserFactory newSAXParserFactory() { * resulting {@link javax.xml.validation.Schema}.</p> * * @return a hardened factory. - * @throws IllegalStateException if no registered {@link XmlProvider} recognises the underlying Schema implementation, or if the recognised provider cannot - * apply the hardening settings to it. + * @throws IllegalStateException if the underlying Schema implementation is not recognised by any bundled hardening recipe, or if the matching recipe + * cannot apply its settings to it. */ public static SchemaFactory newSchemaFactory() { - return CompositeProvider.getInstance().configure(SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI)); + return dispatch(SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI)); } /** @@ -138,11 +213,11 @@ public static SchemaFactory newSchemaFactory() { * {@code Transformer.transform(Source, Result)} time.</p> * * @return a hardened factory. - * @throws IllegalStateException if no registered {@link XmlProvider} recognises the underlying TrAX implementation, or if the recognised provider cannot - * apply the hardening settings to it. + * @throws IllegalStateException if the underlying TrAX implementation is not recognised by any bundled hardening recipe, or if the matching recipe cannot + * apply its settings to it. */ public static TransformerFactory newTransformerFactory() { - return CompositeProvider.getInstance().configure(TransformerFactory.newInstance()); + return dispatch(TransformerFactory.newInstance()); } /** @@ -151,11 +226,11 @@ public static TransformerFactory newTransformerFactory() { * <p>The three universal guarantees on {@link XmlFactories} apply; StAX exposes no additional vectors beyond them.</p> * * @return a hardened factory. - * @throws IllegalStateException if no registered {@link XmlProvider} recognises the underlying StAX implementation, or if the recognised provider cannot - * apply the hardening settings to it. + * @throws IllegalStateException if the underlying StAX implementation is not recognised by any bundled hardening recipe, or if the matching recipe cannot + * apply its settings to it. */ public static XMLInputFactory newXMLInputFactory() { - return CompositeProvider.getInstance().configure(XMLInputFactory.newInstance()); + return dispatch(XMLInputFactory.newInstance()); } /** @@ -165,11 +240,15 @@ public static XMLInputFactory newXMLInputFactory() { * {@code unparsed-text()}) are not resolved.</p> * * @return a hardened factory. - * @throws IllegalStateException if no registered {@link XmlProvider} recognises the underlying XPath implementation, or if the recognised provider cannot - * apply the hardening settings to it. + * @throws IllegalStateException if the underlying XPath implementation is not recognised by any bundled hardening recipe, or if the matching recipe cannot + * apply its settings to it. */ public static XPathFactory newXPathFactory() { - return CompositeProvider.getInstance().configure(XPathFactory.newInstance()); + return dispatch(XPathFactory.newInstance()); + } + + private static HardeningException noProvider(final Object factory) { + return new HardeningException("No hardening recipe for JAXP factory class " + factory.getClass().getName()); } private XmlFactories() { diff --git a/src/main/java/org/apache/commons/xml/factory/XmlProvider.java b/src/main/java/org/apache/commons/xml/factory/XmlProvider.java deleted file mode 100644 index d49d30d..0000000 --- a/src/main/java/org/apache/commons/xml/factory/XmlProvider.java +++ /dev/null @@ -1,153 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.commons.xml.factory; - -import javax.xml.parsers.DocumentBuilderFactory; -import javax.xml.parsers.SAXParserFactory; -import javax.xml.stream.XMLInputFactory; -import javax.xml.transform.TransformerFactory; -import javax.xml.validation.SchemaFactory; -import javax.xml.xpath.XPathFactory; - -import org.xml.sax.XMLReader; - -/** - * Service-provider interface implemented by a single JAXP implementation's hardening logic. - * - * <p>Each provider understands one or more concrete factory classes and applies the implementation-specific feature and property settings needed to turn that - * factory into a safe-by-default parser.</p> - * - * <h2>Dispatch</h2> - * - * <p>Providers are consulted in a fixed order by {@link org.apache.commons.xml.factory.XmlFactories}. The first provider whose {@link #supports(Class)} method - * returns {@code true} for a vanilla factory's concrete class is asked to configure that factory. Bundled providers are consulted before - * {@link java.util.ServiceLoader}-discovered providers, so a user-supplied provider cannot shadow hardening for a JAXP implementation that this library ships - * support for.</p> - * - * <h2>Implementor contract</h2> - * - * <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 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> - * </ul> - * - * <p>The default {@code configure} implementations throw {@link UnsupportedOperationException}. A provider should override only the methods corresponding to - * factory types it actually supports.</p> - */ -interface XmlProvider { - - /** - * Indicates whether this provider can harden a factory of the given concrete class. - * - * <p>Implementations typically match on {@link Class#getName()}; they must not return {@code true} for a class they cannot fully configure.</p> - * - * @param factoryClass the concrete factory class to test; never {@code null}. - * @return {@code true} if this provider is responsible for factories of this class. - */ - boolean supports(Class<?> factoryClass); - - /** - * Hardens a {@link DocumentBuilderFactory}. - * - * <p>The default implementation throws {@link UnsupportedOperationException}; providers that match DOM factories must override.</p> - * - * @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 DocumentBuilderFactory configure(final DocumentBuilderFactory factory) { - throw new UnsupportedOperationException(); - } - - /** - * Hardens a {@link SAXParserFactory}. - * - * <p>The default implementation throws {@link UnsupportedOperationException}; providers that match SAX factories must override.</p> - * - * @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 SAXParserFactory configure(final SAXParserFactory factory) { - throw new UnsupportedOperationException(); - } - - /** - * Hardens an {@link XMLReader}. - * - * <p>The default implementation throws {@link UnsupportedOperationException}.</p> - * - * @param reader the vanilla reader to harden; never {@code null}. - * @return the hardened reader (typically the same instance, configured in place). - */ - default XMLReader configure(final XMLReader reader) { - throw new UnsupportedOperationException(); - } - - /** - * Hardens an {@link XMLInputFactory}. - * - * <p>The default implementation throws {@link UnsupportedOperationException}; providers that match StAX input factories must override.</p> - * - * @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 XMLInputFactory configure(final XMLInputFactory factory) { - throw new UnsupportedOperationException(); - } - - /** - * Hardens a {@link TransformerFactory}. - * - * <p>The default implementation throws {@link UnsupportedOperationException}; providers that match TrAX factories must override.</p> - * - * @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 TransformerFactory configure(final TransformerFactory factory) { - throw new UnsupportedOperationException(); - } - - /** - * Hardens an {@link XPathFactory}. - * - * <p>The default implementation throws {@link UnsupportedOperationException}; providers that match XPath factories must override.</p> - * - * @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 XPathFactory configure(final XPathFactory factory) { - throw new UnsupportedOperationException(); - } - - /** - * Hardens a {@link SchemaFactory}. - * - * <p>The default implementation throws {@link UnsupportedOperationException}; providers that match Schema factories must override.</p> - * - * @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 SchemaFactory configure(final SchemaFactory factory) { - throw new UnsupportedOperationException(); - } -} diff --git a/src/site/markdown/index.md b/src/site/markdown/index.md index 4fe6566..f73b356 100644 --- a/src/site/markdown/index.md +++ b/src/site/markdown/index.md @@ -35,10 +35,10 @@ such as standalone Xerces, Woodstox, or Saxon's TrAX, need further configuration library author has no control over which implementation is on the classpath at runtime, so the effective security posture of their code depends on a deployment decision made elsewhere. -This library provides that baseline. Each `XmlFactories` call returns a fresh factory hardened by a provider-specific -SPI, so the returned object behaves the same way security-wise regardless of which JAXP implementation resolved. -Security becomes a property of the call, not of the classpath, and there is one place to update when a new hardening -setting becomes available or a default changes. +This library provides that baseline. Each `XmlFactories` call returns a fresh factory hardened by an +implementation-specific recipe, so the returned object behaves the same way security-wise regardless of which JAXP +implementation resolved. Security becomes a property of the call, not of the classpath, and there is one place to +update when a new hardening setting becomes available or a default changes. ## Usage @@ -59,13 +59,9 @@ stylesheet) is blocked, and DOCTYPE input is rejected wherever the underlying im ### Supported implementations Out of the box the library recognises the stock JDK JAXP implementations, Apache Xerces 2.x, Woodstox, and Saxon-HE. If -a factory resolves to an implementation not covered by any bundled or registered provider, every `XmlFactories` method -throws `IllegalStateException` with a message naming the unsupported class. - -Support for additional implementations can be plugged in by publishing an -`org.apache.commons.xml.factory.spi.XmlProvider` through the standard Java `ServiceLoader` mechanism. Bundled providers -always take precedence, so a third-party provider cannot hijack hardening for a factory class this library already -supports. +a factory resolves to an implementation not covered by any bundled hardening recipe, every `XmlFactories` method throws +`IllegalStateException` with a message naming the unsupported class. Adding support for a new JAXP implementation +requires a code change to this library. **DOM parsing** via `DocumentBuilderFactory`: diff --git a/src/test/java/org/apache/commons/xml/factory/CompositeProviderTest.java b/src/test/java/org/apache/commons/xml/factory/CompositeProviderTest.java deleted file mode 100644 index 62dd43e..0000000 --- a/src/test/java/org/apache/commons/xml/factory/CompositeProviderTest.java +++ /dev/null @@ -1,145 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.commons.xml.factory; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import java.util.Arrays; -import java.util.Collections; - -import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; - -import org.junit.jupiter.api.Test; - -/** - * Verifies composite dispatch without relying on any bundled provider. - * - * <p>Stub providers are injected through the package-private {@link CompositeProvider#CompositeProvider(java.util.List)} constructor so the tests exercise the - * lookup, dispatch and cache logic in isolation from the ServiceLoader-discovered classpath.</p> - */ -class CompositeProviderTest { - - /** - * Stand-in factory whose concrete class is not matched by any bundled provider on the test classpath. - */ - private static final class FakeFactory extends DocumentBuilderFactory { - - @Override - public DocumentBuilder newDocumentBuilder() { - throw new UnsupportedOperationException(); - } - - @Override - public void setAttribute(final String name, final Object value) { - // no-op - } - - @Override - public Object getAttribute(final String name) { - return null; - } - - @Override - public void setFeature(final String name, final boolean value) { - // no-op - } - - @Override - public boolean getFeature(final String name) { - return false; - } - } - - /** Stub that records every {@code supports} and {@code configure} invocation and claims to support all classes. */ - private static final class CountingMatchAll implements XmlProvider { - - int supportsCount; - - int configureCount; - - @Override - public boolean supports(final Class<?> factoryClass) { - supportsCount++; - return true; - } - - @Override - public DocumentBuilderFactory configure(final DocumentBuilderFactory factory) { - configureCount++; - return factory; - } - } - - /** Stub that matches nothing. */ - private static final class MatchesNone implements XmlProvider { - @Override - public boolean supports(final Class<?> factoryClass) { - return false; - } - } - - @Test - void unknownFactoryClassThrowsHardeningException() { - final CompositeProvider composite = new CompositeProvider(Collections.emptyList()); - final FakeFactory factory = new FakeFactory(); - final HardeningException thrown = assertThrows(HardeningException.class, () -> composite.configure(factory)); - assertTrue(thrown.getMessage().contains(FakeFactory.class.getName()), - "Exception message must name the unsupported class: " + thrown.getMessage()); - } - - @Test - void scansProvidersOnEveryInvocation() { - final CountingMatchAll provider = new CountingMatchAll(); - final CompositeProvider composite = new CompositeProvider(Collections.singletonList(provider)); - final FakeFactory factory = new FakeFactory(); - composite.configure(factory); - composite.configure(factory); - assertEquals(2, provider.supportsCount, "supports() must be invoked once per configure call (no caching)"); - assertEquals(2, provider.configureCount); - } - - @Test - void bundledProvidersAreConsultedInDeclaredOrder() { - final CountingMatchAll winner = new CountingMatchAll(); - final CountingMatchAll shadowed = new CountingMatchAll(); - final CompositeProvider composite = new CompositeProvider(Arrays.asList(winner, shadowed)); - final FakeFactory factory = new FakeFactory(); - assertSame(factory, composite.configure(factory)); - assertEquals(1, winner.configureCount); - assertEquals(0, shadowed.configureCount); - } - - @Test - void nonMatchingBundledProviderFallsThroughToNext() { - final XmlProvider noMatch = new MatchesNone(); - final CountingMatchAll match = new CountingMatchAll(); - final CompositeProvider composite = new CompositeProvider(Arrays.asList(noMatch, match)); - composite.configure(new FakeFactory()); - assertEquals(1, match.configureCount); - } - - @Test - void supportsAlwaysReturnsFalse() { - final CompositeProvider composite = new CompositeProvider(Collections.singletonList(new CountingMatchAll())); - assertFalse(composite.supports(FakeFactory.class)); - } -} diff --git a/src/test/java/org/apache/commons/xml/factory/SaxonXPathExternalCallsTest.java b/src/test/java/org/apache/commons/xml/factory/SaxonXPathExternalCallsTest.java index ed46c33..5936919 100644 --- a/src/test/java/org/apache/commons/xml/factory/SaxonXPathExternalCallsTest.java +++ b/src/test/java/org/apache/commons/xml/factory/SaxonXPathExternalCallsTest.java @@ -89,7 +89,7 @@ private static String evaluateAsString(final XPathFactory factory, final String } private static XPathFactory hardenedSaxonXPathFactory() { - return CompositeProvider.getInstance().configure(saxonXPathFactory()); + return SaxonProvider.configure(saxonXPathFactory()); } private static String jsonDocExpression() { diff --git a/src/test/java/org/apache/commons/xml/factory/UnsupportedXmlImplementationTest.java b/src/test/java/org/apache/commons/xml/factory/UnsupportedXmlImplementationTest.java index 7b28903..c50e5af 100644 --- a/src/test/java/org/apache/commons/xml/factory/UnsupportedXmlImplementationTest.java +++ b/src/test/java/org/apache/commons/xml/factory/UnsupportedXmlImplementationTest.java @@ -26,12 +26,12 @@ import org.junit.jupiter.api.Test; /** - * Verifies that an unknown factory class surfaces {@link IllegalStateException} with a message naming the class and pointing at the remediation path. + * Verifies that an unknown factory class surfaces {@link IllegalStateException} with a message naming the class. */ class UnsupportedXmlImplementationTest { /** - * A stand-in factory class whose fully qualified name is not matched by any bundled provider or {@code ServiceLoader} service on the test classpath. + * A stand-in factory class whose fully qualified name is not matched by any bundled hardening recipe. */ public static final class FakeDocumentBuilderFactory extends DocumentBuilderFactory { @@ -62,15 +62,12 @@ public boolean getFeature(final String name) { } @Test - void registryRejectsUnknownFactory() { + void dispatchRejectsUnknownFactory() { final IllegalStateException thrown = assertThrows( IllegalStateException.class, - () -> CompositeProvider.getInstance().configure(new FakeDocumentBuilderFactory())); + () -> XmlFactories.dispatch(new FakeDocumentBuilderFactory())); assertNotNull(thrown.getMessage()); assertTrue(thrown.getMessage().contains(FakeDocumentBuilderFactory.class.getName()), "Exception message must name the unsupported class: " + thrown.getMessage()); - assertTrue(thrown.getMessage().contains("ServiceLoader") - || thrown.getMessage().contains("XmlProvider"), - "Message should hint at the remediation path: " + thrown.getMessage()); } }
