Strengthen OSGi factory tests

android-tests/build.gradle.kts

@@ -105,7 +105,12 @@ android {
 
 // These are JVM-only test classes; exclude them from the Android test compile.
 tasks.withType<JavaCompile>().configureEach {
-    exclude("**/ShadingFootprintTest.java", "**/JaxbTest.java", "**/OsgiIT.java")
+    exclude(
+        "**/ShadingFootprintTest.java",
+        "**/JaxbTest.java",
+        "**/OsgiIT.java",
+        "**/FallbackIgnoreLSResourceResolverInitializationTest.java"
+    )
 }
 
 // Skip JAXP groups whose factories Android does not ship

src/test/java/org/apache/commons/xml/osgi/OsgiIT.java

@@ -24,6 +24,7 @@
 
 import java.io.PrintWriter;
 import java.io.StringWriter;
+import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.net.URL;
 import java.nio.file.Files;
@@ -67,7 +68,7 @@
 import org.osgi.framework.wiring.BundleWiring;
 
 /**
- * Runs the unit test suite inside an OSGi framework, against the built bundle.
+ * Runs the applicable unit tests inside an OSGi framework, against the built bundle.
  *
  * <p>
  * The tests catch class-loading assumptions that only hold on a flat classpath: an OSGi bundle class loader sees its own classes, the packages it imports and
@@ -80,6 +81,11 @@
  * implementations are used, as in the {@code test-stockjdk} execution: provider lookups cannot see other bundles without a mediator such as SPI Fly.
  * </p>
  * <p>
+ * Only the initial default-factory checks use the bundle class loader as the thread context class loader (TCCL). The reused tests retain the launcher's
+ * TCCL: XSLTC-generated translets need a parent loader that can see JDK internals. They therefore do not prove that all operations work with a bundle TCCL.
+ * The reused suite excludes {@code xpath3}, {@link #NOT_IN_OSGI} and {@link #METHODS_NOT_IN_OSGI}.
+ * </p>
+ * <p>
  * The framework is whichever single {@link FrameworkFactory} the class path provides; the build runs this class once with Apache Felix and once with Eclipse
  * Equinox.
  * </p>
@@ -91,9 +97,18 @@ class OsgiIT {
      * Factory methods that reach the JDK's built-in implementations without a provider lookup: {@code SecureSchemaFactory} also initializes the
      * {@code LSResourceResolver} floor.
      */
-    private static final String[][] DEFAULT_FACTORIES = {{"SecureDocumentBuilderFactory", "newDefaultInstance"},
-        {"SecureSAXParserFactory", "newDefaultInstance"}, {"SecureSchemaFactory", "newDefaultInstance"}, {"SecureTransformerFactory", "newDefaultInstance"},
-        {"SecureXPathFactory", "newDefaultInstance"}, {"SecureXMLInputFactory", "newDefaultFactory"}};
+    private static final String[][] DEFAULT_FACTORIES = {
+        {"SecureDocumentBuilderFactory", "newDefaultInstance", "javax.xml.parsers.DocumentBuilderFactory",
+            "com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl"},
+        {"SecureSAXParserFactory", "newDefaultInstance", "javax.xml.parsers.SAXParserFactory",
+            "com.sun.org.apache.xerces.internal.jaxp.SAXParserFactoryImpl"},
+        {"SecureSchemaFactory", "newDefaultInstance", "javax.xml.validation.SchemaFactory:http://www.w3.org/2001/XMLSchema",
+            "com.sun.org.apache.xerces.internal.jaxp.validation.XMLSchemaFactory"},
+        {"SecureTransformerFactory", "newDefaultInstance", "javax.xml.transform.TransformerFactory",
+            "com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl"},
+        {"SecureXPathFactory", "newDefaultInstance", "javax.xml.xpath.XPathFactory:http://java.sun.com/jaxp/xpath/dom",
+            "com.sun.org.apache.xpath.internal.jaxp.XPathFactoryImpl"},
+        {"SecureXMLInputFactory", "newDefaultFactory", "javax.xml.stream.XMLInputFactory", "com.sun.xml.internal.stream.XMLInputFactoryImpl"}};
 
     /**
      * Package roots the framework delegates to the class path: the test harness only, never the library or JDK internals.
@@ -214,10 +229,26 @@ void defaultFactoriesIgnoreContextClassLoader() throws Exception {
         thread.setContextClassLoader(host.adapt(BundleWiring.class).getClassLoader());
         try {
             assertAll(Stream.of(DEFAULT_FACTORIES).map(factory -> () -> {
+                final String className = factory[0];
+                final String methodName = factory[1];
+                final String property = factory[2];
+                final String provider = factory[3];
+                // A broken provider selection makes an accidental fallback to ordinary discovery fail, even on a stock-JDK class path.
+                final String previous = System.setProperty(property, "missing.Provider");
                 try {
-                    assertNotNull(host.loadClass(HOST_SYMBOLIC_NAME + "." + factory[0]).getMethod(factory[1]).invoke(null), factory[0] + "." + factory[1]);
+                    final Object secured = host.loadClass(HOST_SYMBOLIC_NAME + "." + className).getMethod(methodName).invoke(null);
+                    assertNotNull(secured, className + "." + methodName);
+                    final Field delegate = secured.getClass().getDeclaredField("delegate");
+                    delegate.setAccessible(true);
+                    assertEquals(provider, delegate.get(secured).getClass().getName(), className);
                 } catch (final InvocationTargetException e) {
                     throw e.getCause();
+                } finally {
+                    if (previous == null) {
+                        System.clearProperty(property);
+                    } else {
+                        System.setProperty(property, previous);
+                    }
                 }
             }));
         } finally {

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

@@ -0,0 +1,132 @@
+/*
+ * 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.secure;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.ByteArrayOutputStream;
+import java.io.InputStream;
+import java.lang.reflect.Field;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledInNativeImage;
+import org.w3c.dom.bootstrap.DOMImplementationRegistry;
+
+/**
+ * Tests resolver initialization using fresh JVM class definitions and the JDK DOM implementation.
+ *
+ * <p>
+ * Excluded from Android compilation because Android does not support this JVM class-loading mechanism.
+ * </p>
+ */
+@DisabledInNativeImage
+class FallbackIgnoreLSResourceResolverInitializationTest {
+
+    private static void checkInitialization(final boolean failRetry) throws Exception {
+        final Thread thread = Thread.currentThread();
+        final ClassLoader original = thread.getContextClassLoader();
+        final String source = failRetry ? "missing.DOMImplementationSource" : "com.sun.org.apache.xerces.internal.dom.DOMXSImplementationSourceImpl";
+        final ClassNotFoundException firstFailure = new ClassNotFoundException("Rejected by test TCCL");
+        final ClassLoader restricted = new ClassLoader(original) {
+
+            @Override
+            protected Class<?> loadClass(final String name, final boolean resolve) throws ClassNotFoundException {
+                if (source.equals(name)) {
+                    throw firstFailure;
+                }
+                return super.loadClass(name, resolve);
+            }
+        };
+        final String previous = System.setProperty(DOMImplementationRegistry.PROPERTY, source);
+        thread.setContextClassLoader(restricted);
+        try {
+            if (failRetry) {
+                final ExceptionInInitializerError error = assertThrows(ExceptionInInitializerError.class,
+                        FallbackIgnoreLSResourceResolverInitializationTest::initializeResolver);
+                final Throwable retry = error.getCause();
+                assertInstanceOf(ClassNotFoundException.class, retry);
+                assertEquals(1, retry.getSuppressed().length);
+                assertSame(firstFailure, retry.getSuppressed()[0]);
+            } else {
+                final Field domLs = initializeResolver().getDeclaredField("DOM_LS");
+                domLs.setAccessible(true);
+                assertNotNull(domLs.get(null));
+            }
+            assertSame(restricted, thread.getContextClassLoader(), "Initialization must restore the caller's TCCL");
+        } finally {
+            thread.setContextClassLoader(original);
+            if (previous == null) {
+                System.clearProperty(DOMImplementationRegistry.PROPERTY);
+            } else {
+                System.setProperty(DOMImplementationRegistry.PROPERTY, previous);
+            }
+        }
+    }
+
+    /**
+     * Defines only the resolver in a fresh loader, so each test executes its static initializer independently.
+     */
+    private static Class<?> initializeResolver() throws Exception {
+        final String name = FallbackIgnoreLSResourceResolver.class.getName();
+        final byte[] bytes;
+        try (InputStream input = FallbackIgnoreLSResourceResolver.class.getResourceAsStream("FallbackIgnoreLSResourceResolver.class");
+                ByteArrayOutputStream output = new ByteArrayOutputStream()) {
+            assertNotNull(input);
+            final byte[] buffer = new byte[4096];
+            int count;
+            while ((count = input.read(buffer)) != -1) {
+                output.write(buffer, 0, count);
+            }
+            bytes = output.toByteArray();
+        }
+        final ClassLoader isolated = new ClassLoader(FallbackIgnoreLSResourceResolver.class.getClassLoader()) {
+
+            @Override
+            protected Class<?> loadClass(final String className, final boolean resolve) throws ClassNotFoundException {
+                if (!name.equals(className)) {
+                    return super.loadClass(className, resolve);
+                }
+                synchronized (getClassLoadingLock(className)) {
+                    Class<?> result = findLoadedClass(className);
+                    if (result == null) {
+                        result = defineClass(className, bytes, 0, bytes.length);
+                    }
+                    if (resolve) {
+                        resolveClass(result);
+                    }
+                    return result;
+                }
+            }
+        };
+        return Class.forName(name, true, isolated);
+    }
+
+    @Test
+    void initializationRestoresContextLoaderAfterRetry() throws Exception {
+        checkInitialization(false);
+    }
+
+    @Test
+    void initializationRetainsBothFailuresAndRestoresContextLoader() throws Exception {
+        checkInitialization(true);
+    }
+}