This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 57f9399bdbf3d016fc0e44de5dcfeaf1af15fc1f Author: opencode <[email protected]> AuthorDate: Tue Oct 20 11:35:44 2026 +0200 Fix the class loader used to check the type of a JNDI lookup reference LookupFactory loaded the class named as the type of the reference with the one-argument Class.forName. That uses the class loader which loaded the factory itself. The factory ships in catalina.jar and is therefore loaded by a class loader which is a parent of the class loaders of the web applications, so it cannot see any class which is local to a web application. Every reference which declared such a class as its type failed with a ClassNotFoundException even when the looked up object was of the expected type. This includes the type of a field annotated with Resource if that type is a class of the web application, which is the normal case. The type is the type the application expects the object to have, so it is now loaded with the class loader of the application, which is the context class loader on the threads of a web application. This is also how the factory class of the reference is loaded a few lines above and how the sibling ResourceLinkFactory loads the type it checks against. If the class cannot be loaded, a NamingException with a message naming the class and the reference now reports the failure instead of letting the ClassNotFoundException propagate into the generic wrapping of the JNDI machinery. A reference which declares no type at all now skips the type check; previously, the null type led to a NullPointerException and the empty type to a ClassNotFoundException. With this, the type of a descriptor also has to name the expected type. For an EJB reference, the type element of the descriptor is the type of the reference itself, which is "Entity" or "Session" and never a class name, so NamingContextListener.addEjb now uses the bean interface which the application expects - the remote interface or, if the descriptor names none, the home interface - as the type of a lookup reference. If the descriptor names neither, the type of the looked up object is left unchecked. Previously, every EJB reference with a lookup-name failed because the class "Session" or "Entity" cannot be loaded. Add a regression test which compiles a class into the web application so that only its own class loader can see it, pinning both the type load with the class loader of the application and the use of the remote interface as the type of an EJB reference. --- .../catalina/core/NamingContextListener.java | 12 +- .../apache/naming/factory/LocalStrings.properties | 1 + java/org/apache/naming/factory/LookupFactory.java | 46 +++-- .../apache/naming/factory/TestLookupFactory.java | 230 +++++++++++++++++++++ 4 files changed, 275 insertions(+), 14 deletions(-) diff --git a/java/org/apache/catalina/core/NamingContextListener.java b/java/org/apache/catalina/core/NamingContextListener.java index 8ea24c293c..ea94bcb2d7 100644 --- a/java/org/apache/catalina/core/NamingContextListener.java +++ b/java/org/apache/catalina/core/NamingContextListener.java @@ -649,7 +649,17 @@ public class NamingContextListener implements LifecycleListener, PropertyChangeL */ public void addEjb(ContextEjb ejb) { - Reference ref = lookForLookupRef(ejb); + // The type of an EJB reference descriptor is the type of the reference + // itself, which is "Entity" or "Session" and never a class name, so the + // interface which the application expects is used as the type of a + // lookup reference. No type is given when the descriptor names no + // interface, which leaves the type of the looked up object unchecked. + String lookupName = ejb.getLookupName(); + Reference ref = null; + if (lookupName != null && !lookupName.isEmpty()) { + String beanType = ejb.getRemote() != null ? ejb.getRemote() : ejb.getHome(); + ref = new LookupRef(beanType, lookupName); + } if (ref == null) { // Create a reference to the EJB. diff --git a/java/org/apache/naming/factory/LocalStrings.properties b/java/org/apache/naming/factory/LocalStrings.properties index fe61bc5d2e..4a54e94849 100644 --- a/java/org/apache/naming/factory/LocalStrings.properties +++ b/java/org/apache/naming/factory/LocalStrings.properties @@ -29,6 +29,7 @@ factoryBase.factoryCreationError=Could not create resource factory instance factoryBase.instanceCreationError=Could not create resource instance lookupFactory.circularReference=Found a circular reference involving [{0}] +lookupFactory.classNotFound=Could not load the class [{0}] which is declared as the type of the JNDI reference [{1}] lookupFactory.createFailed=Could not create instance of JNDI lookup factory class lookupFactory.loadFailed=Could not load JNDI lookup factory class lookupFactory.typeMismatch=The JNDI reference [{0}] was expected to be of type [{1}] but the lookup [{2}] return an object of type [{3}] diff --git a/java/org/apache/naming/factory/LookupFactory.java b/java/org/apache/naming/factory/LookupFactory.java index 90f5a7440e..4d8499e8a2 100644 --- a/java/org/apache/naming/factory/LookupFactory.java +++ b/java/org/apache/naming/factory/LookupFactory.java @@ -128,21 +128,41 @@ public class LookupFactory implements ObjectFactory { } } - Class<?> clazz = Class.forName(ref.getClassName()); - if (result != null && !clazz.isAssignableFrom(result.getClass())) { - String msg = sm.getString("lookupFactory.typeMismatch", name, ref.getClassName(), lookupName, - result.getClass().getName()); - NamingException ne = new NamingException(msg); - log.warn(msg, ne); - // Close the resource we no longer need if we know how to do so - if (result instanceof AutoCloseable) { - try { - ((AutoCloseable) result).close(); - } catch (Exception e) { - // Ignore + String className = ref.getClassName(); + if (className != null && !className.isEmpty()) { + // The type is the type of the object the application + // expects, so it is loaded with the class loader of the + // application rather than with the one which loaded this + // factory. + Class<?> clazz; + ClassLoader tcl = Thread.currentThread().getContextClassLoader(); + try { + if (tcl != null) { + clazz = tcl.loadClass(className); + } else { + clazz = Class.forName(className); } + } catch (ClassNotFoundException e) { + NamingException ex = new NamingException( + sm.getString("lookupFactory.classNotFound", className, name)); + ex.initCause(e); + throw ex; + } + if (result != null && !clazz.isAssignableFrom(result.getClass())) { + String msg = sm.getString("lookupFactory.typeMismatch", name, className, lookupName, + result.getClass().getName()); + NamingException ne = new NamingException(msg); + log.warn(msg, ne); + // Close the resource we no longer need if we know how to do so + if (result instanceof AutoCloseable) { + try { + ((AutoCloseable) result).close(); + } catch (Exception e) { + // Ignore + } + } + throw ne; } - throw ne; } } finally { names.get().remove(lookupName); diff --git a/test/org/apache/naming/factory/TestLookupFactory.java b/test/org/apache/naming/factory/TestLookupFactory.java new file mode 100644 index 0000000000..4af5e7ebeb --- /dev/null +++ b/test/org/apache/naming/factory/TestLookupFactory.java @@ -0,0 +1,230 @@ +/* + * 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 + * + * http://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.naming.factory; + +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import javax.naming.Context; +import javax.tools.JavaCompiler; +import javax.tools.ToolProvider; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Assume; +import org.junit.Test; + +import org.apache.catalina.LifecycleListener; +import org.apache.catalina.core.NamingContextListener; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.naming.ContextAccessController; +import org.apache.naming.ContextBindings; +import org.apache.naming.LookupRef; +import org.apache.tomcat.util.descriptor.web.ContextEjb; + +/* + * The type of a lookup reference is the type which the application expects, so + * it has to be loaded with the class loader of the application. The factory + * itself is loaded by a class loader which does not see the classes of the + * web application, so using the class loader of the factory left the type + * unloadable and the lookup failed for every type which was local to the web + * application, which includes the type of a field annotated with Resource. + */ +public class TestLookupFactory extends TomcatBaseTest { + + private static final String MARKER_CLASS = "tester.Marker"; + + private ClassLoader originalContextClassLoader; + + + @Test + public void testTypeOfWebappLocalClassIsLoaded() throws Exception { + TesterWebapp webapp = startWebapp(); + + Context container = ContextBindings.getClassLoader(); + container.bind("comp/env/ref", new LookupRef(MARKER_CLASS, "java:comp/env/target")); + + Object found = new javax.naming.InitialContext().lookup("java:comp/env/ref"); + Assert.assertSame(webapp.marker(), found); + } + + + @Test + public void testEjbLookupRefIsCheckedAgainstTheBeanInterface() throws Exception { + TesterWebapp webapp = startWebapp(); + + ContextEjb ejb = new ContextEjb(); + ejb.setName("ejb"); + // The type of an EJB reference descriptor is the type of the + // reference, which is neither a class name nor a type the looked up + // object can have + ejb.setType("Session"); + ejb.setLookupName("java:comp/env/target"); + ejb.setRemote(MARKER_CLASS); + webapp.listener().addEjb(ejb); + + Object found = new javax.naming.InitialContext().lookup("java:comp/env/ejb"); + Assert.assertSame(webapp.marker(), found); + } + + + @Test + public void testTypeMismatchIsStillReported() throws Exception { + TesterWebapp webapp = startWebapp(); + + Context container = ContextBindings.getClassLoader(); + container.bind("comp/env/ref", new LookupRef(String.class.getName(), "java:comp/env/target")); + + try { + Object found = new javax.naming.InitialContext().lookup("java:comp/env/ref"); + Assert.fail("The reference of the wrong type was returned: " + found); + } catch (javax.naming.NamingException e) { + Assert.assertTrue("The wrong failure was reported: " + e.getMessage(), + e.getMessage().contains("expected to be of type")); + } + } + + + /* + * The type of an EJB reference descriptor names the type of the reference + * itself and is never loadable, so the remote interface has to be the type + * which the looked up object is checked against. + */ + @Test + public void testEjbLookupRefIsCheckedAgainstTheRemoteInterface() throws Exception { + TesterWebapp webapp = startWebapp(); + + ContextEjb ejb = new ContextEjb(); + ejb.setName("ejb"); + ejb.setType("Session"); + ejb.setLookupName("java:comp/env/target"); + // The object bound as the target is not an instance of this interface + ejb.setRemote(String.class.getName()); + webapp.listener().addEjb(ejb); + + try { + Object found = new javax.naming.InitialContext().lookup("java:comp/env/ejb"); + Assert.fail("The reference of the wrong type was returned: " + found); + } catch (javax.naming.NamingException e) { + Assert.assertTrue("The wrong failure was reported: " + e.getMessage(), + e.getMessage().contains("expected to be of type")); + } + } + + + @Test + public void testTypeWhichCannotBeLoadedIsReported() throws Exception { + startWebapp(); + + String missingClass = MARKER_CLASS + "Missing"; + Context container = ContextBindings.getClassLoader(); + container.bind("comp/env/ref", new LookupRef(missingClass, "java:comp/env/target")); + + try { + Object found = new javax.naming.InitialContext().lookup("java:comp/env/ref"); + Assert.fail("The reference of an unloadable type was returned: " + found); + } catch (javax.naming.NamingException e) { + Assert.assertTrue("The wrong failure was reported: " + e.getMessage(), + e.getMessage().contains("Could not load the class")); + Assert.assertTrue("The class which cannot be loaded is not named: " + e.getMessage(), + e.getMessage().contains(missingClass)); + Assert.assertTrue("The reason for the failure is not reported", + e.getCause() instanceof ClassNotFoundException); + } + } + + + /** + * Starts a web application with a class which only its own class loader + * can see and binds an instance of it as the target of the lookup + * references. The context class loader is set to the one of the web + * application, as it is for the threads of a web application, and restored + * after the test. + * + * @return the listener of the naming resources and the instance of the class of the web application + * + * @throws Exception if the web application cannot be started + */ + private TesterWebapp startWebapp() throws Exception { + JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + Assume.assumeTrue("The compiler of the JDK is not available", compiler != null); + + File docBase = Files.createTempDirectory(getTemporaryDirectory().toPath(), "docBase").toFile(); + Path classes = new File(docBase, "WEB-INF/classes/").toPath(); + Files.createDirectories(classes); + Path source = new File(docBase, "tester/Marker.java").toPath(); + Files.createDirectories(source.getParent()); + Files.write(source, ("package tester;\npublic class Marker {\n}\n".getBytes(StandardCharsets.UTF_8))); + ByteArrayOutputStream compilerOutput = new ByteArrayOutputStream(); + int result = compiler.run(null, compilerOutput, compilerOutput, "-d", classes.toString(), + source.toString()); + Assert.assertEquals("The class of the web application could not be compiled: " + compilerOutput, 0, result); + + Tomcat tomcat = getTomcatInstance(); + tomcat.enableNaming(); + org.apache.catalina.Context ctx = tomcat.addContext("", docBase.getAbsolutePath()); + tomcat.start(); + + // The context of the web application is read only once it is started + NamingContextListener listener = null; + for (LifecycleListener lifecycleListener : ctx.findLifecycleListeners()) { + if (lifecycleListener instanceof NamingContextListener namingContextListener) { + listener = namingContextListener; + } + } + Assert.assertNotNull(listener); + ContextAccessController.setWritable(listener.getName(), ctx.getNamingToken()); + + ClassLoader webappLoader = ctx.getLoader().getClassLoader(); + // The naming context of a web application is bound to its class loader + originalContextClassLoader = Thread.currentThread().getContextClassLoader(); + Thread.currentThread().setContextClassLoader(webappLoader); + + Object marker = webappLoader.loadClass(MARKER_CLASS).getDeclaredConstructor().newInstance(); + // The target of the references is bound in the context of the web + // application, which is the context a lookup resolves names in + Context container = ContextBindings.getClassLoader(); + container.bind("comp/env/target", marker); + + return new TesterWebapp(listener, marker); + } + + + /* + * The lookup is done on the test thread with the class loader of the web + * application as the context class loader, as it is on the threads of a + * web application. It is restored here, which runs before the tear down of + * the base class stops the web application, so that neither the stopping + * nor the other tests run with the class loader of the web application. + */ + @After + public void restoreContextClassLoader() { + Thread.currentThread().setContextClassLoader(originalContextClassLoader); + } + + + /** + * The listener which binds the naming resources of the web application and + * the instance of the class which only that web application can see. + */ + private record TesterWebapp(NamingContextListener listener, Object marker) { + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
