Issue #3729 Ensure java:comp/env creation threadsafe.

Also fixed and added some tests for locking of java:comp/env

Signed-off-by: Jan Bartel <janb@webtide.com>
This commit is contained in:
Jan Bartel 2019-06-05 12:19:33 +02:00
parent 4db934a809
commit bc3c650822
2 changed files with 100 additions and 41 deletions

View File

@ -19,8 +19,10 @@
package org.eclipse.jetty.jndi;
import java.io.IOException;
import java.util.Collections;
import java.util.Hashtable;
import java.util.WeakHashMap;
import java.util.Map;
import javax.naming.Context;
import javax.naming.Name;
import javax.naming.NameParser;
@ -62,7 +64,7 @@ public class ContextFactory implements ObjectFactory
/**
* Map of classloaders to contexts.
*/
private static final WeakHashMap __contextMap = new WeakHashMap();
private static final Map<ClassLoader, Context> __contextMap = Collections.synchronizedMap(new WeakHashMap<>());
/**
* Threadlocal for injecting a context to use
@ -102,7 +104,8 @@ public class ContextFactory implements ObjectFactory
Context ctx = (Context)__threadContext.get();
if (ctx != null)
{
if(__log.isDebugEnabled()) __log.debug("Using the Context that is bound on the thread");
if(__log.isDebugEnabled())
__log.debug("Using the Context that is bound on the thread");
return ctx;
}
@ -111,37 +114,47 @@ public class ContextFactory implements ObjectFactory
ClassLoader loader = (ClassLoader)__threadClassLoader.get();
if (loader != null)
{
if (__log.isDebugEnabled() && loader != null) __log.debug("Using threadlocal classloader");
ctx = getContextForClassLoader(loader);
if (ctx == null)
if (__log.isDebugEnabled())
__log.debug("Using threadlocal classloader");
synchronized(__contextMap)
{
ctx = newNamingContext(obj, loader, env, name, nameCtx);
__contextMap.put (loader, ctx);
if(__log.isDebugEnabled())__log.debug("Made context "+name.get(0)+" for classloader: "+loader);
ctx = getContextForClassLoader(loader);
if (ctx == null)
{
ctx = newNamingContext(obj, loader, env, name, nameCtx);
__contextMap.put (loader, ctx);
if(__log.isDebugEnabled())
__log.debug("Made context {} for classloader {}",name.get(0),loader);
}
return ctx;
}
return ctx;
}
//If the thread context classloader is set, then try its hierarchy to find a matching context
ClassLoader tccl = Thread.currentThread().getContextClassLoader();
loader = tccl;
if (loader != null)
{
if (__log.isDebugEnabled() && loader != null) __log.debug("Trying thread context classloader");
while (ctx == null && loader != null)
if (__log.isDebugEnabled())
__log.debug("Trying thread context classloader");
synchronized(__contextMap)
{
ctx = getContextForClassLoader(loader);
if (ctx == null && loader != null)
loader = loader.getParent();
}
while (ctx == null && loader != null)
{
ctx = getContextForClassLoader(loader);
if (ctx == null && loader != null)
loader = loader.getParent();
}
if (ctx == null)
{
ctx = newNamingContext(obj, tccl, env, name, nameCtx);
__contextMap.put (tccl, ctx);
if(__log.isDebugEnabled())__log.debug("Made context "+name.get(0)+" for classloader: "+tccl);
if (ctx == null)
{
ctx = newNamingContext(obj, tccl, env, name, nameCtx);
__contextMap.put (tccl, ctx);
if(__log.isDebugEnabled())
__log.debug("Made context {} for classloader {}", name.get(0), tccl);
}
return ctx;
}
return ctx;
}
@ -149,19 +162,23 @@ public class ContextFactory implements ObjectFactory
//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)
if (__log.isDebugEnabled() && loader != null)
__log.debug("Trying classloader of current org.eclipse.jetty.server.handler.ContextHandler");
synchronized(__contextMap)
{
ctx = newNamingContext(obj, loader, env, name, nameCtx);
__contextMap.put (loader, ctx);
if(__log.isDebugEnabled())__log.debug("Made context "+name.get(0)+" for classloader: "+loader);
}
loader = ContextHandler.getCurrentContext().getContextHandler().getClassLoader();
ctx = (Context)__contextMap.get(loader);
return ctx;
if (ctx == null && loader != null)
{
ctx = newNamingContext(obj, loader, env, name, nameCtx);
__contextMap.put (loader, ctx);
if(__log.isDebugEnabled())
__log.debug("Made context {} for classloader {} ", name.get(0), loader);
}
return ctx;
}
}
return null;
}
@ -239,6 +256,9 @@ public class ContextFactory implements ObjectFactory
public static void dump(Appendable out, String indent) throws IOException
{
Dumpable.dumpObjects(out, indent, String.format("o.e.j.jndi.ContextFactory@",__contextMap.hashCode()), __contextMap);
synchronized (__contextMap)
{
Dumpable.dumpObjects(out, indent, String.format("o.e.j.jndi.ContextFactory@",__contextMap.hashCode()), __contextMap);
}
}
}

View File

@ -18,7 +18,9 @@
package org.eclipse.jetty.jndi.java;
import static org.hamcrest.MatcherAssert.assertThat;
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.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
@ -49,6 +51,7 @@ import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.server.handler.HandlerList;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Test;
/**
*
@ -288,7 +291,7 @@ public class TestJNDI
ClassLoader currentLoader = currentThread.getContextClassLoader();
ClassLoader childLoader1 = new URLClassLoader(new URL[0], currentLoader);
ClassLoader childLoader2 = new URLClassLoader(new URL[0], currentLoader);
InitialContext initCtx = null;
try
{
@ -325,7 +328,7 @@ public class TestJNDI
//Set up the tccl before doing any jndi operations
currentThread.setContextClassLoader(childLoader1);
InitialContext initCtx = new InitialContext();
initCtx = new InitialContext();
//Test we can lookup the root java: naming tree
Context sub0 = (Context)initCtx.lookup("java:");
@ -439,30 +442,66 @@ public class TestJNDI
//check locking the context
Context ectx = (Context)initCtx.lookup("java:comp");
//make a deep structure lie ttt/ttt2 for later use
Context ttt = ectx.createSubcontext("ttt");
ttt.createSubcontext("ttt2");
//bind a value
ectx.bind("crud", "xxx");
ectx.addToEnvironment("org.eclipse.jndi.immutable", "TRUE");
//lock
ectx.addToEnvironment("org.eclipse.jetty.jndi.lock", "TRUE");
//check we can't get the lock
assertFalse(ectx.getEnvironment().containsKey("org.eclipse.jetty.jndi.lock"));
//check once locked we can still do lookups
assertEquals ("xxx", initCtx.lookup("java:comp/crud"));
assertNotNull(initCtx.lookup("java:comp/ttt/ttt2"));
//test trying to bind into java:comp after lock
InitialContext zzz = null;
try
{
ectx.bind("crud2", "xxx2");
zzz = new InitialContext();
((Context)zzz.lookup("java:comp")).bind("crud2", "xxx2");
fail("Should not be able to write to locked context");
}
catch (NamingException ne)
{
//expected failure to modify immutable context
assertThat(ne.getMessage(), Matchers.containsString("immutable"));
}
finally
{
zzz.close();
}
initCtx.close();
//test trying to bind into a deep structure inside java:comp after lock
try
{
zzz = new InitialContext();
((Context)zzz.lookup("java:comp/ttt/ttt2")).bind("zzz2", "zzz2");
fail("Should not be able to write to locked context");
}
catch (NamingException ne)
{
assertThat(ne.getMessage(), Matchers.containsString("immutable"));
}
finally
{
zzz.close();
}
}
finally
{
//make some effort to clean up
initCtx.close();
InitialContext ic = new InitialContext();
Context java = (Context)ic.lookup("java:");
java.destroySubcontext("zero");
java.destroySubcontext("fee");
currentThread.setContextClassLoader(childLoader1);
Context comp = (Context)ic.lookup("java:comp");
comp.addToEnvironment("org.eclipse.jetty.jndi.unlock", "TRUE");
comp.destroySubcontext("env");
comp.unbind("crud");
comp.unbind("crud2");