From 5593e5b479291c36ab3703d37753663c90c4cce6 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Wed, 26 Sep 2018 11:27:57 +1000 Subject: [PATCH] Jetty 9.4.x 2903 delay listener instantiation (#2930) * Issue #2903 Ensure listeners not instantiated for quickstart gen Signed-off-by: Jan Bartel --- .../annotations/WebListenerAnnotation.java | 6 +- jetty-maven-plugin/pom.xml | 20 ++++ .../QuickStartDescriptorGenerator.java | 9 +- .../jetty/quickstart/TestQuickStart.java | 7 +- .../eclipse/jetty/servlet/ListenerHolder.java | 94 +++++++++++++++---- .../jetty/servlet/ServletContextHandler.java | 9 +- .../webapp/StandardDescriptorProcessor.java | 24 +---- 7 files changed, 118 insertions(+), 51 deletions(-) diff --git a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/WebListenerAnnotation.java b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/WebListenerAnnotation.java index 5478d5637ab..89f9ba4f704 100644 --- a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/WebListenerAnnotation.java +++ b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/WebListenerAnnotation.java @@ -81,12 +81,10 @@ public class WebListenerAnnotation extends DiscoveredAnnotation { MetaData metaData = _context.getMetaData(); if (metaData.getOrigin(clazz.getName()+".listener") == Origin.NotSet) - { - java.util.EventListener listener = (java.util.EventListener)_context.getServletContext().createInstance(clazz); + { ListenerHolder h = _context.getServletHandler().newListenerHolder(new Source(Source.Origin.ANNOTATION, clazz.getName())); - h.setListener(listener); + h.setHeldClass(clazz); _context.getServletHandler().addListener(h); - _context.addEventListener(listener); } } else diff --git a/jetty-maven-plugin/pom.xml b/jetty-maven-plugin/pom.xml index c4d02c88a29..d631db23eb0 100644 --- a/jetty-maven-plugin/pom.xml +++ b/jetty-maven-plugin/pom.xml @@ -185,6 +185,26 @@ jetty-server ${project.version} + + org.eclipse.jetty + jetty-servlet + ${project.version} + + + org.eclipse.jetty + jetty-client + ${project.version} + + + org.eclipse.jetty + jetty-http + ${project.version} + + + org.eclipse.jetty + jetty-io + ${project.version} + org.eclipse.jetty jetty-jmx diff --git a/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/QuickStartDescriptorGenerator.java b/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/QuickStartDescriptorGenerator.java index 9a27e1e36c2..896bc4f84bf 100644 --- a/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/QuickStartDescriptorGenerator.java +++ b/jetty-quickstart/src/main/java/org/eclipse/jetty/quickstart/QuickStartDescriptorGenerator.java @@ -51,6 +51,7 @@ import org.eclipse.jetty.security.authentication.FormAuthenticator; import org.eclipse.jetty.servlet.ErrorPageErrorHandler; import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.FilterMapping; +import org.eclipse.jetty.servlet.ListenerHolder; import org.eclipse.jetty.servlet.ServletContextHandler.JspConfig; import org.eclipse.jetty.servlet.ServletHandler; import org.eclipse.jetty.servlet.ServletHolder; @@ -176,10 +177,10 @@ public class QuickStartDescriptorGenerator .tag("param-value",_webApp.getInitParameter(p)) .closeTag(); - if (_webApp.getEventListeners() != null) - for (EventListener e : _webApp.getEventListeners()) - out.openTag("listener",origin(md,e.getClass().getCanonicalName() + ".listener")) - .tag("listener-class",e.getClass().getCanonicalName()) + if (_webApp.getServletHandler().getListeners() != null) + for (ListenerHolder e : _webApp.getServletHandler().getListeners()) + out.openTag("listener",origin(md,e.getClassName() + ".listener")) + .tag("listener-class",e.getClassName()) .closeTag(); ServletHandler servlets = _webApp.getServletHandler(); diff --git a/jetty-quickstart/src/test/java/org/eclipse/jetty/quickstart/TestQuickStart.java b/jetty-quickstart/src/test/java/org/eclipse/jetty/quickstart/TestQuickStart.java index 0636f18b921..df0a5aa91d1 100644 --- a/jetty-quickstart/src/test/java/org/eclipse/jetty/quickstart/TestQuickStart.java +++ b/jetty-quickstart/src/test/java/org/eclipse/jetty/quickstart/TestQuickStart.java @@ -24,6 +24,7 @@ import static org.junit.jupiter.api.Assertions.*; import java.io.File; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.servlet.ListenerHolder; import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; @@ -69,8 +70,10 @@ public class TestQuickStart ServletHolder fooHolder = new ServletHolder(); fooHolder.setServlet(new FooServlet()); fooHolder.setName("foo"); - quickstart.getServletHandler().addServlet(fooHolder); - quickstart.addEventListener(new FooContextListener()); + quickstart.getServletHandler().addServlet(fooHolder); + ListenerHolder lholder = new ListenerHolder(); + lholder.setListener(new FooContextListener()); + quickstart.getServletHandler().addListener(lholder); server.setHandler(quickstart); server.start(); server.stop(); diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java index d09e298e7e8..9e94f463f62 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java @@ -21,54 +21,112 @@ package org.eclipse.jetty.servlet; import java.util.EventListener; +import javax.servlet.ServletContext; +import javax.servlet.ServletException; + /** * ListenerHolder * - * Specialization of AbstractHolder for servlet listeners. This + * Specialization of BaseHolder for servlet listeners. This * allows us to record where the listener originated - web.xml, * annotation, api etc. */ public class ListenerHolder extends BaseHolder { private EventListener _listener; + private boolean _initialized = false; + public ListenerHolder () + { + this (Source.EMBEDDED); + } + public ListenerHolder(Source source) { super(source); } - - public void setListener(EventListener listener) - { - _listener = listener; - setClassName(listener.getClass().getName()); - setHeldClass(listener.getClass()); - _extInstance=true; - } - + public EventListener getListener() { return _listener; } + /** + * Set an explicit instance. In this case, + * just like ServletHolder and FilterHolder, + * the listener will not be introspected for + * annotations like Resource etc. + * + * @param listener + */ + public void setListener (EventListener listener) + { + _listener = listener; + _extInstance=true; + setHeldClass(_listener.getClass()); + setClassName(_listener.getClass().getName()); + } + + + public void initialize (ServletContext context) throws Exception + { + if (!_initialized) + { + initialize(); + + if (_listener == null) + { + //create an instance of the listener and decorate it + try + { + _listener = (context instanceof ServletContextHandler.Context) + ?((ServletContextHandler.Context)context).createListener(getHeldClass()) + :getHeldClass().getDeclaredConstructor().newInstance(); + + } + catch (ServletException se) + { + Throwable cause = se.getRootCause(); + if (cause instanceof InstantiationException) + throw (InstantiationException)cause; + if (cause instanceof IllegalAccessException) + throw (IllegalAccessException)cause; + throw se; + } + } + _initialized = true; + } + } + @Override public void doStart() throws Exception { - //Listeners always have an instance eagerly created, it cannot be deferred to the doStart method - if (_listener == null) - throw new IllegalStateException("No listener instance"); - super.doStart(); + if (!java.util.EventListener.class.isAssignableFrom(_class)) + { + String msg = _class+" is not a java.util.EventListener"; + super.stop(); + throw new IllegalStateException(msg); + } } + + @Override + public void doStop() throws Exception + { + super.doStop(); + if (!_extInstance) + _listener = null; + _initialized = false; + } + @Override public String toString() { - return super.toString()+(_listener == null?"":": "+getClassName()); - } - - + return super.toString()+": "+getClassName(); + } } diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java index ec0b6e853d2..8729025410b 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java @@ -356,13 +356,16 @@ public class ServletContextHandler extends ContextHandler if (_servletHandler != null) { - // Call decorators on all holders, and also on any EventListeners before - // decorators are called on any other classes (like servlets and filters) + //Ensure listener instances are created, added to ContextHandler if(_servletHandler.getListeners() != null) { for (ListenerHolder holder:_servletHandler.getListeners()) { - _objFactory.decorate(holder.getListener()); + holder.start(); + //we need to pass in the context because the ServletHandler has not + //yet got a reference to the ServletContext (happens in super.startContext) + holder.initialize(_scontext); + addEventListener(holder.getListener()); } } } diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/StandardDescriptorProcessor.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/StandardDescriptorProcessor.java index cae3782b160..fdfaede611b 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/StandardDescriptorProcessor.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/StandardDescriptorProcessor.java @@ -1911,17 +1911,11 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor } ((WebDescriptor)descriptor).addClassName(className); - - Class listenerClass = (Class)context.loadClass(className); - listener = newListenerInstance(context,listenerClass, descriptor); - if (!(listener instanceof EventListener)) - { - LOG.warn("Not an EventListener: " + listener); - return; - } - context.addEventListener(listener); + + ListenerHolder h = context.getServletHandler().newListenerHolder(new Source (Source.Origin.DESCRIPTOR, descriptor.getResource().toString())); + h.setClassName(className); + context.getServletHandler().addListener(h); context.getMetaData().setOrigin(className+".listener", descriptor); - } } catch (Exception e) @@ -1960,14 +1954,4 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor ((ConstraintAware)context.getSecurityHandler()).setDenyUncoveredHttpMethods(true); } - - public EventListener newListenerInstance(WebAppContext context,Class clazz, Descriptor descriptor) throws Exception - { - ListenerHolder h = context.getServletHandler().newListenerHolder(new Source (Source.Origin.DESCRIPTOR, descriptor.getResource().toString())); - EventListener l = context.getServletContext().createInstance(clazz); - h.setListener(l); - context.getServletHandler().addListener(h); - return l; - - } }