This is an automated email from the ASF dual-hosted git repository.
ppkarwasz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/commons-xml.git
The following commit(s) were added to refs/heads/main by this push:
new 2a67b2f Remove the harden(XMLReader) and harden(Source) methods from
the public API (#37)
2a67b2f is described below
commit 2a67b2f1fdb66f4980273f3197e5cfa62e76e407
Author: Piotr P. Karwasz <[email protected]>
AuthorDate: Tue Aug 18 17:56:25 2026 +0200
Remove the harden(XMLReader) and harden(Source) methods from the public API
(#37)
Keep the public surface of XmlFactories minimal for the first release:
callers who need a hardened XMLReader can obtain one from a hardened SAX
factory, and rewriting a Source is an edge case that can live in another
library. Both methods were one-line delegations to the package-private
SAXParserHardener workhorses, which every internal caller already uses
directly.
Javadoc references now point at SAXParserHardener#hardenSource and
SAXParserHardener#hardenReader, and the tests that exercised the
hardened-in-place reader scenario (still relied on by SaxonProvider)
call the package-private hardenReader directly.
Assisted-By: Claude Fable 5 <[email protected]>
---
.../org/apache/commons/xml/HardeningSchema.java | 2 +-
.../apache/commons/xml/HardeningSchemaFactory.java | 4 +--
.../apache/commons/xml/HardeningTransformer.java | 2 +-
.../commons/xml/HardeningTransformerFactory.java | 4 +--
.../org/apache/commons/xml/HardeningValidator.java | 2 +-
.../org/apache/commons/xml/SAXParserHardener.java | 2 +-
.../java/org/apache/commons/xml/SaxonProvider.java | 2 +-
.../java/org/apache/commons/xml/XmlFactories.java | 29 ----------------------
.../org/apache/commons/xml/AttackTestSupport.java | 10 ++++----
.../java/org/apache/commons/xml/XIncludeTest.java | 14 +++++------
10 files changed, 21 insertions(+), 50 deletions(-)
diff --git a/src/main/java/org/apache/commons/xml/HardeningSchema.java
b/src/main/java/org/apache/commons/xml/HardeningSchema.java
index 87244f9..f3294c1 100644
--- a/src/main/java/org/apache/commons/xml/HardeningSchema.java
+++ b/src/main/java/org/apache/commons/xml/HardeningSchema.java
@@ -23,7 +23,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
XmlFactories#harden(javax.xml.transform.Source)} and installs the resolver
floor), and
+ * {@link HardeningValidator} (which rewrites the Source through {@link
SAXParserHardener#hardenSource(javax.xml.transform.Source)} 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.
*/
diff --git a/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java
b/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java
index ac000c2..a21ee58 100644
--- a/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java
+++ b/src/main/java/org/apache/commons/xml/HardeningSchemaFactory.java
@@ -38,13 +38,13 @@
* <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 XmlFactories#harden(Source)}.</li>
+ * through {@link SAXParserHardener#hardenSource(Source)}.</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 XmlFactories#harden(Source)}
already carries {@code FEATURE_SECURE_PROCESSING} and the processing limits, so
a
+ * <p>The hardened reader supplied by {@link
SAXParserHardener#hardenSource(Source)} 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
diff --git a/src/main/java/org/apache/commons/xml/HardeningTransformer.java
b/src/main/java/org/apache/commons/xml/HardeningTransformer.java
index 6ab4f97..48587b5 100644
--- a/src/main/java/org/apache/commons/xml/HardeningTransformer.java
+++ b/src/main/java/org/apache/commons/xml/HardeningTransformer.java
@@ -29,7 +29,7 @@
/**
* {@link Transformer} wrapper that rewrites the Source on every {@link
Transformer#transform(Source, Result)} call through
- * {@link XmlFactories#harden(Source)} before delegating, and keeps an
ignore-all {@link URIResolver} floor so runtime {@code document()} calls a
caller does not
+ * {@link SAXParserHardener#hardenSource(Source)} 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)}
diff --git
a/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java
b/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java
index ed8ac0f..8c7ebfa 100644
--- a/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java
+++ b/src/main/java/org/apache/commons/xml/HardeningTransformerFactory.java
@@ -32,7 +32,7 @@
import org.xml.sax.XMLReader;
/**
- * {@link javax.xml.transform.TransformerFactory} wrapper that rewrites every
Source-taking entry point through {@link XmlFactories#harden(Source)} before
+ * {@link javax.xml.transform.TransformerFactory} wrapper that rewrites every
Source-taking entry point through {@link
SAXParserHardener#hardenSource(Source)} before
* delegating.
*
* <p>Used by providers whose underlying TrAX implementation pulls a fresh
{@code SAXParserFactory.newInstance()} for any Source that is not already a
@@ -50,7 +50,7 @@
* <h2>Caveats</h2>
* <ul>
* <li>A {@link SAXSource} that carries its own {@link XMLReader} is trusted
as-is: the caller is expected to supply a hardened reader (via
- * {@link XmlFactories#newSAXParserFactory()} or {@link
XmlFactories#harden(XMLReader)}) in that case.</li>
+ * {@link XmlFactories#newSAXParserFactory()}) in that case.</li>
* <li>{@link TransformerHandler} returned from {@code
newTransformerHandler} is not wrapped: it processes incoming SAX events instead
of reading a Source, so
* it has no inner Source-parsing path. A caller who pulls the inner
{@link Transformer} via {@link TransformerHandler#getTransformer()} bypasses the
* runtime source rewrite.</li>
diff --git a/src/main/java/org/apache/commons/xml/HardeningValidator.java
b/src/main/java/org/apache/commons/xml/HardeningValidator.java
index e67f1e0..730230f 100644
--- a/src/main/java/org/apache/commons/xml/HardeningValidator.java
+++ b/src/main/java/org/apache/commons/xml/HardeningValidator.java
@@ -32,7 +32,7 @@
/**
* {@link Validator} wrapper that rewrites the Source on every {@link
Validator#validate(Source)} and {@link Validator#validate(Source, Result)} call
through
- * {@link XmlFactories#harden(Source)} before delegating, and keeps an
ignore-all {@link LSResourceResolver} floor so {@code xsi:schemaLocation} is
not resolved at
+ * {@link SAXParserHardener#hardenSource(Source)} 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 {
diff --git a/src/main/java/org/apache/commons/xml/SAXParserHardener.java
b/src/main/java/org/apache/commons/xml/SAXParserHardener.java
index 4cd784f..5a309c2 100644
--- a/src/main/java/org/apache/commons/xml/SAXParserHardener.java
+++ b/src/main/java/org/apache/commons/xml/SAXParserHardener.java
@@ -103,7 +103,7 @@ static SAXParserFactory harden(final SAXParserFactory
factory) {
*/
static XMLReader hardenReader(final XMLReader reader) {
if (reader instanceof HardeningXMLReader) {
- // Already hardened (for example, handed back through
XmlFactories.harden(XMLReader)); the floor is already in place.
+ // Already hardened (for example, a reader from a hardened factory
passed back through hardenReader); the floor is already in place.
return reader;
}
if (ANDROID_EXPAT_READER.equals(reader.getClass().getName())) {
diff --git a/src/main/java/org/apache/commons/xml/SaxonProvider.java
b/src/main/java/org/apache/commons/xml/SaxonProvider.java
index eaec0ca..9abc224 100644
--- a/src/main/java/org/apache/commons/xml/SaxonProvider.java
+++ b/src/main/java/org/apache/commons/xml/SaxonProvider.java
@@ -52,7 +52,7 @@ final class SaxonProvider {
*
* <ol>
* <li><b>SAX layer.</b> {@link #makeParser} hands every {@link
XMLReader} Saxon would otherwise use through
- * {@link XmlFactories#harden(XMLReader)}, which routes it to the
matching bundled hardening recipe. External DTDs, entities and XInclude
+ * {@link SAXParserHardener#hardenReader(XMLReader)}, which routes it to
the matching bundled hardening recipe. External DTDs, entities and XInclude
* resolve to empty content at parse time.</li>
* <li><b>Resource-resolution layer.</b> A non-removable ignore-all
{@link ResourceResolver} floor backs every resolution chain ({@code
xsl:include},
* {@code xsl:import}, {@code xsl:source-document}, and the XPath/XSLT
functions {@code fn:doc}, {@code fn:document}, {@code fn:unparsed-text},
diff --git a/src/main/java/org/apache/commons/xml/XmlFactories.java
b/src/main/java/org/apache/commons/xml/XmlFactories.java
index 270c830..a01b914 100644
--- a/src/main/java/org/apache/commons/xml/XmlFactories.java
+++ b/src/main/java/org/apache/commons/xml/XmlFactories.java
@@ -21,14 +21,10 @@
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.SAXParserFactory;
import javax.xml.stream.XMLInputFactory;
-import javax.xml.transform.Source;
-import javax.xml.transform.TransformerConfigurationException;
import javax.xml.transform.TransformerFactory;
import javax.xml.validation.SchemaFactory;
import javax.xml.xpath.XPathFactory;
-import org.xml.sax.XMLReader;
-
/**
* Entry point for obtaining hardened JAXP factories.
*
@@ -80,31 +76,6 @@ public final class XmlFactories {
*/
static final String THROW_ON_UNRESOLVED =
"org.apache.commons.xml.throwOnUnresolved";
- /**
- * Rewrites a {@link Source} so that any SAX parsing it triggers runs
through an {@link XmlFactories}-hardened {@link XMLReader}.
- *
- * <p>Only a {@code StreamSource} or a {@code SAXSource} without a reader
is enriched with a hardened, namespace-aware reader; other kinds of sources are
- * returned as-is.</p>
- *
- * @param source The source to harden; never {@code null}.
- * @return A hardened source.
- * @throws TransformerConfigurationException if a hardened reader cannot
be obtained.
- */
- public static Source harden(final Source source) throws
TransformerConfigurationException {
- return SAXParserHardener.hardenSource(source);
- }
-
- /**
- * Hardens an existing {@link XMLReader}.
- *
- * @param reader The reader to harden; never {@code null}.
- * @return A hardened reader.
- * @throws IllegalStateException if a required hardening setting cannot be
applied to the underlying implementation.
- */
- public static XMLReader harden(final XMLReader reader) {
- return SAXParserHardener.hardenReader(reader);
- }
-
/**
* Returns a fresh, hardened {@link DocumentBuilderFactory}.
*
diff --git a/src/test/java/org/apache/commons/xml/AttackTestSupport.java
b/src/test/java/org/apache/commons/xml/AttackTestSupport.java
index 8ca31ee..df2c334 100644
--- a/src/test/java/org/apache/commons/xml/AttackTestSupport.java
+++ b/src/test/java/org/apache/commons/xml/AttackTestSupport.java
@@ -691,7 +691,7 @@ static void assertValidatorValidates(final String xml) {
/**
* Asserts a hardened-in-place XMLReader parse of the payload throws.
*
- * <p>{@link XMLReader#parse(InputSource)} on a raw reader hardened via
{@link XmlFactories#harden(XMLReader)}; only a thrown exception passes.</p>
+ * <p>{@link XMLReader#parse(InputSource)} on a raw reader hardened via
{@link SAXParserHardener#hardenReader(XMLReader)}; only a thrown exception
passes.</p>
*/
static void assertXmlReaderBlocks(final String payload) {
assertParseFails(() -> consumeXmlReader(rawHardenedReader(), payload),
"XMLReader", SAXException.class);
@@ -707,7 +707,7 @@ static void assertXmlReaderBlocksOrDoesNotLeak(final String
payload) {
/**
* Asserts a hardened-in-place XMLReader parse completes without throwing
and without leaked content.
*
- * <p>{@link XMLReader#parse(InputSource)} on a raw reader hardened via
{@link XmlFactories#harden(XMLReader)}; use this when the hardening contract
+ * <p>{@link XMLReader#parse(InputSource)} on a raw reader hardened via
{@link SAXParserHardener#hardenReader(XMLReader)}; use this when the hardening
contract
* guarantees the parse succeeds but never resolves the external
resource.</p>
*/
static void assertXmlReaderDoesNotLeak(final String payload) {
@@ -717,7 +717,7 @@ static void assertXmlReaderDoesNotLeak(final String
payload) {
/**
* Asserts a hardened-in-place XMLReader parse succeeds.
*
- * <p>{@link XMLReader#parse(InputSource)} on a raw reader hardened via
{@link XmlFactories#harden(XMLReader)}; positive control for DOCTYPE-only
+ * <p>{@link XMLReader#parse(InputSource)} on a raw reader hardened via
{@link SAXParserHardener#hardenReader(XMLReader)}; positive control for
DOCTYPE-only
* payloads.</p>
*/
static void assertXmlReaderParses(final String payload) {
@@ -915,13 +915,13 @@ private static boolean probeDomResolvesInternalEntities()
{
}
}
- /** Builds a raw {@link XMLReader} from a deliberately permissive {@link
SAXParserFactory} and hardens it via {@link XmlFactories#harden(XMLReader)}. */
+ /** Builds a raw {@link XMLReader} from a deliberately permissive {@link
SAXParserFactory} and hardens it via {@link
SAXParserHardener#hardenReader(XMLReader)}. */
private static XMLReader rawHardenedReader() throws Exception {
final SAXParserFactory factory = SAXParserFactory.newInstance();
if (!IS_ANDROID) {
factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, false);
}
- return XmlFactories.harden(factory.newSAXParser().getXMLReader());
+ return
SAXParserHardener.hardenReader(factory.newSAXParser().getXMLReader());
}
/** Opens the named test resource as a {@link StreamSource} preserving its
system id, so relative includes/imports/redefines resolve normally. */
diff --git a/src/test/java/org/apache/commons/xml/XIncludeTest.java
b/src/test/java/org/apache/commons/xml/XIncludeTest.java
index a75a715..84a3933 100644
--- a/src/test/java/org/apache/commons/xml/XIncludeTest.java
+++ b/src/test/java/org/apache/commons/xml/XIncludeTest.java
@@ -331,7 +331,7 @@ void hardenedSaxNullResolverDoesNotLeak() throws Exception {
//endregion
- //region harden(XMLReader): reader with XInclude enabled before hardening
is blocked
+ //region hardenReader: reader with XInclude enabled before hardening is
blocked (the internal recipe behind the hardened factory and SaxonProvider)
@Test
@Tag("sax")
@@ -342,9 +342,9 @@ void hardenReaderBlocksParseXml() throws Exception {
final SAXParserFactory unhardenedFactory =
SAXParserFactory.newInstance();
unhardenedFactory.setNamespaceAware(true);
assumeXIncludeAware(unhardenedFactory);
- final XMLReader reader =
XmlFactories.harden(unhardenedFactory.newSAXParser().getXMLReader());
+ final XMLReader reader =
SAXParserHardener.hardenReader(unhardenedFactory.newSAXParser().getXMLReader());
assertThrows(SAXException.class, () -> reader.parse(input),
- "harden(reader) should block XInclude parse=xml on reader with
XInclude already enabled");
+ "hardenReader should block XInclude parse=xml on reader with
XInclude already enabled");
}
@Test
@@ -355,10 +355,10 @@ void hardenReaderBlocksParseText() throws Exception {
final SAXParserFactory unhardenedFactory =
SAXParserFactory.newInstance();
unhardenedFactory.setNamespaceAware(true);
assumeXIncludeAware(unhardenedFactory);
- final XMLReader reader =
XmlFactories.harden(unhardenedFactory.newSAXParser().getXMLReader());
+ final XMLReader reader =
SAXParserHardener.hardenReader(unhardenedFactory.newSAXParser().getXMLReader());
final String captured = captureCharacters(reader, input);
assertFalse(captured.contains(LEAKED_MARKER),
- "harden(reader) parse=text must resolve the include to empty,
not leak; got: " + captured);
+ "hardenReader parse=text must resolve the include to empty,
not leak; got: " + captured);
}
@Test
@@ -369,11 +369,11 @@ void hardenReaderAllowListResolvesParseXml() throws
Exception {
final SAXParserFactory unhardenedFactory =
SAXParserFactory.newInstance();
unhardenedFactory.setNamespaceAware(true);
assumeXIncludeAware(unhardenedFactory);
- final XMLReader reader =
XmlFactories.harden(unhardenedFactory.newSAXParser().getXMLReader());
+ final XMLReader reader =
SAXParserHardener.hardenReader(unhardenedFactory.newSAXParser().getXMLReader());
reader.setEntityResolver(new AllowListResolver());
final String captured = captureCharacters(reader, input);
assertEquals(RESOLVED_MARKER, captured.trim(),
- "harden(reader) + allow-list should resolve to the resolver's
content on a reader with XInclude already enabled");
+ "hardenReader + allow-list should resolve to the resolver's
content on a reader with XInclude already enabled");
}
//endregion
