From bd143a674ba7fbb3c7e7e450c8102e9a4bd95172 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 22 Aug 2018 08:29:12 +1000 Subject: [PATCH 01/13] Issue #2787 BadMessage if bad query detected in dispatcher (#2827) Signed-off-by: Greg Wilkins --- .../org/eclipse/jetty/server/Request.java | 11 +++- .../eclipse/jetty/servlet/ServletHandler.java | 8 +++ .../eclipse/jetty/servlet/DispatcherTest.java | 64 ++++++++++++++++++- 3 files changed, 77 insertions(+), 6 deletions(-) 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 6068f7c34d0..b75d0e6548e 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 @@ -2390,8 +2390,6 @@ public class Request implements HttpServletRequest /* ------------------------------------------------------------ */ public void mergeQueryParameters(String oldQuery,String newQuery, boolean updateQueryString) { - // TODO This is seriously ugly - MultiMap newQueryParams = null; // Have to assume ENCODING because we can't know otherwise. if (newQuery!=null) @@ -2404,7 +2402,14 @@ public class Request implements HttpServletRequest if (oldQueryParams == null && oldQuery != null) { oldQueryParams = new MultiMap<>(); - UrlEncoded.decodeTo(oldQuery, oldQueryParams, getQueryEncoding()); + try + { + UrlEncoded.decodeTo(oldQuery, oldQueryParams, getQueryEncoding()); + } + catch(Throwable th) + { + throw new BadMessageException(400,"Bad query encoding",th); + } } MultiMap mergedQueryParams; 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 269ad2b44c1..ee2f0e01d0c 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 @@ -50,6 +50,7 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.PathMap; @@ -647,8 +648,15 @@ public class ServletHandler extends ScopedHandler else response.sendError(HttpServletResponse.SC_SERVICE_UNAVAILABLE); } + else if (th instanceof BadMessageException) + { + BadMessageException bme = (BadMessageException)th; + response.sendError(bme.getCode(),bme.getMessage()); + } else + { response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + } } else { diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java index e7fb37e280c..c2657eba638 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java @@ -18,6 +18,7 @@ package org.eclipse.jetty.servlet; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; @@ -59,6 +60,8 @@ import org.eclipse.jetty.server.handler.ResourceHandler; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.UrlEncoded; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.StacklessLogging; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -156,6 +159,42 @@ public class DispatcherTest assertEquals(expected, responses); } + @Test + public void testForwardWithBadParams() throws Exception + { + try(StacklessLogging nostack = new StacklessLogging(ServletHandler.class)) + { + Log.getLogger(ServletHandler.class).info("Expect Not valid UTF8 warnings..."); + _contextHandler.addServlet(AlwaysForwardServlet.class, "/forward/*"); + _contextHandler.addServlet(EchoServlet.class, "/echo/*"); + + String response; + + response = _connector.getResponse("GET /context/forward/?echo=allgood HTTP/1.0\n\n"); + assertThat(response,containsString(" 200 OK")); + assertThat(response,containsString("allgood")); + + response = _connector.getResponse("GET /context/forward/params?echo=allgood HTTP/1.0\n\n"); + assertThat(response,containsString(" 200 OK")); + assertThat(response,containsString("allgood")); + assertThat(response,containsString("forward")); + + response = _connector.getResponse("GET /context/forward/badparams?echo=badparams HTTP/1.0\n\n"); + assertThat(response,containsString(" 500 ")); + + response = _connector.getResponse("GET /context/forward/?echo=badclient&bad=%88%A4 HTTP/1.0\n\n"); + assertThat(response,containsString(" 400 ")); + + response = _connector.getResponse("GET /context/forward/params?echo=badclient&bad=%88%A4 HTTP/1.0\n\n"); + assertThat(response,containsString(" 400 ")); + + response = _connector.getResponse("GET /context/forward/badparams?echo=badclientandparam&bad=%88%A4 HTTP/1.0\n\n"); + assertThat(response,containsString(" 500 ")); + } + } + + + @Test public void testInclude() throws Exception { @@ -358,6 +397,20 @@ public class DispatcherTest dispatcher.forward(request, response); } } + + public static class AlwaysForwardServlet extends HttpServlet implements Servlet + { + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + { + if ("/params".equals(request.getPathInfo())) + getServletContext().getRequestDispatcher("/echo?echo=forward").forward(request, response); + else if ("/badparams".equals(request.getPathInfo())) + getServletContext().getRequestDispatcher("/echo?echo=forward&fbad=%88%A4").forward(request, response); + else + getServletContext().getRequestDispatcher("/echo").forward(request, response); + } + } public static class ForwardNonUTF8Servlet extends HttpServlet implements Servlet @@ -480,15 +533,20 @@ public class DispatcherTest @Override public void service(ServletRequest req, ServletResponse res) throws ServletException, IOException { - String echoText = req.getParameter("echo"); + String[] echoText = req.getParameterValues("echo"); - if ( echoText == null ) + if ( echoText == null || echoText.length==0) { throw new ServletException("echo is a required parameter"); } + else if (echoText.length==1) + { + res.getWriter().print(echoText[0]); + } else { - res.getWriter().print(echoText); + for (String text:echoText) + res.getWriter().print(text); } } } From 44e57f21701b7fda71542b8af0b448754a84667c Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Thu, 23 Aug 2018 21:42:37 +1000 Subject: [PATCH 02/13] Issue #2775 LowResourceMonitor extendable (#2812) * make LowResourceMonitor extendable #2775 Signed-off-by: olivier lamy --- .../main/config/etc/jetty-lowresources.xml | 4 +- .../jetty/server/LowResourceMonitor.java | 606 ++++++++++++------ .../server/CustomResourcesMonitorTest.java | 194 ++++++ .../jetty/server/LowResourcesMonitorTest.java | 113 ++-- 4 files changed, 678 insertions(+), 239 deletions(-) create mode 100644 jetty-server/src/test/java/org/eclipse/jetty/server/CustomResourcesMonitorTest.java diff --git a/jetty-server/src/main/config/etc/jetty-lowresources.xml b/jetty-server/src/main/config/etc/jetty-lowresources.xml index 273ccd3ce6e..4e9fbd8d1ea 100644 --- a/jetty-server/src/main/config/etc/jetty-lowresources.xml +++ b/jetty-server/src/main/config/etc/jetty-lowresources.xml @@ -8,7 +8,7 @@ - + @@ -16,7 +16,7 @@ - + diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/LowResourceMonitor.java b/jetty-server/src/main/java/org/eclipse/jetty/server/LowResourceMonitor.java index e172b9fbeb7..742958ddba7 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/LowResourceMonitor.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/LowResourceMonitor.java @@ -18,6 +18,17 @@ package org.eclipse.jetty.server; +import org.eclipse.jetty.io.EndPoint; +import org.eclipse.jetty.util.annotation.ManagedAttribute; +import org.eclipse.jetty.util.annotation.ManagedObject; +import org.eclipse.jetty.util.annotation.Name; +import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; +import org.eclipse.jetty.util.thread.Scheduler; +import org.eclipse.jetty.util.thread.ThreadPool; + import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -27,66 +38,46 @@ import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; -import org.eclipse.jetty.io.EndPoint; -import org.eclipse.jetty.util.annotation.ManagedAttribute; -import org.eclipse.jetty.util.annotation.ManagedObject; -import org.eclipse.jetty.util.annotation.Name; -import org.eclipse.jetty.util.component.AbstractLifeCycle; -import org.eclipse.jetty.util.log.Log; -import org.eclipse.jetty.util.log.Logger; -import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; -import org.eclipse.jetty.util.thread.Scheduler; -import org.eclipse.jetty.util.thread.ThreadPool; - /** - * A monitor for low resources - *

- * An instance of this class will monitor all the connectors of a server (or a set of connectors - * configured with {@link #setMonitoredConnectors(Collection)}) for a low resources state. - *

- * Low resources can be detected by: + * + * A monitor for low resources, low resources can be detected by: *

    *
  • {@link ThreadPool#isLowOnThreads()} if {@link Connector#getExecutor()} is * an instance of {@link ThreadPool} and {@link #setMonitorThreads(boolean)} is true.
    • *
  • If {@link #setMaxMemory(long)} is non zero then low resources is detected if the JVMs * {@link Runtime} instance has {@link Runtime#totalMemory()} minus {@link Runtime#freeMemory()} * greater than {@link #getMaxMemory()}
    • - *
  • If {@link #setMaxConnections(int)} is non zero then low resources is dected if the total number + *
  • If {@link #setMaxConnections(int)} is non zero then low resources is detected if the total number * of connections exceeds {@link #getMaxConnections()}. This feature is deprecated and replaced by * {@link ConnectionLimit}
    • *
- *

- * Once low resources state is detected, the cause is logged and all existing connections returned - * by {@link Connector#getConnectedEndPoints()} have {@link EndPoint#setIdleTimeout(long)} set - * to {@link #getLowResourcesIdleTimeout()}. New connections are not affected, however if the low - * resources state persists for more than {@link #getMaxLowResourcesTime()}, then the - * {@link #getLowResourcesIdleTimeout()} to all connections again. Once the low resources state is - * cleared, the idle timeout is reset to the connector default given by {@link Connector#getIdleTimeout()}. - *

- * If {@link #setAcceptingInLowResources(boolean)} is set to false (Default is true), then no new connections - * are accepted when in low resources state. + * */ @ManagedObject ("Monitor for low resource conditions and activate a low resource mode if detected") -public class LowResourceMonitor extends AbstractLifeCycle +public class LowResourceMonitor extends ContainerLifeCycle { private static final Logger LOG = Log.getLogger(LowResourceMonitor.class); - private final Server _server; + + protected final Server _server; private Scheduler _scheduler; private Connector[] _monitoredConnectors; private Set _acceptingConnectors = new HashSet<>(); private int _period=1000; - private int _maxConnections; - private long _maxMemory; + + private int _lowResourcesIdleTimeout=1000; private int _maxLowResourcesTime=0; - private boolean _monitorThreads=true; + private final AtomicBoolean _low = new AtomicBoolean(); - private String _cause; + private String _reasons; + private long _lowStarted; private boolean _acceptingInLowResources = true; + private Set _lowResourceChecks = new HashSet<>(); + private final Runnable _monitor = new Runnable() { @Override @@ -95,14 +86,86 @@ public class LowResourceMonitor extends AbstractLifeCycle if (isRunning()) { monitor(); - _scheduler.schedule(_monitor,_period,TimeUnit.MILLISECONDS); + _scheduler.schedule( _monitor, _period, TimeUnit.MILLISECONDS); } } }; public LowResourceMonitor(@Name("server") Server server) { - _server=server; + _server = server; + } + + @ManagedAttribute("True if low available threads status is monitored") + public boolean getMonitorThreads() + { + return !getBeans(ConnectorsThreadPoolLowResourceCheck.class).isEmpty(); + } + + /** + * @param monitorThreads If true, check connectors executors to see if they are + * {@link ThreadPool} instances that are low on threads. + */ + public void setMonitorThreads(boolean monitorThreads) + { + if(monitorThreads) + // already configured? + if ( !getMonitorThreads() ) addLowResourceCheck( new ConnectorsThreadPoolLowResourceCheck() ); + else + getBeans(ConnectorsThreadPoolLowResourceCheck.class).forEach(this::removeBean); + } + + /** + * @return The maximum connections allowed for the monitored connectors before low resource handling is activated + * @deprecated Replaced by ConnectionLimit + */ + @ManagedAttribute("The maximum connections allowed for the monitored connectors before low resource handling is activated") + @Deprecated + public int getMaxConnections() + { + for(MaxConnectionsLowResourceCheck lowResourceCheck : getBeans(MaxConnectionsLowResourceCheck.class)) + { + if (lowResourceCheck.getMaxConnections()>0) + { + return lowResourceCheck.getMaxConnections(); + } + } + return -1; + } + + /** + * @param maxConnections The maximum connections before low resources state is triggered + * @deprecated Replaced by ConnectionLimit + */ + @Deprecated + public void setMaxConnections(int maxConnections) + { + if (maxConnections>0) + { + if (getBeans(MaxConnectionsLowResourceCheck.class).isEmpty()) + { + addLowResourceCheck(new MaxConnectionsLowResourceCheck(maxConnections)); + } else + { + getBeans(MaxConnectionsLowResourceCheck.class).forEach( c -> c.setMaxConnections( maxConnections ) ); + } + } + else + { + getBeans(ConnectorsThreadPoolLowResourceCheck.class).forEach(this::removeBean); + } + } + + + @ManagedAttribute("The reasons the monitored connectors are low on resources") + public String getReasons() + { + return _reasons; + } + + protected void setReasons(String reasons) + { + _reasons = reasons; } @ManagedAttribute("Are the monitored connectors low on resources?") @@ -111,24 +174,39 @@ public class LowResourceMonitor extends AbstractLifeCycle return _low.get(); } + protected boolean enableLowOnResources(boolean expectedValue, boolean newValue) + { + return _low.compareAndSet(expectedValue, newValue); + } + @ManagedAttribute("The reason(s) the monitored connectors are low on resources") public String getLowResourcesReasons() { return _reasons; } + protected void setLowResourcesReasons(String reasons) + { + _reasons = reasons; + } + @ManagedAttribute("Get the timestamp in ms since epoch that low resources state started") public long getLowResourcesStarted() { return _lowStarted; } + public void setLowResourcesStarted(long lowStarted) + { + _lowStarted = lowStarted; + } + @ManagedAttribute("The monitored connectors. If null then all server connectors are monitored") public Collection getMonitoredConnectors() { if (_monitoredConnectors==null) return Collections.emptyList(); - return Arrays.asList(_monitoredConnectors); + return Arrays.asList( _monitoredConnectors); } /** @@ -142,6 +220,13 @@ public class LowResourceMonitor extends AbstractLifeCycle _monitoredConnectors = monitoredConnectors.toArray(new Connector[monitoredConnectors.size()]); } + protected Connector[] getMonitoredOrServerConnectors() + { + if (_monitoredConnectors!=null && _monitoredConnectors.length>0) + return _monitoredConnectors; + return _server.getConnectors(); + } + @ManagedAttribute("If false, new connections are not accepted while in low resources") public boolean isAcceptingInLowResources() { @@ -152,7 +237,7 @@ public class LowResourceMonitor extends AbstractLifeCycle { _acceptingInLowResources = acceptingInLowResources; } - + @ManagedAttribute("The monitor period in ms") public int getPeriod() { @@ -167,58 +252,6 @@ public class LowResourceMonitor extends AbstractLifeCycle _period = periodMS; } - @ManagedAttribute("True if low available threads status is monitored") - public boolean getMonitorThreads() - { - return _monitorThreads; - } - - /** - * @param monitorThreads If true, check connectors executors to see if they are - * {@link ThreadPool} instances that are low on threads. - */ - public void setMonitorThreads(boolean monitorThreads) - { - _monitorThreads = monitorThreads; - } - - /** - * @return The maximum connections allowed for the monitored connectors before low resource handling is activated - * @deprecated Replaced by ConnectionLimit - */ - @ManagedAttribute("The maximum connections allowed for the monitored connectors before low resource handling is activated") - @Deprecated - public int getMaxConnections() - { - return _maxConnections; - } - - /** - * @param maxConnections The maximum connections before low resources state is triggered - * @deprecated Replaced by ConnectionLimit - */ - @Deprecated - public void setMaxConnections(int maxConnections) - { - if (maxConnections>0) - LOG.warn("LowResourceMonitor.setMaxConnections is deprecated. Use ConnectionLimit."); - _maxConnections = maxConnections; - } - - @ManagedAttribute("The maximum memory (in bytes) that can be used before low resources is triggered. Memory used is calculated as (totalMemory-freeMemory).") - public long getMaxMemory() - { - return _maxMemory; - } - - /** - * @param maxMemoryBytes The maximum memory in bytes in use before low resources is triggered. - */ - public void setMaxMemory(long maxMemoryBytes) - { - _maxMemory = maxMemoryBytes; - } - @ManagedAttribute("The idletimeout in ms to apply to all existing connections when low resources is detected") public int getLowResourcesIdleTimeout() { @@ -247,6 +280,99 @@ public class LowResourceMonitor extends AbstractLifeCycle _maxLowResourcesTime = maxLowResourcesTimeMS; } + @ManagedAttribute("The maximum memory (in bytes) that can be used before low resources is triggered. Memory used is calculated as (totalMemory-freeMemory).") + public long getMaxMemory() + { + Collection beans = getBeans(MemoryLowResourceCheck.class); + if(beans.isEmpty()) + { + return 0; + } + return beans.stream().findFirst().get().getMaxMemory(); + } + + /** + * @param maxMemoryBytes The maximum memory in bytes in use before low resources is triggered. + */ + public void setMaxMemory(long maxMemoryBytes) + { + if(maxMemoryBytes<=0) + { + return; + } + Collection beans = getBeans(MemoryLowResourceCheck.class); + if(beans.isEmpty()) + addLowResourceCheck( new MemoryLowResourceCheck( maxMemoryBytes ) ); + else + beans.forEach( lowResourceCheck -> lowResourceCheck.setMaxMemory( maxMemoryBytes ) ); + } + + public Set getLowResourceChecks() + { + return _lowResourceChecks; + } + + public void setLowResourceChecks( Set lowResourceChecks ) + { + updateBeans(_lowResourceChecks.toArray(),lowResourceChecks.toArray()); + this._lowResourceChecks = lowResourceChecks; + } + + public void addLowResourceCheck( LowResourceCheck lowResourceCheck ) + { + addBean(lowResourceCheck); + this._lowResourceChecks.add(lowResourceCheck); + } + + protected void monitor() + { + + String reasons=null; + + + for(LowResourceCheck lowResourceCheck : _lowResourceChecks) + { + if(lowResourceCheck.isLowOnResources()) + { + reasons = lowResourceCheck.toString(); + break; + } + } + + if (reasons!=null) + { + // Log the reasons if there is any change in the cause + if (!reasons.equals(getReasons())) + { + LOG.warn("Low Resources: {}",reasons); + setReasons(reasons); + } + + // Enter low resources state? + if (enableLowOnResources(false,true)) + { + setLowResourcesReasons(reasons); + setLowResourcesStarted(System.currentTimeMillis()); + setLowResources(); + } + + // Too long in low resources state? + if ( getMaxLowResourcesTime()>0 && (System.currentTimeMillis()-getLowResourcesStarted())>getMaxLowResourcesTime()) + setLowResources(); + } + else + { + if (enableLowOnResources(true,false)) + { + LOG.info("Low Resources cleared"); + setLowResourcesReasons(null); + setLowResourcesStarted(0); + setReasons(null); + clearLowResources(); + } + } + } + @Override protected void doStart() throws Exception { @@ -257,6 +383,7 @@ public class LowResourceMonitor extends AbstractLifeCycle _scheduler=new LRMScheduler(); _scheduler.start(); } + super.doStart(); _scheduler.schedule(_monitor,_period,TimeUnit.MILLISECONDS); @@ -265,94 +392,11 @@ public class LowResourceMonitor extends AbstractLifeCycle @Override protected void doStop() throws Exception { - if (_scheduler instanceof LRMScheduler) + if (_scheduler instanceof LRMScheduler ) _scheduler.stop(); super.doStop(); } - protected Connector[] getMonitoredOrServerConnectors() - { - if (_monitoredConnectors!=null && _monitoredConnectors.length>0) - return _monitoredConnectors; - return _server.getConnectors(); - } - - protected void monitor() - { - String reasons=null; - String cause=""; - int connections=0; - - ThreadPool serverThreads = _server.getThreadPool(); - if (_monitorThreads && serverThreads.isLowOnThreads()) - { - reasons=low(reasons,"Server low on threads: "+serverThreads); - cause+="S"; - } - - for(Connector connector : getMonitoredOrServerConnectors()) - { - connections+=connector.getConnectedEndPoints().size(); - - Executor executor = connector.getExecutor(); - if (executor instanceof ThreadPool && executor!=serverThreads) - { - ThreadPool connectorThreads=(ThreadPool)executor; - if (_monitorThreads && connectorThreads.isLowOnThreads()) - { - reasons=low(reasons,"Connector low on threads: "+connectorThreads); - cause+="T"; - } - } - } - - if (_maxConnections>0 && connections>_maxConnections) - { - reasons=low(reasons,"Max Connections exceeded: "+connections+">"+_maxConnections); - cause+="C"; - } - - long memory=Runtime.getRuntime().totalMemory()-Runtime.getRuntime().freeMemory(); - if (_maxMemory>0 && memory>_maxMemory) - { - reasons=low(reasons,"Max memory exceeded: "+memory+">"+_maxMemory); - cause+="M"; - } - - if (reasons!=null) - { - // Log the reasons if there is any change in the cause - if (!cause.equals(_cause)) - { - LOG.warn("Low Resources: {}",reasons); - _cause=cause; - } - - // Enter low resources state? - if (_low.compareAndSet(false,true)) - { - _reasons=reasons; - _lowStarted=System.currentTimeMillis(); - setLowResources(); - } - - // Too long in low resources state? - if (_maxLowResourcesTime>0 && (System.currentTimeMillis()-_lowStarted)>_maxLowResourcesTime) - setLowResources(); - } - else - { - if (_low.compareAndSet(true,false)) - { - LOG.info("Low Resources cleared"); - _reasons=null; - _lowStarted=0; - _cause=null; - clearLowResources(); - } - } - } - protected void setLowResources() { for(Connector connector : getMonitoredOrServerConnectors()) @@ -366,8 +410,8 @@ public class LowResourceMonitor extends AbstractLifeCycle c.setAccepting(false); } } - - for (EndPoint endPoint : connector.getConnectedEndPoints()) + + for ( EndPoint endPoint : connector.getConnectedEndPoints()) endPoint.setIdleTimeout(_lowResourcesIdleTimeout); } } @@ -379,7 +423,7 @@ public class LowResourceMonitor extends AbstractLifeCycle for (EndPoint endPoint : connector.getConnectedEndPoints()) endPoint.setIdleTimeout(connector.getIdleTimeout()); } - + for (AbstractConnector connector : _acceptingConnectors) { connector.setAccepting(true); @@ -387,15 +431,219 @@ public class LowResourceMonitor extends AbstractLifeCycle _acceptingConnectors.clear(); } - private String low(String reasons, String newReason) + protected String low(String reasons, String newReason) { if (reasons==null) return newReason; return reasons+", "+newReason; } - private static class LRMScheduler extends ScheduledExecutorScheduler { } + + interface LowResourceCheck + { + boolean isLowOnResources(); + + String getReason(); + } + + //------------------------------------------------------ + // default implementations for backward compat + //------------------------------------------------------ + + public class MainThreadPoolLowResourceCheck implements LowResourceCheck + { + private String reason; + + public MainThreadPoolLowResourceCheck() + { + // no op + } + + @Override + public boolean isLowOnResources() + { + ThreadPool serverThreads = _server.getThreadPool(); + if (serverThreads.isLowOnThreads()) + { + reason="Server low on threads: "+serverThreads; + return true; + } + return false; + } + + @Override + public String getReason() + { + return reason; + } + + @Override + public String toString() + { + return "Check if the server ThreadPool is lowOnThreads"; + } + } + + public class ConnectorsThreadPoolLowResourceCheck implements LowResourceCheck + { + private String reason; + + public ConnectorsThreadPoolLowResourceCheck() + { + // no op + } + + @Override + public boolean isLowOnResources() + { + ThreadPool serverThreads = _server.getThreadPool(); + if(serverThreads.isLowOnThreads()) + { + reason ="Server low on threads: "+serverThreads.getThreads()+", idleThreads:"+serverThreads.getIdleThreads(); + return true; + } + for(Connector connector : getMonitoredConnectors()) + { + Executor executor = connector.getExecutor(); + if (executor instanceof ThreadPool && executor!=serverThreads) + { + ThreadPool connectorThreads=(ThreadPool)executor; + if (connectorThreads.isLowOnThreads()) + { + reason ="Connector low on threads: "+connectorThreads; + return true; + } + } + } + return false; + } + + @Override + public String getReason() + { + return reason; + } + + @Override + public String toString() + { + return "Check if the ThreadPool from monitored connectors are lowOnThreads and if all connections number is higher than the allowed maxConnection"; + } + } + + @ManagedObject("Check max allowed connections on connectors") + public class MaxConnectionsLowResourceCheck implements LowResourceCheck + { + private String reason; + private int maxConnections; + + public MaxConnectionsLowResourceCheck(int maxConnections) + { + this.maxConnections = maxConnections; + } + + /** + * @return The maximum connections allowed for the monitored connectors before low resource handling is activated + * @deprecated Replaced by ConnectionLimit + */ + @ManagedAttribute("The maximum connections allowed for the monitored connectors before low resource handling is activated") + @Deprecated + public int getMaxConnections() + { + return maxConnections; + } + + /** + * @param maxConnections The maximum connections before low resources state is triggered + * @deprecated Replaced by ConnectionLimit + */ + @Deprecated + public void setMaxConnections(int maxConnections) + { + if (maxConnections>0) + LOG.warn("LowResourceMonitor.setMaxConnections is deprecated. Use ConnectionLimit."); + this.maxConnections = maxConnections; + } + + @Override + public boolean isLowOnResources() + { + int connections=0; + for(Connector connector : getMonitoredConnectors()) + { + connections+=connector.getConnectedEndPoints().size(); + } + if (maxConnections>0 && connections>maxConnections) + { + reason ="Max Connections exceeded: "+connections+">"+maxConnections; + return true; + } + return false; + } + + @Override + public String getReason() + { + return reason; + } + + @Override + public String toString() + { + return "All connections number is higher than the allowed maxConnection"; + } + } + + public class MemoryLowResourceCheck implements LowResourceCheck + { + private String reason; + private long maxMemory; + + public MemoryLowResourceCheck(long maxMemory) + { + this.maxMemory = maxMemory; + } + + @Override + public boolean isLowOnResources() + { + long memory=Runtime.getRuntime().totalMemory()-Runtime.getRuntime().freeMemory(); + if (maxMemory>0 && memory>maxMemory) + { + reason = "Max memory exceeded: "+memory+">"+maxMemory; + return true; + } + return false; + } + + public long getMaxMemory() + { + return maxMemory; + } + + /** + * @param maxMemoryBytes The maximum memory in bytes in use before low resources is triggered. + */ + public void setMaxMemory( long maxMemoryBytes ) + { + this.maxMemory = maxMemoryBytes; + } + + @Override + public String getReason() + { + return reason; + } + + @Override + public String toString() + { + return "Check if used memory is higher than the allowed max memory"; + } + } + + } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/CustomResourcesMonitorTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/CustomResourcesMonitorTest.java new file mode 100644 index 00000000000..486cb80afe5 --- /dev/null +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/CustomResourcesMonitorTest.java @@ -0,0 +1,194 @@ +// +// ======================================================================== +// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.server; + + +import org.eclipse.jetty.toolchain.test.AdvancedRunner; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.util.thread.TimerScheduler; +import org.hamcrest.Matchers; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.io.IOException; +import java.io.InputStream; +import java.net.Socket; +import java.net.SocketTimeoutException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.junit.Assert.*; + +@RunWith(AdvancedRunner.class) +public class CustomResourcesMonitorTest +{ + Server _server; + ServerConnector _connector; + FileOnDirectoryMonitor _fileOnDirectoryMonitor; + Path _monitoredPath; + LowResourceMonitor _lowResourceMonitor; + + @Before + public void before() throws Exception + { + _server = new Server(); + + _server.addBean(new TimerScheduler()); + + _connector = new ServerConnector(_server); + _connector.setPort(0); + _connector.setIdleTimeout(35000); + _server.addConnector(_connector); + + _server.setHandler(new DumpHandler()); + + _monitoredPath = Files.createTempDirectory( "jetty_test" ); + _fileOnDirectoryMonitor=new FileOnDirectoryMonitor(_monitoredPath); + _lowResourceMonitor = new LowResourceMonitor(_server); + _server.addBean( _lowResourceMonitor ); + _lowResourceMonitor.addLowResourceCheck( _fileOnDirectoryMonitor ); + _server.start(); + } + + @After + public void after() throws Exception + { + _server.stop(); + } + + @Test + public void testFileOnDirectoryMonitor() throws Exception + { + int monitorPeriod = _lowResourceMonitor.getPeriod(); + int lowResourcesIdleTimeout = _lowResourceMonitor.getLowResourcesIdleTimeout(); + assertThat(lowResourcesIdleTimeout, Matchers.lessThanOrEqualTo(monitorPeriod)); + + int maxLowResourcesTime = 5 * monitorPeriod; + _lowResourceMonitor.setMaxLowResourcesTime(maxLowResourcesTime); + assertFalse(_fileOnDirectoryMonitor.isLowOnResources()); + + try(Socket socket0 = new Socket("localhost",_connector.getLocalPort())) + { + Path tmpFile = Files.createTempFile( _monitoredPath, "yup", ".tmp" ); + // Write a file + Files.write(tmpFile, "foobar".getBytes()); + + // Wait a couple of monitor periods so that + // fileOnDirectoryMonitor detects it is in low mode. + Thread.sleep(2 * monitorPeriod); + assertTrue(_fileOnDirectoryMonitor.isLowOnResources()); + + + // We already waited enough for fileOnDirectoryMonitor to close socket0. + assertEquals(-1, socket0.getInputStream().read()); + + // New connections are not affected by the + // low mode until maxLowResourcesTime elapses. + try(Socket socket1 = new Socket("localhost",_connector.getLocalPort())) + { + // Set a very short read timeout so we can test if the server closed. + socket1.setSoTimeout(1); + InputStream input1 = socket1.getInputStream(); + + assertTrue(_fileOnDirectoryMonitor.isLowOnResources()); + try + { + input1.read(); + fail(); + } + catch (SocketTimeoutException expected) + { + } + + // Wait a couple of lowResources idleTimeouts. + Thread.sleep(2 * lowResourcesIdleTimeout); + + // Verify the new socket is still open. + assertTrue(_fileOnDirectoryMonitor.isLowOnResources()); + try + { + input1.read(); + fail(); + } + catch (SocketTimeoutException expected) + { + } + + Files.delete( tmpFile ); + + // Let the maxLowResourcesTime elapse. + Thread.sleep(maxLowResourcesTime); + assertFalse(_fileOnDirectoryMonitor.isLowOnResources()); + } + } + } + + + static class FileOnDirectoryMonitor implements LowResourceMonitor.LowResourceCheck + { + private static final Logger LOG = Log.getLogger( FileOnDirectoryMonitor.class); + + private final Path _pathToMonitor; + + private String reason; + + public FileOnDirectoryMonitor(Path pathToMonitor ) + { + _pathToMonitor = pathToMonitor; + } + + @Override + public boolean isLowOnResources() + { + try + { + Stream paths = Files.list( _pathToMonitor ); + List content = paths.collect( Collectors.toList() ); + if(!content.isEmpty()) + { + reason= "directory not empty so enable low resources"; + return true; + } + } + catch ( IOException e ) + { + LOG.info( "ignore issue looking at directory content", e ); + } + return false; + } + + @Override + public String getReason() + { + return reason; + } + + @Override + public String toString() + { + return getClass().getName(); + } + } +} diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/LowResourcesMonitorTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/LowResourcesMonitorTest.java index c95821a2487..7b70fbf1dab 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/LowResourcesMonitorTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/LowResourcesMonitorTest.java @@ -19,27 +19,27 @@ package org.eclipse.jetty.server; -import java.io.InputStream; -import java.net.Socket; -import java.net.SocketTimeoutException; -import java.nio.charset.StandardCharsets; -import java.util.Arrays; -import java.util.concurrent.CountDownLatch; - import org.eclipse.jetty.toolchain.test.AdvancedRunner; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.eclipse.jetty.util.thread.TimerScheduler; import org.hamcrest.Matchers; import org.junit.After; -import org.junit.Assert; import org.junit.Before; import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; +import java.io.InputStream; +import java.net.Socket; +import java.net.SocketTimeoutException; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Collections; +import java.util.concurrent.CountDownLatch; + import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.*; @RunWith(AdvancedRunner.class) public class LowResourcesMonitorTest @@ -48,7 +48,7 @@ public class LowResourcesMonitorTest Server _server; ServerConnector _connector; LowResourceMonitor _lowResourcesMonitor; - + @Before public void before() throws Exception { @@ -71,6 +71,7 @@ public class LowResourcesMonitorTest _lowResourcesMonitor.setLowResourcesIdleTimeout(200); _lowResourcesMonitor.setMaxConnections(20); _lowResourcesMonitor.setPeriod(900); + _lowResourcesMonitor.setMonitoredConnectors( Collections.singleton( _connector ) ); _server.addBean(_lowResourcesMonitor); _server.start(); @@ -86,38 +87,35 @@ public class LowResourcesMonitorTest @Test public void testLowOnThreads() throws Exception { + _lowResourcesMonitor.setMonitorThreads(true); Thread.sleep(1200); _threadPool.setMaxThreads(_threadPool.getThreads()-_threadPool.getIdleThreads()+10); Thread.sleep(1200); - Assert.assertFalse(_lowResourcesMonitor.isLowOnResources()); + assertFalse(_lowResourcesMonitor.getReasons(), _lowResourcesMonitor.isLowOnResources()); final CountDownLatch latch = new CountDownLatch(1); for (int i=0;i<100;i++) { - _threadPool.execute(new Runnable() + _threadPool.execute(() -> { - @Override - public void run() + try { - try - { - latch.await(); - } - catch (InterruptedException e) - { - e.printStackTrace(); - } + latch.await(); + } + catch (InterruptedException e) + { + e.printStackTrace(); } }); } Thread.sleep(1200); - Assert.assertTrue(_lowResourcesMonitor.isLowOnResources()); + assertTrue(_lowResourcesMonitor.isLowOnResources()); latch.countDown(); Thread.sleep(1200); - Assert.assertFalse(_lowResourcesMonitor.isLowOnResources()); + assertFalse(_lowResourcesMonitor.getReasons(),_lowResourcesMonitor.isLowOnResources()); } @@ -125,10 +123,13 @@ public class LowResourcesMonitorTest public void testNotAccepting() throws Exception { _lowResourcesMonitor.setAcceptingInLowResources(false); + _lowResourcesMonitor.setMonitorThreads( true ); Thread.sleep(1200); - _threadPool.setMaxThreads(_threadPool.getThreads()-_threadPool.getIdleThreads()+10); + int maxThreads = _threadPool.getThreads()-_threadPool.getIdleThreads()+10; + System.out.println("maxThreads:"+maxThreads); + _threadPool.setMaxThreads(maxThreads); Thread.sleep(1200); - Assert.assertFalse(_lowResourcesMonitor.isLowOnResources()); + assertFalse(_lowResourcesMonitor.getReasons(),_lowResourcesMonitor.isLowOnResources()); for (AbstractConnector c : _server.getBeans(AbstractConnector.class)) assertThat(c.isAccepting(),Matchers.is(true)); @@ -136,31 +137,27 @@ public class LowResourcesMonitorTest final CountDownLatch latch = new CountDownLatch(1); for (int i=0;i<100;i++) { - _threadPool.execute(new Runnable() + _threadPool.execute(() -> { - @Override - public void run() + try { - try - { - latch.await(); - } - catch (InterruptedException e) - { - e.printStackTrace(); - } + latch.await(); + } + catch (InterruptedException e) + { + e.printStackTrace(); } }); } Thread.sleep(1200); - Assert.assertTrue(_lowResourcesMonitor.isLowOnResources()); + assertTrue(_lowResourcesMonitor.isLowOnResources()); for (AbstractConnector c : _server.getBeans(AbstractConnector.class)) assertThat(c.isAccepting(),Matchers.is(false)); latch.countDown(); Thread.sleep(1200); - Assert.assertFalse(_lowResourcesMonitor.isLowOnResources()); + assertFalse(_lowResourcesMonitor.getReasons(),_lowResourcesMonitor.isLowOnResources()); for (AbstractConnector c : _server.getBeans(AbstractConnector.class)) assertThat(c.isAccepting(),Matchers.is(true)); } @@ -172,7 +169,7 @@ public class LowResourcesMonitorTest { _lowResourcesMonitor.setMaxMemory(Runtime.getRuntime().totalMemory()-Runtime.getRuntime().freeMemory()+(100*1024*1024)); Thread.sleep(1200); - Assert.assertFalse(_lowResourcesMonitor.isLowOnResources()); + assertFalse(_lowResourcesMonitor.getReasons(),_lowResourcesMonitor.isLowOnResources()); byte[] data = new byte[100*1024*1024]; Arrays.fill(data,(byte)1); @@ -180,13 +177,13 @@ public class LowResourcesMonitorTest assertThat(hash,not(equalTo(0))); Thread.sleep(1200); - Assert.assertTrue(_lowResourcesMonitor.isLowOnResources()); + assertTrue(_lowResourcesMonitor.isLowOnResources()); data=null; System.gc(); System.gc(); Thread.sleep(1200); - Assert.assertFalse(_lowResourcesMonitor.isLowOnResources()); + assertFalse(_lowResourcesMonitor.getReasons(),_lowResourcesMonitor.isLowOnResources()); } @@ -194,26 +191,27 @@ public class LowResourcesMonitorTest public void testMaxConnectionsAndMaxIdleTime() throws Exception { _lowResourcesMonitor.setMaxMemory(0); - Assert.assertFalse(_lowResourcesMonitor.isLowOnResources()); + assertFalse(_lowResourcesMonitor.getReasons(),_lowResourcesMonitor.isLowOnResources()); + assertEquals( 20, _lowResourcesMonitor.getMaxConnections() ); Socket[] socket = new Socket[_lowResourcesMonitor.getMaxConnections()+1]; for (int i=0;i Date: Fri, 24 Aug 2018 10:15:55 +1000 Subject: [PATCH 03/13] Issue #2798 thread budget warning default to available cores (#2799) * Issue #2798 thread budget warning default to available cores * Issue #2798 - Testcase example for ExecutorThreadPool * Issue #2798 - Better TestCases for ThreadPoolBudget * Simplified * Issue #2798 - ThreadPoolBudget improvements. * Removed unused imports. * Made fields private. * Removed field info. * Changed the logic to print the info on leases only when warning or throwing. Signed-off-by: Greg Wilkins Signed-off-by: Joakim Erdfelt Signed-off-by: Simone Bordet --- .../jetty/util/thread/ExecutorThreadPool.java | 4 +- .../jetty/util/thread/QueuedThreadPool.java | 5 +- .../jetty/util/thread/ThreadPoolBudget.java | 80 ++++++++-------- .../util/thread/AbstractThreadPoolTest.java | 94 +++++++++++++++++++ .../util/thread/ExecutorThreadPoolTest.java | 30 ++++++ .../util/thread/QueuedThreadPoolTest.java | 30 +++--- 6 files changed, 192 insertions(+), 51 deletions(-) create mode 100644 jetty-util/src/test/java/org/eclipse/jetty/util/thread/AbstractThreadPoolTest.java create mode 100644 jetty-util/src/test/java/org/eclipse/jetty/util/thread/ExecutorThreadPoolTest.java diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ExecutorThreadPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ExecutorThreadPool.java index b99314a53b9..0f6a4c98d89 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ExecutorThreadPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ExecutorThreadPool.java @@ -95,7 +95,7 @@ public class ExecutorThreadPool extends ContainerLifeCycle implements ThreadPool _group = group; _minThreads = minThreads; _reservedThreads = reservedThreads; - _budget = new ThreadPoolBudget(this,minThreads); + _budget = new ThreadPoolBudget(this); } /** @@ -140,6 +140,8 @@ public class ExecutorThreadPool extends ContainerLifeCycle implements ThreadPool @Override public void setMaxThreads(int threads) { + if (_budget!=null) + _budget.check(threads); _executor.setCorePoolSize(threads); _executor.setMaximumPoolSize(threads); } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java index ab950cbbc92..72c8e51012b 100755 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java @@ -32,6 +32,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import org.eclipse.jetty.util.BlockingArrayQueue; +import org.eclipse.jetty.util.ProcessorUtils; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.annotation.ManagedOperation; @@ -117,7 +118,7 @@ public class QueuedThreadPool extends ContainerLifeCycle implements SizedThreadP } _jobs=queue; _threadGroup=threadGroup; - setThreadPoolBudget(new ThreadPoolBudget(this,_minThreads)); + setThreadPoolBudget(new ThreadPoolBudget(this)); } @Override @@ -257,6 +258,8 @@ public class QueuedThreadPool extends ContainerLifeCycle implements SizedThreadP @Override public void setMaxThreads(int maxThreads) { + if (_budget!=null) + _budget.check(maxThreads); _maxThreads = maxThreads; if (_minThreads > _maxThreads) _minThreads = _maxThreads; diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPoolBudget.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPoolBudget.java index 2269effe41f..d83f827ab19 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPoolBudget.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPoolBudget.java @@ -19,13 +19,11 @@ package org.eclipse.jetty.util.thread; import java.io.Closeable; -import java.io.IOException; import java.util.Set; import java.util.concurrent.CopyOnWriteArraySet; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicBoolean; -import org.eclipse.jetty.util.ProcessorUtils; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -48,8 +46,8 @@ public class ThreadPoolBudget */ public class Leased implements Lease { - final Object leasee; - final int threads; + private final Object leasee; + private final int threads; private Leased(Object leasee,int threads) { @@ -66,8 +64,7 @@ public class ThreadPoolBudget @Override public void close() { - info.remove(this); - allocations.remove(this); + leases.remove(this); warned.set(false); } } @@ -75,7 +72,7 @@ public class ThreadPoolBudget private static final Lease NOOP_LEASE = new Lease() { @Override - public void close() throws IOException + public void close() { } @@ -86,25 +83,26 @@ public class ThreadPoolBudget } }; - final ThreadPool.SizedThreadPool pool; - final Set allocations = new CopyOnWriteArraySet<>(); - final Set info = new CopyOnWriteArraySet<>(); - final AtomicBoolean warned = new AtomicBoolean(); - final int warnAt; + private final Set leases = new CopyOnWriteArraySet<>(); + private final AtomicBoolean warned = new AtomicBoolean(); + private final ThreadPool.SizedThreadPool pool; + private final int warnAt; /** - * Construct a budget for a SizedThreadPool, with the warning level set by heuristic. + * Construct a budget for a SizedThreadPool. * @param pool The pool to budget thread allocation for. */ public ThreadPoolBudget(ThreadPool.SizedThreadPool pool) { - this(pool,Math.min(ProcessorUtils.availableProcessors(),pool.getMinThreads())); + this.pool = pool; + this.warnAt = -1; } /** * @param pool The pool to budget thread allocation for. * @param warnAt The level of free threads at which a warning is generated. */ + @Deprecated public ThreadPoolBudget(ThreadPool.SizedThreadPool pool, int warnAt) { this.pool = pool; @@ -118,52 +116,60 @@ public class ThreadPoolBudget public void reset() { - allocations.clear(); - info.clear(); + leases.clear(); warned.set(false); } public Lease leaseTo(Object leasee, int threads) { Leased lease = new Leased(leasee,threads); - allocations.add(lease); - check(); - return lease; + leases.add(lease); + try + { + check(pool.getMaxThreads()); + return lease; + } + catch(IllegalStateException e) + { + lease.close(); + throw e; + } } /** - * Check registered allocations against the budget. + *

Checks leases against the given number of {@code maxThreads}.

+ * + * @param maxThreads A proposed change to the maximum threads to check. + * @return true if passes check, false if otherwise (see logs for details) * @throws IllegalStateException if insufficient threads are configured. */ - public void check() throws IllegalStateException + public boolean check(int maxThreads) throws IllegalStateException { - int required = allocations.stream() + int required = leases.stream() .mapToInt(Lease::getThreads) .sum(); - int maximum = pool.getMaxThreads(); - int actual = maximum - required; - - if (actual <= 0) + int left = maxThreads - required; + if (left <= 0) { - infoOnLeases(); - throw new IllegalStateException(String.format("Insufficient configured threads: required=%d < max=%d for %s", required, maximum, pool)); + printInfoOnLeases(); + throw new IllegalStateException(String.format("Insufficient configured threads: required=%d < max=%d for %s", required, maxThreads, pool)); } - if (actual < warnAt) + if (left < warnAt) { - infoOnLeases(); if (warned.compareAndSet(false,true)) - LOG.warn("Low configured threads: (max={} - required={})={} < warnAt={} for {}", maximum, required, actual, warnAt, pool); + { + printInfoOnLeases(); + LOG.info("Low configured threads: (max={} - required={})={} < warnAt={} for {}", maxThreads, required, left, warnAt, pool); + } + return false; } + return true; } - private void infoOnLeases() + private void printInfoOnLeases() { - allocations.stream().filter(lease->!info.contains(lease)) - .forEach(lease->{ - info.add(lease); - LOG.info("{} requires {} threads from {}",lease.leasee,lease.getThreads(),pool); - }); + leases.forEach(lease-> LOG.info("{} requires {} threads from {}",lease.leasee,lease.getThreads(),pool)); } public static Lease leaseFrom(Executor executor, Object leasee, int threads) diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/AbstractThreadPoolTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/AbstractThreadPoolTest.java new file mode 100644 index 00000000000..03ba85e50b1 --- /dev/null +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/AbstractThreadPoolTest.java @@ -0,0 +1,94 @@ +// +// ======================================================================== +// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.util.thread; + +import static org.hamcrest.MatcherAssert.assertThat; + +import org.eclipse.jetty.util.ProcessorUtils; +import org.eclipse.jetty.util.component.LifeCycle; +import org.eclipse.jetty.util.thread.ThreadPool.SizedThreadPool; +import org.hamcrest.Matcher; +import org.hamcrest.Matchers; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; + +public abstract class AbstractThreadPoolTest +{ + private static int originalCoreCount; + + @BeforeClass + public static void saveState() + { + originalCoreCount = ProcessorUtils.availableProcessors(); + } + + @AfterClass + public static void restoreState() + { + ProcessorUtils.setAvailableProcessors(originalCoreCount); + } + + abstract protected SizedThreadPool newPool(int max); + + @Test + public void testBudget_constructMaxThenLease() + { + SizedThreadPool pool = newPool(4); + + pool.getThreadPoolBudget().leaseTo(this,2); + + try + { + pool.getThreadPoolBudget().leaseTo(this,3); + Assert.fail(); + } + catch(IllegalStateException e) + { + Assert.assertThat(e.getMessage(),Matchers.containsString("Insufficient configured threads")); + } + + pool.getThreadPoolBudget().leaseTo(this,1); + } + + @Test + public void testBudget_LeaseThenSetMax() + { + SizedThreadPool pool = newPool(4); + + pool.getThreadPoolBudget().leaseTo(this,2); + + pool.setMaxThreads(3); + + try + { + pool.setMaxThreads(1); + Assert.fail(); + } + catch(IllegalStateException e) + { + Assert.assertThat(e.getMessage(),Matchers.containsString("Insufficient configured threads")); + } + + Assert.assertThat(pool.getMaxThreads(),Matchers.is(3)); + + } + +} diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/ExecutorThreadPoolTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/ExecutorThreadPoolTest.java new file mode 100644 index 00000000000..dcf0dbe28eb --- /dev/null +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/ExecutorThreadPoolTest.java @@ -0,0 +1,30 @@ +// +// ======================================================================== +// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.util.thread; + +import org.eclipse.jetty.util.thread.ThreadPool.SizedThreadPool; + +public class ExecutorThreadPoolTest extends AbstractThreadPoolTest +{ + @Override + protected SizedThreadPool newPool(int max) + { + return new ExecutorThreadPool(max); + } +} diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java index fa261cb5d7a..e00fd5f85f8 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/QueuedThreadPoolTest.java @@ -18,16 +18,6 @@ package org.eclipse.jetty.util.thread; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; - -import org.eclipse.jetty.toolchain.test.AdvancedRunner; -import org.eclipse.jetty.util.log.StacklessLogging; -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; - import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; @@ -35,8 +25,17 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; -@RunWith(AdvancedRunner.class) -public class QueuedThreadPoolTest +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import org.eclipse.jetty.util.ProcessorUtils; +import org.eclipse.jetty.util.log.StacklessLogging; +import org.eclipse.jetty.util.thread.ThreadPool.SizedThreadPool; +import org.junit.Assert; +import org.junit.Test; + +public class QueuedThreadPoolTest extends AbstractThreadPoolTest { private final AtomicInteger _jobs=new AtomicInteger(); @@ -297,4 +296,11 @@ public class QueuedThreadPoolTest { new QueuedThreadPool(4, 8); } + + @Override + protected SizedThreadPool newPool(int max) + { + return new QueuedThreadPool(max); + } + } From 5e07592a692e7400cd641e608decd8e0c942872d Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 23 Aug 2018 18:08:35 -0500 Subject: [PATCH 04/13] Issue #2807 - Updating Default TLS Cipher Suite Exclusions Signed-off-by: Joakim Erdfelt --- .../eclipse/jetty/util/ssl/SslContextFactory.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java index 206f6017930..70a87b6e9dc 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java @@ -212,7 +212,20 @@ public class SslContextFactory extends AbstractLifeCycle implements Dumpable { setTrustAll(trustAll); addExcludeProtocols("SSL", "SSLv2", "SSLv2Hello", "SSLv3"); + + // Exclude weak / insecure ciphers setExcludeCipherSuites("^.*_(MD5|SHA|SHA1)$"); + // Exclude ciphers that don't support forward secrecy + addExcludeCipherSuites("^TLS_RSA_.*$"); + + /* The following exclusions are present to cleanup known bad cipher suites + * that are accidentally added via Include patterns. + * The default enabled cipher list in Java will not include these + * (but they are available in the supported list) */ + addExcludeCipherSuites("^SSL_.*$"); + addExcludeCipherSuites("^.*_NULL_.*$"); + addExcludeCipherSuites("^.*_anon_.*$"); + if (keyStorePath != null) setKeyStorePath(keyStorePath); } From 325f9438346fc2a0d5e48f9a9d568863efe7d5ae Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 24 Aug 2018 17:24:40 +0200 Subject: [PATCH 05/13] Issue #2807 - Updating Default TLS Cipher Suite Exclusions. Clarified comment about exclusions. --- .../org/eclipse/jetty/util/ssl/SslContextFactory.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java index 70a87b6e9dc..b86a10f98e6 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java @@ -217,11 +217,10 @@ public class SslContextFactory extends AbstractLifeCycle implements Dumpable setExcludeCipherSuites("^.*_(MD5|SHA|SHA1)$"); // Exclude ciphers that don't support forward secrecy addExcludeCipherSuites("^TLS_RSA_.*$"); - - /* The following exclusions are present to cleanup known bad cipher suites - * that are accidentally added via Include patterns. - * The default enabled cipher list in Java will not include these - * (but they are available in the supported list) */ + // The following exclusions are present to cleanup known bad cipher + // suites that may be accidentally included via include patterns. + // The default enabled cipher list in Java will not include these + // (but they are available in the supported list). addExcludeCipherSuites("^SSL_.*$"); addExcludeCipherSuites("^.*_NULL_.*$"); addExcludeCipherSuites("^.*_anon_.*$"); From 33a1367325530ec38254a279abb30946505af485 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 24 Aug 2018 17:32:35 +0200 Subject: [PATCH 06/13] Fixes #2821 - AuthenticationProtocolHandler should not always cache Authentication.Result. Now the Result is cached only if getURI() returns non-null. --- .../org/eclipse/jetty/client/HttpAuthenticationStore.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpAuthenticationStore.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpAuthenticationStore.java index db94e28a4e1..359c0a77575 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpAuthenticationStore.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpAuthenticationStore.java @@ -65,7 +65,9 @@ public class HttpAuthenticationStore implements AuthenticationStore @Override public void addAuthenticationResult(Authentication.Result result) { - results.put(result.getURI(), result); + URI uri = result.getURI(); + if (uri != null) + results.put(uri, result); } @Override From 084d6ce4439005299469e6b010e2b855bcf780e0 Mon Sep 17 00:00:00 2001 From: Ivanov Anton Date: Sat, 25 Aug 2018 18:27:18 +0300 Subject: [PATCH 07/13] Issue #2860 Fix leakage of HttpDestinations in HttpClient Signed-off-by: Ivanov Anton --- .../jetty/client/PoolingHttpDestination.java | 32 +++++++++++++------ .../http/HttpDestinationOverHTTPTest.java | 26 +++++++++++++++ 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java b/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java index e56d1787d3a..9e61f404046 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java @@ -210,16 +210,7 @@ public abstract class PoolingHttpDestination extends HttpD if (getHttpExchanges().isEmpty()) { - if (getHttpClient().isRemoveIdleDestinations() && connectionPool.isEmpty()) - { - // There is a race condition between this thread removing the destination - // and another thread queueing a request to this same destination. - // If this destination is removed, but the request queued, a new connection - // will be opened, the exchange will be executed and eventually the connection - // will idle timeout and be closed. Meanwhile a new destination will be created - // in HttpClient and will be used for other requests. - getHttpClient().removeDestination(this); - } + removeDestinationIfIdle(); } else { @@ -237,6 +228,27 @@ public abstract class PoolingHttpDestination extends HttpD connectionPool.close(); } + public void abort(Throwable cause) + { + super.abort(cause); + if (getHttpExchanges().isEmpty()) { + removeDestinationIfIdle(); + } + } + + private void removeDestinationIfIdle() { + if (getHttpClient().isRemoveIdleDestinations() && connectionPool.isEmpty()) + { + // There is a race condition between this thread removing the destination + // and another thread queueing a request to this same destination. + // If this destination is removed, but the request queued, a new connection + // will be opened, the exchange will be executed and eventually the connection + // will idle timeout and be closed. Meanwhile a new destination will be created + // in HttpClient and will be used for other requests. + getHttpClient().removeDestination(this); + } + } + @Override public String toString() { diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java index 9720ce97917..6929be4bac4 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java @@ -39,6 +39,7 @@ import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.hamcrest.Matchers; import org.junit.Assert; +import org.junit.Assume; import org.junit.Before; import org.junit.Test; @@ -280,6 +281,31 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest Assert.assertNotSame(destinationBefore, destinationAfter); } + @Test + public void testDestinationIsRemovedAfterConnectionError() throws Exception + { + String host = "localhost"; + int port = connector.getLocalPort(); + client.setRemoveIdleDestinations(true); + Assume.assumeTrue("Destinations of a fresh client must be empty", client.getDestinations().isEmpty()); + + server.stop(); + Request request = client.newRequest(host, port).scheme(this.scheme); + try + { + request.send(); + Assume.assumeTrue("Request to a closed port must fail", false); + } catch (Exception ignored) { + } + + long deadline = System.currentTimeMillis() + 100; + while (!client.getDestinations().isEmpty() && System.currentTimeMillis() < deadline) + { + Thread.yield(); + } + Assert.assertTrue("Destination must be removed after connection error", client.getDestinations().isEmpty()); + } + private Connection timedPoll(Queue connections, long time, TimeUnit unit) throws InterruptedException { long start = System.nanoTime(); From a458bfaaf4ecde2d02416b8c9a26ea6c0fb7e20d Mon Sep 17 00:00:00 2001 From: Ivanov Anton Date: Sun, 26 Aug 2018 21:34:20 +0300 Subject: [PATCH 08/13] Issue #2860 fixes after review --- .../jetty/client/PoolingHttpDestination.java | 10 +++++----- .../client/http/HttpDestinationOverHTTPTest.java | 14 +++++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java b/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java index 9e61f404046..1fbc1eba9e5 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java @@ -210,7 +210,7 @@ public abstract class PoolingHttpDestination extends HttpD if (getHttpExchanges().isEmpty()) { - removeDestinationIfIdle(); + tryRemoveIdleDestination(); } else { @@ -231,12 +231,12 @@ public abstract class PoolingHttpDestination extends HttpD public void abort(Throwable cause) { super.abort(cause); - if (getHttpExchanges().isEmpty()) { - removeDestinationIfIdle(); - } + if (getHttpExchanges().isEmpty()) + tryRemoveIdleDestination(); } - private void removeDestinationIfIdle() { + private void tryRemoveIdleDestination() + { if (getHttpClient().isRemoveIdleDestinations() && connectionPool.isEmpty()) { // There is a race condition between this thread removing the destination diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java index 6929be4bac4..a1ea9f6f46c 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java @@ -39,7 +39,6 @@ import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.hamcrest.Matchers; import org.junit.Assert; -import org.junit.Assume; import org.junit.Before; import org.junit.Test; @@ -287,21 +286,22 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest String host = "localhost"; int port = connector.getLocalPort(); client.setRemoveIdleDestinations(true); - Assume.assumeTrue("Destinations of a fresh client must be empty", client.getDestinations().isEmpty()); + Assert.assertTrue("Destinations of a fresh client must be empty", client.getDestinations().isEmpty()); server.stop(); Request request = client.newRequest(host, port).scheme(this.scheme); try { request.send(); - Assume.assumeTrue("Request to a closed port must fail", false); - } catch (Exception ignored) { + Assert.fail("Request to a closed port must fail"); + } catch (Exception ignored) + { } - long deadline = System.currentTimeMillis() + 100; - while (!client.getDestinations().isEmpty() && System.currentTimeMillis() < deadline) + long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(100); + while (!client.getDestinations().isEmpty() && System.nanoTime() < deadline) { - Thread.yield(); + Thread.sleep(10); } Assert.assertTrue("Destination must be removed after connection error", client.getDestinations().isEmpty()); } From 3d87265a0a94758bd0e97f51b894c8f83bfc4ceb Mon Sep 17 00:00:00 2001 From: Ivanov Anton Date: Sun, 26 Aug 2018 21:37:50 +0300 Subject: [PATCH 09/13] Issue #2860 fix after review --- .../eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java index a1ea9f6f46c..a068998bef8 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java @@ -294,7 +294,7 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest { request.send(); Assert.fail("Request to a closed port must fail"); - } catch (Exception ignored) + } catch (Exception expected) { } From 03c0d58727a4555aab419577bbf0e25ae0c16211 Mon Sep 17 00:00:00 2001 From: Ivanov Anton Date: Mon, 27 Aug 2018 08:24:26 +0300 Subject: [PATCH 10/13] Issue #2860 fix after review --- .../eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java index a068998bef8..9e345afc093 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java @@ -298,7 +298,7 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest { } - long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(100); + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(1); while (!client.getDestinations().isEmpty() && System.nanoTime() < deadline) { Thread.sleep(10); From 6211564f797ee2a09b8216c9aa0990f913bfa475 Mon Sep 17 00:00:00 2001 From: Ivanov Anton Date: Mon, 27 Aug 2018 08:28:07 +0300 Subject: [PATCH 11/13] Issue #2860 fix after review --- .../eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java index 9e345afc093..58a32f4f636 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java @@ -294,7 +294,8 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest { request.send(); Assert.fail("Request to a closed port must fail"); - } catch (Exception expected) + } + catch (Exception expected) { } From 09be34e08b2cd8a91e4c4a9c62561b2abe53a61a Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Tue, 28 Aug 2018 09:53:06 +1000 Subject: [PATCH 12/13] Issue #2844 Clean up webdefault and DefaultServlet documentation (#2851) Signed-off-by: Jan Bartel --- .../extras/default-servlet.adoc | 43 ++++++++++++++----- .../eclipse/jetty/servlet/DefaultServlet.java | 3 +- .../src/main/config/etc/webdefault.xml | 38 +++++----------- .../demo-base/webapps/test.d/override-web.xml | 8 ++++ 4 files changed, 53 insertions(+), 39 deletions(-) diff --git a/jetty-documentation/src/main/asciidoc/administration/extras/default-servlet.adoc b/jetty-documentation/src/main/asciidoc/administration/extras/default-servlet.adoc index 300b08f7f17..766eb40e5c1 100644 --- a/jetty-documentation/src/main/asciidoc/administration/extras/default-servlet.adoc +++ b/jetty-documentation/src/main/asciidoc/administration/extras/default-servlet.adoc @@ -40,18 +40,39 @@ See the `DefaultServlet` link:{JDURL}/org/eclipse/jetty/servlet/DefaultServlet.h Jetty supports the following `initParameters`: acceptRanges:: -If true, range requests and responses are supported. +If `true`, range requests and responses are supported. dirAllowed:: -If true, directory listings are returned if no welcome file is found. +If `true`, directory listings are returned if no welcome file is found. Otherwise 403 Forbidden displays. redirectWelcome:: -If true, welcome files are redirected rather that forwarded. +If `true`, welcome files are redirected rather that forwarded. +welcomeServlets:: +If `true`, attempt to dispatch to welcome files that are servlets, but only after no matching static +resources could be found. If `false`, then a welcome file must exist on disk. If `exact`, then exact +servlet matches are supported without an existing file. Default is `true`. This must be `false` if you want directory listings, +but have index.jsp in your welcome file list. +precompressed:: +If set to a comma separated list of encoding types (that may be listed in a requests Accept-Encoding header) to file +extension mappings to look for and serve. For example: +`br=.br,gzip=.gz,bzip2=.bz`. +If set to a boolean `true`, then a default set of compressed formats +will be used, otherwise no precompressed formats supported. gzip:: -If set to true, then static content is served as gzip content encoded if a matching resource is found ending with ".gz". +Deprecated. Use `precompressed` instead. If set to `true`, then static content is served as gzip content encoded if a matching resource is found ending with ".gz". resourceBase:: Set to replace the context resource base. -aliases:: -If true, aliases of resources are allowed (that is, symbolic links and caps variations) and may bypass security constraints. +resourceCache:: +If set, this is a context attribute name, which the servlet will use to look for a shared ResourceCache instance. +relativeResourceBase:: +Set with a pathname relative to the base of the servlet context root. Useful for only serving static content out of only specific subdirectories. +cacheControl:: +If set, all static content will have this value set as the cache-control header. +pathInfoOnly:: +If `true`, only the path info will be applied to the resourceBase +stylesheet:: +Set with the location of an optional stylesheet that will be used to decorate the directory listing html. +etags:: +If `true`, weak etags will be generated and handled. maxCacheSize:: Maximum total size of the cache or 0 for no cache. maxCachedFileSize:: @@ -59,9 +80,11 @@ Maximum size of a file to cache. maxCachedFiles:: Maximum number of files to cache. useFileMappedBuffer:: -If set to true, mapped file buffer serves static content. -Setting this value to false means that a direct buffer is used instead of a mapped file buffer. -By default, this is set to true. +If set to `true`, mapped file buffer serves static content. +Setting this value to `false` means that a direct buffer is used instead of a mapped file buffer. +By default, this is set to `true`. otherGzipFileExtensions:: A comma separated list of other file extensions that signify that a file is gzip compressed. -If you don't explicitly set this, it defaults to ".svgz". +If you don't explicitly set this, it defaults to `.svgz`. +encodingHeaderCacheSize:: +Max entries in a cache of ACCEPT-ENCODING headers diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java index d004d838460..90582bb965a 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java @@ -123,7 +123,8 @@ import org.eclipse.jetty.util.resource.ResourceFactory; * otherGzipFileExtensions * Other file extensions that signify that a file is already compressed. Eg ".svgz" * - * + * encodingHeaderCacheSize + * Max entries in a cache of ACCEPT-ENCODING headers. * * */ diff --git a/jetty-webapp/src/main/config/etc/webdefault.xml b/jetty-webapp/src/main/config/etc/webdefault.xml index ecc7d199fcf..891bd28727a 100644 --- a/jetty-webapp/src/main/config/etc/webdefault.xml +++ b/jetty-webapp/src/main/config/etc/webdefault.xml @@ -95,9 +95,10 @@ * found ending with ".gz" (default false) * (deprecated: use precompressed) * - * precompressed If set to a comma separated list of file extensions, these - * indicate compressed formats that are known to map to a mime-type - * that may be listed in a requests Accept-Encoding header. + * precompressed If set to a comma separated list of encoding types (that may be + * listed in a requests Accept-Encoding header) to file + * extension mappings to look for and serve. For example: + * "br=.br,gzip=.gz,bzip2=.bz". * If set to a boolean True, then a default set of compressed formats * will be used, otherwise no precompressed formats. * @@ -115,9 +116,6 @@ * * stylesheet Set with the location of an optional stylesheet that will be used * to decorate the directory listing html. - * - * aliases If True, aliases of resources are allowed (eg. symbolic - * links and caps variations). May bypass security constraints. * * etags If True, weak etags will be generated and handled. * @@ -134,15 +132,17 @@ * cacheControl If set, all static content will have this value set as the cache-control * header. * + * encodingHeaderCacheSize + * Max entries in a cache of ACCEPT-ENCODING headers. + * + * otherGzipFileExtensions + * defaults to .svgz but a comma separated list of gzip equivalent file extensions can be supplied + * --> default org.eclipse.jetty.servlet.DefaultServlet - - aliases - false - acceptRanges true @@ -171,12 +171,6 @@ maxCachedFiles 2048 - etags false @@ -185,18 +179,6 @@ useFileMappedBuffer true - - 0 diff --git a/tests/test-webapps/test-jetty-webapp/src/main/config/demo-base/webapps/test.d/override-web.xml b/tests/test-webapps/test-jetty-webapp/src/main/config/demo-base/webapps/test.d/override-web.xml index 8a58a83f6a3..81d2ccc56da 100644 --- a/tests/test-webapps/test-jetty-webapp/src/main/config/demo-base/webapps/test.d/override-web.xml +++ b/tests/test-webapps/test-jetty-webapp/src/main/config/demo-base/webapps/test.d/override-web.xml @@ -15,6 +15,14 @@ a context value + + + default + + precompressed + true + + From 9f6747d741da368ef363724771cd86515ded9a6f Mon Sep 17 00:00:00 2001 From: WalkerWatch Date: Mon, 27 Aug 2018 22:52:58 -0400 Subject: [PATCH 13/13] Updated gzip doc. Resolves #2845 --- .../extras/default-servlet.adoc | 8 +++---- .../administration/extras/gzip-filter.adoc | 21 +++++++++++++------ 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/jetty-documentation/src/main/asciidoc/administration/extras/default-servlet.adoc b/jetty-documentation/src/main/asciidoc/administration/extras/default-servlet.adoc index 766eb40e5c1..f91059a8e6c 100644 --- a/jetty-documentation/src/main/asciidoc/administration/extras/default-servlet.adoc +++ b/jetty-documentation/src/main/asciidoc/administration/extras/default-servlet.adoc @@ -52,11 +52,9 @@ resources could be found. If `false`, then a welcome file must exist on disk. If servlet matches are supported without an existing file. Default is `true`. This must be `false` if you want directory listings, but have index.jsp in your welcome file list. precompressed:: -If set to a comma separated list of encoding types (that may be listed in a requests Accept-Encoding header) to file -extension mappings to look for and serve. For example: -`br=.br,gzip=.gz,bzip2=.bz`. -If set to a boolean `true`, then a default set of compressed formats -will be used, otherwise no precompressed formats supported. +If set to a comma separated list of encoding types (that may be listed in a requests Accept-Encoding header) to file extension mappings to look for and serve. +For example: `br=.br,gzip=.gz,bzip2=.bz`. +If set to a boolean `true`, then a default set of compressed formats will be used, otherwise no precompressed formats supported. gzip:: Deprecated. Use `precompressed` instead. If set to `true`, then static content is served as gzip content encoded if a matching resource is found ending with ".gz". resourceBase:: diff --git a/jetty-documentation/src/main/asciidoc/administration/extras/gzip-filter.adoc b/jetty-documentation/src/main/asciidoc/administration/extras/gzip-filter.adoc index ff4c25256e4..7d962dae2aa 100644 --- a/jetty-documentation/src/main/asciidoc/administration/extras/gzip-filter.adoc +++ b/jetty-documentation/src/main/asciidoc/administration/extras/gzip-filter.adoc @@ -34,13 +34,13 @@ It fixes many of the bugs in commonly available compression filters: it works wi It has been tested with Jetty continuations and suspending requests. Some user-agents might be excluded from compression to avoid common browser bugs (yes, this means IE!). -The `GzipHandler` is added to the entire server by the `etc/jetty-gzip.xml` file from the `gzip.mod` module. +The `GzipHandler` can be added to the entire server by enabling the `gzip.mod` module. It may also be added to individual contexts in a context xml file. ____ [NOTE] Jetty 9 only compresses using GZip. -Using deflate http compression is not supported and will not function. +Using deflate HTTP compression is not supported and will not function. ____ [[gzip-filter-rules]] @@ -59,16 +59,25 @@ ____ Compressing the content can greatly improve the network bandwidth usage, but at the cost of memory and CPU cycles. The link:#default-servlet[DefaultServlet] is capable of serving pre-compressed static content, which saves memory and CPU. -By default, the `GzipHandler` will check to see if pre-compressed content exists, and pass the request through to be handled by the `DefaultServlet`. + +The `GzipHandler` installs an output interceptor which passes through to the `DefaultServlet`. +If the content served by `DefaultServlet` is already compressed, the `GzipHandler` does nothing; if it is not compressed, the content is compressed on-the-fly. + +____ +[NOTE] +Automatic precompression by the `DefaultServlet` can be configured. +Read more about the `DefaultServlet` link:#default-servlet[here.] +____ + [[gzip-filter-init]] ==== Gzip Configuration minGzipSize:: Content will only be compressed if content length is either unknown or greater than `minGzipSize`. -checkGzExists:: -True by default. -If set to false, the handler will not check for pre-compressed content. +checkGzExists (Deprecated):: +False by default. +If set to true, the handler will check for pre-compressed content. includedMethods:: List of HTTP methods to compress. If not set, only `GET` requests are compressed.