400457 Thread context classloader hierarchy not searched when finding webapp's java:comp/env

jetty-jndi/src/main/java/org/eclipse/jetty/jndi/ContextFactory.java

@@ -52,9 +52,11 @@ import org.eclipse.jetty.util.log.Logger;
  *  specific to a webapp).
  *  
  *  The context selected is based on classloaders. First
- *  we try looking in at the classloader that is associated
+ *  we try looking at the thread context classloader if it is set, and walk its
- *  with the current webapp context (if there is one). If
+ *  hierarchy, creating a context if none is found. If the thread context classloader
- *  not, we use the thread context classloader.
+ *  is not set, then we use the classloader associated with the current Context.
+ *  
+ *  If there is no current context, or no classloader, we return null.
  * 
  * Created: Fri Jun 27 09:26:40 2003
  *
@@ -80,9 +82,16 @@ public class ContextFactory implements ObjectFactory
     /** 
      * Find or create a context which pertains to a classloader.
      * 
-     * We use either the classloader for the current ContextHandler if
+     * If the thread context classloader is set, we try to find an already-created naming context
-     * we are handling a request, OR we use the thread context classloader
+     * for it. If one does not exist, we walk its classloader hierarchy until one is found, or we 
-     * if we are not processing a request.
+     * run out of parent classloaders. In the latter case, we will create a new naming context associated
+     * with the original thread context classloader.
+     * 
+     * If the thread context classloader is not set, we obtain the classloader from the current 
+     * jetty Context, and look for an already-created naming context. 
+     * 
+     * If there is no current jetty Context, or it has no associated classloader, we 
+     * return null.
      * @see javax.naming.spi.ObjectFactory#getObjectInstance(java.lang.Object, javax.naming.Name, javax.naming.Context, java.util.Hashtable)
      */
     public Object getObjectInstance (Object obj,
@@ -99,41 +108,89 @@ public class ContextFactory implements ObjectFactory
             return ctx;
         }
         
-        ClassLoader loader = null;
        
-        loader = Thread.currentThread().getContextClassLoader();
+        ClassLoader tccl = Thread.currentThread().getContextClassLoader();
-        if (__log.isDebugEnabled() && loader != null) __log.debug("Using thread context classloader");
+        ClassLoader loader = tccl;
-
+        //If the thread context classloader is set, then try its hierarchy to find a matching context
-        if (loader == null && ContextHandler.getCurrentContext() != null)
+        if (loader != null)
         {
-            loader = ContextHandler.getCurrentContext().getContextHandler().getClassLoader();
+            if (__log.isDebugEnabled() && loader != null) __log.debug("Trying thread context classloader");
-            if (__log.isDebugEnabled() && loader != null) __log.debug("Using classloader of current org.eclipse.jetty.server.handler.ContextHandler");
+            while (ctx == null && loader != null)
+            {
+                ctx = getContextForClassLoader(loader);
+                if (ctx == null && loader != null)
+                    loader = loader.getParent();
             }
 
-        //Get the context matching the classloader
-        ctx = (Context)__contextMap.get(loader);
-
-        //The map does not contain an entry for this classloader
             if (ctx == null)
             {
-            //Didn't find a context to match, make one
+                ctx = newNamingContext(obj, tccl, env, name, nameCtx);
+                __contextMap.put (tccl, ctx);
+                if(__log.isDebugEnabled())__log.debug("Made context "+name.get(0)+" for classloader: "+tccl);
+            }
+            return ctx;
+        }
+
+
+        //If trying thread context classloader hierarchy failed, try the
+        //classloader associated with the current context
+        if (ContextHandler.getCurrentContext() != null)
+        {
+            
+            if (__log.isDebugEnabled() && loader != null) __log.debug("Trying classloader of current org.eclipse.jetty.server.handler.ContextHandler");
+            loader = ContextHandler.getCurrentContext().getContextHandler().getClassLoader();
+            ctx = (Context)__contextMap.get(loader);    
+
+            if (ctx == null && loader != null)
+            {
+                ctx = newNamingContext(obj, loader, env, name, nameCtx);
+                __contextMap.put (loader, ctx);
+                if(__log.isDebugEnabled())__log.debug("Made context "+name.get(0)+" for classloader: "+loader);
+            }
+
+            return ctx;
+        }
+        return null;
+    }
+
+
+    /**
+     * Create a new NamingContext.
+     * @param obj
+     * @param loader
+     * @param env
+     * @param name
+     * @param parentCtx
+     * @return
+     * @throws Exception
+     */
+    public NamingContext newNamingContext(Object obj, ClassLoader loader, Hashtable env, Name name, Context parentCtx)
+    throws Exception
+    {
         Reference ref = (Reference)obj;
         StringRefAddr parserAddr = (StringRefAddr)ref.get("parser");
         String parserClassName = (parserAddr==null?null:(String)parserAddr.getContent());
         NameParser parser = (NameParser)(parserClassName==null?null:loader.loadClass(parserClassName).newInstance());
 
-            ctx = new NamingContext (env,
+        return new NamingContext (env,
                                   name.get(0),
-                                     (NamingContext)nameCtx,
+                                  (NamingContext)parentCtx,
                                   parser);
-            if(__log.isDebugEnabled())__log.debug("Made context "+name.get(0)+" for classloader: "+loader);
-            __contextMap.put (loader, ctx);
-        }
-
-        return ctx;
     }
 
   
+    /**
+     * Find the naming Context for the given classloader
+     * @param loader
+     * @return
+     */
+    public Context getContextForClassLoader(ClassLoader loader)
+    {
+        if (loader == null)
+            return null;
+        
+        return (Context)__contextMap.get(loader);
+    }
     
 
     /**

jetty-jndi/src/test/java/org/eclipse/jetty/jndi/java/TestJNDI.java

@@ -36,13 +36,17 @@ import javax.naming.NamingException;
 import javax.naming.Reference;
 import javax.naming.StringRefAddr;
 import javax.naming.spi.ObjectFactory;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
 
 import org.eclipse.jetty.jndi.ContextFactory;
 import org.eclipse.jetty.jndi.NamingContext;
 import org.eclipse.jetty.jndi.NamingUtil;
 import org.eclipse.jetty.jndi.local.localContextRoot;
+import org.eclipse.jetty.server.handler.ContextHandler;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
@@ -73,69 +77,134 @@ public class TestJNDI
         
     }
     
+    
     @Test
-    public void testIt() throws Exception
+    public void testThreadContextClassloaderAndCurrentContext()
+    throws Exception
     {
-        //set up some classloaders
+        //create a jetty context, and start it so that its classloader it created
-        Thread currentThread = Thread.currentThread();
+        //and it is the current context
-        ClassLoader currentLoader = currentThread.getContextClassLoader();
+        ContextHandler ch = new ContextHandler();
-        ClassLoader childLoader1 = new URLClassLoader(new URL[0], currentLoader);
+        URLClassLoader chLoader = new URLClassLoader(new URL[0], Thread.currentThread().getContextClassLoader());
-        ClassLoader childLoader2 = new URLClassLoader(new URL[0], currentLoader);
+        ch.setClassLoader(chLoader);
+        
+        //Create another one
+        ContextHandler ch2 = new ContextHandler();
+        URLClassLoader ch2Loader = new URLClassLoader(new URL[0], Thread.currentThread().getContextClassLoader());
+        ch2.setClassLoader(ch2Loader);
         
         try
         {
-
+            ch.setContextPath("/ch");
-            //Uncomment to aid with debug
+            ch.addEventListener(new ServletContextListener()
-            /*
-            javaRootURLContext.getRoot().addListener(new NamingContext.Listener()
             {
-                public void unbind(NamingContext ctx, Binding binding)
+                private Context comp;
+                private Object testObj = new Object();
+                
+                public void contextInitialized(ServletContextEvent sce)
                 {
-                    System.err.println("java unbind "+binding+" from "+ctx.getName());
-                }
-                
-                public Binding bind(NamingContext ctx, Binding binding)
-                {
-                    System.err.println("java bind "+binding+" to "+ctx.getName());
-                    return binding;
-                }
-            });
-            
-            localContextRoot.getRoot().addListener(new NamingContext.Listener()
-            {
-                public void unbind(NamingContext ctx, Binding binding)
-                {
-                    System.err.println("local unbind "+binding+" from "+ctx.getName());
-                }
-                
-                public Binding bind(NamingContext ctx, Binding binding)
-                {
-                    System.err.println("local bind "+binding+" to "+ctx.getName());
-                    return binding;
-                }
-            });
-            */
-            
-            
-            //set the current thread's classloader
-            currentThread.setContextClassLoader(childLoader1);
-
-            InitialContext initCtxA = new InitialContext();
-            initCtxA.bind ("blah", "123");
-            assertEquals ("123", initCtxA.lookup("blah"));
-            
-            initCtxA.destroySubcontext("blah");
                     try
                     {
-                initCtxA.lookup("blah");
+                        InitialContext initCtx = new InitialContext();
-                fail("context blah was not destroyed");
+                        Context java = (Context)initCtx.lookup("java:");
+                        assertNotNull(java);
+                        comp = (Context)initCtx.lookup("java:comp");
+                        assertNotNull(comp);
+                        Context env = ((Context)comp).createSubcontext("env");
+                        assertNotNull(env);
+                        env.bind("ch", testObj);
+                    }
+                    catch (Exception e)
+                    {
+                        throw new IllegalStateException(e);
+                    }
+                }
+
+                public void contextDestroyed(ServletContextEvent sce)
+                {
+                    try
+                    {
+                        assertNotNull(comp);
+                        assertEquals(testObj,comp.lookup("env/ch"));
+                        comp.destroySubcontext("env");
+                    }
+                    catch (Exception e)
+                    {
+                        throw new IllegalStateException(e);
+                    }
+                }
+            });
+            //Starting the context makes it current and creates a classloader for it
+            ch.start();
+            
+            ch2.setContextPath("/ch2");
+            ch2.addEventListener(new ServletContextListener()
+            {
+                private Context comp;
+                private Object testObj = new Object();
+
+                public void contextInitialized(ServletContextEvent sce)
+                {
+                    try
+                    {
+                        InitialContext initCtx = new InitialContext();
+                        comp = (Context)initCtx.lookup("java:comp");
+                        assertNotNull(comp);
+                       
+                        //another context's bindings should not be visible
+                        Context env = ((Context)comp).createSubcontext("env");
+                        try
+                        {
+                            env.lookup("ch");
+                            fail("java:comp/env visible from another context!");
                         }
                         catch (NameNotFoundException e)
                         {
                             //expected
                         }
+                    }
+                    catch (Exception e)
+                    {
+                        throw new IllegalStateException(e);
+                    }
+                }
+                
+                public void contextDestroyed(ServletContextEvent sce)
+                {
+                    try
+                    {
+                        assertNotNull(comp);
+                        comp.destroySubcontext("env");
+                    }
+                    catch (Exception e)
+                    {
+                        throw new IllegalStateException(e);
+                    }
+                }
+            });
+            //make the new context the current one
+            ch2.start();
+        }
+        finally
+        {
+            ch.stop();
+            ch2.stop();
+        }
+    }
+    
+    @Test
+    public void testJavaNameParsing() throws Exception
+    {
+        Thread currentThread = Thread.currentThread();
+        ClassLoader currentLoader = currentThread.getContextClassLoader();
+        ClassLoader childLoader1 = new URLClassLoader(new URL[0], currentLoader);
         
         
+        //set the current thread's classloader
+        currentThread.setContextClassLoader(childLoader1);
+
+        try
+        {
             InitialContext initCtx = new InitialContext();
             Context sub0 = (Context)initCtx.lookup("java:");
 
@@ -185,10 +254,73 @@ public class TestJNDI
 
             Context fee = ncontext.createSubcontext("fee");
             fee.bind ("fi", "88");
-            assertEquals("88", initCtxA.lookup("java:/fee/fi"));
+            assertEquals("88", initCtx.lookup("java:/fee/fi"));
-            assertEquals("88", initCtxA.lookup("java:/fee/fi/"));
+            assertEquals("88", initCtx.lookup("java:/fee/fi/"));
-            assertTrue (initCtxA.lookup("java:/fee/") instanceof javax.naming.Context);
+            assertTrue (initCtx.lookup("java:/fee/") instanceof javax.naming.Context);
+        }
+        finally
+        {
+            InitialContext ic = new InitialContext();
+            Context java = (Context)ic.lookup("java:");
+            java.destroySubcontext("fee");
+        }
+    }
     
+    
+
+    @Test
+    public void testIt() throws Exception
+    {
+        //set up some classloaders
+        Thread currentThread = Thread.currentThread();
+        ClassLoader currentLoader = currentThread.getContextClassLoader();
+        ClassLoader childLoader1 = new URLClassLoader(new URL[0], currentLoader);
+        ClassLoader childLoader2 = new URLClassLoader(new URL[0], currentLoader);
+        
+        try
+        {
+
+            //Uncomment to aid with debug
+            /*
+            javaRootURLContext.getRoot().addListener(new NamingContext.Listener()
+            {
+                public void unbind(NamingContext ctx, Binding binding)
+                {
+                    System.err.println("java unbind "+binding+" from "+ctx.getName());
+                }
+                
+                public Binding bind(NamingContext ctx, Binding binding)
+                {
+                    System.err.println("java bind "+binding+" to "+ctx.getName());
+                    return binding;
+                }
+            });
+            
+            localContextRoot.getRoot().addListener(new NamingContext.Listener()
+            {
+                public void unbind(NamingContext ctx, Binding binding)
+                {
+                    System.err.println("local unbind "+binding+" from "+ctx.getName());
+                }
+                
+                public Binding bind(NamingContext ctx, Binding binding)
+                {
+                    System.err.println("local bind "+binding+" to "+ctx.getName());
+                    return binding;
+                }
+            });
+            */
+            
+            //Set up the tccl before doing any jndi operations
+            currentThread.setContextClassLoader(childLoader1);
+            InitialContext initCtx = new InitialContext();
+            
+            //Test we can lookup the root java: naming tree
+            Context sub0 = (Context)initCtx.lookup("java:");
+            assertNotNull(sub0);
+            
+            //Test that we cannot bind java:comp as it should
+            //already be bound 
             try
             {
                 Context sub1 = sub0.createSubcontext ("comp");
@@ -201,8 +333,10 @@ public class TestJNDI
 
             //check bindings at comp
             Context sub1 = (Context)initCtx.lookup("java:comp");
+            assertNotNull(sub1);
 
             Context sub2 = sub1.createSubcontext ("env");
+            assertNotNull(sub2);
 
             initCtx.bind ("java:comp/env/rubbish", "abc");
             assertEquals ("abc", initCtx.lookup("java:comp/env/rubbish"));
@@ -305,7 +439,6 @@ public class TestJNDI
                 //expected failure to modify immutable context
             }
             
-            //test what happens when you close an initial context that was used
             initCtx.close();
         }
         finally
@@ -322,59 +455,4 @@ public class TestJNDI
             comp.unbind("crud2");
         }
     }
-    
-    
-    @Test
-    public void testParent()
-    throws Exception
-    {
-        //set up some classloaders
-        Thread currentThread = Thread.currentThread();
-        ClassLoader parentLoader = currentThread.getContextClassLoader();
-        ClassLoader childLoader1 = new URLClassLoader(new URL[0], parentLoader);
-        
-        try
-        {
-            //Test creating a comp for the parent loader does not leak to child
-            InitialContext initCtx = new InitialContext();
-            Context comp = (Context)initCtx.lookup("java:comp");
-            assertNotNull(comp);
-            
-            Context env = (Context)comp.createSubcontext("env");
-            assertNotNull(env);
-            
-            env.bind("foo", "aaabbbcccddd");
-            assertEquals("aaabbbcccddd", (String)initCtx.lookup("java:comp/env/foo"));
-            
-            //Change to child loader
-            currentThread.setContextClassLoader(childLoader1);
-            comp = (Context)initCtx.lookup("java:comp");
-            
-            Context childEnv = (Context)comp.createSubcontext("env");
-            assertNotSame(env, childEnv);
-            
-            childEnv.bind("foo", "eeefffggghhh");
-            assertEquals("eeefffggghhh", (String)initCtx.lookup("java:comp/env/foo"));
-            
-            //Change back to parent
-            currentThread.setContextClassLoader(parentLoader);
-            assertEquals("aaabbbcccddd", (String)initCtx.lookup("java:comp/env/foo"));
-            
-            
-        }
-        finally
-        {
-            //make some effort to clean up
-            InitialContext ic = new InitialContext();
-            currentThread.setContextClassLoader(parentLoader);
-            Context comp = (Context)ic.lookup("java:comp");
-            comp.destroySubcontext("env");
-            
-            currentThread.setContextClassLoader(childLoader1);
-            comp = (Context)ic.lookup("java:comp");
-            comp.destroySubcontext("env");
-         
-         
-        }
-    }
 }

jetty-jndi/src/test/java/org/eclipse/jetty/jndi/java/TestLocalJNDI.java

@@ -224,6 +224,15 @@ public class TestLocalJNDI
         assertEquals("333", (String)o);
         assertEquals("333", ic.lookup(name));
         ic.destroySubcontext("a");
+        try
+        {
+            ic.lookup("a");
+            fail("context a was not destroyed");
+        }
+        catch (NameNotFoundException e)
+        {
+            //expected
+        }
 
         name = parser.parse("");
         name.add("x");