Install a fresh StAX resolver floor per hook instead of mutating one

src/main/java/org/apache/commons/xml/secure/SecureXMLInputFactory.java

@@ -60,12 +60,17 @@ public final class SecureXMLInputFactory {
      *
      * <p>Every resolver-valued entry point ({@link #setXMLResolver(XMLResolver)}, {@code setProperty(XMLInputFactory.RESOLVER, ...)} and the Woodstox
      * {@code com.ctc.wstx.*Resolver} keys) is routed uniformly: a caller who supplies their own {@link FallbackIgnoreXMLResolver} takes control and it is
-     * passed straight to the delegate; otherwise the current resolver on that hook is read, and if it is one of our floors the caller's resolver is set as its
-     * {@link FallbackIgnoreXMLResolver#setDelegate delegate} (an opt-in the floor cannot be removed by), or, if the hook is empty, the caller's resolver is
-     * wrapped in a new floor. This matters because Woodstox does not chain resolvers: when a resolver returns {@code null}, {@code DefaultInputResolver} falls
+     * passed straight to the delegate; otherwise the caller's resolver is wrapped in a new floor installed on that hook, an opt-in the floor cannot be removed
+     * by. This matters because Woodstox does not chain resolvers: when a resolver returns {@code null}, {@code DefaultInputResolver} falls
      * through to fetching the systemId URL itself, so a caller-set resolver that returns {@code null} must still land behind the floor. {@link #getXMLResolver()} and
      * {@code getProperty} report the caller's resolver unwrapped.</p>
      *
+     * <p>The floor on a hook is replaced rather than mutated, which is what keeps each hook and each reader independent. Woodstox routes
+     * {@code setXMLResolver} to both its DTD-subset and entity hooks, so one floor object sits on several of them, and it copies that reference into every
+     * reader it creates; setting a delegate on the object in place would therefore also answer hooks the caller never named, and would change the resolution
+     * policy of readers already created, including ones parsing on another thread. Installing a new floor leaves both untouched: a hook keeps whatever floor it
+     * was given, and a reader keeps the one it captured when it was created.</p>
+     *
      * @see org.apache.commons.xml.secure
      */
     private static final class Wrapper extends XMLInputFactory {
@@ -225,12 +230,9 @@ private void setResolverProperty(final String name, final XMLResolver resolver)
                 // The caller supplies their own floor: hand it to the delegate as-is.
                 delegate.setProperty(name, resolver);
             } else {
-                final Object current = delegate.getProperty(name);
-                if (current instanceof FallbackIgnoreXMLResolver) {
-                    ((FallbackIgnoreXMLResolver) current).setDelegate(resolver);
-                } else {
-                    delegate.setProperty(name, new FallbackIgnoreXMLResolver(resolver));
-                }
+                // A fresh floor for this hook rather than a new delegate on the floor already there: Woodstox puts one floor object on several hooks and copies
+                // the reference into every reader it creates, so mutating it would reach hooks, and readers already parsing, that this call never named.
+                delegate.setProperty(name, new FallbackIgnoreXMLResolver(resolver));
             }
         }
 

src/test/java/org/apache/commons/xml/secure/SecureXMLInputFactoryTest.java

@@ -20,6 +20,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -492,14 +493,14 @@ void setXMLResolverNullClearsCallerDelegate() {
     void setXMLResolverRoutesCallerBehindInstalledFloor() {
         final RecordingXMLInputFactory fake = new RecordingXMLInputFactory();
         final XMLInputFactory secure = SecureXMLInputFactory.secure(fake);
-        final FallbackIgnoreXMLResolver floor = (FallbackIgnoreXMLResolver) fake.resolverHook;
         final XMLResolver caller = (publicID, systemID, baseURI, namespace) -> null;
         secure.setXMLResolver(caller);
-        assertSame(caller, floor.getDelegate(), "the caller's resolver must become the delegate of the installed floor");
+        // The hook keeps a floor with the caller behind it; whether that is the floor already there or a fresh one is the subject of
+        // settingAResolverInstallsAFreshFloorInsteadOfMutatingTheInstalledOne.
+        assertTrue(fake.resolverHook instanceof FallbackIgnoreXMLResolver, "a caller resolver must land behind a floor, not replace it on the delegate's hook");
+        assertSame(caller, ((FallbackIgnoreXMLResolver) fake.resolverHook).getDelegate(), "the caller's resolver must be the floor's delegate");
         assertSame(caller, secure.getXMLResolver(), "getXMLResolver must report the caller's resolver unwrapped");
         assertSame(caller, secure.getProperty(XMLInputFactory.RESOLVER), "getProperty must report the caller's resolver unwrapped");
-        assertFalse(fake.calls.stream().anyMatch(c -> c.startsWith("setProperty(" + XMLInputFactory.RESOLVER)),
-                "a caller resolver must not replace the floor on the delegate's hook");
     }
 
     @Test
@@ -609,6 +610,34 @@ void wrapperDelegatesReaderCreationToDelegate() throws Exception {
                 "the exact stream filter must be forwarded");
     }
 
+    @Test
+    void settingAResolverInstallsAFreshFloorInsteadOfMutatingTheInstalledOne() {
+        // The implementations copy the floor reference into every reader they create, so mutating the installed floor would change the resolution policy of
+        // readers created before the call, including ones already parsing. Replacing it leaves what those readers captured alone.
+        final RecordingXMLInputFactory fake = new RecordingXMLInputFactory();
+        final XMLInputFactory secure = SecureXMLInputFactory.secure(fake);
+        final Object captured = fake.resolverHook;
+        secure.setXMLResolver((publicID, systemID, baseURI, namespace) -> null);
+        assertNotSame(captured, fake.resolverHook, "setting a resolver must install a fresh floor, not re-delegate the one already on the hook");
+        assertNull(((FallbackIgnoreXMLResolver) captured).getDelegate(), "the floor an existing reader captured must keep resolving to empty");
+    }
+
+    @Test
+    void woodstoxResolverHooksStayIndependent() {
+        // Woodstox routes setXMLResolver to both its DTD-subset and entity hooks, so one floor object sits on several of them. Setting one hook must not
+        // answer the others, which it would if the shared floor were mutated in place.
+        final XMLInputFactory secure = SecureXMLInputFactory.newInstance();
+        final XMLResolver dtd = (publicID, systemID, baseURI, namespace) -> null;
+        try {
+            secure.setProperty(SecureXMLInputFactory.WSTX_DTD_RESOLVER, dtd);
+        } catch (final IllegalArgumentException notWoodstox) {
+            Assumptions.abort("the implementation does not support " + SecureXMLInputFactory.WSTX_DTD_RESOLVER);
+            return;
+        }
+        assertSame(dtd, secure.getProperty(SecureXMLInputFactory.WSTX_DTD_RESOLVER), "the hook the caller named must report their resolver");
+        assertNull(secure.getProperty(SecureXMLInputFactory.WSTX_ENTITY_RESOLVER), "a resolver set on the DTD hook must not answer the entity hook");
+    }
+
     @Test
     void wrapperInstallsFloorOnDelegateHook() {
         final RecordingXMLInputFactory fake = new RecordingXMLInputFactory();