From 86c786c7944c030680b2aa60dc09501f49e2add6 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 28 Aug 2026 10:58:53 +0200 Subject: [PATCH 01/13] Honor jdk.xml.overrideDefaultParser in the hardened source rewrites The wrappers parse every reader-less Source themselves, so the wrapped implementations' internal parsers are never used and the feature was silently ignored. Instead of setting it on the underlying implementation (the XPath hardener no longer does), the TrAX, XPath and schema wrappers now read it at product creation, like the JDK: where the implementation recognizes the feature and its value is false (the JDK's default), the rewrites pin the platform's built-in parser via newDefaultNSInstance; otherwise they keep the pluggable newNSInstance lookup. The javax.xml.parsers.*Factory system properties override the pin, matching the JDK's own internal parser choice. Adds the test-jdk-xerces surefire execution, the only cell pairing the JDK TrAX and XPath implementations with a third-party ServiceLoader parser, where the two rewrite parser families genuinely differ. Assisted-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ErRKq7RUeQ9LGrSboSUyYm --- pom.xml | 32 ++++- src/changes/changes.xml | 1 + .../xml/FallbackIgnoreURIResolver.java | 17 ++- .../xml/HardeningDocumentBuilderFactory.java | 16 +++ .../xml/HardeningSAXParserFactory.java | 28 +++- .../apache/commons/xml/HardeningSchema.java | 15 ++- .../commons/xml/HardeningSchemaFactory.java | 32 +++-- .../commons/xml/HardeningTemplates.java | 18 ++- .../xml/HardeningTemplatesHandler.java | 12 +- .../commons/xml/HardeningTransformer.java | 22 ++- .../xml/HardeningTransformerFactory.java | 69 +++++++--- .../xml/HardeningTransformerHandler.java | 6 +- .../commons/xml/HardeningValidator.java | 16 ++- .../commons/xml/HardeningXMLFilter.java | 2 +- .../commons/xml/HardeningXMLReader.java | 9 ++ .../apache/commons/xml/HardeningXPath.java | 24 ++-- .../commons/xml/HardeningXPathExpression.java | 15 ++- .../commons/xml/HardeningXPathFactory.java | 45 +++---- .../org/apache/commons/xml/SaxonProvider.java | 2 +- src/site/markdown/threat_model.md | 9 +- .../commons/xml/DenyUnresolvedTest.java | 2 +- .../xml/OverrideDefaultParserTest.java | 126 ++++++++++++++++++ .../commons/xml/ShadingFootprintTest.java | 2 +- .../commons/xml/XPathInputSourceTest.java | 2 +- 24 files changed, 419 insertions(+), 103 deletions(-) create mode 100644 src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java diff --git a/pom.xml b/pom.xml index 389972fe..984a6a17 100644 --- a/pom.xml +++ b/pom.xml @@ -146,8 +146,10 @@ limitations under the License. + trax,xpath,schema + + + xerces + xercesImpl + ${commons.xerces.version} + + + + net.sf.saxon:Saxon-HE + + + @@ -548,6 +574,10 @@ limitations under the License. test-xerces none + + test-jdk-xerces + none + diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 769d8635..15662efc 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -43,6 +43,7 @@ The type attribute can be add, update, fix, or remove. Mirror on each factory class every JAXP static factory method, including the Java 9 newDefaultInstance and Java 13 newNSInstance families, all usable on Java 8. Block XInclude (xi:include) href resolution by default, since the JAXP external-access properties do not govern it. + Honor jdk.xml.overrideDefaultParser on TrAX, XPath and schema factories that recognize it: the hardened source rewrites use the platform's built-in parser while the feature is false (the JDK's default), instead of always using the pluggable lookup. Restore the hardened configuration when a factory or parser is reset() instead of reverting to the implementation defaults. Parse a Source opted in by a caller-supplied URIResolver using a hardened parser. Harden the document parse behind the InputSource-taking XPath evaluation entry points. diff --git a/src/main/java/org/apache/commons/xml/FallbackIgnoreURIResolver.java b/src/main/java/org/apache/commons/xml/FallbackIgnoreURIResolver.java index cf3e920c..6c2a38d0 100644 --- a/src/main/java/org/apache/commons/xml/FallbackIgnoreURIResolver.java +++ b/src/main/java/org/apache/commons/xml/FallbackIgnoreURIResolver.java @@ -17,6 +17,7 @@ package org.apache.commons.xml; +import java.util.function.BooleanSupplier; import java.util.function.Supplier; import javax.xml.parsers.DocumentBuilderFactory; @@ -85,15 +86,23 @@ private static Document newEmptyDocument() { */ private final Supplier emptySource; + /** + * Whether the opted-in rewrite should pin the platform's built-in parser; read per resolution so the factory-level floor tracks a later + * {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} toggle. + */ + private final BooleanSupplier useDefaultParser; + /** * Constructs a new resolver. * - * @param delegate the resolver to delegate resolution to; may be {@code null}. - * @param emptySource the empty-{@link Source} supplier for the ignore outcome, or {@code null} for the default empty DOM document. + * @param delegate the resolver to delegate resolution to; may be {@code null}. + * @param emptySource the empty-{@link Source} supplier for the ignore outcome, or {@code null} for the default empty DOM document. + * @param useDefaultParser whether the opted-in rewrite should pin the platform's built-in parser, read at each resolution. */ - FallbackIgnoreURIResolver(final URIResolver delegate, final Supplier emptySource) { + FallbackIgnoreURIResolver(final URIResolver delegate, final Supplier emptySource, final BooleanSupplier useDefaultParser) { this.delegate = delegate; this.emptySource = emptySource != null ? emptySource : () -> new DOMSource(EMPTY_DOCUMENT); + this.useDefaultParser = useDefaultParser; } /** @@ -116,7 +125,7 @@ public Source resolve(final String href, final String base) throws TransformerEx final Source resolved = delegate != null ? delegate.resolve(href, base) : null; if (resolved != null) { // The implementation parses the opted-in handle with an internal reader at its own defaults; the rewrite hands it a hardened reader instead. - return HardeningSAXParserFactory.harden(resolved); + return HardeningSAXParserFactory.harden(resolved, useDefaultParser.getAsBoolean()); } if (HardeningException.throwOnUnresolved()) { throw new TransformerException(HardeningException.forbidden("uri", null, null, href, base)); diff --git a/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java b/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java index 1c16de63..719358ed 100644 --- a/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java @@ -193,6 +193,22 @@ public static DocumentBuilderFactory newNSInstance() { return makeNSAware(newInstance()); } + /** + * Returns the hardened, namespace-aware factory the Source-rewriting wrappers parse with, mirroring the JDK's own internal parser choice: with + * {@code useDefaultParser} the platform's built-in implementation is pinned, unless the {@code javax.xml.parsers.DocumentBuilderFactory} system property + * explicitly requests another implementation, which overrides the pin like it does inside the JDK. + * + * @param useDefaultParser whether {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} on the originating factory asks for the platform's built-in + * implementation. + * @return A hardened, namespace-aware factory. + * @throws IllegalStateException Thrown if a required hardening setting cannot be applied to the underlying implementation. + * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service configuration error} or if the + * implementation is not available or cannot be instantiated. + */ + static DocumentBuilderFactory newNSInstance(final boolean useDefaultParser) { + return useDefaultParser && System.getProperty("javax.xml.parsers.DocumentBuilderFactory") == null ? newDefaultNSInstance() : newNSInstance(); + } + /** * Returns a new, hardened, namespace-aware {@link DocumentBuilderFactory} of the given implementation class, enabling namespace awareness on * {@link #newInstance(String, ClassLoader)}, the behavior {@code DocumentBuilderFactory.newNSInstance(String, ClassLoader)} (Java 13 or later) is specified diff --git a/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java b/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java index 9101acd1..1e1d5599 100644 --- a/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java @@ -64,6 +64,12 @@ public final class HardeningSAXParserFactory { /** Class name of the JDK's built-in default implementation, the Java 8 fallback for {@link #newDefaultInstance()}. */ private static final String JDK_SAX_PARSER_FACTORY = "com.sun.org.apache.xerces.internal.jaxp.SAXParserFactoryImpl"; + /** + * The JDK feature governing whether an implementation's internal parser lookup may resolve a third-party parser. The hardening wrappers parse every source + * themselves, so instead of configuring the implementation the TrAX, XPath and schema wrappers read this feature and pick the rewrite parser accordingly. + */ + static final String OVERRIDE_DEFAULT_PARSER = "jdk.xml.overrideDefaultParser"; + private static final MethodHandle NEW_DEFAULT_INSTANCE = findStatic("newDefaultInstance", MethodType.methodType(SAXParserFactory.class)); private static MethodHandle findStatic(final String name, final MethodType type) { @@ -114,16 +120,17 @@ static SAXParserFactory harden(final SAXParserFactory factory) { * as-is. Used by the TrAX and schema wrappers to route every source they parse through the SAX hardening path. *

* - * @param source the source to harden; never {@code null}. + * @param source the source to harden; never {@code null}. + * @param useDefaultParser whether {@value #OVERRIDE_DEFAULT_PARSER} on the originating factory asks for the platform's built-in parser. * @return a hardened source. * @throws TransformerConfigurationException if a hardened reader cannot be obtained. * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service * configuration error} or if the implementation is not available or cannot be instantiated. */ - static Source harden(final Source source) throws TransformerConfigurationException { + static Source harden(final Source source, final boolean useDefaultParser) throws TransformerConfigurationException { if (source instanceof StreamSource || source instanceof SAXSource && ((SAXSource) source).getXMLReader() == null) { final InputSource inputSource = SAXSource.sourceToInputSource(source); - return inputSource == null ? source : new SAXSource(newHardenedReader(), inputSource); + return inputSource == null ? source : new SAXSource(newHardenedReader(useDefaultParser), inputSource); } return source; } @@ -209,16 +216,25 @@ public static SAXParserFactory newDefaultNSInstance() { } /** - * Creates a new hardened, namespace-aware {@link XMLReader} for the TrAX wrappers to parse sources with. + * Creates a new hardened, namespace-aware {@link XMLReader} for the TrAX, XPath and schema wrappers to parse sources with. + *

+ * Mirrors the JDK's own internal parser choice: with {@code useDefaultParser} the platform's built-in parser is pinned, unless the + * {@code javax.xml.parsers.SAXParserFactory} system property explicitly requests another implementation, which overrides the pin like it does inside the + * JDK. + *

* + * @param useDefaultParser whether {@value #OVERRIDE_DEFAULT_PARSER} on the originating factory asks for the platform's built-in parser. * @return a hardened reader. * @throws TransformerConfigurationException if a hardened reader cannot be obtained. * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service * configuration error} or if the implementation is not available or cannot be instantiated. */ - static XMLReader newHardenedReader() throws TransformerConfigurationException { + static XMLReader newHardenedReader(final boolean useDefaultParser) throws TransformerConfigurationException { try { - return newNSInstance().newSAXParser().getXMLReader(); + final SAXParserFactory factory = useDefaultParser && System.getProperty("javax.xml.parsers.SAXParserFactory") == null + ? newDefaultNSInstance() + : newNSInstance(); + return factory.newSAXParser().getXMLReader(); } catch (final ParserConfigurationException | SAXException e) { throw new TransformerConfigurationException("Failed to obtain a hardened XMLReader for source parsing", e); } diff --git a/src/main/java/org/apache/commons/xml/HardeningSchema.java b/src/main/java/org/apache/commons/xml/HardeningSchema.java index e92374c1..c45cbd5b 100644 --- a/src/main/java/org/apache/commons/xml/HardeningSchema.java +++ b/src/main/java/org/apache/commons/xml/HardeningSchema.java @@ -25,7 +25,7 @@ /** * {@link Schema} wrapper that hardens every {@link Validator} and {@link ValidatorHandler} the inner Schema produces: each {@link Validator} is wrapped in - * {@link HardeningValidator} (which rewrites the Source through {@link HardeningSAXParserFactory#harden(javax.xml.transform.Source)} and installs the resolver + * {@link HardeningValidator} (which rewrites the Source through {@link HardeningSAXParserFactory#harden(javax.xml.transform.Source, boolean)} and installs the resolver * floor), and each {@link ValidatorHandler} is wrapped in a {@link HardeningValidatorHandler} that keeps the same ignore-all resolver floor so * {@code xsi:schemaLocation} is not resolved during SAX-driven validation. */ @@ -33,19 +33,26 @@ final class HardeningSchema extends Schema { private final Schema delegate; + /** + * Snapshot of the factory's {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} outcome, carried onto every produced Validator. + */ + final boolean useDefaultParser; + /** * Constructs a new instance. * - * @param delegate the delegate to wrap; must not be {@code null}. + * @param delegate the delegate to wrap; must not be {@code null}. + * @param useDefaultParser whether the produced Validators' source rewrites should pin the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ - HardeningSchema(final Schema delegate) { + HardeningSchema(final Schema delegate, final boolean useDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); + this.useDefaultParser = useDefaultParser; } @Override public Validator newValidator() { - return new HardeningValidator(delegate.newValidator()); + return new HardeningValidator(delegate.newValidator(), useDefaultParser); } @Override diff --git a/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java b/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java index 539b8cfa..44bf12f6 100644 --- a/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java @@ -80,7 +80,7 @@ private static MethodHandle findNewDefaultInstance() { *

Unlike the other factory types there is no per-implementation branching and no feature or limit configuration on the factory itself: schema compilation * and validation reach external resources only through the resolver hook, so wrapping the factory with a non-removable ignore-all resolver floor is enough on * every implementation. The reader used to parse schema and instance documents is hardened separately, through - * {@link HardeningSAXParserFactory#harden(javax.xml.transform.Source)}.

+ * {@link HardeningSAXParserFactory#harden(javax.xml.transform.Source, boolean)}.

* * @param factory the factory to harden; never {@code null}. * @return a hardened factory. @@ -160,14 +160,14 @@ private HardeningSchemaFactory() { *
    *
  1. {@link HardeningSchemaFactory} installs an ignore-all {@link FallbackIgnoreLSResourceResolver} floor on the factory (blocking * {@code xs:import}/{@code xs:include}/{@code xs:redefine} at compile time) and rewrites the Source on every {@code newSchema(Source[])} entry point - * through {@link HardeningSAXParserFactory#harden(Source)}.
    2. + * through {@link HardeningSAXParserFactory#harden(Source, boolean)}. *
  2. {@link HardeningSchema} wraps every Validator/ValidatorHandler the inner Schema produces and re-installs the floor on each (blocking * {@code xsi:schemaLocation} at validation time), since neither the JDK nor Xerces reliably propagates it through {@code Schema}.
    3. *
  3. {@link HardeningValidator} rewrites the Source on every {@link Validator#validate(Source)} call.
    4. *
* *

- * The hardened reader supplied by {@link HardeningSAXParserFactory#harden(Source)} already carries {@code FEATURE_SECURE_PROCESSING} and the processing limits, so a + * The hardened reader supplied by {@link HardeningSAXParserFactory#harden(Source, boolean)} already carries {@code FEATURE_SECURE_PROCESSING} and the processing limits, so a * DOCTYPE, external entity or Billion Laughs payload in the schema or instance document is bounded there rather than on this factory. The JAXP 1.5 * {@code ACCESS_EXTERNAL_*} properties are deliberately not set: the resolver floor already blocks the same fetches on every implementation, and the JDK 8 * {@code SchemaFactory} has a bug whereby those properties keep blocking even when a caller's own resolver would grant the access. The floor is a non-removable @@ -180,7 +180,7 @@ private HardeningSchemaFactory() { private static final class Wrapper extends SchemaFactory { /** - * Hardens every schema source through {@link HardeningSAXParserFactory#harden(Source)}. + * Hardens every schema source through {@link HardeningSAXParserFactory#harden(Source, boolean)}. * * @param schemas the schema sources to harden; must not be {@code null}. * @return a new array of hardened sources. @@ -188,11 +188,12 @@ private static final class Wrapper extends SchemaFactory { * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service * configuration error} or if the implementation is not available or cannot be instantiated. */ - private static Source[] harden(final Source[] schemas) throws SAXException { + private Source[] harden(final Source[] schemas) throws SAXException { final Source[] hardened = new Source[schemas.length]; + final boolean useDefaultParser = useDefaultParser(); try { for (int i = 0; i < schemas.length; i++) { - hardened[i] = HardeningSAXParserFactory.harden(schemas[i]); + hardened[i] = HardeningSAXParserFactory.harden(schemas[i], useDefaultParser); } } catch (final TransformerConfigurationException e) { throw new SAXException("Failed to harden schema source", e); @@ -244,7 +245,7 @@ public boolean isSchemaLanguageSupported(final String schemaLanguage) { @Override public Schema newSchema() throws SAXException { - return new HardeningSchema(delegate.newSchema()); + return new HardeningSchema(delegate.newSchema(), useDefaultParser()); } /** @@ -255,7 +256,22 @@ public Schema newSchema() throws SAXException { */ @Override public Schema newSchema(final Source[] schemas) throws SAXException { - return new HardeningSchema(delegate.newSchema(harden(schemas))); + return new HardeningSchema(delegate.newSchema(harden(schemas)), useDefaultParser()); + } + + /** + * Whether {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} on the delegate currently asks for the platform's built-in parser; the + * implementation's internal parsers are never used, so the feature instead selects which hardened parser family performs the source rewrites. + * An implementation that does not recognize the feature reports it by exception and keeps the pluggable lookup. + * + * @return Whether the rewrites should pin the platform's built-in parser. + */ + private boolean useDefaultParser() { + try { + return !delegate.getFeature(HardeningSAXParserFactory.OVERRIDE_DEFAULT_PARSER); + } catch (final SAXNotRecognizedException | SAXNotSupportedException e) { + return false; + } } @Override diff --git a/src/main/java/org/apache/commons/xml/HardeningTemplates.java b/src/main/java/org/apache/commons/xml/HardeningTemplates.java index 56b339ee..667f984b 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTemplates.java +++ b/src/main/java/org/apache/commons/xml/HardeningTemplates.java @@ -51,18 +51,26 @@ final class HardeningTemplates implements Templates { */ private final Supplier emptySource; + /** + * Snapshot of the factory's {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} outcome, carried onto every produced Transformer and self-provisioned + * filter reader. + */ + final boolean useDefaultParser; + /** * Constructs a new instance. * - * @param delegate the delegate to wrap; must not be {@code null}. - * @param uriResolver the compile-time URIResolver snapshot to restore onto Transformers produced from the compiled Templates; may be {@code null}. - * @param emptySource the empty-{@link Source} supplier for the produced Transformers + * @param delegate the delegate to wrap; must not be {@code null}. + * @param uriResolver the compile-time URIResolver snapshot to restore onto Transformers produced from the compiled Templates; may be {@code null}. + * @param emptySource the empty-{@link Source} supplier for the produced Transformers + * @param useDefaultParser whether the produced Transformers' source rewrites should pin the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ - HardeningTemplates(final Templates delegate, final URIResolver uriResolver, final Supplier emptySource) { + HardeningTemplates(final Templates delegate, final URIResolver uriResolver, final Supplier emptySource, final boolean useDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); this.uriResolver = uriResolver; this.emptySource = emptySource; + this.useDefaultParser = useDefaultParser; } /** @@ -85,6 +93,6 @@ public Transformer newTransformer() throws TransformerConfigurationException { if (transformer == null) { return null; } - return new HardeningTransformer(transformer, uriResolver, emptySource); + return new HardeningTransformer(transformer, uriResolver, emptySource, useDefaultParser); } } diff --git a/src/main/java/org/apache/commons/xml/HardeningTemplatesHandler.java b/src/main/java/org/apache/commons/xml/HardeningTemplatesHandler.java index 814d9671..25e63030 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTemplatesHandler.java +++ b/src/main/java/org/apache/commons/xml/HardeningTemplatesHandler.java @@ -51,18 +51,26 @@ final class HardeningTemplatesHandler implements TemplatesHandler { */ private final Supplier emptySource; + /** + * Snapshot of the factory's {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} outcome, carried onto the produced Templates. + */ + private final boolean useDefaultParser; + /** * Constructs a new instance. * * @param delegate the delegate to wrap; must not be {@code null}. * @param uriResolver the compile-time URIResolver snapshot to restore onto Transformers produced from the compiled Templates; may be {@code null}. * @param emptySource the empty-{@link Source} supplier for the produced Templates; may be {@code null} for the default empty DOM document. + * @param useDefaultParser whether the produced Templates' source rewrites should pin the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ - HardeningTemplatesHandler(final TemplatesHandler delegate, final URIResolver uriResolver, final Supplier emptySource) { + HardeningTemplatesHandler(final TemplatesHandler delegate, final URIResolver uriResolver, final Supplier emptySource, + final boolean useDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); this.uriResolver = uriResolver; this.emptySource = emptySource; + this.useDefaultParser = useDefaultParser; } @Override @@ -94,7 +102,7 @@ public String getSystemId() { public Templates getTemplates() { // Null before the stylesheet's endDocument (and on a failed compile in some implementations). final Templates templates = delegate.getTemplates(); - return templates == null ? null : new HardeningTemplates(templates, uriResolver, emptySource); + return templates == null ? null : new HardeningTemplates(templates, uriResolver, emptySource, useDefaultParser); } @Override diff --git a/src/main/java/org/apache/commons/xml/HardeningTransformer.java b/src/main/java/org/apache/commons/xml/HardeningTransformer.java index 39669d65..381c3b75 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTransformer.java +++ b/src/main/java/org/apache/commons/xml/HardeningTransformer.java @@ -32,7 +32,7 @@ /** * {@link Transformer} wrapper that rewrites the Source on every {@link Transformer#transform(Source, Result)} call through - * {@link HardeningSAXParserFactory#harden(Source)} before delegating, and keeps an ignore-all {@link URIResolver} floor so runtime {@code document()} calls a + * {@link HardeningSAXParserFactory#harden(Source, boolean)} before delegating, and keeps an ignore-all {@link URIResolver} floor so runtime {@code document()} calls a * caller does not resolve return empty rather than being fetched. *

* The floor is installed on the delegate transformer at construction, seeded with the factory's compile-time resolver; {@link #setURIResolver(URIResolver)} @@ -51,18 +51,26 @@ final class HardeningTransformer extends Transformer { private final FallbackIgnoreURIResolver floor; + /** + * Snapshot of the factory's {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} outcome at creation, like the JDK copies the feature onto the + * transformers it creates. + */ + private final boolean useDefaultParser; + /** * Constructs a new instance. * - * @param delegate the delegate to wrap; must not be {@code null}. - * @param uriResolver the compile-time URIResolver snapshot to seed the floor with; may be {@code null}. - * @param emptySource the empty-{@link Source} supplier for the produced Transformers; {@code null} for the default empty DOM document. + * @param delegate the delegate to wrap; must not be {@code null}. + * @param uriResolver the compile-time URIResolver snapshot to seed the floor with; may be {@code null}. + * @param emptySource the empty-{@link Source} supplier for the produced Transformers; {@code null} for the default empty DOM document. + * @param useDefaultParser whether the source rewrites should pin the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ - HardeningTransformer(final Transformer delegate, final URIResolver uriResolver, final Supplier emptySource) { + HardeningTransformer(final Transformer delegate, final URIResolver uriResolver, final Supplier emptySource, final boolean useDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); this.uriResolver = uriResolver; - this.floor = new FallbackIgnoreURIResolver(uriResolver, emptySource); + this.useDefaultParser = useDefaultParser; + this.floor = new FallbackIgnoreURIResolver(uriResolver, emptySource, () -> useDefaultParser); delegate.setURIResolver(floor); } @@ -137,7 +145,7 @@ public void setURIResolver(final URIResolver resolver) { @Override public void transform(final Source xmlSource, final Result outputTarget) throws TransformerException { try { - delegate.transform(HardeningSAXParserFactory.harden(xmlSource), outputTarget); + delegate.transform(HardeningSAXParserFactory.harden(xmlSource, useDefaultParser), outputTarget); } catch (final TransformerConfigurationException e) { throw new TransformerException(e); } diff --git a/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java b/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java index 7f030c56..90c7e654 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java @@ -103,6 +103,9 @@ private static MethodHandle findNewDefaultInstance() { * difference is the empty-{@link Source} shape the floor returns, {@code EmptySource} for Saxon rather than the default empty DOM document. *

  • FSP ({@link XMLConstants#FEATURE_SECURE_PROCESSING}): required. On XSLTC it enables the runtime evaluator limits; on Xalan it disables * reflection-based extension functions.
    • + *
  • FODP ({@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER}): read, not set. The implementation's internal parsers are never + * used (the wrapper rewrites every source), so on implementations that recognize the feature its value selects which hardened parser family performs + * the rewrites: the platform's built-in parser when {@code false} (the JDK's default), the pluggable lookup when {@code true} or unrecognized.
    • *
  • {@link FallbackIgnoreURIResolver} floor: required. An ignore-all {@link URIResolver} floor, installed by * the nested wrapper and carried onto every produced {@link Transformer}, resolves {@code xsl:import}/{@code xsl:include} at compile * time and {@code document()} at runtime to an empty document, the one channel both XSLTC and Xalan route through. A caller-set {@link URIResolver} is @@ -195,7 +198,7 @@ private HardeningTransformerFactory() { } /** - * {@link TransformerFactory} wrapper that rewrites every Source-taking entry point through {@link HardeningSAXParserFactory#harden(Source)} before + * {@link TransformerFactory} wrapper that rewrites every Source-taking entry point through {@link HardeningSAXParserFactory#harden(Source, boolean)} before * delegating. * *

    Used by providers whose underlying TrAX implementation pulls a new {@code SAXParserFactory.newInstance()} for any Source that is not already a @@ -230,21 +233,21 @@ private static final class Wrapper extends SAXTransformerFactory { /** * Parses a reader-less source into a DOM through a hardened, namespace-aware {@link javax.xml.parsers.DocumentBuilder} and returns a {@link DOMSource} * carrying its system id, so the consumer walks the tree instead of provisioning its own reader. Any other source is left to - * {@link HardeningSAXParserFactory#harden(Source)}. + * {@link HardeningSAXParserFactory#harden(Source, boolean)}. * * @param source The source to scan for an associated stylesheet. - * @return A {@link DOMSource} for a reader-less source, otherwise the result of {@link HardeningSAXParserFactory#harden(Source)}. + * @return A {@link DOMSource} for a reader-less source, otherwise the result of {@link HardeningSAXParserFactory#harden(Source, boolean)}. * @throws TransformerConfigurationException if the source cannot be parsed. * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service * configuration error} or if the implementation is not available or cannot be instantiated. * @throws HardeningException Thrown if a (non-Andoid) factory cannot support the secure processing feature {@link XMLConstants#FEATURE_SECURE_PROCESSING}. */ - private static Source hardenSourceToDom(final Source source) throws TransformerConfigurationException { + private Source hardenSourceToDom(final Source source) throws TransformerConfigurationException { if (source instanceof StreamSource || source instanceof SAXSource && ((SAXSource) source).getXMLReader() == null) { final InputSource inputSource = SAXSource.sourceToInputSource(source); if (inputSource != null) { try { - final DocumentBuilderFactory factory = HardeningDocumentBuilderFactory.newNSInstance(); + final DocumentBuilderFactory factory = HardeningDocumentBuilderFactory.newNSInstance(useDefaultParser()); final Document document = factory.newDocumentBuilder().parse(inputSource); return new DOMSource(document, inputSource.getSystemId()); } catch (final ParserConfigurationException | SAXException | IOException e) { @@ -252,7 +255,7 @@ private static Source hardenSourceToDom(final Source source) throws TransformerC } } } - return HardeningSAXParserFactory.harden(source); + return HardeningSAXParserFactory.harden(source, useDefaultParser()); } /** @@ -265,6 +268,24 @@ private static boolean isXalan(final SAXTransformerFactory factory) { return factory.getClass().getName().startsWith("org.apache.xalan."); } + /** + * Whether the delegate recognizes {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER}, probed with a same-value {@code setFeature}: + * {@code TransformerFactory.getFeature} cannot signal an unrecognized name (it returns {@code false}), while every implementation rejects a + * {@code setFeature} for a name it does not support (Xalan with {@link TransformerConfigurationException}, Saxon with its own unchecked exception). + * + * @param factory The delegate factory. + * @return Whether the delegate recognizes the feature. + */ + private static boolean probeOverrideDefaultParser(final SAXTransformerFactory factory) { + try { + factory.setFeature(HardeningSAXParserFactory.OVERRIDE_DEFAULT_PARSER, + factory.getFeature(HardeningSAXParserFactory.OVERRIDE_DEFAULT_PARSER)); + return true; + } catch (final Exception e) { + return false; + } + } + private static Templates unwrap(final Templates templates) { return templates instanceof HardeningTemplates ? ((HardeningTemplates) templates).getDelegate() : templates; } @@ -278,6 +299,9 @@ private static Templates unwrap(final Templates templates) { private final FallbackIgnoreURIResolver floor; + /** Whether the delegate recognizes {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER}; its value is read per created product, like the JDK. */ + private final boolean supportsOverrideDefaultParser; + /** * Constructs a new instance. * @@ -299,7 +323,8 @@ private Wrapper(final SAXTransformerFactory delegate) { private Wrapper(final SAXTransformerFactory delegate, final Supplier emptySource) { this.delegate = Objects.requireNonNull(delegate, "delegate"); this.emptySource = emptySource; - this.floor = new FallbackIgnoreURIResolver(null, emptySource); + this.supportsOverrideDefaultParser = probeOverrideDefaultParser(delegate); + this.floor = new FallbackIgnoreURIResolver(null, emptySource, this::useDefaultParser); // Compile-time block for xsl:import/xsl:include and document(); a caller-set resolver is routed through the floor rather than replacing it. delegate.setURIResolver(floor); } @@ -314,7 +339,7 @@ private Wrapper(final SAXTransformerFactory delegate, final Supplier emp public Source getAssociatedStylesheet(final Source source, final String media, final String title, final String charset) throws TransformerConfigurationException { // Xalan's getAssociatedStylesheet drops a SAXSource's reader and self-provisions its own to scan for xml-stylesheet PIs (XALANJ-2849). - final Source hardened = isXalan(delegate) ? hardenSourceToDom(source) : HardeningSAXParserFactory.harden(source); + final Source hardened = isXalan(delegate) ? hardenSourceToDom(source) : HardeningSAXParserFactory.harden(source, useDefaultParser()); return delegate.getAssociatedStylesheet(hardened, media, title, charset); } @@ -339,7 +364,7 @@ public URIResolver getURIResolver() { } private TransformerHandler hardenHandler(final TransformerHandler handler) { - return handler == null ? null : new HardeningTransformerHandler(handler, getURIResolver(), emptySource); + return handler == null ? null : new HardeningTransformerHandler(handler, getURIResolver(), emptySource, useDefaultParser()); } /** @@ -350,21 +375,21 @@ private TransformerHandler hardenHandler(final TransformerHandler handler) { */ @Override public Templates newTemplates(final Source source) throws TransformerConfigurationException { - final Templates templates = delegate.newTemplates(HardeningSAXParserFactory.harden(source)); - return templates == null ? null : new HardeningTemplates(templates, getURIResolver(), emptySource); + final Templates templates = delegate.newTemplates(HardeningSAXParserFactory.harden(source, useDefaultParser())); + return templates == null ? null : new HardeningTemplates(templates, getURIResolver(), emptySource, useDefaultParser()); } @Override public TemplatesHandler newTemplatesHandler() throws TransformerConfigurationException { final TemplatesHandler handler = delegate.newTemplatesHandler(); - return handler == null ? null : new HardeningTemplatesHandler(handler, getURIResolver(), emptySource); + return handler == null ? null : new HardeningTemplatesHandler(handler, getURIResolver(), emptySource, useDefaultParser()); } @Override public Transformer newTransformer() throws TransformerConfigurationException { // Identity transformer: still parses runtime sources, so wrap it to harden Transformer.transform(Source, Result). final Transformer transformer = delegate.newTransformer(); - return transformer == null ? null : new HardeningTransformer(transformer, getURIResolver(), emptySource); + return transformer == null ? null : new HardeningTransformer(transformer, getURIResolver(), emptySource, useDefaultParser()); } /** @@ -375,8 +400,8 @@ public Transformer newTransformer() throws TransformerConfigurationException { */ @Override public Transformer newTransformer(final Source source) throws TransformerConfigurationException { - final Transformer transformer = delegate.newTransformer(HardeningSAXParserFactory.harden(source)); - return transformer == null ? null : new HardeningTransformer(transformer, getURIResolver(), emptySource); + final Transformer transformer = delegate.newTransformer(HardeningSAXParserFactory.harden(source, useDefaultParser())); + return transformer == null ? null : new HardeningTransformer(transformer, getURIResolver(), emptySource, useDefaultParser()); } @Override @@ -392,7 +417,7 @@ public TransformerHandler newTransformerHandler() throws TransformerConfiguratio */ @Override public TransformerHandler newTransformerHandler(final Source source) throws TransformerConfigurationException { - return hardenHandler(delegate.newTransformerHandler(HardeningSAXParserFactory.harden(source))); + return hardenHandler(delegate.newTransformerHandler(HardeningSAXParserFactory.harden(source, useDefaultParser()))); } @Override @@ -416,7 +441,7 @@ public XMLFilter newXMLFilter(final Source source) throws TransformerConfigurati @Override public XMLFilter newXMLFilter(final Templates templates) throws TransformerConfigurationException { return new HardeningXMLFilter(templates instanceof HardeningTemplates ? (HardeningTemplates) templates - : new HardeningTemplates(templates, getURIResolver(), emptySource)); + : new HardeningTemplates(templates, getURIResolver(), emptySource, useDefaultParser())); } @Override @@ -439,5 +464,15 @@ public void setFeature(final String name, final boolean value) throws Transforme public void setURIResolver(final URIResolver resolver) { floor.setDelegate(resolver); } + + /** + * Whether {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} on the delegate currently asks for the platform's built-in parser; the + * implementation's internal parsers are never used, so the feature instead selects which hardened parser family performs the source rewrites. + * + * @return Whether the rewrites should pin the platform's built-in parser. + */ + private boolean useDefaultParser() { + return supportsOverrideDefaultParser && !delegate.getFeature(HardeningSAXParserFactory.OVERRIDE_DEFAULT_PARSER); + } } } diff --git a/src/main/java/org/apache/commons/xml/HardeningTransformerHandler.java b/src/main/java/org/apache/commons/xml/HardeningTransformerHandler.java index 69000e98..c6223f01 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTransformerHandler.java +++ b/src/main/java/org/apache/commons/xml/HardeningTransformerHandler.java @@ -54,11 +54,13 @@ final class HardeningTransformerHandler implements TransformerHandler { * @param delegate the delegate to wrap; must not be {@code null}. * @param uriResolver the compile-time URIResolver snapshot to restore onto the live transformer; may be {@code null}. * @param emptySource the empty-{@link Source} supplier for the produced Transformer's floor; {@code null} means the default empty DOM. + * @param useDefaultParser whether the live transformer's source rewrites should pin the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ - HardeningTransformerHandler(final TransformerHandler delegate, final URIResolver uriResolver, final Supplier emptySource) { + HardeningTransformerHandler(final TransformerHandler delegate, final URIResolver uriResolver, final Supplier emptySource, + final boolean useDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); - this.transformer = new HardeningTransformer(delegate.getTransformer(), uriResolver, emptySource); + this.transformer = new HardeningTransformer(delegate.getTransformer(), uriResolver, emptySource, useDefaultParser); } @Override diff --git a/src/main/java/org/apache/commons/xml/HardeningValidator.java b/src/main/java/org/apache/commons/xml/HardeningValidator.java index 92d06fc8..494f2b73 100644 --- a/src/main/java/org/apache/commons/xml/HardeningValidator.java +++ b/src/main/java/org/apache/commons/xml/HardeningValidator.java @@ -34,7 +34,7 @@ /** * {@link Validator} wrapper that rewrites the Source on every {@link Validator#validate(Source)} and {@link Validator#validate(Source, Result)} call through - * {@link HardeningSAXParserFactory#harden(Source)} before delegating, and keeps an ignore-all {@link LSResourceResolver} floor so {@code xsi:schemaLocation} is not resolved at + * {@link HardeningSAXParserFactory#harden(Source, boolean)} before delegating, and keeps an ignore-all {@link LSResourceResolver} floor so {@code xsi:schemaLocation} is not resolved at * validation time. {@link #reset()} re-establishes the bare ignore-all floor, matching the just-constructed state. */ final class HardeningValidator extends Validator { @@ -43,14 +43,22 @@ final class HardeningValidator extends Validator { private final FallbackIgnoreLSResourceResolver floor = new FallbackIgnoreLSResourceResolver(null); + /** + * Snapshot of the factory's {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} outcome at creation, like the JDK copies the feature onto its + * validators. + */ + private final boolean useDefaultParser; + /** * Constructs a new instance. * - * @param delegate the delegate to wrap; must not be {@code null}. + * @param delegate the delegate to wrap; must not be {@code null}. + * @param useDefaultParser whether the source rewrites should pin the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ - HardeningValidator(final Validator delegate) { + HardeningValidator(final Validator delegate, final boolean useDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); + this.useDefaultParser = useDefaultParser; // Block xsi:schemaLocation resolution; neither the JDK nor Xerces reliably propagates the factory's resolver to its Validators. The floor is a // non-removable lower bound: a caller opts specific lookups in by setting their own resolver, but cannot drop the ignore-all block. delegate.setResourceResolver(floor); @@ -113,7 +121,7 @@ public void setResourceResolver(final LSResourceResolver resourceResolver) { @Override public void validate(final Source source, final Result result) throws SAXException, IOException { try { - delegate.validate(HardeningSAXParserFactory.harden(source), result); + delegate.validate(HardeningSAXParserFactory.harden(source, useDefaultParser), result); } catch (final TransformerConfigurationException e) { throw new SAXException("Failed to harden source for validation", e); } diff --git a/src/main/java/org/apache/commons/xml/HardeningXMLFilter.java b/src/main/java/org/apache/commons/xml/HardeningXMLFilter.java index 1a8533e8..4dd42990 100644 --- a/src/main/java/org/apache/commons/xml/HardeningXMLFilter.java +++ b/src/main/java/org/apache/commons/xml/HardeningXMLFilter.java @@ -70,7 +70,7 @@ public void parse(final InputSource input) throws SAXException, IOException { } if (getParent() == null) { try { - setParent(HardeningSAXParserFactory.newHardenedReader()); + setParent(HardeningSAXParserFactory.newHardenedReader(templates.useDefaultParser)); } catch (final TransformerException e) { throw new SAXException(e); } diff --git a/src/main/java/org/apache/commons/xml/HardeningXMLReader.java b/src/main/java/org/apache/commons/xml/HardeningXMLReader.java index 856aa673..9eae201d 100644 --- a/src/main/java/org/apache/commons/xml/HardeningXMLReader.java +++ b/src/main/java/org/apache/commons/xml/HardeningXMLReader.java @@ -63,6 +63,15 @@ public ContentHandler getContentHandler() { return delegate.getContentHandler(); } + /** + * Gets the wrapped reader, so tests can observe which parser implementation a rewrite picked. + * + * @return The wrapped reader. + */ + XMLReader getDelegate() { + return delegate; + } + @Override public DTDHandler getDTDHandler() { return delegate.getDTDHandler(); diff --git a/src/main/java/org/apache/commons/xml/HardeningXPath.java b/src/main/java/org/apache/commons/xml/HardeningXPath.java index c843fea6..07cda123 100644 --- a/src/main/java/org/apache/commons/xml/HardeningXPath.java +++ b/src/main/java/org/apache/commons/xml/HardeningXPath.java @@ -55,7 +55,8 @@ final class HardeningXPath implements XPath { * Parses the source through a hardened, namespace-aware {@link javax.xml.parsers.DocumentBuilder}, mirroring the namespace awareness of the parser the * engine would have provisioned. * - * @param source The document to evaluate against. + * @param source The document to evaluate against. + * @param useDefaultParser Whether the document build should pin the platform's built-in parser. * @return The parsed document. * @throws NullPointerException if {@code source} is {@code null}, per the {@link XPath} contract. * @throws XPathExpressionException if the source cannot be parsed. @@ -63,10 +64,10 @@ final class HardeningXPath implements XPath { * configuration error} or if the implementation is not available or cannot be instantiated. * @throws HardeningException Thrown if a (non-Andoid) factory cannot support the secure processing feature {@link XMLConstants#FEATURE_SECURE_PROCESSING}. */ - static Document parse(final InputSource source) throws XPathExpressionException { + static Document parse(final InputSource source, final boolean useDefaultParser) throws XPathExpressionException { Objects.requireNonNull(source, "source"); try { - final DocumentBuilderFactory factory = HardeningDocumentBuilderFactory.newNSInstance(); + final DocumentBuilderFactory factory = HardeningDocumentBuilderFactory.newNSInstance(useDefaultParser); return factory.newDocumentBuilder().parse(source); } catch (final ParserConfigurationException | SAXException | IOException e) { throw new XPathExpressionException(e); @@ -75,20 +76,27 @@ static Document parse(final InputSource source) throws XPathExpressionException private final XPath delegate; + /** + * Snapshot of the factory's {@code jdk.xml.overrideDefaultParser} outcome at creation, like the JDK copies the feature onto the XPath objects it creates. + */ + final boolean useDefaultParser; + /** * Constructs a new instance. * - * @param delegate the delegate to wrap; must not be {@code null}. + * @param delegate the delegate to wrap; must not be {@code null}. + * @param useDefaultParser whether the {@link InputSource} document builds should pin the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ - HardeningXPath(final XPath delegate) { + HardeningXPath(final XPath delegate, final boolean useDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); + this.useDefaultParser = useDefaultParser; } @Override public XPathExpression compile(final String expression) throws XPathExpressionException { final XPathExpression compiled = delegate.compile(expression); - return compiled == null ? null : new HardeningXPathExpression(compiled); + return compiled == null ? null : new HardeningXPathExpression(compiled, useDefaultParser); } /** @@ -99,7 +107,7 @@ public XPathExpression compile(final String expression) throws XPathExpressionEx */ @Override public String evaluate(final String expression, final InputSource source) throws XPathExpressionException { - return delegate.evaluate(expression, parse(source)); + return delegate.evaluate(expression, parse(source, useDefaultParser)); } /** @@ -110,7 +118,7 @@ public String evaluate(final String expression, final InputSource source) throws */ @Override public Object evaluate(final String expression, final InputSource source, final QName returnType) throws XPathExpressionException { - return delegate.evaluate(expression, parse(source), returnType); + return delegate.evaluate(expression, parse(source, useDefaultParser), returnType); } @Override diff --git a/src/main/java/org/apache/commons/xml/HardeningXPathExpression.java b/src/main/java/org/apache/commons/xml/HardeningXPathExpression.java index 579bfa94..020a0792 100644 --- a/src/main/java/org/apache/commons/xml/HardeningXPathExpression.java +++ b/src/main/java/org/apache/commons/xml/HardeningXPathExpression.java @@ -38,14 +38,21 @@ final class HardeningXPathExpression implements XPathExpression { private final XPathExpression delegate; + /** + * Snapshot of the factory's {@code jdk.xml.overrideDefaultParser} outcome, inherited from the {@link HardeningXPath} that compiled this expression. + */ + private final boolean useDefaultParser; + /** * Constructs a new instance. * - * @param delegate the delegate to wrap; must not be {@code null}. + * @param delegate the delegate to wrap; must not be {@code null}. + * @param useDefaultParser whether the {@link InputSource} document builds should pin the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ - HardeningXPathExpression(final XPathExpression delegate) { + HardeningXPathExpression(final XPathExpression delegate, final boolean useDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); + this.useDefaultParser = useDefaultParser; } /** @@ -56,7 +63,7 @@ final class HardeningXPathExpression implements XPathExpression { */ @Override public String evaluate(final InputSource source) throws XPathExpressionException { - return delegate.evaluate(HardeningXPath.parse(source)); + return delegate.evaluate(HardeningXPath.parse(source, useDefaultParser)); } /** @@ -67,7 +74,7 @@ public String evaluate(final InputSource source) throws XPathExpressionException */ @Override public Object evaluate(final InputSource source, final QName returnType) throws XPathExpressionException { - return delegate.evaluate(HardeningXPath.parse(source), returnType); + return delegate.evaluate(HardeningXPath.parse(source, useDefaultParser), returnType); } @Override diff --git a/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java b/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java index cde63abb..2e241ae8 100644 --- a/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java @@ -75,10 +75,12 @@ private static MethodHandle findNewDefaultInstance() { * functions and reflection-based extension calls are reachable only through a locked-down Saxon {@code Configuration}, not the standard JAXP knobs; this * is the XPath counterpart of the Saxon exception in {@link HardeningTransformerFactory#harden(javax.xml.transform.TransformerFactory)}, kept as a * documented package-prefix exception because the required hardening surface is reachable only through a vendor API.

    • - *
  • FODP ({@code jdk.xml.overrideDefaultParser}, set to {@code false}): best-effort. On the stock JDK it pins the internal parser lookup to - * the bundled SAX parser, blocking a system property swap to a third-party parser (defense-in-depth); Xalan rejects the feature and is left unchanged.
    • *
  • FSP ({@link javax.xml.XMLConstants#FEATURE_SECURE_PROCESSING}): required. It is the only knob both the stock JDK and Xalan XPath * engines expose, and switches on their secure-processing limits. {@link XPathFactory} has no attribute API for finer control.
    • + *
  • FODP ({@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER}): read, not set. The engine's internal parser is never used (the + * wrapper builds the {@link org.xml.sax.InputSource} documents itself), so on implementations that recognize the feature its value selects which + * hardened parser family performs that build: the platform's built-in parser when {@code false} (the JDK's default), the pluggable lookup when + * {@code true} or unrecognized.
    • *
  • The nested wrapper: required. FSP governs only the engine, not the parser it provisions internally for the * {@link org.xml.sax.InputSource}-taking {@code evaluate} entry points; the wrapper performs that document build with a hardened parser instead, so * the engine never parses.
    • @@ -93,8 +95,6 @@ static XPathFactory harden(final XPathFactory factory) { // Saxon: only a locked-down Configuration can close its URI-fetching functions and extension-function surface. return SaxonProvider.configure(factory); } - // Best-effort: the stock JDK pins its bundled SAX parser (defense-in-depth); Xalan rejects the feature. - setOptionalFeature(factory, FEATURE_OVERRIDE_DEFAULT_PARSER, false); // Required: enables the engine's secure-processing limits; XPathFactory has no attribute API for finer control. setFeature(factory, XMLConstants.FEATURE_SECURE_PROCESSING, true); // Required: FSP does not reach the parser the engine provisions for InputSource-taking evaluate calls; the wrapper parses those itself. @@ -196,26 +196,6 @@ private static void setFeature(final XPathFactory factory, final String feature, } } - /** - * Sets an optional feature on the given factory, ignoring it if the implementation does not recognize it. - * - * @param factory The factory to harden. - * @param feature The feature to set. - * @param value The value to set. - */ - private static void setOptionalFeature(final XPathFactory factory, final String feature, final boolean value) { - try { - factory.setFeature(feature, value); - } catch (final XPathFactoryConfigurationException e) { - // Ignored: the implementation does not recognize this optional feature. - } - } - - /** - * {@code jdk.xml.overrideDefaultParser}: pin to the JDK's bundled SAX parser; defense-in-depth against a system property swap to a third-party parser. - */ - private static final String FEATURE_OVERRIDE_DEFAULT_PARSER = "jdk.xml.overrideDefaultParser"; - private HardeningXPathFactory() { // static only } @@ -256,7 +236,7 @@ public boolean isObjectModelSupported(final String objectModel) { @Override public XPath newXPath() { final XPath xpath = delegate.newXPath(); - return xpath == null ? null : new HardeningXPath(xpath); + return xpath == null ? null : new HardeningXPath(xpath, useDefaultParser()); } @Override @@ -264,6 +244,21 @@ public void setFeature(final String name, final boolean value) throws XPathFacto delegate.setFeature(name, value); } + /** + * Whether {@code jdk.xml.overrideDefaultParser} on the delegate currently asks for the platform's built-in parser; the engine's internal parser is + * never used, so the feature instead selects which hardened parser family performs the {@link org.xml.sax.InputSource} document build. An + * implementation that does not recognize the feature reports it by exception and keeps the pluggable lookup. + * + * @return Whether the document builds should pin the platform's built-in parser. + */ + private boolean useDefaultParser() { + try { + return !delegate.getFeature(HardeningSAXParserFactory.OVERRIDE_DEFAULT_PARSER); + } catch (final XPathFactoryConfigurationException e) { + return false; + } + } + @Override public void setXPathFunctionResolver(final XPathFunctionResolver resolver) { delegate.setXPathFunctionResolver(resolver); diff --git a/src/main/java/org/apache/commons/xml/SaxonProvider.java b/src/main/java/org/apache/commons/xml/SaxonProvider.java index 935b422d..13a2c221 100644 --- a/src/main/java/org/apache/commons/xml/SaxonProvider.java +++ b/src/main/java/org/apache/commons/xml/SaxonProvider.java @@ -111,7 +111,7 @@ private static TransformerFactory configure(final TransformerFactory factory) { private static XPathFactory configure(final XPathFactory factory) { final HardenedConfiguration config = new HardenedConfiguration(); // XPath has no factory wrapper, so the ignore-all floor lives on the Configuration; reuse FallbackIgnoreURIResolver, adapted to a ResourceResolver. - config.setResourceResolver(new ResourceResolverWrappingURIResolver(new FallbackIgnoreURIResolver(null, emptySourceSupplier()))); + config.setResourceResolver(new ResourceResolverWrappingURIResolver(new FallbackIgnoreURIResolver(null, emptySourceSupplier(), () -> false))); ((XPathFactoryImpl) factory).setConfiguration(config); return factory; } diff --git a/src/site/markdown/threat_model.md b/src/site/markdown/threat_model.md index 1532cbf1..2d97d37f 100644 --- a/src/site/markdown/threat_model.md +++ b/src/site/markdown/threat_model.md @@ -161,7 +161,6 @@ produces, breaks the hardening for that instance. - `http://xml.org/sax/features/external-parameter-entities` - `javax.xml.stream.isSupportingExternalEntities` - `javax.xml.stream.supportDTD` -- `jdk.xml.overrideDefaultParser` - the implementation's secure-processing limits (entity expansion, element depth, attribute count, and similar) This list is not exhaustive: @@ -219,6 +218,14 @@ enforced by the reserved settings above, which a caller cannot lift. As in the previous case, you need to provide a secure resolver. +- **Parser pinning.** You may set `jdk.xml.overrideDefaultParser` on a TrAX, XPath or schema factory that recognizes it. + The implementation's internal parsers are never used + (the hardening wrappers parse every source themselves), + so the feature carries no security weight here: + it selects which hardened parser family performs those parses — + the platform's built-in parser when `false` (the JDK's default), + the pluggable lookup when `true` or where the feature is not recognized. + ### What is out of scope A returned factory is hardened as delivered; reconfiguring it is a decision to take over hardening for that instance, diff --git a/src/test/java/org/apache/commons/xml/DenyUnresolvedTest.java b/src/test/java/org/apache/commons/xml/DenyUnresolvedTest.java index 54b25b7f..62bee1d9 100644 --- a/src/test/java/org/apache/commons/xml/DenyUnresolvedTest.java +++ b/src/test/java/org/apache/commons/xml/DenyUnresolvedTest.java @@ -56,7 +56,7 @@ void floorsThrowOnUnresolved() { "XMLResolver floor should throw on an unresolved entity"); assertThrows(LSException.class, () -> new FallbackIgnoreLSResourceResolver(null).resolveResource(null, null, null, SYSTEM_ID, null), "LSResourceResolver floor should throw on an unresolved resource"); - assertThrows(TransformerException.class, () -> new FallbackIgnoreURIResolver(null, null).resolve(SYSTEM_ID, null), + assertThrows(TransformerException.class, () -> new FallbackIgnoreURIResolver(null, null, () -> false).resolve(SYSTEM_ID, null), "URIResolver floor should throw on an unresolved URI"); } } diff --git a/src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java b/src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java new file mode 100644 index 00000000..4dc167ea --- /dev/null +++ b/src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java @@ -0,0 +1,126 @@ +/* + * 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; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeFalse; + +import java.io.StringWriter; + +import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerFactory; +import javax.xml.transform.stream.StreamResult; +import javax.xml.validation.SchemaFactory; +import javax.xml.xpath.XPathFactory; + +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.xml.sax.XMLReader; + +/** + * Checks that {@code jdk.xml.overrideDefaultParser} selects which hardened parser family performs the source rewrites on factories that recognize the feature. + * + *

    The wrapped implementations' internal parsers are never used — the wrappers parse every source themselves — so instead of configuring the delegate the + * wrappers read the feature: {@code false} (the JDK's default) pins the platform's built-in parser, {@code true} (or a delegate that does not recognize the + * feature) keeps the pluggable lookup. Both choices are hardened, so the feature carries no security weight. The tests pin the JDK implementations through + * {@code newDefaultInstance()}, so they discriminate in every JVM execution; under test-jdk-xerces the two parser families genuinely differ.

    + */ +@Tag("trax") +@Tag("xpath") +@Tag("schema") +class OverrideDefaultParserTest { + + private static final String FEATURE = HardeningSAXParserFactory.OVERRIDE_DEFAULT_PARSER; + + /** Package prefix of the JDK's built-in parsers, the family a {@code false} feature value pins. */ + private static final String JDK_INTERNAL_PREFIX = "com.sun.org.apache.xerces.internal."; + + private static String transform(final TransformerFactory factory, final String text) throws Exception { + final Transformer transformer = factory.newTransformer(AttackTestSupport.streamSource(AttackTestSupport.xsltBody(text))); + final StringWriter out = new StringWriter(); + transformer.transform(AttackTestSupport.streamSource(AttackTestSupport.xmlBody("ignored")), new StreamResult(out)); + return out.toString(); + } + + @Test + void hardenedReaderFollowsFlag() throws Exception { + assumeFalse(AttackTestSupport.IS_ANDROID); + final XMLReader pinned = ((HardeningXMLReader) HardeningSAXParserFactory.newHardenedReader(true)).getDelegate(); + assertTrue(pinned.getClass().getName().startsWith(JDK_INTERNAL_PREFIX), pinned.getClass().getName()); + final XMLReader pluggable = ((HardeningXMLReader) HardeningSAXParserFactory.newHardenedReader(false)).getDelegate(); + final XMLReader lookedUp = ((HardeningXMLReader) HardeningSAXParserFactory.newNSInstance().newSAXParser().getXMLReader()).getDelegate(); + assertEquals(lookedUp.getClass(), pluggable.getClass()); + if (xercesOnClasspath()) { + // The two families genuinely differ only where a third-party parser wins the lookup (the test-jdk-xerces execution). + assertNotEquals(pinned.getClass(), pluggable.getClass()); + } + } + + @Test + void schemaFactoryReadsFeatureAtCreation() throws Exception { + assumeFalse(AttackTestSupport.IS_ANDROID); + final SchemaFactory factory = HardeningSchemaFactory.newDefaultInstance(); + assertFalse(factory.getFeature(FEATURE)); + assertTrue(((HardeningSchema) factory.newSchema(AttackTestSupport.streamSource(AttackTestSupport.BENIGN_SCHEMA))).useDefaultParser); + factory.setFeature(FEATURE, true); + assertFalse(((HardeningSchema) factory.newSchema(AttackTestSupport.streamSource(AttackTestSupport.BENIGN_SCHEMA))).useDefaultParser); + } + + @Test + void transformerFactoryReadsFeatureAtCreation() throws Exception { + assumeFalse(AttackTestSupport.IS_ANDROID); + final TransformerFactory factory = HardeningTransformerFactory.newDefaultInstance(); + assertFalse(factory.getFeature(FEATURE)); + assertTrue(((HardeningTemplates) factory.newTemplates(AttackTestSupport.streamSource(AttackTestSupport.xsltBody("probe")))).useDefaultParser); + factory.setFeature(FEATURE, true); + assertFalse(((HardeningTemplates) factory.newTemplates(AttackTestSupport.streamSource(AttackTestSupport.xsltBody("probe")))).useDefaultParser); + } + + @Test + void transformSucceedsUnderBothParserFamilies() throws Exception { + assumeFalse(AttackTestSupport.IS_ANDROID); + final TransformerFactory factory = HardeningTransformerFactory.newDefaultInstance(); + // Feature false (the JDK's default): stylesheet and source parse through the pinned platform parser. + assertTrue(transform(factory, "pinned").contains("pinned")); + factory.setFeature(FEATURE, true); + // Feature true: same result through the pluggable lookup. + assertTrue(transform(factory, "pluggable").contains("pluggable")); + } + + private static boolean xercesOnClasspath() { + try { + Class.forName("org.apache.xerces.jaxp.SAXParserFactoryImpl"); + return true; + } catch (final ClassNotFoundException e) { + return false; + } + } + + @Test + void xPathFactoryReadsFeatureAtCreation() throws Exception { + assumeFalse(AttackTestSupport.IS_ANDROID); + final XPathFactory factory = HardeningXPathFactory.newDefaultInstance(); + assertFalse(factory.getFeature(FEATURE)); + assertTrue(((HardeningXPath) factory.newXPath()).useDefaultParser); + factory.setFeature(FEATURE, true); + assertFalse(((HardeningXPath) factory.newXPath()).useDefaultParser); + } +} diff --git a/src/test/java/org/apache/commons/xml/ShadingFootprintTest.java b/src/test/java/org/apache/commons/xml/ShadingFootprintTest.java index 57ea76f8..a96a83d2 100644 --- a/src/test/java/org/apache/commons/xml/ShadingFootprintTest.java +++ b/src/test/java/org/apache/commons/xml/ShadingFootprintTest.java @@ -87,7 +87,7 @@ class ShadingFootprintTest { // @formatter:on /** - * TrAX, XPath and schema re-harden their sub-parsers through {@link HardeningSAXParserFactory#harden(Source)}, so each builds on the full SAX closure below; + * TrAX, XPath and schema re-harden their sub-parsers through {@link HardeningSAXParserFactory#harden(Source, boolean)}, so each builds on the full SAX closure below; * TrAX additionally parses the Xalan {@code getAssociatedStylesheet} source and XPath its InputSource-taking evaluate calls through the DOM entry point, so * their closures carry that set too. */ diff --git a/src/test/java/org/apache/commons/xml/XPathInputSourceTest.java b/src/test/java/org/apache/commons/xml/XPathInputSourceTest.java index 18231b83..198ad484 100644 --- a/src/test/java/org/apache/commons/xml/XPathInputSourceTest.java +++ b/src/test/java/org/apache/commons/xml/XPathInputSourceTest.java @@ -33,7 +33,7 @@ *

    The stock JDK and Apache Xalan implement the {@link org.xml.sax.InputSource}-taking {@code evaluate} entry points by provisioning an internal document * parser that {@code FEATURE_SECURE_PROCESSING} on the {@link XPathFactory} does not reach. The {@link HardeningXPathFactory} wrapper parses the input * through a hardened {@code DocumentBuilder} instead, so the external reference resolves to empty on the floor, while the - * evaluation itself still works. Tagged {@code xpath}, so it runs under test-stockjdk, test-xalan and test-xalan-xerces; the Saxon engine takes the separate + * evaluation itself still works. Tagged {@code xpath}, so it runs under test-stockjdk, test-jdk-xerces, test-xalan and test-xalan-xerces; the Saxon engine takes the separate * {@code SaxonProvider} path covered by {@code SaxonXPathExternalCallsTest}.

    */ @Tag("xpath") From f992e26903df606eff2360663aa2d4e4b4713082 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 28 Aug 2026 11:01:00 +0200 Subject: [PATCH 02/13] Shorten the overrideDefaultParser changes.xml entry Assisted-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ErRKq7RUeQ9LGrSboSUyYm --- src/changes/changes.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 15662efc..943f1302 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -43,7 +43,7 @@ The type attribute can be add, update, fix, or remove. Mirror on each factory class every JAXP static factory method, including the Java 9 newDefaultInstance and Java 13 newNSInstance families, all usable on Java 8. Block XInclude (xi:include) href resolution by default, since the JAXP external-access properties do not govern it. - Honor jdk.xml.overrideDefaultParser on TrAX, XPath and schema factories that recognize it: the hardened source rewrites use the platform's built-in parser while the feature is false (the JDK's default), instead of always using the pluggable lookup. + Honor jdk.xml.overrideDefaultParser on TrAX, XPath and schema factories that recognize it. Restore the hardened configuration when a factory or parser is reset() instead of reverting to the implementation defaults. Parse a Source opted in by a caller-supplied URIResolver using a hardened parser. Harden the document parse behind the InputSource-taking XPath evaluation entry points. From cd0f1232b8906acef1c86dad8f4eaf6741877fb1 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 28 Aug 2026 11:57:22 +0200 Subject: [PATCH 03/13] Skip the XSLTC-pinning transform test in the native image newDefaultInstance() pins the JDK's XSLTC, which defines the compiled translet class at run time; a closed-world native image cannot, which is the reason the native-xalan profile substitutes Xalan for TrAX. The capture tests stay enabled: newTemplates never loads the translet. Assisted-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ErRKq7RUeQ9LGrSboSUyYm --- .../org/apache/commons/xml/OverrideDefaultParserTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java b/src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java index 4dc167ea..edf317f8 100644 --- a/src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java +++ b/src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java @@ -33,6 +33,7 @@ import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledInNativeImage; import org.xml.sax.XMLReader; /** @@ -95,6 +96,9 @@ void transformerFactoryReadsFeatureAtCreation() throws Exception { } @Test + // The JDK default TrAX pinned by newDefaultInstance() is XSLTC, which defines the compiled translet class at run time — impossible in a closed-world + // native image (the reason the native profile substitutes Xalan). The capture tests above stay enabled: newTemplates never loads the translet. + @DisabledInNativeImage void transformSucceedsUnderBothParserFamilies() throws Exception { assumeFalse(AttackTestSupport.IS_ANDROID); final TransformerFactory factory = HardeningTransformerFactory.newDefaultInstance(); From 8e490c06ba9d38ca9f551a4c8a96f71094b862da Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 28 Aug 2026 14:42:56 +0200 Subject: [PATCH 04/13] Document that the returned factories are behavior-preserving wrappers A returned factory is not necessarily an instance of the underlying implementation, but everything except the security behavior is preserved, including each implementation's internal parser choice: the stock JDK's jdk.xml.overrideDefaultParser feature and system property are honored, and Saxon keeps its own parser selection. Assisted-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ErRKq7RUeQ9LGrSboSUyYm --- .../org/apache/commons/xml/package-info.java | 17 +++++++++++++++ src/site/markdown/index.md | 21 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/main/java/org/apache/commons/xml/package-info.java b/src/main/java/org/apache/commons/xml/package-info.java index c837c8c1..835207ed 100644 --- a/src/main/java/org/apache/commons/xml/package-info.java +++ b/src/main/java/org/apache/commons/xml/package-info.java @@ -21,6 +21,23 @@ * Every method returns new, hardened factory instances. No caching or pooling is performed; callers on a hot path are responsible for their own * caching. *

    + *

    + * A returned factory is not necessarily an instance of the underlying implementation — it might be (and usually is) a wrapper around it, so it cannot be cast + * to the implementation's own class. Everything else about the implementation's behavior is preserved — features, properties, and attributes delegate to it — + * and only the security behavior is hardened. + *

    + *

    + * Preserved behavior includes the choice of internal parsers. Each TrAX, XPath, or schema implementation has its own way of instantiating them, and the + * library respects it: + *

    + *
      + *
    • Stock JDK factories use the JDK parsers by default, and expose the {@code jdk.xml.overrideDefaultParser} feature (and Java system property of the same + * name) to switch to parsers instantiated through {@link java.util.ServiceLoader}.
      • + *
    • Saxon selects its parsers through its own configuration.
      • + *
    + *

    + * Whichever parser is selected, it is hardened. + *

    *

    Hardening guarantees

    *

    * Every factory returned by makes the same three guarantees, regardless of which JAXP implementation is on the classpath: diff --git a/src/site/markdown/index.md b/src/site/markdown/index.md index f8801f0a..408288e4 100644 --- a/src/site/markdown/index.md +++ b/src/site/markdown/index.md @@ -146,6 +146,27 @@ HardeningSchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI) .validate(new StreamSource(inputStream)); ``` +### Wrappers, not the original factories + +A returned factory is not necessarily an instance of the underlying implementation: +it might be (and usually is) a wrapper around it, +so it cannot be cast to the implementation's own class. +Everything else about the implementation's behavior is preserved — +features, properties, and attributes delegate to it — +and only the security behavior is hardened. + +Preserved behavior includes the choice of internal parsers. +Each TrAX, XPath, or schema implementation has its own way of instantiating them, +and the library respects it: + +- Stock JDK factories use the JDK parsers by default, + and expose the `jdk.xml.overrideDefaultParser` feature + (and Java system property of the same name) + to switch to parsers instantiated through `ServiceLoader`. +- Saxon selects its parsers through its own configuration. + +Whichever parser is selected, it is hardened. + ### Factory methods Each factory class mirrors every static factory method its JAXP counterpart offers, From 0395537665438945ffbac20337c0534b458db6a6 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 28 Aug 2026 14:49:38 +0200 Subject: [PATCH 05/13] Reword the threat model's internal parser selection entry Assisted-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ErRKq7RUeQ9LGrSboSUyYm --- src/site/markdown/threat_model.md | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/site/markdown/threat_model.md b/src/site/markdown/threat_model.md index 2d97d37f..79056886 100644 --- a/src/site/markdown/threat_model.md +++ b/src/site/markdown/threat_model.md @@ -218,13 +218,12 @@ enforced by the reserved settings above, which a caller cannot lift. As in the previous case, you need to provide a secure resolver. -- **Parser pinning.** You may set `jdk.xml.overrideDefaultParser` on a TrAX, XPath or schema factory that recognizes it. - The implementation's internal parsers are never used - (the hardening wrappers parse every source themselves), - so the feature carries no security weight here: - it selects which hardened parser family performs those parses — - the platform's built-in parser when `false` (the JDK's default), - the pluggable lookup when `true` or where the feature is not recognized. +- **Internal parser selection.** + On the stock JDK TrAX, XPath, and schema implementations + you may set [`jdk.xml.overrideDefaultParser`](https://docs.oracle.com/en/java/javase/25/docs/api/java.xml/module-summary.html#jdk.xml.overrideDefaultParser) + to switch their internal parses from the JDK parsers to a `ServiceLoader`-resolved parser. + Whichever parser is selected, it is hardened, + so the setting carries no security weight. ### What is out of scope From 5a2d7039a73eca45bc095d99edea023f07af75c6 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 28 Aug 2026 15:02:26 +0200 Subject: [PATCH 06/13] Simplify the useDefaultParser Javadoc Assisted-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ErRKq7RUeQ9LGrSboSUyYm --- .../org/apache/commons/xml/HardeningSchemaFactory.java | 9 +++++---- .../apache/commons/xml/HardeningTransformerFactory.java | 8 +++++--- .../org/apache/commons/xml/HardeningXPathFactory.java | 9 +++++---- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java b/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java index b3b8b6be..a9a03d23 100644 --- a/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java @@ -250,11 +250,12 @@ public Schema newSchema(final Source[] schemas) throws SAXException { } /** - * Whether {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} on the delegate currently asks for the platform's built-in parser; the - * implementation's internal parsers are never used, so the feature instead selects which hardened parser family performs the source rewrites. - * An implementation that does not recognize the feature reports it by exception and keeps the pluggable lookup. + * Checks whether parsers should be instantiated via {@code newDefaultInstance()} instead of {@code newInstance()}. * - * @return Whether the rewrites should pin the platform's built-in parser. + *

    The JDK implementation of {@link SchemaFactory} uses the JDK parsers while {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} is unset or + * {@code false}.

    + * + * @return {@code true} if parsers should be created via {@code newDefaultInstance()}. */ private boolean useDefaultParser() { try { diff --git a/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java b/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java index 69162b34..21c2e2d5 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java @@ -456,10 +456,12 @@ public void setURIResolver(final URIResolver resolver) { } /** - * Whether {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} on the delegate currently asks for the platform's built-in parser; the - * implementation's internal parsers are never used, so the feature instead selects which hardened parser family performs the source rewrites. + * Checks whether parsers should be instantiated via {@code newDefaultInstance()} instead of {@code newInstance()}. * - * @return Whether the rewrites should pin the platform's built-in parser. + *

    The JDK implementation of {@link TransformerFactory} uses the JDK parsers while {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} is unset + * or {@code false}.

    + * + * @return {@code true} if parsers should be created via {@code newDefaultInstance()}. */ private boolean useDefaultParser() { return supportsOverrideDefaultParser && !delegate.getFeature(HardeningSAXParserFactory.OVERRIDE_DEFAULT_PARSER); diff --git a/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java b/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java index 78c453d7..2658cb85 100644 --- a/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java @@ -235,11 +235,12 @@ public void setFeature(final String name, final boolean value) throws XPathFacto } /** - * Whether {@code jdk.xml.overrideDefaultParser} on the delegate currently asks for the platform's built-in parser; the engine's internal parser is - * never used, so the feature instead selects which hardened parser family performs the {@link org.xml.sax.InputSource} document build. An - * implementation that does not recognize the feature reports it by exception and keeps the pluggable lookup. + * Checks whether parsers should be instantiated via {@code newDefaultInstance()} instead of {@code newInstance()}. * - * @return Whether the document builds should pin the platform's built-in parser. + *

    The JDK implementation of {@link XPathFactory} uses the JDK parsers while {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} is unset or + * {@code false}.

    + * + * @return {@code true} if parsers should be created via {@code newDefaultInstance()}. */ private boolean useDefaultParser() { try { From a987c0688a23a43311410696729f7aab75b2eb60 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 28 Aug 2026 15:19:01 +0200 Subject: [PATCH 07/13] Document the default-parser selection in newNSInstance(boolean) Both parser factories now expose the selection as a package-private newNSInstance(boolean) whose Javadoc explains the role of the javax.xml.parsers.*Factory system properties: they are the JDK's own mechanism for reconfiguring what "default parser" means, so they are honored through the standard lookup rather than bypassed. Assisted-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ErRKq7RUeQ9LGrSboSUyYm --- .../xml/HardeningDocumentBuilderFactory.java | 12 ++++--- .../xml/HardeningSAXParserFactory.java | 31 +++++++++++++------ 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java b/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java index c653c257..5ceb2975 100644 --- a/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java @@ -185,12 +185,14 @@ public static DocumentBuilderFactory newNSInstance() { } /** - * Returns the hardened, namespace-aware factory the Source-rewriting wrappers parse with, mirroring the JDK's own internal parser choice: with - * {@code useDefaultParser} the platform's built-in implementation is pinned, unless the {@code javax.xml.parsers.DocumentBuilderFactory} system property - * explicitly requests another implementation, which overrides the pin like it does inside the JDK. + * Returns the hardened, namespace-aware factory the Source-rewriting wrappers parse with. + *

    + * With {@code useDefaultParser} the factory is the JDK's "default parser" factory, determined the way the JDK itself determines it: the built-in + * implementation, unless the {@code javax.xml.parsers.DocumentBuilderFactory} system property is set — that property is the JDK's own mechanism for + * reconfiguring the default parser, so it is honored through the standard lookup rather than bypassed. + *

    * - * @param useDefaultParser whether {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} on the originating factory asks for the platform's built-in - * implementation. + * @param useDefaultParser whether {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} on the originating factory asks for the JDK's default parser. * @return A hardened, namespace-aware factory. * @throws IllegalStateException Thrown if a required hardening setting cannot be applied to the underlying implementation. * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service configuration error} or if the diff --git a/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java b/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java index 61cbf22a..5f15ee13 100644 --- a/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java @@ -207,12 +207,8 @@ public static SAXParserFactory newDefaultNSInstance() { } /** - * Creates a new hardened, namespace-aware {@link XMLReader} for the TrAX, XPath and schema wrappers to parse sources with. - *

    - * Mirrors the JDK's own internal parser choice: with {@code useDefaultParser} the platform's built-in parser is pinned, unless the - * {@code javax.xml.parsers.SAXParserFactory} system property explicitly requests another implementation, which overrides the pin like it does inside the - * JDK. - *

    + * Creates a new hardened, namespace-aware {@link XMLReader} for the TrAX, XPath and schema wrappers to parse sources with, from the factory + * {@link #newNSInstance(boolean)} selects. * * @param useDefaultParser whether {@value #OVERRIDE_DEFAULT_PARSER} on the originating factory asks for the platform's built-in parser. * @return a hardened reader. @@ -222,10 +218,7 @@ public static SAXParserFactory newDefaultNSInstance() { */ static XMLReader newHardenedReader(final boolean useDefaultParser) throws TransformerConfigurationException { try { - final SAXParserFactory factory = useDefaultParser && System.getProperty("javax.xml.parsers.SAXParserFactory") == null - ? newDefaultNSInstance() - : newNSInstance(); - return factory.newSAXParser().getXMLReader(); + return newNSInstance(useDefaultParser).newSAXParser().getXMLReader(); } catch (final ParserConfigurationException | SAXException e) { throw new TransformerConfigurationException("Failed to obtain a hardened XMLReader for source parsing", e); } @@ -269,6 +262,24 @@ public static SAXParserFactory newNSInstance() { return makeNSAware(newInstance()); } + /** + * Returns the hardened, namespace-aware factory the Source-rewriting wrappers parse with. + *

    + * With {@code useDefaultParser} the factory is the JDK's "default parser" factory, determined the way the JDK itself determines it: the built-in parser, + * unless the {@code javax.xml.parsers.SAXParserFactory} system property is set — that property is the JDK's own mechanism for reconfiguring the default + * parser, so it is honored through the standard lookup rather than bypassed. + *

    + * + * @param useDefaultParser whether {@value #OVERRIDE_DEFAULT_PARSER} on the originating factory asks for the JDK's default parser. + * @return A hardened, namespace-aware factory. + * @throws IllegalStateException Thrown if a required hardening setting cannot be applied to the underlying implementation. + * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service configuration error} or if the + * implementation is not available or cannot be instantiated. + */ + static SAXParserFactory newNSInstance(final boolean useDefaultParser) { + return useDefaultParser && System.getProperty("javax.xml.parsers.SAXParserFactory") == null ? newDefaultNSInstance() : newNSInstance(); + } + /** * Returns a new, hardened, namespace-aware {@link SAXParserFactory} of the given implementation class, enabling namespace awareness on * {@link #newInstance(String, ClassLoader)}, the behavior {@code SAXParserFactory.newNSInstance(String, ClassLoader)} (Java 13 or later) is specified to have. From 2589e357852fef9b7069c34dd53758f688b19a27 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 28 Aug 2026 16:34:59 +0200 Subject: [PATCH 08/13] Rename useDefaultParser to overrideDefaultParser, matching the JDK feature The flag now carries the jdk.xml.overrideDefaultParser value itself instead of its negation, so wrappers, floors and tests read the same polarity as the JDK. Assisted-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ErRKq7RUeQ9LGrSboSUyYm --- .../xml/FallbackIgnoreURIResolver.java | 12 +++---- .../xml/HardeningDocumentBuilderFactory.java | 8 ++--- .../xml/HardeningSAXParserFactory.java | 20 +++++------ .../apache/commons/xml/HardeningSchema.java | 10 +++--- .../commons/xml/HardeningSchemaFactory.java | 18 +++++----- .../commons/xml/HardeningTemplates.java | 10 +++--- .../xml/HardeningTemplatesHandler.java | 10 +++--- .../commons/xml/HardeningTransformer.java | 12 +++---- .../xml/HardeningTransformerFactory.java | 34 +++++++++---------- .../xml/HardeningTransformerHandler.java | 6 ++-- .../commons/xml/HardeningValidator.java | 10 +++--- .../commons/xml/HardeningXMLFilter.java | 2 +- .../apache/commons/xml/HardeningXPath.java | 20 +++++------ .../commons/xml/HardeningXPathExpression.java | 12 +++---- .../commons/xml/HardeningXPathFactory.java | 12 +++---- .../xml/OverrideDefaultParserTest.java | 16 ++++----- 16 files changed, 106 insertions(+), 106 deletions(-) diff --git a/src/main/java/org/apache/commons/xml/FallbackIgnoreURIResolver.java b/src/main/java/org/apache/commons/xml/FallbackIgnoreURIResolver.java index 6c2a38d0..0b6c781c 100644 --- a/src/main/java/org/apache/commons/xml/FallbackIgnoreURIResolver.java +++ b/src/main/java/org/apache/commons/xml/FallbackIgnoreURIResolver.java @@ -87,22 +87,22 @@ private static Document newEmptyDocument() { private final Supplier emptySource; /** - * Whether the opted-in rewrite should pin the platform's built-in parser; read per resolution so the factory-level floor tracks a later + * Whether the opted-in rewrite should use the pluggable parser lookup instead of the platform's built-in parser; read per resolution so the factory-level floor tracks a later * {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} toggle. */ - private final BooleanSupplier useDefaultParser; + private final BooleanSupplier overrideDefaultParser; /** * Constructs a new resolver. * * @param delegate the resolver to delegate resolution to; may be {@code null}. * @param emptySource the empty-{@link Source} supplier for the ignore outcome, or {@code null} for the default empty DOM document. - * @param useDefaultParser whether the opted-in rewrite should pin the platform's built-in parser, read at each resolution. + * @param overrideDefaultParser whether the opted-in rewrite should use the pluggable parser lookup instead of the platform's built-in parser, read at each resolution. */ - FallbackIgnoreURIResolver(final URIResolver delegate, final Supplier emptySource, final BooleanSupplier useDefaultParser) { + FallbackIgnoreURIResolver(final URIResolver delegate, final Supplier emptySource, final BooleanSupplier overrideDefaultParser) { this.delegate = delegate; this.emptySource = emptySource != null ? emptySource : () -> new DOMSource(EMPTY_DOCUMENT); - this.useDefaultParser = useDefaultParser; + this.overrideDefaultParser = overrideDefaultParser; } /** @@ -125,7 +125,7 @@ public Source resolve(final String href, final String base) throws TransformerEx final Source resolved = delegate != null ? delegate.resolve(href, base) : null; if (resolved != null) { // The implementation parses the opted-in handle with an internal reader at its own defaults; the rewrite hands it a hardened reader instead. - return HardeningSAXParserFactory.harden(resolved, useDefaultParser.getAsBoolean()); + return HardeningSAXParserFactory.harden(resolved, overrideDefaultParser.getAsBoolean()); } if (HardeningException.throwOnUnresolved()) { throw new TransformerException(HardeningException.forbidden("uri", null, null, href, base)); diff --git a/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java b/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java index 5ceb2975..81b00a65 100644 --- a/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java @@ -187,19 +187,19 @@ public static DocumentBuilderFactory newNSInstance() { /** * Returns the hardened, namespace-aware factory the Source-rewriting wrappers parse with. *

    - * With {@code useDefaultParser} the factory is the JDK's "default parser" factory, determined the way the JDK itself determines it: the built-in + * While {@code overrideDefaultParser} is {@code false} the factory is the JDK's "default parser" factory, determined the way the JDK itself determines it: the built-in * implementation, unless the {@code javax.xml.parsers.DocumentBuilderFactory} system property is set — that property is the JDK's own mechanism for * reconfiguring the default parser, so it is honored through the standard lookup rather than bypassed. *

    * - * @param useDefaultParser whether {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} on the originating factory asks for the JDK's default parser. + * @param overrideDefaultParser whether {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} on the originating factory asks to override the JDK's default parser. * @return A hardened, namespace-aware factory. * @throws IllegalStateException Thrown if a required hardening setting cannot be applied to the underlying implementation. * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service configuration error} or if the * implementation is not available or cannot be instantiated. */ - static DocumentBuilderFactory newNSInstance(final boolean useDefaultParser) { - return useDefaultParser && System.getProperty("javax.xml.parsers.DocumentBuilderFactory") == null ? newDefaultNSInstance() : newNSInstance(); + static DocumentBuilderFactory newNSInstance(final boolean overrideDefaultParser) { + return overrideDefaultParser || System.getProperty("javax.xml.parsers.DocumentBuilderFactory") != null ? newNSInstance() : newDefaultNSInstance(); } /** diff --git a/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java b/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java index 5f15ee13..461477e6 100644 --- a/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java @@ -112,16 +112,16 @@ static SAXParserFactory harden(final SAXParserFactory factory) { *

    * * @param source the source to harden; never {@code null}. - * @param useDefaultParser whether {@value #OVERRIDE_DEFAULT_PARSER} on the originating factory asks for the platform's built-in parser. + * @param overrideDefaultParser whether {@value #OVERRIDE_DEFAULT_PARSER} on the originating factory asks to override the JDK's default parser. * @return a hardened source. * @throws TransformerConfigurationException if a hardened reader cannot be obtained. * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service * configuration error} or if the implementation is not available or cannot be instantiated. */ - static Source harden(final Source source, final boolean useDefaultParser) throws TransformerConfigurationException { + static Source harden(final Source source, final boolean overrideDefaultParser) throws TransformerConfigurationException { if (source instanceof StreamSource || source instanceof SAXSource && ((SAXSource) source).getXMLReader() == null) { final InputSource inputSource = SAXSource.sourceToInputSource(source); - return inputSource == null ? source : new SAXSource(newHardenedReader(useDefaultParser), inputSource); + return inputSource == null ? source : new SAXSource(newHardenedReader(overrideDefaultParser), inputSource); } return source; } @@ -210,15 +210,15 @@ public static SAXParserFactory newDefaultNSInstance() { * Creates a new hardened, namespace-aware {@link XMLReader} for the TrAX, XPath and schema wrappers to parse sources with, from the factory * {@link #newNSInstance(boolean)} selects. * - * @param useDefaultParser whether {@value #OVERRIDE_DEFAULT_PARSER} on the originating factory asks for the platform's built-in parser. + * @param overrideDefaultParser whether {@value #OVERRIDE_DEFAULT_PARSER} on the originating factory asks to override the JDK's default parser. * @return a hardened reader. * @throws TransformerConfigurationException if a hardened reader cannot be obtained. * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service * configuration error} or if the implementation is not available or cannot be instantiated. */ - static XMLReader newHardenedReader(final boolean useDefaultParser) throws TransformerConfigurationException { + static XMLReader newHardenedReader(final boolean overrideDefaultParser) throws TransformerConfigurationException { try { - return newNSInstance(useDefaultParser).newSAXParser().getXMLReader(); + return newNSInstance(overrideDefaultParser).newSAXParser().getXMLReader(); } catch (final ParserConfigurationException | SAXException e) { throw new TransformerConfigurationException("Failed to obtain a hardened XMLReader for source parsing", e); } @@ -265,19 +265,19 @@ public static SAXParserFactory newNSInstance() { /** * Returns the hardened, namespace-aware factory the Source-rewriting wrappers parse with. *

    - * With {@code useDefaultParser} the factory is the JDK's "default parser" factory, determined the way the JDK itself determines it: the built-in parser, + * While {@code overrideDefaultParser} is {@code false} the factory is the JDK's "default parser" factory, determined the way the JDK itself determines it: the built-in parser, * unless the {@code javax.xml.parsers.SAXParserFactory} system property is set — that property is the JDK's own mechanism for reconfiguring the default * parser, so it is honored through the standard lookup rather than bypassed. *

    * - * @param useDefaultParser whether {@value #OVERRIDE_DEFAULT_PARSER} on the originating factory asks for the JDK's default parser. + * @param overrideDefaultParser whether {@value #OVERRIDE_DEFAULT_PARSER} on the originating factory asks to override the JDK's default parser. * @return A hardened, namespace-aware factory. * @throws IllegalStateException Thrown if a required hardening setting cannot be applied to the underlying implementation. * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service configuration error} or if the * implementation is not available or cannot be instantiated. */ - static SAXParserFactory newNSInstance(final boolean useDefaultParser) { - return useDefaultParser && System.getProperty("javax.xml.parsers.SAXParserFactory") == null ? newDefaultNSInstance() : newNSInstance(); + static SAXParserFactory newNSInstance(final boolean overrideDefaultParser) { + return overrideDefaultParser || System.getProperty("javax.xml.parsers.SAXParserFactory") != null ? newNSInstance() : newDefaultNSInstance(); } /** diff --git a/src/main/java/org/apache/commons/xml/HardeningSchema.java b/src/main/java/org/apache/commons/xml/HardeningSchema.java index c45cbd5b..19385d95 100644 --- a/src/main/java/org/apache/commons/xml/HardeningSchema.java +++ b/src/main/java/org/apache/commons/xml/HardeningSchema.java @@ -36,23 +36,23 @@ final class HardeningSchema extends Schema { /** * Snapshot of the factory's {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} outcome, carried onto every produced Validator. */ - final boolean useDefaultParser; + final boolean overrideDefaultParser; /** * Constructs a new instance. * * @param delegate the delegate to wrap; must not be {@code null}. - * @param useDefaultParser whether the produced Validators' source rewrites should pin the platform's built-in parser. + * @param overrideDefaultParser whether the produced Validators' source rewrites should use the pluggable parser lookup instead of the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ - HardeningSchema(final Schema delegate, final boolean useDefaultParser) { + HardeningSchema(final Schema delegate, final boolean overrideDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); - this.useDefaultParser = useDefaultParser; + this.overrideDefaultParser = overrideDefaultParser; } @Override public Validator newValidator() { - return new HardeningValidator(delegate.newValidator(), useDefaultParser); + return new HardeningValidator(delegate.newValidator(), overrideDefaultParser); } @Override diff --git a/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java b/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java index a9a03d23..8732bdbd 100644 --- a/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java @@ -180,10 +180,10 @@ private static final class Wrapper extends SchemaFactory { */ private Source[] harden(final Source[] schemas) throws SAXException { final Source[] hardened = new Source[schemas.length]; - final boolean useDefaultParser = useDefaultParser(); + final boolean overrideDefaultParser = overrideDefaultParser(); try { for (int i = 0; i < schemas.length; i++) { - hardened[i] = HardeningSAXParserFactory.harden(schemas[i], useDefaultParser); + hardened[i] = HardeningSAXParserFactory.harden(schemas[i], overrideDefaultParser); } } catch (final TransformerConfigurationException e) { throw new SAXException("Failed to harden schema source", e); @@ -235,7 +235,7 @@ public boolean isSchemaLanguageSupported(final String schemaLanguage) { @Override public Schema newSchema() throws SAXException { - return new HardeningSchema(delegate.newSchema(), useDefaultParser()); + return new HardeningSchema(delegate.newSchema(), overrideDefaultParser()); } /** @@ -246,22 +246,22 @@ public Schema newSchema() throws SAXException { */ @Override public Schema newSchema(final Source[] schemas) throws SAXException { - return new HardeningSchema(delegate.newSchema(harden(schemas)), useDefaultParser()); + return new HardeningSchema(delegate.newSchema(harden(schemas)), overrideDefaultParser()); } /** - * Checks whether parsers should be instantiated via {@code newDefaultInstance()} instead of {@code newInstance()}. + * Checks whether parsers should be instantiated via {@code newInstance()} instead of {@code newDefaultInstance()}. * *

    The JDK implementation of {@link SchemaFactory} uses the JDK parsers while {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} is unset or * {@code false}.

    * - * @return {@code true} if parsers should be created via {@code newDefaultInstance()}. + * @return {@code true} if parsers should be created via {@code newInstance()}. */ - private boolean useDefaultParser() { + private boolean overrideDefaultParser() { try { - return !delegate.getFeature(HardeningSAXParserFactory.OVERRIDE_DEFAULT_PARSER); + return delegate.getFeature(HardeningSAXParserFactory.OVERRIDE_DEFAULT_PARSER); } catch (final SAXNotRecognizedException | SAXNotSupportedException e) { - return false; + return true; } } diff --git a/src/main/java/org/apache/commons/xml/HardeningTemplates.java b/src/main/java/org/apache/commons/xml/HardeningTemplates.java index 667f984b..5b800928 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTemplates.java +++ b/src/main/java/org/apache/commons/xml/HardeningTemplates.java @@ -55,7 +55,7 @@ final class HardeningTemplates implements Templates { * Snapshot of the factory's {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} outcome, carried onto every produced Transformer and self-provisioned * filter reader. */ - final boolean useDefaultParser; + final boolean overrideDefaultParser; /** * Constructs a new instance. @@ -63,14 +63,14 @@ final class HardeningTemplates implements Templates { * @param delegate the delegate to wrap; must not be {@code null}. * @param uriResolver the compile-time URIResolver snapshot to restore onto Transformers produced from the compiled Templates; may be {@code null}. * @param emptySource the empty-{@link Source} supplier for the produced Transformers - * @param useDefaultParser whether the produced Transformers' source rewrites should pin the platform's built-in parser. + * @param overrideDefaultParser whether the produced Transformers' source rewrites should use the pluggable parser lookup instead of the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ - HardeningTemplates(final Templates delegate, final URIResolver uriResolver, final Supplier emptySource, final boolean useDefaultParser) { + HardeningTemplates(final Templates delegate, final URIResolver uriResolver, final Supplier emptySource, final boolean overrideDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); this.uriResolver = uriResolver; this.emptySource = emptySource; - this.useDefaultParser = useDefaultParser; + this.overrideDefaultParser = overrideDefaultParser; } /** @@ -93,6 +93,6 @@ public Transformer newTransformer() throws TransformerConfigurationException { if (transformer == null) { return null; } - return new HardeningTransformer(transformer, uriResolver, emptySource, useDefaultParser); + return new HardeningTransformer(transformer, uriResolver, emptySource, overrideDefaultParser); } } diff --git a/src/main/java/org/apache/commons/xml/HardeningTemplatesHandler.java b/src/main/java/org/apache/commons/xml/HardeningTemplatesHandler.java index 25e63030..537d2add 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTemplatesHandler.java +++ b/src/main/java/org/apache/commons/xml/HardeningTemplatesHandler.java @@ -54,7 +54,7 @@ final class HardeningTemplatesHandler implements TemplatesHandler { /** * Snapshot of the factory's {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} outcome, carried onto the produced Templates. */ - private final boolean useDefaultParser; + private final boolean overrideDefaultParser; /** * Constructs a new instance. @@ -62,15 +62,15 @@ final class HardeningTemplatesHandler implements TemplatesHandler { * @param delegate the delegate to wrap; must not be {@code null}. * @param uriResolver the compile-time URIResolver snapshot to restore onto Transformers produced from the compiled Templates; may be {@code null}. * @param emptySource the empty-{@link Source} supplier for the produced Templates; may be {@code null} for the default empty DOM document. - * @param useDefaultParser whether the produced Templates' source rewrites should pin the platform's built-in parser. + * @param overrideDefaultParser whether the produced Templates' source rewrites should use the pluggable parser lookup instead of the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ HardeningTemplatesHandler(final TemplatesHandler delegate, final URIResolver uriResolver, final Supplier emptySource, - final boolean useDefaultParser) { + final boolean overrideDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); this.uriResolver = uriResolver; this.emptySource = emptySource; - this.useDefaultParser = useDefaultParser; + this.overrideDefaultParser = overrideDefaultParser; } @Override @@ -102,7 +102,7 @@ public String getSystemId() { public Templates getTemplates() { // Null before the stylesheet's endDocument (and on a failed compile in some implementations). final Templates templates = delegate.getTemplates(); - return templates == null ? null : new HardeningTemplates(templates, uriResolver, emptySource, useDefaultParser); + return templates == null ? null : new HardeningTemplates(templates, uriResolver, emptySource, overrideDefaultParser); } @Override diff --git a/src/main/java/org/apache/commons/xml/HardeningTransformer.java b/src/main/java/org/apache/commons/xml/HardeningTransformer.java index 381c3b75..2229168e 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTransformer.java +++ b/src/main/java/org/apache/commons/xml/HardeningTransformer.java @@ -55,7 +55,7 @@ final class HardeningTransformer extends Transformer { * Snapshot of the factory's {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} outcome at creation, like the JDK copies the feature onto the * transformers it creates. */ - private final boolean useDefaultParser; + private final boolean overrideDefaultParser; /** * Constructs a new instance. @@ -63,14 +63,14 @@ final class HardeningTransformer extends Transformer { * @param delegate the delegate to wrap; must not be {@code null}. * @param uriResolver the compile-time URIResolver snapshot to seed the floor with; may be {@code null}. * @param emptySource the empty-{@link Source} supplier for the produced Transformers; {@code null} for the default empty DOM document. - * @param useDefaultParser whether the source rewrites should pin the platform's built-in parser. + * @param overrideDefaultParser whether the source rewrites should use the pluggable parser lookup instead of the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ - HardeningTransformer(final Transformer delegate, final URIResolver uriResolver, final Supplier emptySource, final boolean useDefaultParser) { + HardeningTransformer(final Transformer delegate, final URIResolver uriResolver, final Supplier emptySource, final boolean overrideDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); this.uriResolver = uriResolver; - this.useDefaultParser = useDefaultParser; - this.floor = new FallbackIgnoreURIResolver(uriResolver, emptySource, () -> useDefaultParser); + this.overrideDefaultParser = overrideDefaultParser; + this.floor = new FallbackIgnoreURIResolver(uriResolver, emptySource, () -> overrideDefaultParser); delegate.setURIResolver(floor); } @@ -145,7 +145,7 @@ public void setURIResolver(final URIResolver resolver) { @Override public void transform(final Source xmlSource, final Result outputTarget) throws TransformerException { try { - delegate.transform(HardeningSAXParserFactory.harden(xmlSource, useDefaultParser), outputTarget); + delegate.transform(HardeningSAXParserFactory.harden(xmlSource, overrideDefaultParser), outputTarget); } catch (final TransformerConfigurationException e) { throw new TransformerException(e); } diff --git a/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java b/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java index 21c2e2d5..29450165 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java @@ -237,7 +237,7 @@ private Source hardenSourceToDom(final Source source) throws TransformerConfigur final InputSource inputSource = SAXSource.sourceToInputSource(source); if (inputSource != null) { try { - final DocumentBuilderFactory factory = HardeningDocumentBuilderFactory.newNSInstance(useDefaultParser()); + final DocumentBuilderFactory factory = HardeningDocumentBuilderFactory.newNSInstance(overrideDefaultParser()); final Document document = factory.newDocumentBuilder().parse(inputSource); return new DOMSource(document, inputSource.getSystemId()); } catch (final ParserConfigurationException | SAXException | IOException e) { @@ -245,7 +245,7 @@ private Source hardenSourceToDom(final Source source) throws TransformerConfigur } } } - return HardeningSAXParserFactory.harden(source, useDefaultParser()); + return HardeningSAXParserFactory.harden(source, overrideDefaultParser()); } /** @@ -314,7 +314,7 @@ private Wrapper(final SAXTransformerFactory delegate, final Supplier emp this.delegate = Objects.requireNonNull(delegate, "delegate"); this.emptySource = emptySource; this.supportsOverrideDefaultParser = probeOverrideDefaultParser(delegate); - this.floor = new FallbackIgnoreURIResolver(null, emptySource, this::useDefaultParser); + this.floor = new FallbackIgnoreURIResolver(null, emptySource, this::overrideDefaultParser); // Compile-time block for xsl:import/xsl:include and document(); a caller-set resolver is routed through the floor rather than replacing it. delegate.setURIResolver(floor); } @@ -329,7 +329,7 @@ private Wrapper(final SAXTransformerFactory delegate, final Supplier emp public Source getAssociatedStylesheet(final Source source, final String media, final String title, final String charset) throws TransformerConfigurationException { // Xalan's getAssociatedStylesheet drops a SAXSource's reader and self-provisions its own to scan for xml-stylesheet PIs (XALANJ-2849). - final Source hardened = isXalan(delegate) ? hardenSourceToDom(source) : HardeningSAXParserFactory.harden(source, useDefaultParser()); + final Source hardened = isXalan(delegate) ? hardenSourceToDom(source) : HardeningSAXParserFactory.harden(source, overrideDefaultParser()); return delegate.getAssociatedStylesheet(hardened, media, title, charset); } @@ -354,7 +354,7 @@ public URIResolver getURIResolver() { } private TransformerHandler hardenHandler(final TransformerHandler handler) { - return handler == null ? null : new HardeningTransformerHandler(handler, getURIResolver(), emptySource, useDefaultParser()); + return handler == null ? null : new HardeningTransformerHandler(handler, getURIResolver(), emptySource, overrideDefaultParser()); } /** @@ -365,21 +365,21 @@ private TransformerHandler hardenHandler(final TransformerHandler handler) { */ @Override public Templates newTemplates(final Source source) throws TransformerConfigurationException { - final Templates templates = delegate.newTemplates(HardeningSAXParserFactory.harden(source, useDefaultParser())); - return templates == null ? null : new HardeningTemplates(templates, getURIResolver(), emptySource, useDefaultParser()); + final Templates templates = delegate.newTemplates(HardeningSAXParserFactory.harden(source, overrideDefaultParser())); + return templates == null ? null : new HardeningTemplates(templates, getURIResolver(), emptySource, overrideDefaultParser()); } @Override public TemplatesHandler newTemplatesHandler() throws TransformerConfigurationException { final TemplatesHandler handler = delegate.newTemplatesHandler(); - return handler == null ? null : new HardeningTemplatesHandler(handler, getURIResolver(), emptySource, useDefaultParser()); + return handler == null ? null : new HardeningTemplatesHandler(handler, getURIResolver(), emptySource, overrideDefaultParser()); } @Override public Transformer newTransformer() throws TransformerConfigurationException { // Identity transformer: still parses runtime sources, so wrap it to harden Transformer.transform(Source, Result). final Transformer transformer = delegate.newTransformer(); - return transformer == null ? null : new HardeningTransformer(transformer, getURIResolver(), emptySource, useDefaultParser()); + return transformer == null ? null : new HardeningTransformer(transformer, getURIResolver(), emptySource, overrideDefaultParser()); } /** @@ -390,8 +390,8 @@ public Transformer newTransformer() throws TransformerConfigurationException { */ @Override public Transformer newTransformer(final Source source) throws TransformerConfigurationException { - final Transformer transformer = delegate.newTransformer(HardeningSAXParserFactory.harden(source, useDefaultParser())); - return transformer == null ? null : new HardeningTransformer(transformer, getURIResolver(), emptySource, useDefaultParser()); + final Transformer transformer = delegate.newTransformer(HardeningSAXParserFactory.harden(source, overrideDefaultParser())); + return transformer == null ? null : new HardeningTransformer(transformer, getURIResolver(), emptySource, overrideDefaultParser()); } @Override @@ -407,7 +407,7 @@ public TransformerHandler newTransformerHandler() throws TransformerConfiguratio */ @Override public TransformerHandler newTransformerHandler(final Source source) throws TransformerConfigurationException { - return hardenHandler(delegate.newTransformerHandler(HardeningSAXParserFactory.harden(source, useDefaultParser()))); + return hardenHandler(delegate.newTransformerHandler(HardeningSAXParserFactory.harden(source, overrideDefaultParser()))); } @Override @@ -431,7 +431,7 @@ public XMLFilter newXMLFilter(final Source source) throws TransformerConfigurati @Override public XMLFilter newXMLFilter(final Templates templates) throws TransformerConfigurationException { return new HardeningXMLFilter(templates instanceof HardeningTemplates ? (HardeningTemplates) templates - : new HardeningTemplates(templates, getURIResolver(), emptySource, useDefaultParser())); + : new HardeningTemplates(templates, getURIResolver(), emptySource, overrideDefaultParser())); } @Override @@ -456,15 +456,15 @@ public void setURIResolver(final URIResolver resolver) { } /** - * Checks whether parsers should be instantiated via {@code newDefaultInstance()} instead of {@code newInstance()}. + * Checks whether parsers should be instantiated via {@code newInstance()} instead of {@code newDefaultInstance()}. * *

    The JDK implementation of {@link TransformerFactory} uses the JDK parsers while {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} is unset * or {@code false}.

    * - * @return {@code true} if parsers should be created via {@code newDefaultInstance()}. + * @return {@code true} if parsers should be created via {@code newInstance()}. */ - private boolean useDefaultParser() { - return supportsOverrideDefaultParser && !delegate.getFeature(HardeningSAXParserFactory.OVERRIDE_DEFAULT_PARSER); + private boolean overrideDefaultParser() { + return !supportsOverrideDefaultParser || delegate.getFeature(HardeningSAXParserFactory.OVERRIDE_DEFAULT_PARSER); } } } diff --git a/src/main/java/org/apache/commons/xml/HardeningTransformerHandler.java b/src/main/java/org/apache/commons/xml/HardeningTransformerHandler.java index c6223f01..0f467591 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTransformerHandler.java +++ b/src/main/java/org/apache/commons/xml/HardeningTransformerHandler.java @@ -54,13 +54,13 @@ final class HardeningTransformerHandler implements TransformerHandler { * @param delegate the delegate to wrap; must not be {@code null}. * @param uriResolver the compile-time URIResolver snapshot to restore onto the live transformer; may be {@code null}. * @param emptySource the empty-{@link Source} supplier for the produced Transformer's floor; {@code null} means the default empty DOM. - * @param useDefaultParser whether the live transformer's source rewrites should pin the platform's built-in parser. + * @param overrideDefaultParser whether the live transformer's source rewrites should use the pluggable parser lookup instead of the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ HardeningTransformerHandler(final TransformerHandler delegate, final URIResolver uriResolver, final Supplier emptySource, - final boolean useDefaultParser) { + final boolean overrideDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); - this.transformer = new HardeningTransformer(delegate.getTransformer(), uriResolver, emptySource, useDefaultParser); + this.transformer = new HardeningTransformer(delegate.getTransformer(), uriResolver, emptySource, overrideDefaultParser); } @Override diff --git a/src/main/java/org/apache/commons/xml/HardeningValidator.java b/src/main/java/org/apache/commons/xml/HardeningValidator.java index 494f2b73..d6a34164 100644 --- a/src/main/java/org/apache/commons/xml/HardeningValidator.java +++ b/src/main/java/org/apache/commons/xml/HardeningValidator.java @@ -47,18 +47,18 @@ final class HardeningValidator extends Validator { * Snapshot of the factory's {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} outcome at creation, like the JDK copies the feature onto its * validators. */ - private final boolean useDefaultParser; + private final boolean overrideDefaultParser; /** * Constructs a new instance. * * @param delegate the delegate to wrap; must not be {@code null}. - * @param useDefaultParser whether the source rewrites should pin the platform's built-in parser. + * @param overrideDefaultParser whether the source rewrites should use the pluggable parser lookup instead of the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ - HardeningValidator(final Validator delegate, final boolean useDefaultParser) { + HardeningValidator(final Validator delegate, final boolean overrideDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); - this.useDefaultParser = useDefaultParser; + this.overrideDefaultParser = overrideDefaultParser; // Block xsi:schemaLocation resolution; neither the JDK nor Xerces reliably propagates the factory's resolver to its Validators. The floor is a // non-removable lower bound: a caller opts specific lookups in by setting their own resolver, but cannot drop the ignore-all block. delegate.setResourceResolver(floor); @@ -121,7 +121,7 @@ public void setResourceResolver(final LSResourceResolver resourceResolver) { @Override public void validate(final Source source, final Result result) throws SAXException, IOException { try { - delegate.validate(HardeningSAXParserFactory.harden(source, useDefaultParser), result); + delegate.validate(HardeningSAXParserFactory.harden(source, overrideDefaultParser), result); } catch (final TransformerConfigurationException e) { throw new SAXException("Failed to harden source for validation", e); } diff --git a/src/main/java/org/apache/commons/xml/HardeningXMLFilter.java b/src/main/java/org/apache/commons/xml/HardeningXMLFilter.java index 4dd42990..a72179b1 100644 --- a/src/main/java/org/apache/commons/xml/HardeningXMLFilter.java +++ b/src/main/java/org/apache/commons/xml/HardeningXMLFilter.java @@ -70,7 +70,7 @@ public void parse(final InputSource input) throws SAXException, IOException { } if (getParent() == null) { try { - setParent(HardeningSAXParserFactory.newHardenedReader(templates.useDefaultParser)); + setParent(HardeningSAXParserFactory.newHardenedReader(templates.overrideDefaultParser)); } catch (final TransformerException e) { throw new SAXException(e); } diff --git a/src/main/java/org/apache/commons/xml/HardeningXPath.java b/src/main/java/org/apache/commons/xml/HardeningXPath.java index 07cda123..f36042d4 100644 --- a/src/main/java/org/apache/commons/xml/HardeningXPath.java +++ b/src/main/java/org/apache/commons/xml/HardeningXPath.java @@ -56,7 +56,7 @@ final class HardeningXPath implements XPath { * engine would have provisioned. * * @param source The document to evaluate against. - * @param useDefaultParser Whether the document build should pin the platform's built-in parser. + * @param overrideDefaultParser Whether the document build should use the pluggable parser lookup instead of the platform's built-in parser. * @return The parsed document. * @throws NullPointerException if {@code source} is {@code null}, per the {@link XPath} contract. * @throws XPathExpressionException if the source cannot be parsed. @@ -64,10 +64,10 @@ final class HardeningXPath implements XPath { * configuration error} or if the implementation is not available or cannot be instantiated. * @throws HardeningException Thrown if a (non-Andoid) factory cannot support the secure processing feature {@link XMLConstants#FEATURE_SECURE_PROCESSING}. */ - static Document parse(final InputSource source, final boolean useDefaultParser) throws XPathExpressionException { + static Document parse(final InputSource source, final boolean overrideDefaultParser) throws XPathExpressionException { Objects.requireNonNull(source, "source"); try { - final DocumentBuilderFactory factory = HardeningDocumentBuilderFactory.newNSInstance(useDefaultParser); + final DocumentBuilderFactory factory = HardeningDocumentBuilderFactory.newNSInstance(overrideDefaultParser); return factory.newDocumentBuilder().parse(source); } catch (final ParserConfigurationException | SAXException | IOException e) { throw new XPathExpressionException(e); @@ -79,24 +79,24 @@ static Document parse(final InputSource source, final boolean useDefaultParser) /** * Snapshot of the factory's {@code jdk.xml.overrideDefaultParser} outcome at creation, like the JDK copies the feature onto the XPath objects it creates. */ - final boolean useDefaultParser; + final boolean overrideDefaultParser; /** * Constructs a new instance. * * @param delegate the delegate to wrap; must not be {@code null}. - * @param useDefaultParser whether the {@link InputSource} document builds should pin the platform's built-in parser. + * @param overrideDefaultParser whether the {@link InputSource} document builds should use the pluggable parser lookup instead of the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ - HardeningXPath(final XPath delegate, final boolean useDefaultParser) { + HardeningXPath(final XPath delegate, final boolean overrideDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); - this.useDefaultParser = useDefaultParser; + this.overrideDefaultParser = overrideDefaultParser; } @Override public XPathExpression compile(final String expression) throws XPathExpressionException { final XPathExpression compiled = delegate.compile(expression); - return compiled == null ? null : new HardeningXPathExpression(compiled, useDefaultParser); + return compiled == null ? null : new HardeningXPathExpression(compiled, overrideDefaultParser); } /** @@ -107,7 +107,7 @@ public XPathExpression compile(final String expression) throws XPathExpressionEx */ @Override public String evaluate(final String expression, final InputSource source) throws XPathExpressionException { - return delegate.evaluate(expression, parse(source, useDefaultParser)); + return delegate.evaluate(expression, parse(source, overrideDefaultParser)); } /** @@ -118,7 +118,7 @@ public String evaluate(final String expression, final InputSource source) throws */ @Override public Object evaluate(final String expression, final InputSource source, final QName returnType) throws XPathExpressionException { - return delegate.evaluate(expression, parse(source, useDefaultParser), returnType); + return delegate.evaluate(expression, parse(source, overrideDefaultParser), returnType); } @Override diff --git a/src/main/java/org/apache/commons/xml/HardeningXPathExpression.java b/src/main/java/org/apache/commons/xml/HardeningXPathExpression.java index 020a0792..851dcb43 100644 --- a/src/main/java/org/apache/commons/xml/HardeningXPathExpression.java +++ b/src/main/java/org/apache/commons/xml/HardeningXPathExpression.java @@ -41,18 +41,18 @@ final class HardeningXPathExpression implements XPathExpression { /** * Snapshot of the factory's {@code jdk.xml.overrideDefaultParser} outcome, inherited from the {@link HardeningXPath} that compiled this expression. */ - private final boolean useDefaultParser; + private final boolean overrideDefaultParser; /** * Constructs a new instance. * * @param delegate the delegate to wrap; must not be {@code null}. - * @param useDefaultParser whether the {@link InputSource} document builds should pin the platform's built-in parser. + * @param overrideDefaultParser whether the {@link InputSource} document builds should use the pluggable parser lookup instead of the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ - HardeningXPathExpression(final XPathExpression delegate, final boolean useDefaultParser) { + HardeningXPathExpression(final XPathExpression delegate, final boolean overrideDefaultParser) { this.delegate = Objects.requireNonNull(delegate, "delegate"); - this.useDefaultParser = useDefaultParser; + this.overrideDefaultParser = overrideDefaultParser; } /** @@ -63,7 +63,7 @@ final class HardeningXPathExpression implements XPathExpression { */ @Override public String evaluate(final InputSource source) throws XPathExpressionException { - return delegate.evaluate(HardeningXPath.parse(source, useDefaultParser)); + return delegate.evaluate(HardeningXPath.parse(source, overrideDefaultParser)); } /** @@ -74,7 +74,7 @@ public String evaluate(final InputSource source) throws XPathExpressionException */ @Override public Object evaluate(final InputSource source, final QName returnType) throws XPathExpressionException { - return delegate.evaluate(HardeningXPath.parse(source, useDefaultParser), returnType); + return delegate.evaluate(HardeningXPath.parse(source, overrideDefaultParser), returnType); } @Override diff --git a/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java b/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java index 2658cb85..87db0101 100644 --- a/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java @@ -226,7 +226,7 @@ public boolean isObjectModelSupported(final String objectModel) { @Override public XPath newXPath() { final XPath xpath = delegate.newXPath(); - return xpath == null ? null : new HardeningXPath(xpath, useDefaultParser()); + return xpath == null ? null : new HardeningXPath(xpath, overrideDefaultParser()); } @Override @@ -235,18 +235,18 @@ public void setFeature(final String name, final boolean value) throws XPathFacto } /** - * Checks whether parsers should be instantiated via {@code newDefaultInstance()} instead of {@code newInstance()}. + * Checks whether parsers should be instantiated via {@code newInstance()} instead of {@code newDefaultInstance()}. * *

    The JDK implementation of {@link XPathFactory} uses the JDK parsers while {@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER} is unset or * {@code false}.

    * - * @return {@code true} if parsers should be created via {@code newDefaultInstance()}. + * @return {@code true} if parsers should be created via {@code newInstance()}. */ - private boolean useDefaultParser() { + private boolean overrideDefaultParser() { try { - return !delegate.getFeature(HardeningSAXParserFactory.OVERRIDE_DEFAULT_PARSER); + return delegate.getFeature(HardeningSAXParserFactory.OVERRIDE_DEFAULT_PARSER); } catch (final XPathFactoryConfigurationException e) { - return false; + return true; } } diff --git a/src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java b/src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java index edf317f8..ef67fef2 100644 --- a/src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java +++ b/src/test/java/org/apache/commons/xml/OverrideDefaultParserTest.java @@ -64,9 +64,9 @@ private static String transform(final TransformerFactory factory, final String t @Test void hardenedReaderFollowsFlag() throws Exception { assumeFalse(AttackTestSupport.IS_ANDROID); - final XMLReader pinned = ((HardeningXMLReader) HardeningSAXParserFactory.newHardenedReader(true)).getDelegate(); + final XMLReader pinned = ((HardeningXMLReader) HardeningSAXParserFactory.newHardenedReader(false)).getDelegate(); assertTrue(pinned.getClass().getName().startsWith(JDK_INTERNAL_PREFIX), pinned.getClass().getName()); - final XMLReader pluggable = ((HardeningXMLReader) HardeningSAXParserFactory.newHardenedReader(false)).getDelegate(); + final XMLReader pluggable = ((HardeningXMLReader) HardeningSAXParserFactory.newHardenedReader(true)).getDelegate(); final XMLReader lookedUp = ((HardeningXMLReader) HardeningSAXParserFactory.newNSInstance().newSAXParser().getXMLReader()).getDelegate(); assertEquals(lookedUp.getClass(), pluggable.getClass()); if (xercesOnClasspath()) { @@ -80,9 +80,9 @@ void schemaFactoryReadsFeatureAtCreation() throws Exception { assumeFalse(AttackTestSupport.IS_ANDROID); final SchemaFactory factory = HardeningSchemaFactory.newDefaultInstance(); assertFalse(factory.getFeature(FEATURE)); - assertTrue(((HardeningSchema) factory.newSchema(AttackTestSupport.streamSource(AttackTestSupport.BENIGN_SCHEMA))).useDefaultParser); + assertFalse(((HardeningSchema) factory.newSchema(AttackTestSupport.streamSource(AttackTestSupport.BENIGN_SCHEMA))).overrideDefaultParser); factory.setFeature(FEATURE, true); - assertFalse(((HardeningSchema) factory.newSchema(AttackTestSupport.streamSource(AttackTestSupport.BENIGN_SCHEMA))).useDefaultParser); + assertTrue(((HardeningSchema) factory.newSchema(AttackTestSupport.streamSource(AttackTestSupport.BENIGN_SCHEMA))).overrideDefaultParser); } @Test @@ -90,9 +90,9 @@ void transformerFactoryReadsFeatureAtCreation() throws Exception { assumeFalse(AttackTestSupport.IS_ANDROID); final TransformerFactory factory = HardeningTransformerFactory.newDefaultInstance(); assertFalse(factory.getFeature(FEATURE)); - assertTrue(((HardeningTemplates) factory.newTemplates(AttackTestSupport.streamSource(AttackTestSupport.xsltBody("probe")))).useDefaultParser); + assertFalse(((HardeningTemplates) factory.newTemplates(AttackTestSupport.streamSource(AttackTestSupport.xsltBody("probe")))).overrideDefaultParser); factory.setFeature(FEATURE, true); - assertFalse(((HardeningTemplates) factory.newTemplates(AttackTestSupport.streamSource(AttackTestSupport.xsltBody("probe")))).useDefaultParser); + assertTrue(((HardeningTemplates) factory.newTemplates(AttackTestSupport.streamSource(AttackTestSupport.xsltBody("probe")))).overrideDefaultParser); } @Test @@ -123,8 +123,8 @@ void xPathFactoryReadsFeatureAtCreation() throws Exception { assumeFalse(AttackTestSupport.IS_ANDROID); final XPathFactory factory = HardeningXPathFactory.newDefaultInstance(); assertFalse(factory.getFeature(FEATURE)); - assertTrue(((HardeningXPath) factory.newXPath()).useDefaultParser); + assertFalse(((HardeningXPath) factory.newXPath()).overrideDefaultParser); factory.setFeature(FEATURE, true); - assertFalse(((HardeningXPath) factory.newXPath()).useDefaultParser); + assertTrue(((HardeningXPath) factory.newXPath()).overrideDefaultParser); } } From ffe5b92279eb8f2bd062b090a858d7df15bd586f Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 28 Aug 2026 17:08:24 +0200 Subject: [PATCH 09/13] Name the factory system properties like the JDK's JdkXmlUtils Assisted-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ErRKq7RUeQ9LGrSboSUyYm --- .../commons/xml/HardeningDocumentBuilderFactory.java | 6 ++++-- .../org/apache/commons/xml/HardeningSAXParserFactory.java | 7 +++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java b/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java index 81b00a65..2421305f 100644 --- a/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningDocumentBuilderFactory.java @@ -50,6 +50,8 @@ public final class HardeningDocumentBuilderFactory { /** Class name of Android's Harmony-based {@link DocumentBuilderFactory}, which exposes no hardening surface. */ private static final String ANDROID_DOCUMENT_BUILDER_FACTORY = "org.apache.harmony.xml.parsers.DocumentBuilderFactoryImpl"; + /** System property naming the {@link DocumentBuilderFactory} implementation, the JDK's own mechanism for reconfiguring the default parser. */ + private static final String DOM_FACTORY_ID = "javax.xml.parsers.DocumentBuilderFactory"; /** Class name of the JDK's built-in default implementation, the Java 8 fallback for {@link #newDefaultInstance()}. */ private static final String JDK_DOCUMENT_BUILDER_FACTORY = "com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl"; @@ -188,7 +190,7 @@ public static DocumentBuilderFactory newNSInstance() { * Returns the hardened, namespace-aware factory the Source-rewriting wrappers parse with. *

    * While {@code overrideDefaultParser} is {@code false} the factory is the JDK's "default parser" factory, determined the way the JDK itself determines it: the built-in - * implementation, unless the {@code javax.xml.parsers.DocumentBuilderFactory} system property is set — that property is the JDK's own mechanism for + * implementation, unless the {@value #DOM_FACTORY_ID} system property is set — that property is the JDK's own mechanism for * reconfiguring the default parser, so it is honored through the standard lookup rather than bypassed. *

    * @@ -199,7 +201,7 @@ public static DocumentBuilderFactory newNSInstance() { * implementation is not available or cannot be instantiated. */ static DocumentBuilderFactory newNSInstance(final boolean overrideDefaultParser) { - return overrideDefaultParser || System.getProperty("javax.xml.parsers.DocumentBuilderFactory") != null ? newNSInstance() : newDefaultNSInstance(); + return overrideDefaultParser || System.getProperty(DOM_FACTORY_ID) != null ? newNSInstance() : newDefaultNSInstance(); } /** diff --git a/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java b/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java index 461477e6..11a8034e 100644 --- a/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningSAXParserFactory.java @@ -69,6 +69,9 @@ public final class HardeningSAXParserFactory { */ static final String OVERRIDE_DEFAULT_PARSER = "jdk.xml.overrideDefaultParser"; + /** System property naming the {@link SAXParserFactory} implementation, the JDK's own mechanism for reconfiguring the default parser. */ + private static final String SAX_FACTORY_ID = "javax.xml.parsers.SAXParserFactory"; + private static final MethodHandle NEW_DEFAULT_INSTANCE = MethodHandleFactory.findStatic(SAXParserFactory.class, "newDefaultInstance", MethodType.methodType(SAXParserFactory.class)); @@ -266,7 +269,7 @@ public static SAXParserFactory newNSInstance() { * Returns the hardened, namespace-aware factory the Source-rewriting wrappers parse with. *

    * While {@code overrideDefaultParser} is {@code false} the factory is the JDK's "default parser" factory, determined the way the JDK itself determines it: the built-in parser, - * unless the {@code javax.xml.parsers.SAXParserFactory} system property is set — that property is the JDK's own mechanism for reconfiguring the default + * unless the {@value #SAX_FACTORY_ID} system property is set — that property is the JDK's own mechanism for reconfiguring the default * parser, so it is honored through the standard lookup rather than bypassed. *

    * @@ -277,7 +280,7 @@ public static SAXParserFactory newNSInstance() { * implementation is not available or cannot be instantiated. */ static SAXParserFactory newNSInstance(final boolean overrideDefaultParser) { - return overrideDefaultParser || System.getProperty("javax.xml.parsers.SAXParserFactory") != null ? newNSInstance() : newDefaultNSInstance(); + return overrideDefaultParser || System.getProperty(SAX_FACTORY_ID) != null ? newNSInstance() : newDefaultNSInstance(); } /** From aa9d38597e773131b569dbdbb6883af8538b1f17 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 28 Aug 2026 17:12:55 +0200 Subject: [PATCH 10/13] Drop the FODP bullet from the harden() Javadoc Neither harden() reads the feature; the selection logic is documented on the wrappers' overrideDefaultParser() methods. Assisted-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01ErRKq7RUeQ9LGrSboSUyYm --- .../org/apache/commons/xml/HardeningTransformerFactory.java | 3 --- .../java/org/apache/commons/xml/HardeningXPathFactory.java | 4 ---- 2 files changed, 7 deletions(-) diff --git a/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java b/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java index 29450165..0908aa6e 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java @@ -93,9 +93,6 @@ public final class HardeningTransformerFactory { * difference is the empty-{@link Source} shape the floor returns, {@code EmptySource} for Saxon rather than the default empty DOM document. *
  • FSP ({@link XMLConstants#FEATURE_SECURE_PROCESSING}): required. On XSLTC it enables the runtime evaluator limits; on Xalan it disables * reflection-based extension functions.
    • - *
  • FODP ({@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER}): read, not set. The implementation's internal parsers are never - * used (the wrapper rewrites every source), so on implementations that recognize the feature its value selects which hardened parser family performs - * the rewrites: the platform's built-in parser when {@code false} (the JDK's default), the pluggable lookup when {@code true} or unrecognized.
    • *
  • {@link FallbackIgnoreURIResolver} floor: required. An ignore-all {@link URIResolver} floor, installed by * the nested wrapper and carried onto every produced {@link Transformer}, resolves {@code xsl:import}/{@code xsl:include} at compile * time and {@code document()} at runtime to an empty document, the one channel both XSLTC and Xalan route through. A caller-set {@link URIResolver} is diff --git a/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java b/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java index 87db0101..0bf18ce8 100644 --- a/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java +++ b/src/main/java/org/apache/commons/xml/HardeningXPathFactory.java @@ -67,10 +67,6 @@ public final class HardeningXPathFactory { * documented package-prefix exception because the required hardening surface is reachable only through a vendor API.
    • *
  • FSP ({@link javax.xml.XMLConstants#FEATURE_SECURE_PROCESSING}): required. It is the only knob both the stock JDK and Xalan XPath * engines expose, and switches on their secure-processing limits. {@link XPathFactory} has no attribute API for finer control.
    • - *
  • FODP ({@value HardeningSAXParserFactory#OVERRIDE_DEFAULT_PARSER}): read, not set. The engine's internal parser is never used (the - * wrapper builds the {@link org.xml.sax.InputSource} documents itself), so on implementations that recognize the feature its value selects which - * hardened parser family performs that build: the platform's built-in parser when {@code false} (the JDK's default), the pluggable lookup when - * {@code true} or unrecognized.
    • *
  • The nested wrapper: required. FSP governs only the engine, not the parser it provisions internally for the * {@link org.xml.sax.InputSource}-taking {@code evaluate} entry points; the wrapper performs that document build with a hardened parser instead, so * the engine never parses.
    • From fed3a4dacb867b42db70627bd11ec33274c4a6a9 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Fri, 28 Aug 2026 12:38:22 -0400 Subject: [PATCH 11/13] Javadoc tweak. --- src/main/java/org/apache/commons/xml/HardeningTemplates.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/apache/commons/xml/HardeningTemplates.java b/src/main/java/org/apache/commons/xml/HardeningTemplates.java index 5b800928..8aa0fa71 100644 --- a/src/main/java/org/apache/commons/xml/HardeningTemplates.java +++ b/src/main/java/org/apache/commons/xml/HardeningTemplates.java @@ -62,7 +62,7 @@ final class HardeningTemplates implements Templates { * * @param delegate the delegate to wrap; must not be {@code null}. * @param uriResolver the compile-time URIResolver snapshot to restore onto Transformers produced from the compiled Templates; may be {@code null}. - * @param emptySource the empty-{@link Source} supplier for the produced Transformers + * @param emptySource the empty-{@link Source} supplier for the produced Transformers. * @param overrideDefaultParser whether the produced Transformers' source rewrites should use the pluggable parser lookup instead of the platform's built-in parser. * @throws NullPointerException if {@code delegate} is {@code null}. */ From af89c933575c900ca16d0293add997877db8e38f Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Fri, 28 Aug 2026 12:41:37 -0400 Subject: [PATCH 12/13] Javadoc tweaks. Clarified description of factory instances and their behavior. --- src/main/java/org/apache/commons/xml/package-info.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/xml/package-info.java b/src/main/java/org/apache/commons/xml/package-info.java index 835207ed..aaeaf318 100644 --- a/src/main/java/org/apache/commons/xml/package-info.java +++ b/src/main/java/org/apache/commons/xml/package-info.java @@ -22,8 +22,8 @@ * caching. *

    *

    - * A returned factory is not necessarily an instance of the underlying implementation — it might be (and usually is) a wrapper around it, so it cannot be cast - * to the implementation's own class. Everything else about the implementation's behavior is preserved — features, properties, and attributes delegate to it — + * A returned factory is not necessarily an instance of the underlying implementation. It might be (and usually is) a wrapper around it, so it cannot be cast + * to the implementation's own class. Everything else about the implementation's behavior is preserved: features, properties, and attributes delegate to it, * and only the security behavior is hardened. *

    *

    From 73c452e0e6d6e380128aef504055b69ffc82f22e Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Fri, 28 Aug 2026 12:43:08 -0400 Subject: [PATCH 13/13] Match previous commit in text style. Clarify the explanation about factory instances and their behavior. --- src/site/markdown/index.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/site/markdown/index.md b/src/site/markdown/index.md index 408288e4..f3f4960a 100644 --- a/src/site/markdown/index.md +++ b/src/site/markdown/index.md @@ -148,11 +148,11 @@ HardeningSchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI) ### Wrappers, not the original factories -A returned factory is not necessarily an instance of the underlying implementation: -it might be (and usually is) a wrapper around it, +A returned factory is not necessarily an instance of the underlying implementation. +It might be (and usually is) a wrapper around it, so it cannot be cast to the implementation's own class. -Everything else about the implementation's behavior is preserved — -features, properties, and attributes delegate to it — +Everything else about the implementation's behavior is preserved: +features, properties, and attributes delegate to it, and only the security behavior is hardened. Preserved behavior includes the choice of internal parsers.