This is an automated email from the ASF dual-hosted git repository. ppkarwasz pushed a commit to branch feature/restore-override-default-parser in repository https://gitbox.apache.org/repos/asf/commons-xml.git
commit 7247fc43d7fe99ce37c67b13e2e3873598ce8098 Author: Piotr P. Karwasz <[email protected]> AuthorDate: Fri Aug 28 10:58:53 2026 +0200 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 <[email protected]> Claude-Session: https://claude.ai/code/session_01ErRKq7RUeQ9LGrSboSUyYm --- pom.xml | 32 +++++- src/changes/changes.xml | 1 + .../commons/xml/FallbackIgnoreURIResolver.java | 17 ++- .../xml/HardeningDocumentBuilderFactory.java | 16 +++ .../commons/xml/HardeningSAXParserFactory.java | 28 ++++- .../org/apache/commons/xml/HardeningSchema.java | 15 ++- .../apache/commons/xml/HardeningSchemaFactory.java | 32 ++++-- .../org/apache/commons/xml/HardeningTemplates.java | 18 ++- .../commons/xml/HardeningTemplatesHandler.java | 12 +- .../apache/commons/xml/HardeningTransformer.java | 22 ++-- .../commons/xml/HardeningTransformerFactory.java | 69 ++++++++--- .../commons/xml/HardeningTransformerHandler.java | 6 +- .../org/apache/commons/xml/HardeningValidator.java | 16 ++- .../org/apache/commons/xml/HardeningXMLFilter.java | 2 +- .../org/apache/commons/xml/HardeningXMLReader.java | 9 ++ .../org/apache/commons/xml/HardeningXPath.java | 24 ++-- .../commons/xml/HardeningXPathExpression.java | 15 ++- .../apache/commons/xml/HardeningXPathFactory.java | 45 ++++---- .../java/org/apache/commons/xml/SaxonProvider.java | 2 +- src/site/markdown/threat_model.md | 9 +- .../org/apache/commons/xml/DenyUnresolvedTest.java | 2 +- .../commons/xml/OverrideDefaultParserTest.java | 126 +++++++++++++++++++++ .../apache/commons/xml/ShadingFootprintTest.java | 2 +- .../apache/commons/xml/XPathInputSourceTest.java | 2 +- 24 files changed, 419 insertions(+), 103 deletions(-) diff --git a/pom.xml b/pom.xml index 389972f..984a6a1 100644 --- a/pom.xml +++ b/pom.xml @@ -146,8 +146,10 @@ limitations under the License. </pluginManagement> <plugins> <!-- - Run the test suite seven times, each with a distinct JAXP combination: + Run the test suite eight times, each with a distinct JAXP combination: + - test-jdk-xerces: stock JDK TrAX and XPath over Apache Xerces SAX/DOM + (the only cell where jdk.xml.overrideDefaultParser decides which parser rewrites sources) - test-saxon: Saxon-HE (TrAX, XPath) over stock JDK SAX - test-saxon-xerces: Saxon-HE (TrAX, XPath) over stock JDK SAX or Apache Xerces SAX (Saxon prefers the JDK SAX provider but may resolve to Xerces) @@ -322,6 +324,30 @@ limitations under the License. </classpathDependencyExcludes> </configuration> </execution> + <execution> + <id>test-jdk-xerces</id> + <goals> + <goal>test</goal> + </goals> + <configuration> + <reportsDirectory>${project.build.directory}/surefire-reports/jdk-xerces</reportsDirectory> + <!-- The only cell pairing the JDK TrAX and XPath implementations with a third-party ServiceLoader parser: Xerces resolves the + SAXParserFactory/DocumentBuilderFactory lookups (and the SchemaFactory one, which is why the schema-group FODP tests pin the JDK + implementation via newDefaultInstance), so jdk.xml.overrideDefaultParser genuinely decides which parser performs the hardened + source rewrites. --> + <groups>trax,xpath,schema</groups> + <additionalClasspathDependencies> + <dependency> + <groupId>xerces</groupId> + <artifactId>xercesImpl</artifactId> + <version>${commons.xerces.version}</version> + </dependency> + </additionalClasspathDependencies> + <classpathDependencyExcludes> + <exclude>net.sf.saxon:Saxon-HE</exclude> + </classpathDependencyExcludes> + </configuration> + </execution> </executions> </plugin> <!-- Do not publish a test-jar: the test classes are not part of the consumer contract. --> @@ -548,6 +574,10 @@ limitations under the License. <id>test-xerces</id> <phase>none</phase> </execution> + <execution> + <id>test-jdk-xerces</id> + <phase>none</phase> + </execution> </executions> </plugin> <plugin> diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 769d863..15662ef 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -43,6 +43,7 @@ The <action> type attribute can be add, update, fix, or remove. <action type="add" dev="ppkarwasz" due-to="Piotr P. Karwasz, Gary Gregory">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.</action> <!-- FIX --> <action type="fix" dev="ppkarwasz" due-to="Ta Duc Thien, Piotr P. Karwasz, Gary Gregory" issue="COMMONSXML-10">Block XInclude (xi:include) href resolution by default, since the JAXP external-access properties do not govern it.</action> + <action type="fix" dev="ppkarwasz" due-to="Piotr P. Karwasz, Gary Gregory">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.</action> <action type="fix" dev="ppkarwasz" due-to="Piotr P. Karwasz, Gary Gregory">Restore the hardened configuration when a factory or parser is reset() instead of reverting to the implementation defaults.</action> <action type="fix" dev="ppkarwasz" due-to="Piotr P. Karwasz, Gary Gregory">Parse a Source opted in by a caller-supplied URIResolver using a hardened parser.</action> <action type="fix" dev="ppkarwasz" due-to="Piotr P. Karwasz, Gary Gregory">Harden the document parse behind the InputSource-taking XPath evaluation entry points.</action> diff --git a/src/main/java/org/apache/commons/xml/FallbackIgnoreURIResolver.java b/src/main/java/org/apache/commons/xml/FallbackIgnoreURIResolver.java index cf3e920..6c2a38d 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<Source> 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<Source> emptySource) { + FallbackIgnoreURIResolver(final URIResolver delegate, final Supplier<Source> 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 1c16de6..719358e 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 9101acd..1e1d559 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. * </p> * - * @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. + * <p> + * 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. + * </p> * + * @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 e92374c..c45cbd5 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 539b8cf..44bf12f 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() { * <p>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)}.</p> + * {@link HardeningSAXParserFactory#harden(javax.xml.transform.Source, boolean)}.</p> * * @param factory the factory to harden; never {@code null}. * @return a hardened factory. @@ -160,14 +160,14 @@ private HardeningSchemaFactory() { * <ol> * <li>{@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)}.</li> + * through {@link HardeningSAXParserFactory#harden(Source, boolean)}.</li> * <li>{@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}.</li> * <li>{@link HardeningValidator} rewrites the Source on every {@link Validator#validate(Source)} call.</li> * </ol> * * <p> - * 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 56b339e..667f984 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<Source> 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<Source> emptySource) { + HardeningTemplates(final Templates delegate, final URIResolver uriResolver, final Supplier<Source> 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 814d967..25e6303 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<Source> 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<Source> emptySource) { + HardeningTemplatesHandler(final TemplatesHandler delegate, final URIResolver uriResolver, final Supplier<Source> 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 39669d6..381c3b7 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. * <p> * 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<Source> emptySource) { + HardeningTransformer(final Transformer delegate, final URIResolver uriResolver, final Supplier<Source> 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 7f030c5..90c7e65 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.</li> * <li><strong>FSP</strong> ({@link XMLConstants#FEATURE_SECURE_PROCESSING}): required. On XSLTC it enables the runtime evaluator limits; on Xalan it disables * reflection-based extension functions.</li> + * <li><strong>FODP</strong> ({@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.</li> * <li><strong>{@link FallbackIgnoreURIResolver} floor</strong>: 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. * * <p>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<Source> 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<Source> 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 69000e9..c6223f0 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<Source> emptySource) { + HardeningTransformerHandler(final TransformerHandler delegate, final URIResolver uriResolver, final Supplier<Source> 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 92d06fc..494f2b7 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 1a8533e..4dd4299 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 856aa67..9eae201 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 c843fea..07cda12 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 579bfa9..020a079 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 cde63ab..2e241ae 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.</li> - * <li><strong>FODP</strong> ({@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.</li> * <li><strong>FSP</strong> ({@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.</li> + * <li><strong>FODP</strong> ({@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.</li> * <li><strong>The nested wrapper</strong>: 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.</li> @@ -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 935b422..13a2c22 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 1532cbf..2d97d37 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 54b25b7..62bee1d 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 0000000..4dc167e --- /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. + * + * <p>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.</p> + */ +@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 57ea76f..a96a83d 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 18231b8..198ad48 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 @@ * <p>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}.</p> */ @Tag("xpath")
