You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by pk...@apache.org on 2026/08/18 07:38:24 UTC

(commons-xml) branch fix/saxparser-reset created (now bcba6ad)

This is an automated email from the ASF dual-hosted git repository.

ppkarwasz pushed a change to branch fix/saxparser-reset
in repository https://gitbox.apache.org/repos/asf/commons-xml.git


      at bcba6ad  fix: Restore hardened state after Transformer.reset()

This branch includes the following new commits:

     new fad8bbc  fix: Restore hardened state after SAXParser.reset()
     new e6cff05  fix: Restore hardened state after Validator.reset()
     new bcba6ad  fix: Restore hardened state after Transformer.reset()

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(commons-xml) 03/03: fix: Restore hardened state after Transformer.reset()

Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ppkarwasz pushed a commit to branch fix/saxparser-reset
in repository https://gitbox.apache.org/repos/asf/commons-xml.git

commit bcba6adba29a7a95ac0ee0cf01e1ea77e5ba61e3
Author: Piotr P. Karwasz <pk...@apache.org>
AuthorDate: Tue Aug 18 09:27:37 2026 +0200

    fix: Restore hardened state after Transformer.reset()
    
    The JAXP reset contract reverts a transformer to its just-created state,
    and several implementations clear the URI resolver installed by the
    constructor. HardeningTransformer.reset() now re-establishes the floor,
    seeded again with the factory's compile-time resolver, matching the other
    hardened wrappers.
    
    Assisted-By: Claude Fable 5 <no...@anthropic.com>
---
 .../apache/commons/xml/HardeningTransformer.java    | 19 +++++++++++++------
 .../org/apache/commons/xml/ResetHardeningTest.java  | 21 +++++++++++++++++++++
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/src/main/java/org/apache/commons/xml/HardeningTransformer.java b/src/main/java/org/apache/commons/xml/HardeningTransformer.java
index a1b9322..6ab4f97 100644
--- a/src/main/java/org/apache/commons/xml/HardeningTransformer.java
+++ b/src/main/java/org/apache/commons/xml/HardeningTransformer.java
@@ -33,20 +33,32 @@
  * 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)}
- * routes a caller's resolver through it rather than replacing it, so the block cannot be dropped.</p>
+ * routes a caller's resolver through it rather than replacing it, so the block cannot be dropped. {@link #reset()} re-establishes the floor, seeded again with
+ * the factory's compile-time resolver, matching the just-constructed state.</p>
  */
 final class HardeningTransformer extends Transformer {
 
     private final Transformer delegate;
 
+    /** Compile-time URIResolver snapshot the floor is seeded with, both at construction and again on {@link #reset()}. */
+    private final URIResolver uriResolver;
+
     private final FallbackIgnoreURIResolver floor;
 
     HardeningTransformer(final Transformer delegate, final URIResolver uriResolver) {
         this.delegate = delegate;
+        this.uriResolver = uriResolver;
         this.floor = new FallbackIgnoreURIResolver(uriResolver);
         delegate.setURIResolver(floor);
     }
 
+    @Override
+    public void reset() {
+        delegate.reset();
+        floor.setDelegate(uriResolver);
+        delegate.setURIResolver(floor);
+    }
+
     @Override
     public void setURIResolver(final URIResolver resolver) {
         floor.setDelegate(resolver);
@@ -92,11 +104,6 @@ public Object getParameter(final String name) {
         return delegate.getParameter(name);
     }
 
-    @Override
-    public void reset() {
-        delegate.reset();
-    }
-
     @Override
     public void setErrorListener(final ErrorListener listener) {
         delegate.setErrorListener(listener);
diff --git a/src/test/java/org/apache/commons/xml/ResetHardeningTest.java b/src/test/java/org/apache/commons/xml/ResetHardeningTest.java
index a7221bd..1c72eeb 100644
--- a/src/test/java/org/apache/commons/xml/ResetHardeningTest.java
+++ b/src/test/java/org/apache/commons/xml/ResetHardeningTest.java
@@ -19,8 +19,13 @@
 
 import static org.junit.jupiter.api.Assertions.assertFalse;
 
+import java.io.StringWriter;
+
 import javax.xml.XMLConstants;
 import javax.xml.parsers.SAXParser;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerException;
+import javax.xml.transform.stream.StreamResult;
 import javax.xml.validation.Validator;
 
 import org.junit.jupiter.api.Tag;
@@ -77,4 +82,20 @@ void validatorResetKeepsResourceResolverFloor() throws Exception {
         AttackTestSupport.assertParseFails(() -> validator.validate(AttackTestSupport.resourceSource("schema-location-instance.xml")),
                 "Validator after reset", SAXException.class, SecurityException.class);
     }
+
+    @Test
+    @Tag("trax")
+    void transformerResetKeepsUriResolverFloor() throws Exception {
+        // with-document.xsl copies document('referenced.xml') into the output at transform time, so a transformer whose floor was stripped leaks the marker.
+        final Transformer transformer = XmlFactories.newTransformerFactory()
+                .newTemplates(AttackTestSupport.resourceSource("with-document.xsl")).newTransformer();
+        AttackTestSupport.assumeDoesNotThrow(transformer::reset);
+        final StringWriter sink = new StringWriter();
+        try {
+            transformer.transform(AttackTestSupport.streamSource("<root/>"), new StreamResult(sink));
+        } catch (final TransformerException blocked) {
+            return; // Acceptable: rejected at transform rather than resolved to empty.
+        }
+        assertFalse(sink.toString().contains(AttackTestSupport.LEAKED_MARKER), "document() leaked after reset:\n" + sink);
+    }
 }


(commons-xml) 01/03: fix: Restore hardened state after SAXParser.reset()

Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ppkarwasz pushed a commit to branch fix/saxparser-reset
in repository https://gitbox.apache.org/repos/asf/commons-xml.git

commit fad8bbc222a245e3e7bc5feec5b74a8ae690a4c7
Author: Piotr P. Karwasz <pk...@apache.org>
AuthorDate: Tue Aug 18 08:58:08 2026 +0200

    fix: Restore hardened state after SAXParser.reset()
    
    The JAXP reset contract reverts a parser to its just-created state, which
    discards the reader-level configuration installed after creation, while the
    wrapper kept serving its stale cached views. HardeningSAXParser.reset() now
    drops the cached reader and parser views, so the next getXMLReader() or
    getParser() call runs the freshly reset reader through the hardening funnel
    again, matching HardeningDocumentBuilder.reset().
    
    Assisted-By: Claude Fable 5 <no...@anthropic.com>
---
 .../org/apache/commons/xml/HardeningSAXParser.java | 14 +++--
 .../org/apache/commons/xml/ResetHardeningTest.java | 64 ++++++++++++++++++++++
 2 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/src/main/java/org/apache/commons/xml/HardeningSAXParser.java b/src/main/java/org/apache/commons/xml/HardeningSAXParser.java
index d8bed29..5a79e74 100644
--- a/src/main/java/org/apache/commons/xml/HardeningSAXParser.java
+++ b/src/main/java/org/apache/commons/xml/HardeningSAXParser.java
@@ -68,6 +68,15 @@ public Parser getParser() throws SAXException {
         return hardenedParser;
     }
 
+    @Override
+    public void reset() {
+        delegate.reset();
+        // The JAXP reset contract reverts the delegate to its just-created state, which strips the post-creation reader hardening.
+        // We reset the cached readers, so hardening can be applied again.
+        hardenedReader = null;
+        hardenedParser = null;
+    }
+
     // <editor-fold defaultstate="collapsed" desc="Trivial delegation">
     @Override
     public Object getProperty(final String name) throws SAXNotRecognizedException, SAXNotSupportedException {
@@ -94,11 +103,6 @@ public boolean isXIncludeAware() {
         return delegate.isXIncludeAware();
     }
 
-    @Override
-    public void reset() {
-        delegate.reset();
-    }
-
     @Override
     public void setProperty(final String name, final Object value) throws SAXNotRecognizedException, SAXNotSupportedException {
         delegate.setProperty(name, value);
diff --git a/src/test/java/org/apache/commons/xml/ResetHardeningTest.java b/src/test/java/org/apache/commons/xml/ResetHardeningTest.java
new file mode 100644
index 0000000..b48fa6c
--- /dev/null
+++ b/src/test/java/org/apache/commons/xml/ResetHardeningTest.java
@@ -0,0 +1,64 @@
+/*
+ * 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.assertFalse;
+
+import javax.xml.parsers.SAXParser;
+
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLReader;
+
+/**
+ * Checks that the JAXP {@code reset()} lifecycle methods do not strip the hardening floors.
+ *
+ * <p>The JAXP reset contract returns an object to its just-created state, and the stock JDK / Xerces implementations take that literally: they re-install
+ * their initial (null) resolvers, silently removing any floor the hardened wrappers installed after creation. Each test resets a hardened object and asserts
+ * that an external reference is still either blocked at parse or resolved to empty content afterwards; the tests are skipped on platforms whose
+ * implementation does not support {@code reset()} at all (there the hardening cannot be stripped in the first place).</p>
+ */
+class ResetHardeningTest {
+
+    /** systemId of the external general entity the floor must keep covering after a reset (its content carries {@link AttackTestSupport#LEAKED_MARKER}). */
+    private static final String UNLISTED = AttackTestSupport.resourceUrl("referenced.xml").toString();
+
+    private static String entityPayload(final String entitySystemId) {
+        return "<?xml version=\"1.0\"?>\n"
+                + "<!DOCTYPE root [\n  <!ENTITY xxe SYSTEM \"" + entitySystemId + "\">\n]>\n"
+                + "<root>&xxe;</root>";
+    }
+
+    @Test
+    @Tag("sax")
+    void saxParserResetKeepsEntityResolverFloor() throws Exception {
+        final SAXParser parser = XmlFactories.newSAXParserFactory().newSAXParser();
+        // Materialize the hardened reader before the reset, so a stale cached wrapper would be observable.
+        parser.getXMLReader();
+        AttackTestSupport.assumeDoesNotThrow(parser::reset);
+        final XMLReader reader = parser.getXMLReader();
+        final String text;
+        try {
+            text = AttackTestSupport.captureCharacters(reader, entityPayload(UNLISTED));
+        } catch (final SAXException blocked) {
+            return; // Acceptable: rejected at parse rather than resolved to empty.
+        }
+        assertFalse(text.contains(AttackTestSupport.LEAKED_MARKER), "external entity leaked after reset:\n" + text);
+    }
+}


(commons-xml) 02/03: fix: Restore hardened state after Validator.reset()

Posted by pk...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ppkarwasz pushed a commit to branch fix/saxparser-reset
in repository https://gitbox.apache.org/repos/asf/commons-xml.git

commit e6cff05fc12cb337ef824d270366583fc43e82cf
Author: Piotr P. Karwasz <pk...@apache.org>
AuthorDate: Tue Aug 18 09:09:08 2026 +0200

    fix: Restore hardened state after Validator.reset()
    
    The JAXP reset contract reverts a validator to its just-created state,
    which removes the resource-resolver floor installed by the constructor.
    HardeningValidator.reset() now re-establishes the bare floor, matching
    HardeningDocumentBuilder.reset().
    
    Assisted-By: Claude Fable 5 <no...@anthropic.com>
---
 .../java/org/apache/commons/xml/HardeningValidator.java  |  4 +++-
 .../java/org/apache/commons/xml/ResetHardeningTest.java  | 16 ++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/apache/commons/xml/HardeningValidator.java b/src/main/java/org/apache/commons/xml/HardeningValidator.java
index cb30279..e67f1e0 100644
--- a/src/main/java/org/apache/commons/xml/HardeningValidator.java
+++ b/src/main/java/org/apache/commons/xml/HardeningValidator.java
@@ -33,7 +33,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
- * validation time.
+ * validation time. {@link #reset()} re-establishes the bare ignore-all floor, matching the just-constructed state.
  */
 final class HardeningValidator extends Validator {
 
@@ -71,6 +71,8 @@ public LSResourceResolver getResourceResolver() {
     @Override
     public void reset() {
         delegate.reset();
+        floor.setDelegate(null);
+        delegate.setResourceResolver(floor);
     }
 
     @Override
diff --git a/src/test/java/org/apache/commons/xml/ResetHardeningTest.java b/src/test/java/org/apache/commons/xml/ResetHardeningTest.java
index b48fa6c..a7221bd 100644
--- a/src/test/java/org/apache/commons/xml/ResetHardeningTest.java
+++ b/src/test/java/org/apache/commons/xml/ResetHardeningTest.java
@@ -19,7 +19,9 @@
 
 import static org.junit.jupiter.api.Assertions.assertFalse;
 
+import javax.xml.XMLConstants;
 import javax.xml.parsers.SAXParser;
+import javax.xml.validation.Validator;
 
 import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.api.Test;
@@ -61,4 +63,18 @@ void saxParserResetKeepsEntityResolverFloor() throws Exception {
         }
         assertFalse(text.contains(AttackTestSupport.LEAKED_MARKER), "external entity leaked after reset:\n" + text);
     }
+
+    @Test
+    @Tag("schema")
+    void validatorResetKeepsResourceResolverFloor() throws Exception {
+        // A Schema built without sources validates against the instance's xsi:schemaLocation hints, so the resolver floor is the only barrier between the
+        // validator and the external schema fetch.
+        final Validator validator = XmlFactories.newSchemaFactory(XMLConstants.W3C_XML_SCHEMA_NS_URI).newSchema().newValidator();
+        AttackTestSupport.assumeDoesNotThrow(validator::reset);
+        validator.setErrorHandler(AttackTestSupport.STRICT_REPORTER);
+        // schema-location-instance.xml hints at schema-location.xsd, which declares its root: a validator whose floor was stripped fetches it and validates
+        // cleanly, while the floor resolves the hint to empty content, which fails the validation.
+        AttackTestSupport.assertParseFails(() -> validator.validate(AttackTestSupport.resourceSource("schema-location-instance.xml")),
+                "Validator after reset", SAXException.class, SecurityException.class);
+    }
 }