Rework the DOM default parser selection test.

Splits the explicit factory class case out, collapses the selection
cases into one test that observes the wrapped delegate, and gates both
on Android, where the secure factory returns the platform
implementation unwrapped.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CLnTBsvmYtxzNTWVGNyz33

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

diff --git a/src/main/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactory.java b/src/main/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactory.java
index b51ea87..d693412 100644
--- a/src/main/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactory.java
+++ b/src/main/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactory.java
@@ -182,7 +182,7 @@ public void setXIncludeAware(final boolean state) {
     private static final String DOM_FACTORY_ID = "javax.xml.parsers.DocumentBuilderFactory";
 
     /** Class name of the JDK's built-in default implementation, the Java 8 fallback for {@link #newDefaultInstance()}. */
-    private static final String JDK_DOCUMENT_BUILDER_FACTORY = "com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl";
+    static final String JDK_DOCUMENT_BUILDER_FACTORY = "com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl";
 
     private static final MethodHandle MH_newDefaultInstance = MethodHandleFactory.findStatic(DocumentBuilderFactory.class, "newDefaultInstance");
 

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

diff --git a/src/test/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactoryTest.java b/src/test/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactoryTest.java
index c2f8e57..e7899a8 100644
--- a/src/test/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactoryTest.java
+++ b/src/test/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactoryTest.java
@@ -24,6 +24,8 @@
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.lang.reflect.Field;
+
 import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilderFactory;
 
@@ -34,6 +36,37 @@
 @Tag("dom")
 class SecureDocumentBuilderFactoryTest {
 
+    /** System property naming the {@link DocumentBuilderFactory} implementation, the JVM's mechanism for reconfiguring the default parser. */
+    private static final String FACTORY_ID = "javax.xml.parsers.DocumentBuilderFactory";
+
+    /**
+     * Gets the implementation a secure factory delegates to, so the selection tests can observe which parser implementation a lookup picked.
+     *
+     * @param factory a secure factory returned by one of the {@code new*Instance} methods; never {@code null}.
+     * @return The wrapped factory.
+     */
+    private static DocumentBuilderFactory getDelegate(final DocumentBuilderFactory factory) throws ReflectiveOperationException {
+        final Field delegate = factory.getClass().getDeclaredField("delegate");
+        delegate.setAccessible(true);
+        return (DocumentBuilderFactory) delegate.get(factory);
+    }
+
+    /**
+     * Selects the implementation {@link DocumentBuilderFactory#newInstance()} returns by setting the {@value #FACTORY_ID} system property.
+     *
+     * @param factoryClassName the implementation class name to install, or {@code null} to clear the property and restore the platform lookup.
+     * @return The previous property value, {@code null} if it was not set; pass it back here to restore the original lookup.
+     */
+    private static String setFactoryIdProperty(final String factoryClassName) {
+        final String previous = System.getProperty(FACTORY_ID);
+        if (factoryClassName == null) {
+            System.clearProperty(FACTORY_ID);
+        } else {
+            System.setProperty(FACTORY_ID, factoryClassName);
+        }
+        return previous;
+    }
+
     @Test
     void createsSecureBuildersFromEveryStaticEntryPoint() throws Exception {
         Assumptions.assumeTrue(AttackTestSupport.DOM_RESOLVES_INTERNAL_ENTITIES, "the platform DOM is left unwrapped: it does not resolve user-defined entities");
@@ -44,6 +77,15 @@ void createsSecureBuildersFromEveryStaticEntryPoint() throws Exception {
     }
 
     @Test
+    void explicitFactoryClassSelectsThatImplementation() throws Exception {
+        Assumptions.assumeFalse(AttackTestSupport.IS_ANDROID, "Skipped on Android: the platform factory is used unwrapped");
+        final Class<?> discovered = DocumentBuilderFactory.newInstance().getClass();
+        final DocumentBuilderFactory factory = SecureDocumentBuilderFactory.newNSInstance(discovered.getName(), null);
+        assertEquals(discovered, getDelegate(factory).getClass());
+        assertTrue(factory.isNamespaceAware());
+    }
+
+    @Test
     void forwardsEverySupportedFactoryConfiguration() throws Exception {
         Assumptions.assumeTrue(AttackTestSupport.DOM_RESOLVES_INTERNAL_ENTITIES, "the platform DOM is left unwrapped: it does not resolve user-defined entities");
         final DocumentBuilderFactory factory = SecureDocumentBuilderFactory.newInstance();
@@ -71,21 +113,20 @@ void forwardsEverySupportedFactoryConfiguration() throws Exception {
     }
 
     @Test
-    void honorsExplicitFactoryClassAndDefaultParserOverrides() throws Exception {
-        final String className = DocumentBuilderFactory.newInstance().getClass().getName();
-        assertTrue(SecureDocumentBuilderFactory.newNSInstance(className, null).isNamespaceAware());
-        assertTrue(SecureDocumentBuilderFactory.newNSInstance(true).isNamespaceAware());
-        final String property = "javax.xml.parsers.DocumentBuilderFactory";
-        final String previous = System.getProperty(property);
+    void newNSInstanceFollowsParserSelection() throws Exception {
+        Assumptions.assumeFalse(AttackTestSupport.IS_ANDROID, "Skipped on Android: the platform factory is used unwrapped");
+        final Class<?> discovered = DocumentBuilderFactory.newInstance().getClass();
+        // no property: the JDK built-in default, unless an override is requested
+        assertEquals(SecureDocumentBuilderFactory.JDK_DOCUMENT_BUILDER_FACTORY,
+                getDelegate(SecureDocumentBuilderFactory.newNSInstance(false)).getClass().getName());
+        assertEquals(discovered, getDelegate(SecureDocumentBuilderFactory.newNSInstance(true)).getClass());
+        // the factory id property is the JDK's own default reconfiguration; both selections honor it
+        final String previous = setFactoryIdProperty(discovered.getName());
         try {
-            System.setProperty(property, className);
-            assertTrue(SecureDocumentBuilderFactory.newNSInstance(false).isNamespaceAware());
+            assertEquals(discovered, getDelegate(SecureDocumentBuilderFactory.newNSInstance(false)).getClass());
+            assertEquals(discovered, getDelegate(SecureDocumentBuilderFactory.newNSInstance(true)).getClass());
         } finally {
-            if (previous == null) {
-                System.clearProperty(property);
-            } else {
-                System.setProperty(property, previous);
-            }
+            setFactoryIdProperty(previous);
         }
     }
 }