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 <[email protected]> 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 <[email protected]> --- .../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); + } +}
