From c59de808f1edaa3ac26650680ec741aaa66a6e0b Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 22 Mar 2021 18:28:25 +0800 Subject: [PATCH] Fix #5835 Durable filters and servlets (#6027) Fix #5835 Durable filters and servlets with a general ServletHandler cleanup update indexes after updating mapping update mappings/indexes before destroyed listeners Signed-off-by: Greg Wilkins --- .../jetty/server/handler/ContextHandler.java | 45 +- .../server/handler/ContextHandlerTest.java | 67 ++ .../eclipse/jetty/servlet/ServletHandler.java | 581 ++++++++---------- .../jetty/servlet/DefaultServletTest.java | 2 + .../jetty/servlet/ServletHandlerTest.java | 317 ++++++---- .../util/component/ContainerLifeCycle.java | 47 +- .../util/component/DumpableCollection.java | 5 + 7 files changed, 566 insertions(+), 498 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 2a5391f0cc9..ef4eb96ab6a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -651,7 +651,24 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu } if (listener instanceof ServletContextListener) + { + if (_contextStatus == ContextStatus.INITIALIZED) + { + ServletContextListener scl = (ServletContextListener)listener; + _destroyServletContextListeners.add(scl); + if (isStarting()) + { + LOG.warn("ContextListener {} added whilst starting {}", scl, this); + callContextInitialized(scl, new ServletContextEvent(_scontext)); + } + else + { + LOG.warn("ContextListener {} added after starting {}", scl, this); + } + } + _servletContextListeners.add((ServletContextListener)listener); + } if (listener instanceof ServletContextAttributeListener) _servletContextAttributeListeners.add((ServletContextAttributeListener)listener); @@ -933,31 +950,19 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu public void contextInitialized() throws Exception { // Call context listeners - switch (_contextStatus) + if (_contextStatus == ContextStatus.NOTSET) { - case NOTSET: + _contextStatus = ContextStatus.INITIALIZED; + _destroyServletContextListeners.clear(); + if (!_servletContextListeners.isEmpty()) { - try + ServletContextEvent event = new ServletContextEvent(_scontext); + for (ServletContextListener listener : _servletContextListeners) { - _destroyServletContextListeners.clear(); - if (!_servletContextListeners.isEmpty()) - { - ServletContextEvent event = new ServletContextEvent(_scontext); - for (ServletContextListener listener : _servletContextListeners) - { - callContextInitialized(listener, event); - _destroyServletContextListeners.add(listener); - } - } + callContextInitialized(listener, event); + _destroyServletContextListeners.add(listener); } - finally - { - _contextStatus = ContextStatus.INITIALIZED; - } - break; } - default: - break; } } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java index 86756cc3f5b..0de5ac136fc 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java @@ -22,6 +22,7 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; import javax.servlet.ServletContextEvent; import javax.servlet.ServletContextListener; import javax.servlet.http.HttpServletRequest; @@ -39,6 +40,7 @@ import org.eclipse.jetty.util.resource.Resource; import org.hamcrest.Matchers; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.slf4j.LoggerFactory; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; @@ -778,6 +780,71 @@ public class ContextHandlerTest assertThat("classpath", classpath, containsString(jar.toString())); } + @Test + public void testNonDurableContextListener() throws Exception + { + Server server = new Server(); + ContextHandler context = new ContextHandler(); + server.setHandler(context); + AtomicInteger initialized = new AtomicInteger(); + AtomicInteger destroyed = new AtomicInteger(); + + context.addEventListener(new ServletContextListener() + { + @Override + public void contextInitialized(ServletContextEvent sce) + { + initialized.incrementAndGet(); + context.addEventListener(new ServletContextListener() + { + @Override + public void contextInitialized(ServletContextEvent sce) + { + initialized.incrementAndGet(); + } + + @Override + public void contextDestroyed(ServletContextEvent sce) + { + destroyed.incrementAndGet(); + } + }); + } + + @Override + public void contextDestroyed(ServletContextEvent sce) + { + destroyed.incrementAndGet(); + } + }); + + LoggerFactory.getLogger(ContextHandler.class).info("Expect WARN ContextListener ... add whilst starting ..."); + server.start(); + assertThat(initialized.get(), is(2)); + + LoggerFactory.getLogger(ContextHandler.class).info("Expect WARN ContextListener ... add after starting ..."); + context.addEventListener(new ServletContextListener() + { + @Override + public void contextInitialized(ServletContextEvent sce) + { + // This should not get called because added after started + initialized.incrementAndGet(); + } + + @Override + public void contextDestroyed(ServletContextEvent sce) + { + destroyed.incrementAndGet(); + } + }); + + assertThat(initialized.get(), is(2)); + + server.stop(); + assertThat(destroyed.get(), is(3)); + } + private void checkResourcePathsForExampleWebApp(String root) throws IOException { File testDirectory = setupTestDirectory(); 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 81551fbcff9..bdf2c962799 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 @@ -16,6 +16,7 @@ package org.eclipse.jetty.servlet; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.EnumSet; import java.util.EventListener; @@ -27,6 +28,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.function.Consumer; +import java.util.stream.Collectors; import java.util.stream.Stream; import javax.servlet.DispatcherType; import javax.servlet.Filter; @@ -87,8 +89,8 @@ public class ServletHandler extends ScopedHandler private final AutoLock _lock = new AutoLock(); private ServletContextHandler _contextHandler; private ServletContext _servletContext; - private FilterHolder[] _filters = new FilterHolder[0]; - private FilterMapping[] _filterMappings; + private final List _filters = new ArrayList<>(); + private final List _filterMappings = new ArrayList<>(); private int _matchBeforeIndex = -1; //index of last programmatic FilterMapping with isMatchAfter=false private int _matchAfterIndex = -1; //index of 1st programmatic FilterMapping with isMatchAfter=true private boolean _filterChainsCached = true; @@ -98,17 +100,18 @@ public class ServletHandler extends ScopedHandler private IdentityService _identityService; private boolean _allowDuplicateMappings = false; - private ServletHolder[] _servlets = new ServletHolder[0]; - private ServletMapping[] _servletMappings; + private final List _servlets = new ArrayList<>(); + private final List _servletMappings = new ArrayList<>(); private final Map _filterNameMap = new HashMap<>(); private List _filterPathMappings; private MultiMap _filterNameMappings; private List _wildFilterNameMappings; + private final List> _durable = new ArrayList<>(); private final Map _servletNameMap = new HashMap<>(); private PathMappings _servletPathMap; - private ListenerHolder[] _listeners = new ListenerHolder[0]; + private final List _listeners = new ArrayList<>(); private boolean _initialized = false; @SuppressWarnings("unchecked") @@ -126,6 +129,13 @@ public class ServletHandler extends ScopedHandler return _lock.lock(); } + private void updateAndSet(Collection target, Collection values) + { + updateBeans(target, values); + target.clear(); + target.addAll(values); + } + @Override public boolean isDumpable(Object o) { @@ -136,17 +146,18 @@ public class ServletHandler extends ScopedHandler public void dump(Appendable out, String indent) throws IOException { dumpObjects(out, indent, - DumpableCollection.fromArray("listeners " + this, _listeners), - DumpableCollection.fromArray("filters " + this, _filters), - DumpableCollection.fromArray("filterMappings " + this, _filterMappings), - DumpableCollection.fromArray("servlets " + this, _servlets), - DumpableCollection.fromArray("servletMappings " + this, _servletMappings)); + DumpableCollection.from("listeners " + this, _listeners), + DumpableCollection.from("filters " + this, _filters), + DumpableCollection.from("filterMappings " + this, _filterMappings), + DumpableCollection.from("servlets " + this, _servlets), + DumpableCollection.from("servletMappings " + this, _servletMappings), + DumpableCollection.from("durable " + this, _durable)); } @Override protected void doStart() throws Exception { - try (AutoLock l = lock()) + try (AutoLock ignored = lock()) { ContextHandler.Context context = ContextHandler.getCurrentContext(); _servletContext = context == null ? new ContextHandler.StaticContext() : context; @@ -159,6 +170,11 @@ public class ServletHandler extends ScopedHandler _identityService = securityHandler.getIdentityService(); } + _durable.clear(); + _durable.addAll(Arrays.asList(getFilters())); + _durable.addAll(Arrays.asList(getServlets())); + _durable.addAll(Arrays.asList(getListeners())); + updateNameMappings(); updateMappings(); @@ -226,113 +242,93 @@ public class ServletHandler extends ScopedHandler @Override protected void doStop() throws Exception { - try (AutoLock l = lock()) + try (AutoLock ignored = lock()) { super.doStop(); // Stop filters List filterHolders = new ArrayList<>(); - List filterMappings = ArrayUtil.asMutableList(_filterMappings); - if (_filters != null) + for (int i = _filters.size(); i-- > 0; ) { - for (int i = _filters.length; i-- > 0; ) + FilterHolder filter = _filters.get(i); + try { - FilterHolder filter = _filters[i]; - try - { - filter.stop(); - } - catch (Exception e) - { - LOG.warn("Unable to stop filter {}", filter, e); - } - if (filter.getSource() != Source.EMBEDDED) - { - //remove all of the mappings that were for non-embedded filters - _filterNameMap.remove(filter.getName()); - //remove any mappings associated with this filter - filterMappings.removeIf(fm -> fm.getFilterName().equals(filter.getName())); - } - else - filterHolders.add(filter); //only retain embedded + filter.stop(); + } + catch (Exception e) + { + LOG.warn("Unable to stop filter {}", filter, e); + } + if (_durable.contains(filter)) + { + filterHolders.add(filter); //only retain durable } } - //Retain only filters and mappings that were added using jetty api (ie Source.EMBEDDED) - FilterHolder[] fhs = filterHolders.toArray(new FilterHolder[0]); - updateBeans(_filters, fhs); - _filters = fhs; - FilterMapping[] fms = filterMappings.toArray(new FilterMapping[0]); - updateBeans(_filterMappings, fms); - _filterMappings = fms; - - _matchAfterIndex = (_filterMappings.length == 0 ? -1 : _filterMappings.length - 1); - _matchBeforeIndex = -1; + //Retain only durable filters + updateBeans(_filters, filterHolders); + _filters.clear(); + _filters.addAll(filterHolders); // Stop servlets List servletHolders = new ArrayList<>(); //will be remaining servlets - List servletMappings = ArrayUtil.asMutableList(_servletMappings); //will be remaining mappings - if (_servlets != null) + for (int i = _servlets.size(); i-- > 0; ) { - for (int i = _servlets.length; i-- > 0; ) + ServletHolder servlet = _servlets.get(i); + try { - ServletHolder servlet = _servlets[i]; - try - { - servlet.stop(); - } - catch (Exception e) - { - LOG.warn("Unable to stop servlet {}", servlet, e); - } + servlet.stop(); + } + catch (Exception e) + { + LOG.warn("Unable to stop servlet {}", servlet, e); + } - if (servlet.getSource() != Source.EMBEDDED) - { - //remove from servlet name map - _servletNameMap.remove(servlet.getName()); - //remove any mappings associated with this servlet - servletMappings.removeIf(sm -> sm.getServletName().equals(servlet.getName())); - } - else - servletHolders.add(servlet); //only retain embedded + if (_durable.contains(servlet)) + { + servletHolders.add(servlet); //only retain embedded } } - //Retain only Servlets and mappings added via jetty apis (ie Source.EMBEDDED) - ServletHolder[] shs = servletHolders.toArray(new ServletHolder[0]); - updateBeans(_servlets, shs); - _servlets = shs; - ServletMapping[] sms = servletMappings.toArray(new ServletMapping[0]); - updateBeans(_servletMappings, sms); - _servletMappings = sms; + //Retain only durable Servlets + updateBeans(_servlets, servletHolders); + _servlets.clear(); + _servlets.addAll(servletHolders); + + updateNameMappings(); + updateAndSet(_servletMappings, _servletMappings.stream().filter(m -> _servletNameMap.containsKey(m.getServletName())).collect(Collectors.toList())); + updateAndSet(_filterMappings, _filterMappings.stream().filter(m -> _filterNameMap.containsKey(m.getFilterName())).collect(Collectors.toList())); + updateMappings(); if (_contextHandler != null) _contextHandler.contextDestroyed(); //Retain only Listeners added via jetty apis (is Source.EMBEDDED) List listenerHolders = new ArrayList<>(); - if (_listeners != null) + for (int i = _listeners.size(); i-- > 0; ) { - for (int i = _listeners.length; i-- > 0; ) + ListenerHolder listener = _listeners.get(i); + try { - ListenerHolder listener = _listeners[i]; - try - { - listener.stop(); - } - catch (Exception e) - { - LOG.warn("Unable to stop listener {}", listener, e); - } - if (listener.getSource() == Source.EMBEDDED) - listenerHolders.add(listener); + listener.stop(); } + catch (Exception e) + { + LOG.warn("Unable to stop listener {}", listener, e); + } + if (_durable.contains(listener)) + listenerHolders.add(listener); } - ListenerHolder[] listeners = listenerHolders.toArray(new ListenerHolder[0]); - updateBeans(_listeners, listeners); - _listeners = listeners; - //will be regenerated on next start + updateBeans(_listeners, listenerHolders); + _listeners.clear(); + _listeners.addAll(listenerHolders); + + // Update indexes for prepending filters + _matchAfterIndex = (_filterMappings.size() == 0 ? -1 : _filterMappings.size() - 1); + _matchBeforeIndex = -1; + + _durable.clear(); _filterPathMappings = null; _filterNameMappings = null; _servletPathMap = null; @@ -348,13 +344,13 @@ public class ServletHandler extends ScopedHandler @ManagedAttribute(value = "filters", readonly = true) public FilterMapping[] getFilterMappings() { - return _filterMappings; + return _filterMappings.toArray(new FilterMapping[0]); } @ManagedAttribute(value = "filters", readonly = true) public FilterHolder[] getFilters() { - return _filters; + return _filters.toArray(new FilterHolder[0]); } public ServletContext getServletContext() @@ -370,7 +366,7 @@ public class ServletHandler extends ScopedHandler @ManagedAttribute(value = "mappings of servlets", readonly = true) public ServletMapping[] getServletMappings() { - return _servletMappings; + return _servletMappings.toArray(new ServletMapping[0]); } /** @@ -381,13 +377,13 @@ public class ServletHandler extends ScopedHandler */ public ServletMapping getServletMapping(String pathSpec) { - if (pathSpec == null || _servletMappings == null) + if (pathSpec == null) return null; ServletMapping mapping = null; - for (int i = 0; i < _servletMappings.length && mapping == null; i++) + for (int i = 0; i < _servletMappings.size() && mapping == null; i++) { - ServletMapping m = _servletMappings[i]; + ServletMapping m = _servletMappings.get(i); if (m.getPathSpecs() != null) { for (String p : m.getPathSpecs()) @@ -406,7 +402,7 @@ public class ServletHandler extends ScopedHandler @ManagedAttribute(value = "servlets", readonly = true) public ServletHolder[] getServlets() { - return _servlets; + return _servlets.toArray(new ServletHolder[0]); } public List getServlets(Class clazz) @@ -483,7 +479,7 @@ public class ServletHandler extends ScopedHandler FilterChain chain = null; // find the servlet - if (servletHolder != null && _filterMappings != null && _filterMappings.length > 0) + if (servletHolder != null && _filterMappings.size() > 0) chain = getFilterChain(baseRequest, target.startsWith("/") ? target : null, servletHolder); if (LOG.isDebugEnabled()) @@ -702,7 +698,7 @@ public class ServletHandler extends ScopedHandler }; //Start the listeners so we can call them - Arrays.stream(_listeners).forEach(c); + _listeners.forEach(c); //call listeners contextInitialized if (_contextHandler != null) @@ -713,8 +709,8 @@ public class ServletHandler extends ScopedHandler //Start the filters then the servlets Stream.concat( - Arrays.stream(_filters), - Arrays.stream(_servlets).sorted()) + _filters.stream(), + _servlets.stream().sorted()) .forEach(c); mx.ifExceptionThrow(); @@ -728,7 +724,7 @@ public class ServletHandler extends ScopedHandler return _initialized; } - protected void initializeHolders(BaseHolder[] holders) + protected void initializeHolders(Collection> holders) { for (BaseHolder holder : holders) { @@ -772,15 +768,16 @@ public class ServletHandler extends ScopedHandler public ListenerHolder[] getListeners() { - return _listeners; + return _listeners.toArray(new ListenerHolder[0]); } - public void setListeners(ListenerHolder[] listeners) + public void setListeners(ListenerHolder[] holders) { - if (listeners != null) - initializeHolders(listeners); + List listeners = holders == null ? Collections.emptyList() : Arrays.asList(holders); + initializeHolders(listeners); updateBeans(_listeners, listeners); - _listeners = listeners; + _listeners.clear(); + _listeners.addAll(listeners); } public ListenerHolder newListenerHolder(Source source) @@ -842,7 +839,7 @@ public class ServletHandler extends ScopedHandler ServletHolder[] holders = getServlets(); try { - try (AutoLock l = lock()) + try (AutoLock ignored = lock()) { if (!containsServletHolder(servlet)) setServlets(ArrayUtil.addToArray(holders, servlet, ServletHolder.class)); @@ -870,7 +867,7 @@ public class ServletHandler extends ScopedHandler if (holder == null) return; - try (AutoLock l = lock()) + try (AutoLock ignored = lock()) { if (!containsServletHolder(holder)) setServlets(ArrayUtil.addToArray(getServlets(), holder, ServletHolder.class)); @@ -954,7 +951,7 @@ public class ServletHandler extends ScopedHandler try { - try (AutoLock l = lock()) + try (AutoLock ignored = lock()) { if (!containsFilterHolder(holder)) setFilters(ArrayUtil.addToArray(holders, holder, FilterHolder.class)); @@ -1023,7 +1020,7 @@ public class ServletHandler extends ScopedHandler try { - try (AutoLock l = lock()) + try (AutoLock ignored = lock()) { if (!containsFilterHolder(holder)) setFilters(ArrayUtil.addToArray(holders, holder, FilterHolder.class)); @@ -1052,7 +1049,7 @@ public class ServletHandler extends ScopedHandler { if (filter != null) { - try (AutoLock l = lock()) + try (AutoLock ignored = lock()) { if (!containsFilterHolder(filter)) setFilters(ArrayUtil.addToArray(getFilters(), filter, FilterHolder.class)); @@ -1072,7 +1069,7 @@ public class ServletHandler extends ScopedHandler if (filter == null) return; - try (AutoLock l = lock()) + try (AutoLock ignored = lock()) { if (!containsFilterHolder(filter)) setFilters(ArrayUtil.addToArray(getFilters(), filter, FilterHolder.class)); @@ -1089,7 +1086,7 @@ public class ServletHandler extends ScopedHandler if (filter == null) return; - try (AutoLock l = lock()) + try (AutoLock ignored = lock()) { if (!containsFilterHolder(filter)) setFilters(ArrayUtil.prependToArray(filter, getFilters(), FilterHolder.class)); @@ -1103,24 +1100,27 @@ public class ServletHandler extends ScopedHandler */ public void addFilterMapping(FilterMapping mapping) { - if (mapping != null) + if (mapping == null) + return; + + try (AutoLock ignored = lock()) { Source source = (mapping.getFilterHolder() == null ? null : mapping.getFilterHolder().getSource()); - FilterMapping[] mappings = getFilterMappings(); - if (mappings == null || mappings.length == 0) + + if (_filterMappings.isEmpty()) { - setFilterMappings(insertFilterMapping(mapping, 0, false)); + _filterMappings.add(mapping); if (source == Source.JAVAX_API) _matchAfterIndex = 0; } else { //there are existing entries. If this is a programmatic filtermapping, it is added at the end of the list. - //If this is a normal filtermapping, it is inserted after all the other filtermappings (matchBefores and normals), + //If this is a normal filtermapping, it is inserted after all the other filtermappings (matchBefores and normals), //but before the first matchAfter filtermapping. if (Source.JAVAX_API == source) { - setFilterMappings(insertFilterMapping(mapping, mappings.length - 1, false)); + _filterMappings.add(mapping); if (_matchAfterIndex < 0) _matchAfterIndex = getFilterMappings().length - 1; } @@ -1128,15 +1128,15 @@ public class ServletHandler extends ScopedHandler { //insert non-programmatic filter mappings before any matchAfters, if any if (_matchAfterIndex < 0) - setFilterMappings(insertFilterMapping(mapping, mappings.length - 1, false)); + _filterMappings.add(mapping); else - { - FilterMapping[] newMappings = insertFilterMapping(mapping, _matchAfterIndex, true); - ++_matchAfterIndex; - setFilterMappings(newMappings); - } + _filterMappings.add(_matchAfterIndex++, mapping); } } + addBean(mapping); + if (isRunning()) + updateMappings(); + invalidateChainsCache(); } } @@ -1147,13 +1147,15 @@ public class ServletHandler extends ScopedHandler */ public void prependFilterMapping(FilterMapping mapping) { - if (mapping != null) + if (mapping == null) + return; + + try (AutoLock ignored = lock()) { Source source = (mapping.getFilterHolder() == null ? null : mapping.getFilterHolder().getSource()); - FilterMapping[] mappings = getFilterMappings(); - if (mappings == null || mappings.length == 0) + if (_filterMappings.isEmpty()) { - setFilterMappings(insertFilterMapping(mapping, 0, false)); + _filterMappings.add(mapping); if (Source.JAVAX_API == source) _matchBeforeIndex = 0; } @@ -1169,235 +1171,168 @@ public class ServletHandler extends ScopedHandler { //no programmatically defined prepended filter mappings yet, prepend this one _matchBeforeIndex = 0; - FilterMapping[] newMappings = insertFilterMapping(mapping, 0, true); - setFilterMappings(newMappings); + _filterMappings.add(0, mapping); } else { - FilterMapping[] newMappings = insertFilterMapping(mapping, _matchBeforeIndex, false); - ++_matchBeforeIndex; - setFilterMappings(newMappings); + _filterMappings.add(1 + _matchBeforeIndex++, mapping); } } else { //non programmatically defined, just prepend to list - FilterMapping[] newMappings = insertFilterMapping(mapping, 0, true); - setFilterMappings(newMappings); + _filterMappings.add(0, mapping); } //adjust matchAfterIndex ptr to take account of the mapping we just prepended if (_matchAfterIndex >= 0) ++_matchAfterIndex; } + addBean(mapping); + if (isRunning()) + updateMappings(); + invalidateChainsCache(); } } - /** - * Insert a filtermapping in the list - * - * @param mapping the FilterMapping to add - * @param pos the position in the existing arry at which to add it - * @param before if true, insert before pos, if false insert after it - * @return the new FilterMappings post-insert - */ - protected FilterMapping[] insertFilterMapping(FilterMapping mapping, int pos, boolean before) - { - if (pos < 0) - throw new IllegalArgumentException("FilterMapping insertion pos < 0"); - FilterMapping[] mappings = getFilterMappings(); - - if (mappings == null || mappings.length == 0) - { - return new FilterMapping[]{mapping}; - } - FilterMapping[] newMappings = new FilterMapping[mappings.length + 1]; - - if (before) - { - //copy existing filter mappings up to but not including the pos - System.arraycopy(mappings, 0, newMappings, 0, pos); - - //add in the new mapping - newMappings[pos] = mapping; - - //copy the old pos mapping and any remaining existing mappings - System.arraycopy(mappings, pos, newMappings, pos + 1, mappings.length - pos); - } - else - { - //copy existing filter mappings up to and including the pos - System.arraycopy(mappings, 0, newMappings, 0, pos + 1); - //add in the new mapping after the pos - newMappings[pos + 1] = mapping; - - //copy the remaining existing mappings - if (mappings.length > pos + 1) - System.arraycopy(mappings, pos + 1, newMappings, pos + 2, mappings.length - (pos + 1)); - } - return newMappings; - } - protected void updateNameMappings() { - try (AutoLock l = lock()) + try (AutoLock ignored = lock()) { // update filter name map _filterNameMap.clear(); - if (_filters != null) + for (FilterHolder filter : _filters) { - for (FilterHolder filter : _filters) - { - _filterNameMap.put(filter.getName(), filter); - filter.setServletHandler(this); - } + _filterNameMap.put(filter.getName(), filter); + filter.setServletHandler(this); } // Map servlet names to holders _servletNameMap.clear(); - if (_servlets != null) + // update the maps + for (ServletHolder servlet : _servlets) { - // update the maps - for (ServletHolder servlet : _servlets) - { - _servletNameMap.put(servlet.getName(), new MappedServlet(null, servlet)); - servlet.setServletHandler(this); - } + _servletNameMap.put(servlet.getName(), new MappedServlet(null, servlet)); + servlet.setServletHandler(this); } } } protected void updateMappings() { - try (AutoLock l = lock()) + try (AutoLock ignored = lock()) { // update filter mappings - if (_filterMappings == null) + _filterPathMappings = new ArrayList<>(); + _filterNameMappings = new MultiMap<>(); + for (FilterMapping filtermapping : _filterMappings) { - _filterPathMappings = null; - _filterNameMappings = null; - _wildFilterNameMappings = Collections.emptyList(); - } - else - { - _filterPathMappings = new ArrayList<>(); - _filterNameMappings = new MultiMap<>(); - for (FilterMapping filtermapping : _filterMappings) - { - FilterHolder filterHolder = _filterNameMap.get(filtermapping.getFilterName()); - if (filterHolder == null) - throw new IllegalStateException("No filter named " + filtermapping.getFilterName()); - filtermapping.setFilterHolder(filterHolder); - if (filtermapping.getPathSpecs() != null) - _filterPathMappings.add(filtermapping); + FilterHolder filterHolder = _filterNameMap.get(filtermapping.getFilterName()); + if (filterHolder == null) + throw new IllegalStateException("No filter named " + filtermapping.getFilterName()); + filtermapping.setFilterHolder(filterHolder); + if (filtermapping.getPathSpecs() != null) + _filterPathMappings.add(filtermapping); - if (filtermapping.getServletNames() != null) + if (filtermapping.getServletNames() != null) + { + String[] names = filtermapping.getServletNames(); + for (String name : names) { - String[] names = filtermapping.getServletNames(); - for (String name : names) - { - if (name != null) - _filterNameMappings.add(name, filtermapping); - } + if (name != null) + _filterNameMappings.add(name, filtermapping); } } - // Reverse filter mappings to apply as wrappers last filter wrapped first - for (Map.Entry> entry : _filterNameMappings.entrySet()) - Collections.reverse(entry.getValue()); - Collections.reverse(_filterPathMappings); - _wildFilterNameMappings = _filterNameMappings.get("*"); - if (_wildFilterNameMappings != null) - Collections.reverse(_wildFilterNameMappings); } + // Reverse filter mappings to apply as wrappers last filter wrapped first + for (Map.Entry> entry : _filterNameMappings.entrySet()) + Collections.reverse(entry.getValue()); + Collections.reverse(_filterPathMappings); + _wildFilterNameMappings = _filterNameMappings.get("*"); + if (_wildFilterNameMappings != null) + Collections.reverse(_wildFilterNameMappings); // Map servlet paths to holders - if (_servletMappings == null) - { - _servletPathMap = null; - } - else - { - PathMappings pm = new PathMappings<>(); + PathMappings pm = new PathMappings<>(); - //create a map of paths to set of ServletMappings that define that mapping - HashMap> sms = new HashMap<>(); - for (ServletMapping servletMapping : _servletMappings) + //create a map of paths to set of ServletMappings that define that mapping + HashMap> sms = new HashMap<>(); + for (ServletMapping servletMapping : _servletMappings) + { + String[] pathSpecs = servletMapping.getPathSpecs(); + if (pathSpecs != null) { - String[] pathSpecs = servletMapping.getPathSpecs(); - if (pathSpecs != null) + for (String pathSpec : pathSpecs) { - for (String pathSpec : pathSpecs) - { - List mappings = sms.computeIfAbsent(pathSpec, k -> new ArrayList<>()); - mappings.add(servletMapping); - } + List mappings = sms.computeIfAbsent(pathSpec, k -> new ArrayList<>()); + mappings.add(servletMapping); } } + } - //evaluate path to servlet map based on servlet mappings - for (String pathSpec : sms.keySet()) + //evaluate path to servlet map based on servlet mappings + for (String pathSpec : sms.keySet()) + { + //for each path, look at the mappings where it is referenced + //if a mapping is for a servlet that is not enabled, skip it + List mappings = sms.get(pathSpec); + + ServletMapping finalMapping = null; + for (ServletMapping mapping : mappings) { - //for each path, look at the mappings where it is referenced - //if a mapping is for a servlet that is not enabled, skip it - List mappings = sms.get(pathSpec); + //Get servlet associated with the mapping and check it is enabled + ServletHolder servletHolder = getServlet(mapping.getServletName()); + if (servletHolder == null) + throw new IllegalStateException("No such servlet: " + mapping.getServletName()); + //if the servlet related to the mapping is not enabled, skip it from consideration + if (!servletHolder.isEnabled()) + continue; - ServletMapping finalMapping = null; - for (ServletMapping mapping : mappings) + //only accept a default mapping if we don't have any other + if (finalMapping == null) + finalMapping = mapping; + else { - //Get servlet associated with the mapping and check it is enabled - ServletHolder servletHolder = getServlet(mapping.getServletName()); - if (servletHolder == null) - throw new IllegalStateException("No such servlet: " + mapping.getServletName()); - //if the servlet related to the mapping is not enabled, skip it from consideration - if (!servletHolder.isEnabled()) - continue; - - //only accept a default mapping if we don't have any other - if (finalMapping == null) + //already have a candidate - only accept another one + //if the candidate is a default, or we're allowing duplicate mappings + if (finalMapping.isFromDefaultDescriptor()) finalMapping = mapping; + else if (isAllowDuplicateMappings()) + { + LOG.warn("Multiple servlets map to path {}: {} and {}, choosing {}", pathSpec, finalMapping.getServletName(), mapping.getServletName(), mapping); + finalMapping = mapping; + } else { - //already have a candidate - only accept another one - //if the candidate is a default, or we're allowing duplicate mappings - if (finalMapping.isFromDefaultDescriptor()) - finalMapping = mapping; - else if (isAllowDuplicateMappings()) + //existing candidate isn't a default, if the one we're looking at isn't a default either, then its an error + if (!mapping.isFromDefaultDescriptor()) { - LOG.warn("Multiple servlets map to path {}: {} and {}, choosing {}", pathSpec, finalMapping.getServletName(), mapping.getServletName(), mapping); - finalMapping = mapping; - } - else - { - //existing candidate isn't a default, if the one we're looking at isn't a default either, then its an error - if (!mapping.isFromDefaultDescriptor()) - { - ServletHolder finalMappedServlet = getServlet(finalMapping.getServletName()); - throw new IllegalStateException("Multiple servlets map to path " + - pathSpec + ": " + - finalMappedServlet.getName() + "[mapped:" + finalMapping.getSource() + "]," + - mapping.getServletName() + "[mapped:" + mapping.getSource() + "]"); - } + ServletHolder finalMappedServlet = getServlet(finalMapping.getServletName()); + throw new IllegalStateException("Multiple servlets map to path " + + pathSpec + ": " + + finalMappedServlet.getName() + "[mapped:" + finalMapping.getSource() + "]," + + mapping.getServletName() + "[mapped:" + mapping.getSource() + "]"); } } } - if (finalMapping == null) - throw new IllegalStateException("No acceptable servlet mappings for " + pathSpec); - - if (LOG.isDebugEnabled()) - LOG.debug("Path={}[{}] mapped to servlet={}[{}]", - pathSpec, - finalMapping.getSource(), - finalMapping.getServletName(), - getServlet(finalMapping.getServletName()).getSource()); - - ServletPathSpec servletPathSpec = new ServletPathSpec(pathSpec); - MappedServlet mappedServlet = new MappedServlet(servletPathSpec, getServlet(finalMapping.getServletName())); - pm.put(servletPathSpec, mappedServlet); } + if (finalMapping == null) + throw new IllegalStateException("No acceptable servlet mappings for " + pathSpec); - _servletPathMap = pm; + if (LOG.isDebugEnabled()) + LOG.debug("Path={}[{}] mapped to servlet={}[{}]", + pathSpec, + finalMapping.getSource(), + finalMapping.getServletName(), + getServlet(finalMapping.getServletName()).getSource()); + + ServletPathSpec servletPathSpec = new ServletPathSpec(pathSpec); + MappedServlet mappedServlet = new MappedServlet(servletPathSpec, getServlet(finalMapping.getServletName())); + pm.put(servletPathSpec, mappedServlet); } + _servletPathMap = pm; + // flush filter chain cache for (int i = _chainCache.length; i-- > 0; ) { @@ -1423,33 +1358,17 @@ public class ServletHandler extends ScopedHandler protected boolean containsFilterHolder(FilterHolder holder) { - try (AutoLock l = lock()) + try (AutoLock ignored = lock()) { - if (_filters == null) - return false; - for (FilterHolder f : _filters) - { - if (f == holder) - return true; - } - return false; + return _filters.contains(holder); } } protected boolean containsServletHolder(ServletHolder holder) { - try (AutoLock l = lock()) + try (AutoLock ignored = lock()) { - if (_servlets == null) - return false; - for (ServletHolder s : _servlets) - { - @SuppressWarnings("ReferenceEquality") - boolean foundServletHolder = (s == holder); - if (foundServletHolder) - return true; - } - return false; + return _servlets.contains(holder); } } @@ -1466,22 +1385,23 @@ public class ServletHandler extends ScopedHandler */ public void setFilterMappings(FilterMapping[] filterMappings) { - updateBeans(_filterMappings, filterMappings); - _filterMappings = filterMappings; - if (isRunning()) - updateMappings(); - invalidateChainsCache(); + try (AutoLock ignored = lock()) + { + List mappings = filterMappings == null ? Collections.emptyList() : Arrays.asList(filterMappings); + updateAndSet(_filterMappings, mappings); + if (isRunning()) + updateMappings(); + invalidateChainsCache(); + } } public void setFilters(FilterHolder[] holders) { - try (AutoLock l = lock()) + try (AutoLock ignored = lock()) { - if (holders != null) - initializeHolders(holders); - - updateBeans(_filters, holders); - _filters = holders; + List filters = holders == null ? Collections.emptyList() : Arrays.asList(holders); + initializeHolders(filters); + updateAndSet(_filters, filters); updateNameMappings(); invalidateChainsCache(); } @@ -1492,8 +1412,8 @@ public class ServletHandler extends ScopedHandler */ public void setServletMappings(ServletMapping[] servletMappings) { - updateBeans(_servletMappings, servletMappings); - _servletMappings = servletMappings; + List mappings = servletMappings == null ? Collections.emptyList() : Arrays.asList(servletMappings); + updateAndSet(_servletMappings, mappings); if (isRunning()) updateMappings(); invalidateChainsCache(); @@ -1506,12 +1426,11 @@ public class ServletHandler extends ScopedHandler */ public void setServlets(ServletHolder[] holders) { - try (AutoLock l = lock()) + try (AutoLock ignored = lock()) { - if (holders != null) - initializeHolders(holders); - updateBeans(_servlets, holders); - _servlets = holders; + List servlets = holders == null ? Collections.emptyList() : Arrays.asList(holders); + initializeHolders(servlets); + updateAndSet(_servlets, servlets); updateNameMappings(); invalidateChainsCache(); } diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java index 1092fc91f2f..66957e4d79e 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java @@ -1448,11 +1448,13 @@ public class DefaultServletTest Path image = docRoot.resolve("image.jpg"); createFile(image, "not an image"); + server.stop(); ServletHolder defholder = context.addServlet(DefaultServlet.class, "/"); defholder.setInitParameter("dirAllowed", "false"); defholder.setInitParameter("redirectWelcome", "false"); defholder.setInitParameter("welcomeServlets", "false"); defholder.setInitParameter("gzip", "false"); + server.start(); String rawResponse; HttpTester.Response response; 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 47343dc0fcb..4b7253cbb03 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 @@ -15,19 +15,23 @@ package org.eclipse.jetty.servlet; 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.FilterChain; import javax.servlet.FilterConfig; +import javax.servlet.ServletContextListener; import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSessionEvent; import javax.servlet.http.HttpSessionListener; -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.Container; @@ -35,7 +39,9 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -107,34 +113,33 @@ public class ServletHandlerTest } @Test - public void testAddFilterIgnoresDuplicates() throws Exception + public void testAddFilterIgnoresDuplicates() { - ServletHandler handler = new ServletHandler(); FilterHolder h = new FilterHolder(); h.setName("x"); handler.addFilter(h); FilterHolder[] holders = handler.getFilters(); assertNotNull(holders); - assertTrue(holders[0] == h); + assertThat(h, is(holders[0])); handler.addFilter(h); holders = handler.getFilters(); assertNotNull(holders); - assertTrue(holders.length == 1); - assertTrue(holders[0] == h); + assertThat(1, is(holders.length)); + assertThat(h, is(holders[0])); FilterHolder h2 = new FilterHolder(); h2.setName("x"); //not allowed by servlet spec, just here to test object equality handler.addFilter(h2); holders = handler.getFilters(); assertNotNull(holders); - assertTrue(holders.length == 2); - assertTrue(holders[1] == h2); + assertThat(2, is(holders.length)); + assertThat(h2, is(holders[1])); } @Test - public void testAddFilterIgnoresDuplicates2() throws Exception + public void testAddFilterIgnoresDuplicates2() { ServletHandler handler = new ServletHandler(); @@ -147,7 +152,7 @@ public class ServletHandlerTest handler.addFilter(h, m); FilterHolder[] holders = handler.getFilters(); assertNotNull(holders); - assertTrue(holders[0] == h); + assertThat(h, is(holders[0])); FilterMapping m2 = new FilterMapping(); m2.setPathSpec("/*"); @@ -155,8 +160,8 @@ public class ServletHandlerTest handler.addFilter(h, m2); holders = handler.getFilters(); assertNotNull(holders); - assertTrue(holders.length == 1); - assertTrue(holders[0] == h); + assertThat(1, is(holders.length)); + assertThat(h, is(holders[0])); FilterHolder h2 = new FilterHolder(); h2.setName("x"); //not allowed by servlet spec, just here to test object equality @@ -167,12 +172,12 @@ public class ServletHandlerTest handler.addFilter(h2, m3); holders = handler.getFilters(); assertNotNull(holders); - assertTrue(holders.length == 2); - assertTrue(holders[1] == h2); + assertThat(2, is(holders.length)); + assertThat(h2, is(holders[1])); } @Test - public void testAddFilterWithMappingIgnoresDuplicateFilters() throws Exception + public void testAddFilterWithMappingIgnoresDuplicateFilters() { ServletHandler handler = new ServletHandler(); FilterHolder h = new FilterHolder(); @@ -181,13 +186,13 @@ public class ServletHandlerTest handler.addFilterWithMapping(h, "/*", 0); FilterHolder[] holders = handler.getFilters(); assertNotNull(holders); - assertTrue(holders[0] == h); + assertThat(h, is(holders[0])); handler.addFilterWithMapping(h, "/*", 1); holders = handler.getFilters(); assertNotNull(holders); - assertTrue(holders.length == 1); - assertTrue(holders[0] == h); + assertThat(1, is(holders.length)); + assertThat(h, is(holders[0])); FilterHolder h2 = new FilterHolder(); h2.setName("x"); //not allowed by servlet spec, just here to test object equality @@ -195,12 +200,12 @@ public class ServletHandlerTest handler.addFilterWithMapping(h2, "/*", 0); holders = handler.getFilters(); assertNotNull(holders); - assertTrue(holders.length == 2); - assertTrue(holders[1] == h2); + assertThat(2, is(holders.length)); + assertThat(h2, is(holders[1])); } @Test - public void testAddFilterWithMappingIngoresDuplicateFilters2() throws Exception + public void testAddFilterWithMappingIngoresDuplicateFilters2() { ServletHandler handler = new ServletHandler(); FilterHolder h = new FilterHolder(); @@ -209,13 +214,13 @@ public class ServletHandlerTest handler.addFilterWithMapping(h, "/*", EnumSet.allOf(DispatcherType.class)); FilterHolder[] holders = handler.getFilters(); assertNotNull(holders); - assertTrue(holders[0] == h); + assertThat(h, is(holders[0])); handler.addFilterWithMapping(h, "/x", EnumSet.allOf(DispatcherType.class)); holders = handler.getFilters(); assertNotNull(holders); - assertTrue(holders.length == 1); - assertTrue(holders[0] == h); + assertThat(1, is(holders.length)); + assertThat(h, is(holders[0])); FilterHolder h2 = new FilterHolder(); h2.setName("x"); //not allowed by servlet spec, just here to test object equality @@ -223,12 +228,12 @@ public class ServletHandlerTest handler.addFilterWithMapping(h2, "/*", EnumSet.allOf(DispatcherType.class)); holders = handler.getFilters(); assertNotNull(holders); - assertTrue(holders.length == 2); - assertTrue(holders[1] == h2); + assertThat(2, is(holders.length)); + assertThat(h2, is(holders[1])); } @Test - public void testDuplicateMappingsForbidden() throws Exception + public void testDuplicateMappingsForbidden() { ServletHandler handler = new ServletHandler(); handler.setAllowDuplicateMappings(false); @@ -250,7 +255,7 @@ public class ServletHandlerTest } @Test - public void testDuplicateMappingsWithDefaults() throws Exception + public void testDuplicateMappingsWithDefaults() { ServletHandler handler = new ServletHandler(); handler.setAllowDuplicateMappings(false); @@ -269,7 +274,7 @@ public class ServletHandlerTest } @Test - public void testDuplicateMappingsSameServlet() throws Exception + public void testDuplicateMappingsSameServlet() { ServletHolder sh4 = new ServletHolder(); @@ -292,7 +297,7 @@ public class ServletHandlerTest } @Test - public void testDuplicateMappingsAllowed() throws Exception + public void testDuplicateMappingsAllowed() { ServletHandler handler = new ServletHandler(); handler.setAllowDuplicateMappings(true); @@ -310,7 +315,7 @@ public class ServletHandlerTest } @Test - public void testAllNonProgrammaticFilterMappings() throws Exception + public void testAllNonProgrammaticFilterMappings() { ServletHandler handler = new ServletHandler(); handler.addFilter(fh1); @@ -322,8 +327,8 @@ public class ServletHandlerTest FilterMapping[] mappings = handler.getFilterMappings(); assertNotNull(mappings); - assertTrue(fm1 == mappings[0]); - assertTrue(fm2 == mappings[1]); + assertThat(mappings[0], is(fm1)); + assertThat(mappings[1], is(fm2)); //add another ordinary mapping FilterHolder of1 = new FilterHolder(new Source(Source.Origin.DESCRIPTOR, "foo.xml")); @@ -336,13 +341,13 @@ public class ServletHandlerTest mappings = handler.getFilterMappings(); assertNotNull(mappings); - assertTrue(fm1 == mappings[0]); - assertTrue(fm2 == mappings[1]); - assertTrue(ofm1 == mappings[2]); + assertThat(mappings[0], is(fm1)); + assertThat(mappings[1], is(fm2)); + assertThat(mappings[2], is(ofm1)); } @Test - public void testAllBeforeFilterMappings() throws Exception + public void testAllBeforeFilterMappings() { ServletHandler handler = new ServletHandler(); @@ -362,12 +367,12 @@ public class ServletHandlerTest assertNotNull(mappings); assertEquals(2, mappings.length); - assertTrue(fm4 == mappings[0]); - assertTrue(fm5 == mappings[1]); + assertThat(mappings[0], is(fm4)); + assertThat(mappings[1], is(fm5)); } @Test - public void testAllAfterFilterMappings() throws Exception + public void testAllAfterFilterMappings() { ServletHandler handler = new ServletHandler(); //do equivalent of FilterRegistration.addMappingForUrlPatterns(isMatchAfter=true) @@ -375,19 +380,19 @@ public class ServletHandlerTest handler.addFilterMapping(fm4); FilterMapping[] mappings = handler.getFilterMappings(); assertEquals(1, mappings.length); - assertTrue(fm4 == mappings[0]); + assertThat(mappings[0], is(fm4)); //do equivalent of FilterRegistration.addMappingForUrlPatterns(isMatchAfter=true) handler.addFilter(fh5); handler.addFilterMapping(fm5); mappings = handler.getFilterMappings(); assertEquals(2, mappings.length); - assertTrue(fm4 == mappings[0]); - assertTrue(fm5 == mappings[1]); + assertThat(mappings[0], is(fm4)); + assertThat(mappings[1], is(fm5)); } @Test - public void testMatchAfterAndBefore() throws Exception + public void testMatchAfterAndBefore() { ServletHandler handler = new ServletHandler(); @@ -397,7 +402,7 @@ public class ServletHandlerTest FilterMapping[] mappings = handler.getFilterMappings(); assertNotNull(mappings); assertEquals(1, mappings.length); - assertTrue(fm3 == mappings[0]); + assertThat(mappings[0], is(fm3)); //add a programmatic one, isMatchAfter=false handler.addFilter(fh4); @@ -405,12 +410,12 @@ public class ServletHandlerTest mappings = handler.getFilterMappings(); assertNotNull(mappings); assertEquals(2, mappings.length); - assertTrue(fm4 == mappings[0]); - assertTrue(fm3 == mappings[1]); + assertThat(mappings[0], is(fm4)); + assertThat(mappings[1], is(fm3)); } @Test - public void testMatchBeforeAndAfter() throws Exception + public void testMatchBeforeAndAfter() { ServletHandler handler = new ServletHandler(); @@ -420,7 +425,7 @@ public class ServletHandlerTest FilterMapping[] mappings = handler.getFilterMappings(); assertNotNull(mappings); assertEquals(1, mappings.length); - assertTrue(fm3 == mappings[0]); + assertThat(mappings[0], is(fm3)); //add a programmatic one, isMatchAfter=true handler.addFilter(fh4); @@ -428,12 +433,12 @@ public class ServletHandlerTest mappings = handler.getFilterMappings(); assertNotNull(mappings); assertEquals(2, mappings.length); - assertTrue(fm3 == mappings[0]); - assertTrue(fm4 == mappings[1]); + assertThat(mappings[0], is(fm3)); + assertThat(mappings[1], is(fm4)); } @Test - public void testExistingFilterMappings() throws Exception + public void testExistingFilterMappings() { ServletHandler handler = new ServletHandler(); handler.addFilter(fh1); @@ -445,26 +450,26 @@ public class ServletHandlerTest FilterMapping[] mappings = handler.getFilterMappings(); assertNotNull(mappings); - assertTrue(fm1 == mappings[0]); - assertTrue(fm2 == mappings[1]); + assertThat(mappings[0], is(fm1)); + assertThat(mappings[1], is(fm2)); //do equivalent of FilterRegistration.addMappingForUrlPatterns(isMatchAfter=false) handler.addFilter(fh4); handler.prependFilterMapping(fm4); mappings = handler.getFilterMappings(); assertEquals(3, mappings.length); - assertTrue(fm4 == mappings[0]); + assertThat(mappings[0], is(fm4)); //do equivalent of FilterRegistration.addMappingForUrlPatterns(isMatchAfter=true) handler.addFilter(fh5); handler.addFilterMapping(fm5); mappings = handler.getFilterMappings(); assertEquals(4, mappings.length); - assertTrue(fm5 == mappings[mappings.length - 1]); + assertThat(mappings[mappings.length - 1], is(fm5)); } @Test - public void testFilterMappingNoFilter() throws Exception + public void testFilterMappingNoFilter() { FilterMapping mapping = new FilterMapping(); mapping.setPathSpec("/*"); @@ -474,7 +479,7 @@ public class ServletHandlerTest } @Test - public void testFilterMappingsMix() throws Exception + public void testFilterMappingsMix() { ServletHandler handler = new ServletHandler(); @@ -483,7 +488,7 @@ public class ServletHandlerTest handler.addFilterMapping(fm1); FilterMapping[] mappings = handler.getFilterMappings(); assertNotNull(mappings); - assertTrue(fm1 == mappings[0]); + assertThat(mappings[0], is(fm1)); //add a programmatic one, isMatchAfter=false handler.addFilter(fh4); @@ -491,8 +496,8 @@ public class ServletHandlerTest mappings = handler.getFilterMappings(); assertNotNull(mappings); assertEquals(2, mappings.length); - assertTrue(fm4 == mappings[0]); - assertTrue(fm1 == mappings[1]); + assertThat(mappings[0], is(fm4)); + assertThat(mappings[1], is(fm1)); //add a programmatic one, isMatchAfter=true handler.addFilter(fh3); @@ -500,9 +505,9 @@ public class ServletHandlerTest mappings = handler.getFilterMappings(); assertNotNull(mappings); assertEquals(3, mappings.length); - assertTrue(fm4 == mappings[0]); - assertTrue(fm1 == mappings[1]); - assertTrue(fm3 == mappings[2]); + assertThat(mappings[0], is(fm4)); + assertThat(mappings[1], is(fm1)); + assertThat(mappings[2], is(fm3)); //add a programmatic one, isMatchAfter=false handler.addFilter(fh5); @@ -510,10 +515,10 @@ public class ServletHandlerTest mappings = handler.getFilterMappings(); assertNotNull(mappings); assertEquals(4, mappings.length); - assertTrue(fm4 == mappings[0]); //isMatchAfter = false; - assertTrue(fm5 == mappings[1]); //isMatchAfter = false; - assertTrue(fm1 == mappings[2]); //ordinary - assertTrue(fm3 == mappings[3]); //isMatchAfter = true; + assertThat(mappings[0], is(fm4)); //isMatchAfter = false; + assertThat(mappings[1], is(fm5)); //isMatchAfter = false; + assertThat(mappings[2], is(fm1)); //ordinary + assertThat(mappings[3], is(fm3)); //isMatchAfter = true; //add a non-programmatic one FilterHolder f = new FilterHolder(Source.EMBEDDED); @@ -526,11 +531,11 @@ public class ServletHandlerTest mappings = handler.getFilterMappings(); assertNotNull(mappings); assertEquals(5, mappings.length); - assertTrue(fm4 == mappings[0]); //isMatchAfter = false; - assertTrue(fm5 == mappings[1]); //isMatchAfter = false; - assertTrue(fm1 == mappings[2]); //ordinary - assertTrue(fm == mappings[3]); //ordinary - assertTrue(fm3 == mappings[4]); //isMatchAfter = true; + assertThat(mappings[0], is(fm4)); //isMatchAfter = false; + assertThat(mappings[1], is(fm5)); //isMatchAfter = false; + assertThat(mappings[2], is(fm1)); //ordinary + assertThat(mappings[3], is(fm)); //ordinary + assertThat(mappings[4], is(fm3)); //isMatchAfter = true; //add a programmatic one, isMatchAfter=true FilterHolder pf = new FilterHolder(Source.JAVAX_API); @@ -543,12 +548,12 @@ public class ServletHandlerTest mappings = handler.getFilterMappings(); assertNotNull(mappings); assertEquals(6, mappings.length); - assertTrue(fm4 == mappings[0]); //isMatchAfter = false; - assertTrue(fm5 == mappings[1]); //isMatchAfter = false; - assertTrue(fm1 == mappings[2]); //ordinary - assertTrue(fm == mappings[3]); //ordinary - assertTrue(fm3 == mappings[4]); //isMatchAfter = true; - assertTrue(pfm == mappings[5]); //isMatchAfter = true; + assertThat(mappings[0], is(fm4)); //isMatchAfter = false; + assertThat(mappings[1], is(fm5)); //isMatchAfter = false; + assertThat(mappings[2], is(fm1)); //ordinary + assertThat(mappings[3], is(fm)); //ordinary + assertThat(mappings[4], is(fm3)); //isMatchAfter = true; + assertThat(mappings[5], is(pfm)); //isMatchAfter = true; //add a programmatic one, isMatchAfter=false FilterHolder pf2 = new FilterHolder(Source.JAVAX_API); @@ -561,17 +566,17 @@ public class ServletHandlerTest mappings = handler.getFilterMappings(); assertNotNull(mappings); assertEquals(7, mappings.length); - assertTrue(fm4 == mappings[0]); //isMatchAfter = false; - assertTrue(fm5 == mappings[1]); //isMatchAfter = false; - assertTrue(pfm2 == mappings[2]); //isMatchAfter = false; - assertTrue(fm1 == mappings[3]); //ordinary - assertTrue(fm == mappings[4]); //ordinary - assertTrue(fm3 == mappings[5]); //isMatchAfter = true; - assertTrue(pfm == mappings[6]); //isMatchAfter = true; + assertThat(mappings[0], is(fm4)); //isMatchAfter = false; + assertThat(mappings[1], is(fm5)); //isMatchAfter = false; + assertThat(mappings[2], is(pfm2)); //isMatchAfter = false; + assertThat(mappings[3], is(fm1)); //ordinary + assertThat(mappings[4], is(fm)); //ordinary + assertThat(mappings[5], is(fm3)); //isMatchAfter = true; + assertThat(mappings[6], is(pfm)); //isMatchAfter = true; } @Test - public void testAddFilterWithMappingAPI() throws Exception + public void testAddFilterWithMappingAPI() { ServletHandler handler = new ServletHandler(); @@ -580,7 +585,7 @@ public class ServletHandlerTest handler.updateMappings(); FilterMapping[] mappings = handler.getFilterMappings(); assertNotNull(mappings); - assertTrue(fh1 == mappings[0].getFilterHolder()); + assertThat(mappings[0].getFilterHolder(), is(fh1)); //add a programmatic one, isMatchAfter=false fh4.setServletHandler(handler); @@ -590,8 +595,8 @@ public class ServletHandlerTest mappings = handler.getFilterMappings(); assertNotNull(mappings); assertEquals(2, mappings.length); - assertTrue(fh4 == mappings[0].getFilterHolder()); - assertTrue(fh1 == mappings[1].getFilterHolder()); + assertThat(mappings[0].getFilterHolder(), is(fh4)); + assertThat(mappings[1].getFilterHolder(), is(fh1)); //add a programmatic one, isMatchAfter=true fh3.setServletHandler(handler); @@ -601,9 +606,9 @@ public class ServletHandlerTest mappings = handler.getFilterMappings(); assertNotNull(mappings); assertEquals(3, mappings.length); - assertTrue(fh4 == mappings[0].getFilterHolder()); - assertTrue(fh1 == mappings[1].getFilterHolder()); - assertTrue(fh3 == mappings[2].getFilterHolder()); + assertThat(mappings[0].getFilterHolder(), is(fh4)); + assertThat(mappings[1].getFilterHolder(), is(fh1)); + assertThat(mappings[2].getFilterHolder(), is(fh3)); //add a programmatic one, isMatchAfter=false fh5.setServletHandler(handler); @@ -613,10 +618,10 @@ public class ServletHandlerTest mappings = handler.getFilterMappings(); assertNotNull(mappings); assertEquals(4, mappings.length); - assertTrue(fh4 == mappings[0].getFilterHolder()); //isMatchAfter = false; - assertTrue(fh5 == mappings[1].getFilterHolder()); //isMatchAfter = false; - assertTrue(fh1 == mappings[2].getFilterHolder()); //ordinary - assertTrue(fh3 == mappings[3].getFilterHolder()); //isMatchAfter = true; + assertThat(mappings[0].getFilterHolder(), is(fh4)); //isMatchAfter = false; + assertThat(mappings[1].getFilterHolder(), is(fh5)); //isMatchAfter = false; + assertThat(mappings[2].getFilterHolder(), is(fh1)); //ordinary + assertThat(mappings[3].getFilterHolder(), is(fh3)); //isMatchAfter = true; //add a non-programmatic one FilterHolder f = new FilterHolder(Source.EMBEDDED); @@ -626,11 +631,11 @@ public class ServletHandlerTest mappings = handler.getFilterMappings(); assertNotNull(mappings); assertEquals(5, mappings.length); - assertTrue(fh4 == mappings[0].getFilterHolder()); //isMatchAfter = false; - assertTrue(fh5 == mappings[1].getFilterHolder()); //isMatchAfter = false; - assertTrue(fh1 == mappings[2].getFilterHolder()); //ordinary - assertTrue(f == mappings[3].getFilterHolder()); //ordinary - assertTrue(fh3 == mappings[4].getFilterHolder()); //isMatchAfter = true; + assertThat(mappings[0].getFilterHolder(), is(fh4)); //isMatchAfter = false; + assertThat(mappings[1].getFilterHolder(), is(fh5)); //isMatchAfter = false; + assertThat(mappings[2].getFilterHolder(), is(fh1)); //ordinary + assertThat(mappings[3].getFilterHolder(), is(f)); //ordinary + assertThat(mappings[4].getFilterHolder(), is(fh3)); //isMatchAfter = true; //add a programmatic one, isMatchAfter=true FilterHolder pf = new FilterHolder(Source.JAVAX_API); @@ -642,12 +647,12 @@ public class ServletHandlerTest mappings = handler.getFilterMappings(); assertNotNull(mappings); assertEquals(6, mappings.length); - assertTrue(fh4 == mappings[0].getFilterHolder()); //isMatchAfter = false; - assertTrue(fh5 == mappings[1].getFilterHolder()); //isMatchAfter = false; - assertTrue(fh1 == mappings[2].getFilterHolder()); //ordinary - assertTrue(f == mappings[3].getFilterHolder()); //ordinary - assertTrue(fh3 == mappings[4].getFilterHolder()); //isMatchAfter = true; - assertTrue(pf == mappings[5].getFilterHolder()); //isMatchAfter = true; + assertThat(mappings[0].getFilterHolder(), is(fh4)); //isMatchAfter = false; + assertThat(mappings[1].getFilterHolder(), is(fh5)); //isMatchAfter = false; + assertThat(mappings[2].getFilterHolder(), is(fh1)); //ordinary + assertThat(mappings[3].getFilterHolder(), is(f)); //ordinary + assertThat(mappings[4].getFilterHolder(), is(fh3)); //isMatchAfter = true; + assertThat(mappings[5].getFilterHolder(), is(pf)); //isMatchAfter = true; //add a programmatic one, isMatchAfter=false FilterHolder pf2 = new FilterHolder(Source.JAVAX_API); @@ -660,17 +665,17 @@ public class ServletHandlerTest assertNotNull(mappings); assertEquals(7, mappings.length); - assertTrue(fh4 == mappings[0].getFilterHolder()); //isMatchAfter = false; - assertTrue(fh5 == mappings[1].getFilterHolder()); //isMatchAfter = false; - assertTrue(pf2 == mappings[2].getFilterHolder()); //isMatchAfter = false; - assertTrue(fh1 == mappings[3].getFilterHolder()); //ordinary - assertTrue(f == mappings[4].getFilterHolder()); //ordinary - assertTrue(fh3 == mappings[5].getFilterHolder()); //isMatchAfter = true; - assertTrue(pf == mappings[6].getFilterHolder()); //isMatchAfter = true; + assertThat(mappings[0].getFilterHolder(), is(fh4)); //isMatchAfter = false; + assertThat(mappings[1].getFilterHolder(), is(fh5)); //isMatchAfter = false; + assertThat(mappings[2].getFilterHolder(), is(pf2)); //isMatchAfter = false; + assertThat(mappings[3].getFilterHolder(), is(fh1)); //ordinary + assertThat(mappings[4].getFilterHolder(), is(f)); //ordinary + assertThat(mappings[5].getFilterHolder(), is(fh3)); //isMatchAfter = true; + assertThat(mappings[6].getFilterHolder(), is(pf)); //isMatchAfter = true; } @Test - public void testFiltersServletsListenersAsBeans() throws Exception + public void testFiltersServletsListenersAsBeans() { ServletContextHandler context = new ServletContextHandler(); @@ -747,7 +752,7 @@ public class ServletHandlerTest handler.addServletWithMapping(new ServletHolder(new HttpServlet() { @Override - protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { resp.getOutputStream().println("mapping='" + mapping + "'"); } @@ -779,7 +784,7 @@ public class ServletHandlerTest ServletHolder foo = new ServletHolder(new HttpServlet() { @Override - protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { resp.getOutputStream().println("FOO"); } @@ -790,7 +795,7 @@ public class ServletHandlerTest ServletHolder def = new ServletHolder(new HttpServlet() { @Override - protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { resp.getOutputStream().println("default"); } @@ -830,6 +835,67 @@ public class ServletHandlerTest assertThat(connector.getResponse("GET /other.bob HTTP/1.0\r\n\r\n"), containsString("path-/*-path-*.bob-default")); } + @Test + public void testDurable() throws Exception + { + Server server = new Server(); + ServletContextHandler context = new ServletContextHandler(); + server.setHandler(context); + ServletHandler handler = new ServletHandler(); + context.setHandler(handler); + ListenerHolder lh1 = new ListenerHolder(HSListener.class); + ListenerHolder lh2 = new ListenerHolder(SCListener.class); + + fh1.setFilter(new SomeFilter()); + fm1.setPathSpec("/sm1"); + fm1.setFilterHolder(fh1); + fh2.setFilter(new SomeFilter(){}); + fm2.setPathSpec("/sm2"); + fm2.setFilterHolder(fh2); + sh1.setServlet(new SomeServlet()); + sm1.setPathSpec("/sm1"); + sm1.setServletName(sh1.getName()); + sh2.setServlet(new SomeServlet()); + sm2.setPathSpec("/sm2"); + sm2.setServletName(sh2.getName()); + + handler.setListeners(new ListenerHolder[] {lh1}); + handler.setFilters(new FilterHolder[] {fh1}); + handler.setFilterMappings(new FilterMapping[] {fm1}); + handler.setServlets(new ServletHolder[] {sh1}); + handler.setServletMappings(new ServletMapping[] {sm1}); + + server.start(); + + handler.setListeners(new ListenerHolder[] {lh1, lh2}); + handler.setFilters(new FilterHolder[] {fh1, fh2}); + handler.setFilterMappings(new FilterMapping[] {fm1, fm2}); + handler.setServlets(new ServletHolder[] {sh1, sh2}); + handler.setServletMappings(new ServletMapping[] {sm1, sm2}); + + assertThat(Arrays.asList(handler.getListeners()), contains(lh1, lh2)); + assertThat(Arrays.asList(handler.getFilters()), contains(fh1, fh2)); + assertThat(Arrays.asList(handler.getFilterMappings()), contains(fm1, fm2)); + assertThat(Arrays.asList(handler.getServlets()), contains(sh1, sh2)); + assertThat(Arrays.asList(handler.getServletMappings()), contains(sm1, sm2)); + + server.stop(); + + assertThat(Arrays.asList(handler.getListeners()), contains(lh1)); + assertThat(Arrays.asList(handler.getFilters()), contains(fh1)); + assertThat(Arrays.asList(handler.getFilterMappings()), contains(fm1)); + assertThat(Arrays.asList(handler.getServlets()), contains(sh1)); + assertThat(Arrays.asList(handler.getServletMappings()), contains(sm1)); + } + + public static class HSListener implements HttpSessionListener + { + } + + public static class SCListener implements ServletContextListener + { + } + private interface TestFilter extends Filter { default void init(FilterConfig filterConfig) throws ServletException @@ -842,4 +908,17 @@ public class ServletHandlerTest } } + public static class SomeFilter implements TestFilter + { + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException + { + chain.doFilter(request, response); + } + } + + public static class SomeServlet extends HttpServlet + { + } + } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java index 65dabedc463..115dd4f2bc8 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java @@ -15,11 +15,13 @@ package org.eclipse.jetty.util.component; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.EventListener; import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; @@ -489,12 +491,13 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container, if (super.removeEventListener(listener)) { removeBean(listener); - if (_listeners.remove(listener)) + if (listener instanceof Container.Listener && _listeners.remove(listener)) { + Container.Listener cl = (Container.Listener)listener; // remove existing beans for (Bean b : _beans) { - ((Container.Listener)listener).beanRemoved(this, b._bean); + cl.beanRemoved(this, b._bean); if (listener instanceof InheritedListener && b.isManaged() && b._bean instanceof Container) ((Container)b._bean).removeBean(listener); @@ -813,40 +816,28 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container, public void updateBeans(Object[] oldBeans, final Object[] newBeans) { + updateBeans( + oldBeans == null ? Collections.emptyList() : Arrays.asList(oldBeans), + newBeans == null ? Collections.emptyList() : Arrays.asList(newBeans)); + } + + public void updateBeans(final Collection oldBeans, final Collection newBeans) + { + Objects.requireNonNull(oldBeans); + Objects.requireNonNull(newBeans); + // remove oldChildren not in newChildren - if (oldBeans != null) + for (Object o : oldBeans) { - loop: - for (Object o : oldBeans) - { - if (newBeans != null) - { - for (Object n : newBeans) - { - if (o == n) - continue loop; - } - } + if (!newBeans.contains(o)) removeBean(o); - } } // add new beans not in old - if (newBeans != null) + for (Object n : newBeans) { - loop: - for (Object n : newBeans) - { - if (oldBeans != null) - { - for (Object o : oldBeans) - { - if (o == n) - continue loop; - } - } + if (!oldBeans.contains(n)) addBean(n); - } } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/DumpableCollection.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/DumpableCollection.java index 56de2add111..e26427fb69b 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/DumpableCollection.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/DumpableCollection.java @@ -39,6 +39,11 @@ public class DumpableCollection implements Dumpable return new DumpableCollection(name, items == null ? Collections.emptyList() : Arrays.asList(items)); } + public static DumpableCollection from(String name, Collection collection) + { + return new DumpableCollection(name, collection); + } + @Override public String dump() {