diff --git a/jetty-osgi/test-jetty-osgi/src/test/java/org/eclipse/jetty/osgi/test/TestJettyOSGiBootHTTP2.java b/jetty-osgi/test-jetty-osgi/src/test/java/org/eclipse/jetty/osgi/test/TestJettyOSGiBootHTTP2.java index 1f971fd0e33c..eefd5b85be27 100644 --- a/jetty-osgi/test-jetty-osgi/src/test/java/org/eclipse/jetty/osgi/test/TestJettyOSGiBootHTTP2.java +++ b/jetty-osgi/test-jetty-osgi/src/test/java/org/eclipse/jetty/osgi/test/TestJettyOSGiBootHTTP2.java @@ -82,7 +82,7 @@ public Option[] config() options.add(mavenBundle().groupId("org.eclipse.jetty.http2").artifactId("http2-http-client-transport").versionAsInProject().start()); options.add(systemProperty("org.ops4j.pax.logging.DefaultServiceLog.level").value(LOG_LEVEL)); - options.add(systemProperty("org.eclipse.jetty.LEVEL").value("DEBUG")); + options.add(systemProperty("org.eclipse.jetty.LEVEL").value("INFO")); options.add(CoreOptions.cleanCaches(true)); return options.toArray(new Option[0]); } @@ -154,7 +154,7 @@ public void testHTTP2() throws Exception httpClient.start(); ContentResponse response = httpClient.GET("https://localhost:" + port + "/jsp/jstl.jsp"); - assertEquals(response.toString(), response.getStatus(), HttpStatus.OK_200); + assertEquals(HttpStatus.OK_200,response.getStatus()); String body = response.getContentAsString(); assertTrue("Body contains \"JSTL Example\": " + body, body.contains("JSTL Example")); } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 5ca2bc8b9338..018feb6a28f6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -195,7 +195,7 @@ public static Request getBaseRequest(ServletRequest request) private String _servletPath; private String _pathInfo; private boolean _secure; - private String _asyncNotSupportedSource = null; + private Object _asyncNotSupportedSource = null; private boolean _newContext; private boolean _cookiesExtracted = false; private boolean _handled = false; @@ -1949,7 +1949,7 @@ public void removeEventListener(final EventListener listener) _requestAttributeListeners.remove(listener); } - public void setAsyncSupported(boolean supported, String source) + public void setAsyncSupported(boolean supported, Object source) { _asyncNotSupportedSource = supported ? null : (source == null ? "unknown" : source); } diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/FilterHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/FilterHolder.java index ded28c8769f9..6f4af7368b78 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/FilterHolder.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/FilterHolder.java @@ -35,6 +35,7 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; +import org.eclipse.jetty.server.Request; import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.component.Dumpable; import org.eclipse.jetty.util.component.DumpableCollection; @@ -186,11 +187,24 @@ public Filter getFilter() return _filter; } - public void doFilter(ServletRequest request, ServletResponse response, - FilterChain chain) - throws IOException, ServletException + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { - _filter.doFilter(request, response, chain); + if (isAsyncSupported() || !request.isAsyncSupported()) + getFilter().doFilter(request, response, chain); + else + { + Request baseRequest = Request.getBaseRequest(request); + Objects.requireNonNull(baseRequest); + try + { + baseRequest.setAsyncSupported(false, this); + getFilter().doFilter(request, response, chain); + } + finally + { + baseRequest.setAsyncSupported(true, null); + } + } } @Override 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 1979d285e6ab..32d80bb99084 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 @@ -28,10 +28,9 @@ import java.util.List; import java.util.ListIterator; import java.util.Map; -import java.util.Queue; +import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ConcurrentMap; import java.util.function.Consumer; import java.util.stream.Stream; @@ -63,7 +62,6 @@ import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.ScopedHandler; import org.eclipse.jetty.util.ArrayUtil; -import org.eclipse.jetty.util.LazyList; import org.eclipse.jetty.util.MultiException; import org.eclipse.jetty.util.MultiMap; import org.eclipse.jetty.util.URIUtil; @@ -101,7 +99,7 @@ public class ServletHandler extends ScopedHandler 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; - private int _maxFilterChainsCacheSize = 512; + private int _maxFilterChainsCacheSize = 1024; private boolean _startWithUnavailable = false; private boolean _ensureDefaultServlet = true; private IdentityService _identityService; @@ -112,9 +110,9 @@ public class ServletHandler extends ScopedHandler private final Map _filterNameMap = new HashMap<>(); private List _filterPathMappings; private MultiMap _filterNameMappings; + private List _wildFilterNameMappings; private final Map _servletNameMap = new HashMap<>(); - // private PathMap _servletPathMap; private PathMappings _servletPathMap; private ListenerHolder[] _listeners = new ListenerHolder[0]; @@ -123,9 +121,6 @@ public class ServletHandler extends ScopedHandler @SuppressWarnings("unchecked") protected final ConcurrentMap[] _chainCache = new ConcurrentMap[FilterMapping.ALL]; - @SuppressWarnings("unchecked") - protected final Queue[] _chainLRU = new Queue[FilterMapping.ALL]; - /** * Constructor. */ @@ -136,7 +131,7 @@ public ServletHandler() @Override public boolean isDumpable(Object o) { - return !(o instanceof Holder || o instanceof BaseHolder || o instanceof FilterMapping || o instanceof ServletMapping); + return !(o instanceof BaseHolder || o instanceof FilterMapping || o instanceof ServletMapping); } @Override @@ -184,12 +179,6 @@ protected synchronized void doStart() _chainCache[FilterMapping.INCLUDE] = new ConcurrentHashMap<>(); _chainCache[FilterMapping.ERROR] = new ConcurrentHashMap<>(); _chainCache[FilterMapping.ASYNC] = new ConcurrentHashMap<>(); - - _chainLRU[FilterMapping.REQUEST] = new ConcurrentLinkedQueue<>(); - _chainLRU[FilterMapping.FORWARD] = new ConcurrentLinkedQueue<>(); - _chainLRU[FilterMapping.INCLUDE] = new ConcurrentLinkedQueue<>(); - _chainLRU[FilterMapping.ERROR] = new ConcurrentLinkedQueue<>(); - _chainLRU[FilterMapping.ASYNC] = new ConcurrentLinkedQueue<>(); } if (_contextHandler == null) @@ -274,14 +263,14 @@ protected synchronized void doStop() } //Retain only filters and mappings that were added using jetty api (ie Source.EMBEDDED) - FilterHolder[] fhs = (FilterHolder[])LazyList.toArray(filterHolders, FilterHolder.class); + FilterHolder[] fhs = filterHolders.toArray(new FilterHolder[0]); updateBeans(_filters, fhs); _filters = fhs; - FilterMapping[] fms = (FilterMapping[])LazyList.toArray(filterMappings, FilterMapping.class); + FilterMapping[] fms = filterMappings.toArray(new FilterMapping[0]); updateBeans(_filterMappings, fms); _filterMappings = fms; - _matchAfterIndex = (_filterMappings == null || _filterMappings.length == 0 ? -1 : _filterMappings.length - 1); + _matchAfterIndex = (_filterMappings.length == 0 ? -1 : _filterMappings.length - 1); _matchBeforeIndex = -1; // Stop servlets @@ -319,10 +308,10 @@ protected synchronized void doStop() } //Retain only Servlets and mappings added via jetty apis (ie Source.EMBEDDED) - ServletHolder[] shs = (ServletHolder[])LazyList.toArray(servletHolders, ServletHolder.class); + ServletHolder[] shs = servletHolders.toArray(new ServletHolder[0]); updateBeans(_servlets, shs); _servlets = shs; - ServletMapping[] sms = (ServletMapping[])LazyList.toArray(servletMappings, ServletMapping.class); + ServletMapping[] sms = servletMappings.toArray(new ServletMapping[0]); updateBeans(_servletMappings, sms); _servletMappings = sms; @@ -347,7 +336,7 @@ protected synchronized void doStop() listenerHolders.add(_listeners[i]); } } - ListenerHolder[] listeners = (ListenerHolder[])LazyList.toArray(listenerHolders, ListenerHolder.class); + ListenerHolder[] listeners = listenerHolders.toArray(new ListenerHolder[0]); updateBeans(_listeners, listeners); _listeners = listeners; @@ -596,8 +585,6 @@ public MappedResource getMappedServlet(String target) return _servletPathMap.getMatch(target); } - if (_servletNameMap == null) - return null; ServletHolder holder = _servletNameMap.get(target); if (holder == null) return null; @@ -609,93 +596,73 @@ protected FilterChain getFilterChain(Request baseRequest, String pathInContext, String key = pathInContext == null ? servletHolder.getName() : pathInContext; int dispatch = FilterMapping.dispatch(baseRequest.getDispatcherType()); - if (_filterChainsCached && _chainCache != null) + if (_filterChainsCached) { FilterChain chain = _chainCache[dispatch].get(key); if (chain != null) return chain; } - // Build list of filters (list of FilterHolder objects) - List filters = new ArrayList<>(); - - // Path filters - if (pathInContext != null && _filterPathMappings != null) - { - for (FilterMapping filterPathMapping : _filterPathMappings) - { - if (filterPathMapping.appliesTo(pathInContext, dispatch)) - filters.add(filterPathMapping.getFilterHolder()); - } - } + // Build the filter chain from the inside out. + // ie first wrap the servlet with the last filter to be applied. + // The mappings lists have been reversed to make this simple and fast. + FilterChain chain = null; - // Servlet name filters if (servletHolder != null && _filterNameMappings != null && !_filterNameMappings.isEmpty()) { - Object o = _filterNameMappings.get(servletHolder.getName()); + if (_wildFilterNameMappings != null) + for (FilterMapping mapping : _wildFilterNameMappings) + chain = newFilterChain(mapping.getFilterHolder(), chain == null ? new ChainEnd(servletHolder) : chain); - for (int i = 0; i < LazyList.size(o); i++) + for (FilterMapping mapping : _filterNameMappings.get(servletHolder.getName())) { - FilterMapping mapping = LazyList.get(o, i); if (mapping.appliesTo(dispatch)) - filters.add(mapping.getFilterHolder()); + chain = newFilterChain(mapping.getFilterHolder(), chain == null ? new ChainEnd(servletHolder) : chain); } + } - o = _filterNameMappings.get("*"); - for (int i = 0; i < LazyList.size(o); i++) + if (pathInContext != null && _filterPathMappings != null) + { + for (FilterMapping mapping : _filterPathMappings) { - FilterMapping mapping = LazyList.get(o, i); - if (mapping.appliesTo(dispatch)) - filters.add(mapping.getFilterHolder()); + if (mapping.appliesTo(pathInContext, dispatch)) + chain = newFilterChain(mapping.getFilterHolder(), chain == null ? new ChainEnd(servletHolder) : chain); } } - if (filters.isEmpty()) - return null; - - FilterChain chain = null; if (_filterChainsCached) { - if (!filters.isEmpty()) - chain = newCachedChain(filters, servletHolder); - final Map cache = _chainCache[dispatch]; - final Queue lru = _chainLRU[dispatch]; - // Do we have too many cached chains? - while (_maxFilterChainsCacheSize > 0 && cache.size() >= _maxFilterChainsCacheSize) + if (_maxFilterChainsCacheSize > 0 && cache.size() >= _maxFilterChainsCacheSize) { - // The LRU list is not atomic with the cache map, so be prepared to invalidate if - // a key is not found to delete. - // Delete by LRU (where U==created) - String k = lru.poll(); - if (k == null) - { - cache.clear(); - break; - } - cache.remove(k); + // flush the cache + LOG.debug("{} flushed filter chain cache for {}", this, baseRequest.getDispatcherType()); + cache.clear(); } - + chain = chain == null ? new ChainEnd(servletHolder) : chain; + // flush the cache + LOG.debug("{} cached filter chain for {}: {}", this, baseRequest.getDispatcherType(), chain); cache.put(key, chain); - lru.add(key); } - else if (!filters.isEmpty()) - chain = new Chain(baseRequest, filters, servletHolder); - return chain; } + /** + * Create a FilterChain that calls the passed filter with the passed chain + * @param filterHolder The filter to invoke + * @param chain The chain to pass to the filter + * @return A FilterChain that invokes the filter with the chain + */ + protected FilterChain newFilterChain(FilterHolder filterHolder, FilterChain chain) + { + return new Chain(filterHolder, chain); + } + protected void invalidateChainsCache() { - if (_chainLRU[FilterMapping.REQUEST] != null) + if (_chainCache[FilterMapping.REQUEST] != null) { - _chainLRU[FilterMapping.REQUEST].clear(); - _chainLRU[FilterMapping.FORWARD].clear(); - _chainLRU[FilterMapping.INCLUDE].clear(); - _chainLRU[FilterMapping.ERROR].clear(); - _chainLRU[FilterMapping.ASYNC].clear(); - _chainCache[FilterMapping.REQUEST].clear(); _chainCache[FilterMapping.FORWARD].clear(); _chainCache[FilterMapping.INCLUDE].clear(); @@ -846,18 +813,6 @@ public ListenerHolder newListenerHolder(Source source) return new ListenerHolder(source); } - /** - * Create a new CachedChain - * - * @param filters the filter chain to be cached as a collection of {@link FilterHolder} - * @param servletHolder the servletHolder - * @return a new {@link CachedChain} instance - */ - public CachedChain newCachedChain(List filters, ServletHolder servletHolder) - { - return new CachedChain(filters, servletHolder); - } - /** * Add a new servlet holder * @@ -908,6 +863,7 @@ public ServletHolder addServletWithMapping(Class servlet, Str */ public void addServletWithMapping(ServletHolder servlet, String pathSpec) { + Objects.requireNonNull(servlet); ServletHolder[] holders = getServlets(); if (holders != null) holders = holders.clone(); @@ -916,7 +872,7 @@ public void addServletWithMapping(ServletHolder servlet, String pathSpec) { synchronized (this) { - if (servlet != null && !containsServletHolder(servlet)) + if (!containsServletHolder(servlet)) setServlets(ArrayUtil.addToArray(holders, servlet, ServletHolder.class)); } @@ -1021,6 +977,7 @@ public FilterHolder addFilterWithMapping(String className, String pathSpec, Enum */ public void addFilterWithMapping(FilterHolder holder, String pathSpec, EnumSet dispatches) { + Objects.requireNonNull(holder); FilterHolder[] holders = getFilters(); if (holders != null) holders = holders.clone(); @@ -1029,7 +986,7 @@ public void addFilterWithMapping(FilterHolder holder, String pathSpec, EnumSet> 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 || _servletNameMap == null) + if (_servletMappings == null) { _servletPathMap = null; } else { PathMappings pm = new PathMappings<>(); - Map servletPathMappings = new HashMap<>(); //create a map of paths to set of ServletMappings that define that mapping HashMap> sms = new HashMap<>(); @@ -1386,12 +1352,7 @@ protected synchronized void updateMappings() { for (String pathSpec : pathSpecs) { - List mappings = sms.get(pathSpec); - if (mappings == null) - { - mappings = new ArrayList<>(); - sms.put(pathSpec, mappings); - } + List mappings = sms.computeIfAbsent(pathSpec, k -> new ArrayList<>()); mappings.add(servletMapping); } } @@ -1453,7 +1414,6 @@ else if (isAllowDuplicateMappings()) finalMapping.getServletName(), _servletNameMap.get(finalMapping.getServletName()).getSource()); - servletPathMappings.put(pathSpec, finalMapping); pm.put(new ServletPathSpec(pathSpec), _servletNameMap.get(finalMapping.getServletName())); } @@ -1461,13 +1421,10 @@ else if (isAllowDuplicateMappings()) } // flush filter chain cache - if (_chainCache != null) + for (int i = _chainCache.length; i-- > 0; ) { - for (int i = _chainCache.length; i-- > 0; ) - { - if (_chainCache[i] != null) - _chainCache[i].clear(); - } + if (_chainCache[i] != null) + _chainCache[i].clear(); } if (LOG.isDebugEnabled()) @@ -1502,28 +1459,26 @@ protected synchronized boolean containsFilterHolder(FilterHolder holder) { if (_filters == null) return false; - boolean found = false; for (FilterHolder f : _filters) { if (f == holder) - found = true; + return true; } - return found; + return false; } protected synchronized boolean containsServletHolder(ServletHolder holder) { if (_servlets == null) return false; - boolean found = false; for (ServletHolder s : _servlets) { @SuppressWarnings("ReferenceEquality") boolean foundServletHolder = (s == holder); if (foundServletHolder) - found = true; + return true; } - return found; + return false; } /** @@ -1589,159 +1544,6 @@ public synchronized void setServlets(ServletHolder[] holders) invalidateChainsCache(); } - protected class CachedChain implements FilterChain - { - FilterHolder _filterHolder; - CachedChain _next; - ServletHolder _servletHolder; - - /** - * @param filters list of {@link FilterHolder} objects - * @param servletHolder the current {@link ServletHolder} - */ - protected CachedChain(List filters, ServletHolder servletHolder) - { - if (!filters.isEmpty()) - { - _filterHolder = filters.get(0); - filters.remove(0); - _next = new CachedChain(filters, servletHolder); - } - else - _servletHolder = servletHolder; - } - - @Override - public void doFilter(ServletRequest request, ServletResponse response) - throws IOException, ServletException - { - final Request baseRequest = Request.getBaseRequest(request); - - // pass to next filter - if (_filterHolder != null) - { - if (LOG.isDebugEnabled()) - LOG.debug("call filter {}", _filterHolder); - - //if the request already does not support async, then the setting for the filter - //is irrelevant. However if the request supports async but this filter does not - //temporarily turn it off for the execution of the filter - if (baseRequest.isAsyncSupported() && !_filterHolder.isAsyncSupported()) - { - try - { - baseRequest.setAsyncSupported(false, _filterHolder.toString()); - _filterHolder.doFilter(request, response, _next); - } - finally - { - baseRequest.setAsyncSupported(true, null); - } - } - else - _filterHolder.doFilter(request, response, _next); - - return; - } - - // Call servlet - HttpServletRequest srequest = (HttpServletRequest)request; - if (_servletHolder == null) - notFound(baseRequest, srequest, (HttpServletResponse)response); - else - { - if (LOG.isDebugEnabled()) - LOG.debug("call servlet " + _servletHolder); - _servletHolder.handle(baseRequest, request, response); - } - } - - @Override - public String toString() - { - if (_filterHolder != null) - return _filterHolder + "->" + _next.toString(); - if (_servletHolder != null) - return _servletHolder.toString(); - return "null"; - } - } - - private class Chain implements FilterChain - { - final Request _baseRequest; - final List _chain; - final ServletHolder _servletHolder; - int _filter = 0; - - private Chain(Request baseRequest, List filters, ServletHolder servletHolder) - { - _baseRequest = baseRequest; - _chain = filters; - _servletHolder = servletHolder; - } - - @Override - public void doFilter(ServletRequest request, ServletResponse response) - throws IOException, ServletException - { - if (LOG.isDebugEnabled()) - LOG.debug("doFilter " + _filter); - - // pass to next filter - if (_filter < _chain.size()) - { - FilterHolder holder = _chain.get(_filter++); - if (LOG.isDebugEnabled()) - LOG.debug("call filter " + holder); - - //if the request already does not support async, then the setting for the filter - //is irrelevant. However if the request supports async but this filter does not - //temporarily turn it off for the execution of the filter - if (!holder.isAsyncSupported() && _baseRequest.isAsyncSupported()) - { - try - { - _baseRequest.setAsyncSupported(false, holder.toString()); - holder.doFilter(request, response, this); - } - finally - { - _baseRequest.setAsyncSupported(true, null); - } - } - else - holder.doFilter(request, response, this); - - return; - } - - // Call servlet - HttpServletRequest srequest = (HttpServletRequest)request; - if (_servletHolder == null) - notFound(Request.getBaseRequest(request), srequest, (HttpServletResponse)response); - else - { - if (LOG.isDebugEnabled()) - LOG.debug("call servlet {}", _servletHolder); - _servletHolder.handle(_baseRequest, request, response); - } - } - - @Override - public String toString() - { - StringBuilder b = new StringBuilder(); - for (FilterHolder f : _chain) - { - b.append(f.toString()); - b.append("->"); - } - b.append(_servletHolder); - return b.toString(); - } - } - /** * @return The maximum entries in a filter chain cache. */ @@ -1790,4 +1592,52 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) resp.sendError(HttpServletResponse.SC_NOT_FOUND); } } + + static class Chain implements FilterChain + { + private final FilterHolder _filterHolder; + private final FilterChain _filterChain; + + Chain(FilterHolder filter, FilterChain chain) + { + _filterHolder = filter; + _filterChain = chain; + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException + { + _filterHolder.doFilter(request, response, _filterChain); + } + + @Override + public String toString() + { + return String.format("Chain@%x(%s)->%s", hashCode(), _filterHolder, _filterChain); + } + } + + static class ChainEnd implements FilterChain + { + private final ServletHolder _servletHolder; + + ChainEnd(ServletHolder holder) + { + _servletHolder = holder; + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException + { + Request baseRequest = Request.getBaseRequest(request); + Objects.requireNonNull(baseRequest); + _servletHolder.handle(baseRequest, request, response); + } + + @Override + public String toString() + { + return String.format("ChainEnd@%x(%s)", hashCode(), _servletHolder); + } + } }