Servlet Context/Handler/Holder cleanups (#3767)

Issue #3749 #3755 Servlet annotation cleanups
Cleanups when checking thread safety of lazy servlet/filter introspection.

 + Handle multiple holders for annotated servlets
 + Replaced duplicate code with common methods

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2019-06-12 08:00:01 +02:00 committed by GitHub
parent 2ea0bca333
commit 819c379975
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 72 additions and 169 deletions

View File

@ -62,9 +62,7 @@ public class MultiPartConfigAnnotationHandler extends AbstractIntrospectableAnno
//How to identify the correct Servlet? If the Servlet has no WebServlet annotation on it, does it mean that this MultipartConfig
//annotation applies to all declared instances in web.xml/programmatically?
//Assuming TRUE for now.
ServletHolder holder = getServletHolderForClass(clazz);
if (holder != null)
for (ServletHolder holder : _context.getServletHandler().getServlets(clazz))
{
Descriptor d = metaData.getOriginDescriptor(holder.getName()+".servlet.multipart-config");
//if a descriptor has already set the value for multipart config, do not
@ -76,21 +74,4 @@ public class MultiPartConfigAnnotationHandler extends AbstractIntrospectableAnno
}
}
}
private ServletHolder getServletHolderForClass (Class clazz)
{
ServletHolder holder = null;
ServletHolder[] holders = _context.getServletHandler().getServlets();
if (holders != null)
{
for (ServletHolder h : holders)
{
if (h.getClassName() != null && h.getClassName().equals(clazz.getName()))
{
holder = h;
}
}
}
return holder;
}
}

View File

@ -56,8 +56,7 @@ public class RunAsAnnotationHandler extends AbstractIntrospectableAnnotationHand
String role = runAs.value();
if (role != null)
{
ServletHolder holder = getServletHolderForClass(clazz);
if (holder != null)
for (ServletHolder holder : _context.getServletHandler().getServlets(clazz))
{
MetaData metaData = _context.getMetaData();
Descriptor d = metaData.getOriginDescriptor(holder.getName()+".servlet.run-as");
@ -82,7 +81,6 @@ public class RunAsAnnotationHandler extends AbstractIntrospectableAnnotationHand
else
LOG.warn("Bad value for @RunAs annotation on class "+clazz.getName());
}
}
public void handleField(String className, String fieldName, int access, String fieldType, String signature, Object value, String annotation)
@ -94,21 +92,4 @@ public class RunAsAnnotationHandler extends AbstractIntrospectableAnnotationHand
{
LOG.warn("@RunAs annotation ignored on method: "+className+"."+methodName+" "+signature);
}
private ServletHolder getServletHolderForClass (Class clazz)
{
ServletHolder holder = null;
ServletHolder[] holders = _context.getServletHandler().getServlets();
if (holders != null)
{
for (ServletHolder h : holders)
{
if (h.getClassName() != null && h.getClassName().equals(clazz.getName()))
{
holder = h;
}
}
}
return holder;
}
}

View File

@ -29,7 +29,6 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import javax.servlet.DispatcherType;
import javax.servlet.Filter;
import javax.servlet.FilterRegistration;
@ -61,12 +60,12 @@ import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.HandlerContainer;
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.server.handler.ErrorHandler;
import org.eclipse.jetty.server.handler.HandlerCollection;
import org.eclipse.jetty.server.handler.HandlerWrapper;
import org.eclipse.jetty.server.handler.gzip.GzipHandler;
import org.eclipse.jetty.server.session.SessionHandler;
import org.eclipse.jetty.util.DecoratedObjectFactory;
import org.eclipse.jetty.util.DeprecationWarning;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.annotation.ManagedAttribute;
import org.eclipse.jetty.util.annotation.ManagedObject;
import org.eclipse.jetty.util.component.LifeCycle;
@ -255,7 +254,6 @@ public class ServletContextHandler extends ContextHandler
handler=_gzipHandler;
}
// link servlet handler
if (getServletHandler()!=null)
{
@ -267,7 +265,6 @@ public class ServletContextHandler extends ContextHandler
doSetHandler(handler,_servletHandler);
handler=_servletHandler;
}
}
/* ------------------------------------------------------------ */
@ -566,21 +563,32 @@ public class ServletContextHandler extends ContextHandler
super.callContextDestroyed(l, e);
}
private boolean replaceHandler(Handler handler,Handler replace)
private void replaceHandler(HandlerWrapper handler, HandlerWrapper replacement)
{
HandlerWrapper wrapper=this;
while(true)
{
if (wrapper.getHandler()==handler)
{
doSetHandler(wrapper,replace);
return true;
}
if (isStarted())
throw new IllegalStateException("STARTED");
if (!(wrapper.getHandler() instanceof HandlerWrapper))
return false;
wrapper = (HandlerWrapper)wrapper.getHandler();
Handler next=null;
if (handler!=null)
{
next=handler.getHandler();
handler.setHandler(null);
HandlerWrapper wrapper=this;
while(wrapper != null)
{
if (wrapper.getHandler() == handler)
{
doSetHandler(wrapper,replacement);
break;
}
wrapper = (wrapper.getHandler() instanceof HandlerWrapper) ? (HandlerWrapper)wrapper.getHandler() : null;
}
}
if (next!=null && replacement.getHandler()==null)
replacement.setHandler(next);
}
/* ------------------------------------------------------------ */
@ -589,20 +597,8 @@ public class ServletContextHandler extends ContextHandler
*/
public void setSessionHandler(SessionHandler sessionHandler)
{
if (isStarted())
throw new IllegalStateException("STARTED");
Handler next=null;
if (_sessionHandler!=null)
{
next=_sessionHandler.getHandler();
_sessionHandler.setHandler(null);
replaceHandler(_sessionHandler,sessionHandler);
}
replaceHandler(_sessionHandler, sessionHandler);
_sessionHandler = sessionHandler;
if (next!=null && _sessionHandler.getHandler()==null)
_sessionHandler.setHandler(next);
relinkHandlers();
}
@ -612,44 +608,19 @@ public class ServletContextHandler extends ContextHandler
*/
public void setSecurityHandler(SecurityHandler securityHandler)
{
if (isStarted())
throw new IllegalStateException("STARTED");
Handler next=null;
if (_securityHandler!=null)
{
next=_securityHandler.getHandler();
_securityHandler.setHandler(null);
replaceHandler(_securityHandler,securityHandler);
}
replaceHandler(_sessionHandler, securityHandler);
_securityHandler = securityHandler;
if (next!=null && _securityHandler.getHandler()==null)
_securityHandler.setHandler(next);
relinkHandlers();
}
/* ------------------------------------------------------------ */
/**
* @param gzipHandler The {@link GzipHandler} to set on this context.
*/
public void setGzipHandler(GzipHandler gzipHandler)
{
if (isStarted())
throw new IllegalStateException("STARTED");
Handler next=null;
if (_gzipHandler!=null)
{
next=_gzipHandler.getHandler();
_gzipHandler.setHandler(null);
replaceHandler(_gzipHandler,gzipHandler);
}
replaceHandler(_gzipHandler, gzipHandler);
_gzipHandler = gzipHandler;
if (next!=null && _gzipHandler.getHandler()==null)
_gzipHandler.setHandler(next);
relinkHandlers();
}
@ -659,23 +630,11 @@ public class ServletContextHandler extends ContextHandler
*/
public void setServletHandler(ServletHandler servletHandler)
{
if (isStarted())
throw new IllegalStateException("STARTED");
Handler next=null;
if (_servletHandler!=null)
{
next=_servletHandler.getHandler();
_servletHandler.setHandler(null);
replaceHandler(_servletHandler,servletHandler);
}
replaceHandler(_servletHandler, servletHandler);
_servletHandler = servletHandler;
if (next!=null && _servletHandler.getHandler()==null)
_servletHandler.setHandler(next);
relinkHandlers();
}
/* ------------------------------------------------------------ */
/**
* Insert a HandlerWrapper before the first Session,Security or ServletHandler
@ -1102,6 +1061,18 @@ public class ServletContextHandler extends ContextHandler
return new Dispatcher(context, name);
}
private void checkDynamicName(String name)
{
if (isStarted())
throw new IllegalStateException();
if (StringUtil.isBlank(name))
throw new IllegalStateException("Missing name");
if (!_enabled)
throw new UnsupportedOperationException();
}
/* ------------------------------------------------------------ */
/**
* @since servlet-api-3.0
@ -1109,14 +1080,7 @@ public class ServletContextHandler extends ContextHandler
@Override
public FilterRegistration.Dynamic addFilter(String filterName, Class<? extends Filter> filterClass)
{
if (isStarted())
throw new IllegalStateException();
if (filterName == null || "".equals(filterName.trim()))
throw new IllegalStateException("Missing filter name");
if (!_enabled)
throw new UnsupportedOperationException();
checkDynamicName(filterName);
final ServletHandler handler = ServletContextHandler.this.getServletHandler();
FilterHolder holder = handler.getFilter(filterName);
@ -1146,14 +1110,7 @@ public class ServletContextHandler extends ContextHandler
@Override
public FilterRegistration.Dynamic addFilter(String filterName, String className)
{
if (isStarted())
throw new IllegalStateException();
if (filterName == null || "".equals(filterName.trim()))
throw new IllegalStateException("Missing filter name");
if (!_enabled)
throw new UnsupportedOperationException();
checkDynamicName(filterName);
final ServletHandler handler = ServletContextHandler.this.getServletHandler();
FilterHolder holder = handler.getFilter(filterName);
@ -1184,14 +1141,7 @@ public class ServletContextHandler extends ContextHandler
@Override
public FilterRegistration.Dynamic addFilter(String filterName, Filter filter)
{
if (isStarted())
throw new IllegalStateException();
if (filterName == null || "".equals(filterName.trim()))
throw new IllegalStateException("Missing filter name");
if (!_enabled)
throw new UnsupportedOperationException();
checkDynamicName(filterName);
final ServletHandler handler = ServletContextHandler.this.getServletHandler();
FilterHolder holder = handler.getFilter(filterName);
@ -1222,14 +1172,7 @@ public class ServletContextHandler extends ContextHandler
@Override
public ServletRegistration.Dynamic addServlet(String servletName, Class<? extends Servlet> servletClass)
{
if (!isStarting())
throw new IllegalStateException();
if (servletName == null || "".equals(servletName.trim()))
throw new IllegalStateException("Missing servlet name");
if (!_enabled)
throw new UnsupportedOperationException();
checkDynamicName(servletName);
final ServletHandler handler = ServletContextHandler.this.getServletHandler();
ServletHolder holder = handler.getServlet(servletName);
@ -1260,15 +1203,7 @@ public class ServletContextHandler extends ContextHandler
@Override
public ServletRegistration.Dynamic addServlet(String servletName, String className)
{
if (!isStarting())
throw new IllegalStateException();
if (servletName == null || "".equals(servletName.trim()))
throw new IllegalStateException("Missing servlet name");
if (!_enabled)
throw new UnsupportedOperationException();
checkDynamicName(servletName);
final ServletHandler handler = ServletContextHandler.this.getServletHandler();
ServletHolder holder = handler.getServlet(servletName);
@ -1299,14 +1234,7 @@ public class ServletContextHandler extends ContextHandler
@Override
public ServletRegistration.Dynamic addServlet(String servletName, Servlet servlet)
{
if (!isStarting())
throw new IllegalStateException();
if (servletName == null || "".equals(servletName.trim()))
throw new IllegalStateException("Missing servlet name");
if (!_enabled)
throw new UnsupportedOperationException();
checkDynamicName(servletName);
final ServletHandler handler = ServletContextHandler.this.getServletHandler();
ServletHolder holder = handler.getServlet(servletName);

View File

@ -33,7 +33,6 @@ import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.ConcurrentMap;
import java.util.stream.Stream;
import javax.servlet.DispatcherType;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
@ -427,6 +426,23 @@ public class ServletHandler extends ScopedHandler
return _servlets;
}
public List<ServletHolder> getServlets(Class<?> clazz)
{
List<ServletHolder> holders = null;
for (ServletHolder holder : _servlets)
{
Class<? extends Servlet> held = holder.getHeldClass();
if ((held == null && holder.getClassName() != null && holder.getClassName().equals(clazz.getName())) ||
(held != null && clazz.isAssignableFrom(holder.getHeldClass())))
{
if (holders == null)
holders = new ArrayList<>();
holders.add(holder);
}
}
return holders == null ? Collections.emptyList() : holders;
}
public ServletHolder getServlet(String name)
{
return _servletNameMap.get(name);

View File

@ -70,7 +70,6 @@ import org.eclipse.jetty.util.log.Logger;
@ManagedObject("Servlet Holder")
public class ServletHolder extends Holder<Servlet> implements UserIdentity.Scope, Comparable<ServletHolder>
{
/* ---------------------------------------------------------------- */
private static final Logger LOG = Log.getLogger(ServletHolder.class);
private int _initOrder = -1;
@ -89,11 +88,9 @@ public class ServletHolder extends Holder<Servlet> implements UserIdentity.Scope
private boolean _enabled = true;
private UnavailableException _unavailableEx;
public static final String APACHE_SENTINEL_CLASS = "org.apache.tomcat.InstanceManager";
public static final String JSP_GENERATED_PACKAGE_NAME = "org.eclipse.jetty.servlet.jspPackagePrefix";
public static final Map<String,String> NO_MAPPED_ROLES = Collections.emptyMap();
public static enum JspContainer {APACHE, OTHER};
public enum JspContainer {APACHE, OTHER}
/* ---------------------------------------------------------------- */
/** Constructor .
@ -1304,9 +1301,9 @@ public class ServletHolder extends Holder<Servlet> implements UserIdentity.Scope
try
{
ServletContext ctx = getServletHandler().getServletContext();
if (ctx instanceof ServletContextHandler.Context)
return ((ServletContextHandler.Context)ctx).createServlet(getHeldClass());
return getHeldClass().getDeclaredConstructor().newInstance();
if (ctx == null)
return getHeldClass().getDeclaredConstructor().newInstance();
return ctx.createServlet(getHeldClass());
}
catch (ServletException se)
{