From 6a1bcea0da78feef4067a1c7c6c2cc01b97a550d Mon Sep 17 00:00:00 2001 From: gregw Date: Tue, 22 Dec 2020 14:33:30 +0100 Subject: [PATCH] Fix #5835 Durable Filters, Servlets and Listeners Filters, Servlets and Listeners are only durable if added prior to starting. --- .../eclipse/jetty/servlet/ServletHandler.java | 63 +++++++++---------- .../jetty/servlet/ServletHandlerTest.java | 62 ++++++++++++++++++ 2 files changed, 92 insertions(+), 33 deletions(-) diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java index 4f29e4e76d83..bbf872bbdc1e 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java @@ -26,7 +26,6 @@ import java.util.EventListener; import java.util.HashMap; import java.util.List; -import java.util.ListIterator; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -111,6 +110,7 @@ public class ServletHandler extends ScopedHandler private List _filterPathMappings; private MultiMap _filterNameMappings; private List _wildFilterNameMappings; + private final List> _durable = new ArrayList<>(); private final Map _servletNameMap = new HashMap<>(); private PathMappings _servletPathMap; @@ -160,6 +160,11 @@ protected synchronized void doStart() _identityService = securityHandler.getIdentityService(); } + _durable.clear(); + _durable.addAll(Arrays.asList(getFilters())); + _durable.addAll(Arrays.asList(getServlets())); + _durable.addAll(Arrays.asList(getListeners())); + updateNameMappings(); updateMappings(); @@ -234,35 +239,31 @@ protected synchronized void doStop() List filterMappings = ArrayUtil.asMutableList(_filterMappings); if (_filters != null) { - for (int i = _filters.length; i-- > 0; ) + for (FilterHolder f : _filters) { try { - _filters[i].stop(); + f.stop(); } catch (Exception e) { LOG.warn(Log.EXCEPTION, e); } - if (_filters[i].getSource() != Source.EMBEDDED) + if (_durable.contains(f)) + { + filterHolders.add(f); //only retain embedded + } + else { //remove all of the mappings that were for non-embedded filters - _filterNameMap.remove(_filters[i].getName()); + _filterNameMap.remove(f.getName()); //remove any mappings associated with this filter - ListIterator fmitor = filterMappings.listIterator(); - while (fmitor.hasNext()) - { - FilterMapping fm = fmitor.next(); - if (fm.getFilterName().equals(_filters[i].getName())) - fmitor.remove(); - } + filterMappings.removeIf(fm -> fm.getFilterName().equals(f.getName())); } - else - filterHolders.add(_filters[i]); //only retain embedded } } - //Retain only filters and mappings that were added using jetty api (ie Source.EMBEDDED) + //Retain only filters and mappings that were added using jetty api prior to starting FilterHolder[] fhs = filterHolders.toArray(new FilterHolder[0]); updateBeans(_filters, fhs); _filters = fhs; @@ -278,36 +279,32 @@ protected synchronized void doStop() List servletMappings = ArrayUtil.asMutableList(_servletMappings); //will be remaining mappings if (_servlets != null) { - for (int i = _servlets.length; i-- > 0; ) + for (ServletHolder s : _servlets) { try { - _servlets[i].stop(); + s.stop(); } catch (Exception e) { LOG.warn(Log.EXCEPTION, e); } - if (_servlets[i].getSource() != Source.EMBEDDED) + if (_durable.contains(s)) + { + servletHolders.add(s); //only retain embedded + } + else { //remove from servlet name map - _servletNameMap.remove(_servlets[i].getName()); + _servletNameMap.remove(s.getName()); //remove any mappings associated with this servlet - ListIterator smitor = servletMappings.listIterator(); - while (smitor.hasNext()) - { - ServletMapping sm = smitor.next(); - if (sm.getServletName().equals(_servlets[i].getName())) - smitor.remove(); - } + servletMappings.removeIf(sm -> sm.getServletName().equals(s.getName())); } - else - servletHolders.add(_servlets[i]); //only retain embedded } } - //Retain only Servlets and mappings added via jetty apis (ie Source.EMBEDDED) + // Retain only Servlets and mappings added via jetty apis prior to starting ServletHolder[] shs = servletHolders.toArray(new ServletHolder[0]); updateBeans(_servlets, shs); _servlets = shs; @@ -322,18 +319,18 @@ protected synchronized void doStop() List listenerHolders = new ArrayList<>(); if (_listeners != null) { - for (int i = _listeners.length; i-- > 0; ) + for (ListenerHolder l : _listeners) { try { - _listeners[i].stop(); + l.stop(); } catch (Exception e) { LOG.warn(Log.EXCEPTION, e); } - if (_listeners[i].getSource() == Source.EMBEDDED) - listenerHolders.add(_listeners[i]); + if (_durable.contains(l)) + listenerHolders.add(l); } } ListenerHolder[] listeners = listenerHolders.toArray(new ListenerHolder[0]); diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletHandlerTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletHandlerTest.java index 5bddd101f309..5638e625b6e7 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletHandlerTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletHandlerTest.java @@ -20,11 +20,14 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.EnumSet; import java.util.List; import javax.servlet.DispatcherType; import javax.servlet.Filter; import javax.servlet.FilterConfig; +import javax.servlet.ServletContextEvent; +import javax.servlet.ServletContextListener; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -35,11 +38,13 @@ import org.eclipse.jetty.http.pathmap.MappedResource; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.eclipse.jetty.util.component.Container; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -836,6 +841,63 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se assertThat(connector.getResponse("GET /other.bob HTTP/1.0\r\n\r\n"), containsString("path-/*-path-*.bob-default")); } + @Test + public void testDurableEmbedded() throws Exception + { + Server server = new Server(); + ServletContextHandler context = new ServletContextHandler(); + server.setHandler(context); + ServletHandler handler = context.getServletHandler(); + + FilterHolder durableFilter = new FilterHolder((TestFilter)(req,res,c) -> c.doFilter(req, res)); + ServletHolder durableServlet = new ServletHolder(new HttpServlet(){}); + ListenerHolder durableListener = new ListenerHolder(MyServletListener.class); + + FilterHolder transientFilter = new FilterHolder((TestFilter)(req,res,c) -> c.doFilter(req, res)); + ServletHolder transientServlet = new ServletHolder(new HttpServlet(){}); + ListenerHolder transientListener = new ListenerHolder(MyServletListener.class); + + handler.addFilter(durableFilter); + handler.addServletWithMapping(durableServlet, "/"); + handler.addListener(durableListener); + + context.addBean(new AbstractLifeCycle() + { + @Override + protected void doStart() throws Exception + { + handler.addFilter(transientFilter); + handler.addServlet(transientServlet); + handler.addListener(transientListener); + } + }); + + server.start(); + + assertThat(Arrays.asList(handler.getFilters()), containsInAnyOrder(durableFilter, transientFilter)); + assertThat(Arrays.asList(handler.getServlets()), containsInAnyOrder(durableServlet, transientServlet)); + assertThat(Arrays.asList(handler.getListeners()), containsInAnyOrder(durableListener, transientListener)); + + server.stop(); + + assertThat(Arrays.asList(handler.getFilters()), containsInAnyOrder(durableFilter)); + assertThat(Arrays.asList(handler.getServlets()), containsInAnyOrder(durableServlet)); + assertThat(Arrays.asList(handler.getListeners()), containsInAnyOrder(durableListener)); + } + + public static class MyServletListener implements ServletContextListener + { + @Override + public void contextInitialized(ServletContextEvent sce) + { + } + + @Override + public void contextDestroyed(ServletContextEvent sce) + { + } + } + private interface TestFilter extends Filter { default void init(FilterConfig filterConfig) throws ServletException