From 3facfb6066eea67dd99b0c0bffb68f77be2a3aef Mon Sep 17 00:00:00 2001 From: Jinho Shin Date: Sat, 5 Mar 2016 12:51:25 +0900 Subject: [PATCH 01/21] Add a overridable method 'onClose' in HttpClientTransportOverHTTP2. - able to look GoAwayFrame through 'onClose' method. Signed-off-by: Jinho Shin --- .../http2/client/http/HttpClientTransportOverHTTP2.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2.java index d722717ac07..8191ca5b5fc 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2.java @@ -137,6 +137,10 @@ public class HttpClientTransportOverHTTP2 extends ContainerLifeCycle implements return new HttpConnectionOverHTTP2(destination, session); } + protected void onClose(HttpConnectionOverHTTP2 connection, GoAwayFrame frame) { + connection.close(); + } + private class SessionListenerPromise extends Session.Listener.Adapter implements Promise { private final HttpDestinationOverHTTP2 destination; @@ -181,7 +185,7 @@ public class HttpClientTransportOverHTTP2 extends ContainerLifeCycle implements @Override public void onClose(Session session, GoAwayFrame frame) { - connection.close(); + HttpClientTransportOverHTTP2.this.onClose(connection, frame); } @Override From a4686bc6a4364910d250c4ce6b175cfea41a3844 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sat, 5 Mar 2016 13:29:31 +0100 Subject: [PATCH 02/21] Issue #353 (Jetty Client doesn't forward authentication headers with redirects when using proxy) Fixed by making sure that URI matches take into account default HTTP ports. --- .../jetty/client/HttpAuthenticationStore.java | 3 +- .../org/eclipse/jetty/client/HttpClient.java | 2 +- .../org/eclipse/jetty/client/HttpRequest.java | 2 +- .../client/util/AbstractAuthentication.java | 80 ++++++++++++++ .../client/util/BasicAuthentication.java | 32 ++---- .../client/util/DigestAuthentication.java | 29 ++--- .../client/HttpAuthenticationStoreTest.java | 100 ++++++++++++++++++ 7 files changed, 204 insertions(+), 44 deletions(-) create mode 100644 jetty-client/src/main/java/org/eclipse/jetty/client/util/AbstractAuthentication.java create mode 100644 jetty-client/src/test/java/org/eclipse/jetty/client/HttpAuthenticationStoreTest.java 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 fd58166003a..4b73c941ec6 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 @@ -26,6 +26,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import org.eclipse.jetty.client.api.Authentication; import org.eclipse.jetty.client.api.AuthenticationStore; +import org.eclipse.jetty.client.util.AbstractAuthentication; public class HttpAuthenticationStore implements AuthenticationStore { @@ -85,7 +86,7 @@ public class HttpAuthenticationStore implements AuthenticationStore // TODO: I should match the longest URI for (Map.Entry entry : results.entrySet()) { - if (uri.toString().startsWith(entry.getKey().toString())) + if (AbstractAuthentication.matchesURI(entry.getKey(), uri)) return entry.getValue(); } return null; diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index 697b30a292c..57d8c3bde4f 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -1000,7 +1000,7 @@ public class HttpClient extends ContainerLifeCycle return host; } - protected int normalizePort(String scheme, int port) + public static int normalizePort(String scheme, int port) { return port > 0 ? port : HttpScheme.HTTPS.is(scheme) ? 443 : 80; } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java index ef2a209d993..4c4648d5cf9 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java @@ -88,7 +88,7 @@ public class HttpRequest implements Request this.conversation = conversation; scheme = uri.getScheme(); host = client.normalizeHost(uri.getHost()); - port = client.normalizePort(scheme, uri.getPort()); + port = HttpClient.normalizePort(scheme, uri.getPort()); path = uri.getRawPath(); query = uri.getRawQuery(); extractParams(query); diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/util/AbstractAuthentication.java b/jetty-client/src/main/java/org/eclipse/jetty/client/util/AbstractAuthentication.java new file mode 100644 index 00000000000..7d5f9738b86 --- /dev/null +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/util/AbstractAuthentication.java @@ -0,0 +1,80 @@ +// +// ======================================================================== +// Copyright (c) 1995-2016 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.client.util; + +import java.net.URI; + +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.api.Authentication; + +public abstract class AbstractAuthentication implements Authentication +{ + private final URI uri; + private final String realm; + + public AbstractAuthentication(URI uri, String realm) + { + this.uri = uri; + this.realm = realm; + } + + public abstract String getType(); + + public URI getURI() + { + return uri; + } + + public String getRealm() + { + return realm; + } + + @Override + public boolean matches(String type, URI uri, String realm) + { + if (!getType().equalsIgnoreCase(type)) + return false; + + if (!this.realm.equals(realm)) + return false; + + return matchesURI(this.uri, uri); + } + + public static boolean matchesURI(URI uri1, URI uri2) + { + String scheme = uri1.getScheme(); + if (uri1.getScheme().equalsIgnoreCase(scheme)) + { + if (uri1.getHost().equalsIgnoreCase(uri2.getHost())) + { + // Handle default HTTP ports. + int thisPort = HttpClient.normalizePort(scheme, uri1.getPort()); + int thatPort = HttpClient.normalizePort(scheme, uri2.getPort()); + if (thisPort == thatPort) + { + // Use decoded URI paths. + return uri2.getPath().startsWith(uri1.getPath()); + } + } + } + return false; + } +} diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/util/BasicAuthentication.java b/jetty-client/src/main/java/org/eclipse/jetty/client/util/BasicAuthentication.java index e37f82b66d3..256ce071ca9 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/util/BasicAuthentication.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/util/BasicAuthentication.java @@ -21,7 +21,8 @@ package org.eclipse.jetty.client.util; import java.net.URI; import java.nio.charset.StandardCharsets; -import org.eclipse.jetty.client.api.Authentication; +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.api.AuthenticationStore; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.http.HttpHeader; @@ -35,10 +36,8 @@ import org.eclipse.jetty.util.B64Code; * {@link AuthenticationStore} retrieved from the {@link HttpClient} * via {@link HttpClient#getAuthenticationStore()}. */ -public class BasicAuthentication implements Authentication +public class BasicAuthentication extends AbstractAuthentication { - private final URI uri; - private final String realm; private final String user; private final String password; @@ -50,48 +49,39 @@ public class BasicAuthentication implements Authentication */ public BasicAuthentication(URI uri, String realm, String user, String password) { - this.uri = uri; - this.realm = realm; + super(uri, realm); this.user = user; this.password = password; } @Override - public boolean matches(String type, URI uri, String realm) + public String getType() { - if (!"basic".equalsIgnoreCase(type)) - return false; - - if (!uri.toString().startsWith(this.uri.toString())) - return false; - - return this.realm.equals(realm); + return "Basic"; } @Override public Result authenticate(Request request, ContentResponse response, HeaderInfo headerInfo, Attributes context) { String value = "Basic " + B64Code.encode(user + ":" + password, StandardCharsets.ISO_8859_1); - return new BasicResult(headerInfo.getHeader(), uri, value); + return new BasicResult(headerInfo.getHeader(), value); } - private static class BasicResult implements Result + private class BasicResult implements Result { private final HttpHeader header; - private final URI uri; private final String value; - public BasicResult(HttpHeader header, URI uri, String value) + public BasicResult(HttpHeader header, String value) { this.header = header; - this.uri = uri; this.value = value; } @Override public URI getURI() { - return uri; + return BasicAuthentication.this.getURI(); } @Override @@ -103,7 +93,7 @@ public class BasicAuthentication implements Authentication @Override public String toString() { - return String.format("Basic authentication result for %s", uri); + return String.format("Basic authentication result for %s", getURI()); } } } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/util/DigestAuthentication.java b/jetty-client/src/main/java/org/eclipse/jetty/client/util/DigestAuthentication.java index 0a131b7dfcf..89dda602d13 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/util/DigestAuthentication.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/util/DigestAuthentication.java @@ -23,7 +23,6 @@ import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Locale; @@ -33,7 +32,8 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.eclipse.jetty.client.api.Authentication; +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.api.AuthenticationStore; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.http.HttpHeader; @@ -48,12 +48,10 @@ import org.eclipse.jetty.util.TypeUtil; * {@link AuthenticationStore} retrieved from the {@link HttpClient} * via {@link HttpClient#getAuthenticationStore()}. */ -public class DigestAuthentication implements Authentication +public class DigestAuthentication extends AbstractAuthentication { private static final Pattern PARAM_PATTERN = Pattern.compile("([^=]+)=(.*)"); - private final URI uri; - private final String realm; private final String user; private final String password; @@ -65,22 +63,15 @@ public class DigestAuthentication implements Authentication */ public DigestAuthentication(URI uri, String realm, String user, String password) { - this.uri = uri; - this.realm = realm; + super(uri, realm); this.user = user; this.password = password; } @Override - public boolean matches(String type, URI uri, String realm) + public String getType() { - if (!"digest".equalsIgnoreCase(type)) - return false; - - if (!uri.toString().startsWith(this.uri.toString())) - return false; - - return this.realm.equals(realm); + return "Digest"; } @Override @@ -108,7 +99,7 @@ public class DigestAuthentication implements Authentication clientQOP = "auth-int"; } - return new DigestResult(headerInfo.getHeader(), uri, response.getContent(), realm, user, password, algorithm, nonce, clientQOP, opaque); + return new DigestResult(headerInfo.getHeader(), response.getContent(), getRealm(), user, password, algorithm, nonce, clientQOP, opaque); } private Map parseParameters(String wwwAuthenticate) @@ -179,7 +170,6 @@ public class DigestAuthentication implements Authentication { private final AtomicInteger nonceCount = new AtomicInteger(); private final HttpHeader header; - private final URI uri; private final byte[] content; private final String realm; private final String user; @@ -189,10 +179,9 @@ public class DigestAuthentication implements Authentication private final String qop; private final String opaque; - public DigestResult(HttpHeader header, URI uri, byte[] content, String realm, String user, String password, String algorithm, String nonce, String qop, String opaque) + public DigestResult(HttpHeader header, byte[] content, String realm, String user, String password, String algorithm, String nonce, String qop, String opaque) { this.header = header; - this.uri = uri; this.content = content; this.realm = realm; this.user = user; @@ -206,7 +195,7 @@ public class DigestAuthentication implements Authentication @Override public URI getURI() { - return uri; + return DigestAuthentication.this.getURI(); } @Override diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpAuthenticationStoreTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpAuthenticationStoreTest.java new file mode 100644 index 00000000000..062986f1c5a --- /dev/null +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpAuthenticationStoreTest.java @@ -0,0 +1,100 @@ +// +// ======================================================================== +// Copyright (c) 1995-2016 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.client; + +import java.net.URI; + +import org.eclipse.jetty.client.api.Authentication; +import org.eclipse.jetty.client.api.AuthenticationStore; +import org.eclipse.jetty.client.api.Request; +import org.eclipse.jetty.client.util.BasicAuthentication; +import org.eclipse.jetty.client.util.DigestAuthentication; +import org.junit.Assert; +import org.junit.Test; + +public class HttpAuthenticationStoreTest +{ + @Test + public void testFindAuthenticationWithDefaultHTTPPort() throws Exception + { + AuthenticationStore store = new HttpAuthenticationStore(); + + URI uri1 = URI.create("http://host:80"); + URI uri2 = URI.create("http://host"); + String realm = "realm"; + store.addAuthentication(new BasicAuthentication(uri1, realm, "user", "password")); + + Authentication result = store.findAuthentication("Basic", uri2, realm); + Assert.assertNotNull(result); + + store.clearAuthentications(); + + // Flip the URIs. + uri1 = URI.create("https://server/"); + uri2 = URI.create("https://server:443/path"); + store.addAuthentication(new DigestAuthentication(uri1, realm, "user", "password")); + result = store.findAuthentication("Digest", uri2, realm); + Assert.assertNotNull(result); + } + + @Test + public void testFindAuthenticationResultWithDefaultHTTPPort() throws Exception + { + AuthenticationStore store = new HttpAuthenticationStore(); + + store.addAuthenticationResult(new Authentication.Result() + { + @Override + public URI getURI() + { + return URI.create("http://host:80"); + } + + @Override + public void apply(Request request) + { + } + }); + + URI uri2 = URI.create("http://host"); + Authentication.Result result = store.findAuthenticationResult(uri2); + Assert.assertNotNull(result); + + store.clearAuthenticationResults(); + + // Flip the URIs. + store.addAuthenticationResult(new Authentication.Result() + { + @Override + public URI getURI() + { + return URI.create("https://server/"); + } + + @Override + public void apply(Request request) + { + } + }); + + uri2 = URI.create("https://server:443/path"); + result = store.findAuthenticationResult(uri2); + Assert.assertNotNull(result); + } +} From 10bd934476e2a4776da34f1e87a4275382484b9a Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sat, 5 Mar 2016 14:26:47 +0100 Subject: [PATCH 03/21] Issue #346 HttpParser RFC2616 Compliance mode Empty header values returned as "" in all compliance modes --- .../org/eclipse/jetty/http/HttpParser.java | 2 +- .../eclipse/jetty/http/HttpParserTest.java | 34 +------------------ .../jetty/server/handler/ContextHandler.java | 12 +++---- 3 files changed, 7 insertions(+), 41 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java index 8093eb61dd6..3b56c18e392 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java @@ -1174,7 +1174,7 @@ public class HttpParser { _value=null; _string.setLength(0); - _valueString=_compliance.ordinal()<=HttpCompliance.RFC2616.ordinal()?"":null; + _valueString=""; _length=-1; setState(State.HEADER); diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java index de60c40b742..62d7723f3a3 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java @@ -334,9 +334,8 @@ public class HttpParserTest } - @Test - public void test2616NoValue() throws Exception + public void testNoValue() throws Exception { ByteBuffer buffer= BufferUtil.toBuffer( "GET / HTTP/1.0\015\012" + @@ -366,37 +365,6 @@ public class HttpParserTest assertEquals(3, _headers); } - @Test - public void test7230NoValue() throws Exception - { - ByteBuffer buffer= BufferUtil.toBuffer( - "GET / HTTP/1.0\015\012" + - "Host: localhost\015\012" + - "Name0: \015\012"+ - "Name1: \015\012"+ - "Connection: close\015\012" + - "\015\012"); - - HttpParser.RequestHandler handler = new Handler(); - HttpParser parser= new HttpParser(handler); - parseAll(parser,buffer); - - assertTrue(_headerCompleted); - assertTrue(_messageCompleted); - assertEquals("GET", _methodOrVersion); - assertEquals("/", _uriOrStatus); - assertEquals("HTTP/1.0", _versionOrReason); - assertEquals("Host", _hdr[0]); - assertEquals("localhost", _val[0]); - assertEquals("Name0", _hdr[1]); - assertEquals(null, _val[1]); - assertEquals("Name1", _hdr[2]); - assertEquals(null, _val[2]); - assertEquals("Connection", _hdr[3]); - assertEquals("close", _val[3]); - assertEquals(3, _headers); - } - @Test public void testHeaderParseDirect() throws Exception { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index dfb2459223b..ec11e9209a9 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -752,22 +752,20 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu _attributes.setAttribute("org.eclipse.jetty.server.Executor",getServer().getThreadPool()); + if (_mimeTypes == null) + _mimeTypes = new MimeTypes(); + try { - // Set the classloader + // Set the classloader, context and enter scope if (_classLoader != null) { current_thread = Thread.currentThread(); old_classloader = current_thread.getContextClassLoader(); current_thread.setContextClassLoader(_classLoader); } - - if (_mimeTypes == null) - _mimeTypes = new MimeTypes(); - old_context = __context.get(); __context.set(_scontext); - enterScope(null, getState()); // defers the calling of super.doStart() @@ -778,8 +776,8 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu } finally { + exitScope(null); __context.set(old_context); - // reset the classloader if (_classLoader != null && current_thread!=null) current_thread.setContextClassLoader(old_classloader); From 303aea96a33c5929c285ce540bd1dd645cc61b65 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Sat, 5 Mar 2016 16:00:34 +0100 Subject: [PATCH 04/21] Implement uniform expiry testing for all datastores, FileDataStore still todo. --- .../session/GCloudSessionDataStore.java | 147 +++++++++++------- .../InfinispanSessionDataStore.java | 53 ++++++- .../nosql/mongodb/MongoSessionDataStore.java | 39 ++--- .../session/AbstractSessionDataStore.java | 57 ++++++- .../server/session/AbstractSessionStore.java | 2 +- .../session/CachingSessionDataStore.java | 2 +- .../server/session/FileSessionDataStore.java | 2 +- .../server/session/JDBCSessionDataStore.java | 143 ++++++++++++----- .../server/session/NullSessionDataStore.java | 2 +- .../jetty/server/session/SessionData.java | 19 +++ .../server/session/SessionDataStore.java | 2 +- ...bstractSessionInvalidateAndCreateTest.java | 35 +++-- 12 files changed, 358 insertions(+), 145 deletions(-) diff --git a/jetty-gcloud/jetty-gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionDataStore.java b/jetty-gcloud/jetty-gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionDataStore.java index 74454132f3a..836c357b99b 100644 --- a/jetty-gcloud/jetty-gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionDataStore.java +++ b/jetty-gcloud/jetty-gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionDataStore.java @@ -30,6 +30,7 @@ import org.eclipse.jetty.server.session.AbstractSessionDataStore; import org.eclipse.jetty.server.session.SessionContext; import org.eclipse.jetty.server.session.SessionData; import org.eclipse.jetty.util.ClassLoadingObjectInputStream; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -78,11 +79,6 @@ public class GCloudSessionDataStore extends AbstractSessionDataStore private KeyFactory _keyFactory; private int _maxResults = DEFAULT_MAX_QUERY_RESULTS; - - - - - @@ -189,57 +185,90 @@ public class GCloudSessionDataStore extends AbstractSessionDataStore * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set) */ @Override - public Set getExpired(Set candidates) + public Set doGetExpired(Set candidates, int expiryTimeoutSec) { - long now = System.currentTimeMillis(); - Set expired = new HashSet(); - - //get up to maxResult number of sessions that have expired - ProjectionEntityQueryBuilder pbuilder = Query.projectionEntityQueryBuilder(); - pbuilder.addProjection(Projection.property(ID)); - pbuilder.filter(CompositeFilter.and(PropertyFilter.gt(EXPIRY, 0), PropertyFilter.le(EXPIRY, now))); - pbuilder.limit(_maxResults); - pbuilder.kind(KIND); - StructuredQuery pquery = pbuilder.build(); - QueryResults presults = _datastore.run(pquery); - - while (presults.hasNext()) - { - ProjectionEntity pe = presults.next(); - String id = pe.getString(ID); - expired.add(id); - } - - //reconcile against ids that the SessionStore thinks are expired - Set tmp = new HashSet(candidates); - tmp.removeAll(expired); - if (!tmp.isEmpty()) - { - //sessionstore thinks these are expired, but they are either no - //longer in the db or not expired in the db, or we exceeded the - //number of records retrieved by the expiry query, so check them - //individually - for (String s:tmp) - { - try - { - KeyQueryBuilder kbuilder = Query.keyQueryBuilder(); - kbuilder.filter(PropertyFilter.eq(ID, s)); - kbuilder.kind(KIND); - StructuredQuery kq = kbuilder.build(); - QueryResults kresults = _datastore.run(kq); - if (!kresults.hasNext()) - expired.add(s); //not in db, can be expired - } - catch (Exception e) - { - LOG.warn(e); - } - } - } - - return expired; - + long now = System.currentTimeMillis(); + Set expired = new HashSet(); + + try + { + //get up to maxResult number of sessions that have expired + ProjectionEntityQueryBuilder pbuilder = Query.projectionEntityQueryBuilder(); + pbuilder.addProjection(Projection.property(ID), Projection.property(LASTNODE), Projection.property(EXPIRY)); + pbuilder.filter(CompositeFilter.and(PropertyFilter.gt(EXPIRY, 0), PropertyFilter.le(EXPIRY, now))); + pbuilder.limit(_maxResults); + pbuilder.kind(KIND); + StructuredQuery pquery = pbuilder.build(); + QueryResults presults = _datastore.run(pquery); + + while (presults.hasNext()) + { + ProjectionEntity pe = presults.next(); + String id = pe.getString(ID); + String lastNode = pe.getString(LASTNODE); + long expiry = pe.getLong(EXPIRY); + + if (StringUtil.isBlank(lastNode)) + expired.add(id); //nobody managing it + else + { + if (_context.getWorkerName().equals(lastNode)) + expired.add(id); //we're managing it, we can expire it + else + { + if (_lastExpiryCheckTime <= 0) + { + //our first check, just look for sessions that we managed by another node that + //expired at least 3 graceperiods ago + if (expiry < (now - (1000L * (3 * _gracePeriodSec)))) + expired.add(id); + } + else + { + //another node was last managing it, only expire it if it expired a graceperiod ago + if (expiry < (now - (1000L * _gracePeriodSec))) + expired.add(id); + } + } + } + } + + //reconcile against ids that the SessionStore thinks are expired + Set tmp = new HashSet(candidates); + tmp.removeAll(expired); + if (!tmp.isEmpty()) + { + //sessionstore thinks these are expired, but they are either no + //longer in the db or not expired in the db, or we exceeded the + //number of records retrieved by the expiry query, so check them + //individually + for (String s:tmp) + { + try + { + KeyQueryBuilder kbuilder = Query.keyQueryBuilder(); + kbuilder.filter(PropertyFilter.eq(ID, s)); + kbuilder.kind(KIND); + StructuredQuery kq = kbuilder.build(); + QueryResults kresults = _datastore.run(kq); + if (!kresults.hasNext()) + expired.add(s); //not in db, can be expired + } + catch (Exception e) + { + LOG.warn(e); + } + } + } + + return expired; + } + catch (Exception e) + { + LOG.warn(e); + return expired; //return what we got + } + } @@ -297,6 +326,8 @@ public class GCloudSessionDataStore extends AbstractSessionDataStore oos.writeObject(session.getAllAttributes()); oos.flush(); + try + { //turn a session into an entity entity = Entity.builder(key) .set(ID, session.getId()) @@ -310,6 +341,12 @@ public class GCloudSessionDataStore extends AbstractSessionDataStore .set(EXPIRY, session.getExpiry()) .set(MAXINACTIVE, session.getMaxInactiveMs()) .set(ATTRIBUTES, Blob.copyFrom(baos.toByteArray())).build(); + } + catch (Exception e) + { + e.printStackTrace(); + throw e; + } return entity; } diff --git a/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionDataStore.java b/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionDataStore.java index e7611aab8d7..64c693a2084 100644 --- a/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionDataStore.java +++ b/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionDataStore.java @@ -134,7 +134,7 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set) */ @Override - public Set getExpired(Set candidates) + public Set doGetExpired(Set candidates, int expiryTimeoutSec) { if (candidates == null || candidates.isEmpty()) return candidates; @@ -143,6 +143,9 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore Set expired = new HashSet(); + //TODO if there is NOT an idle timeout set, need to check other sessions that + //might have expired + for (String candidate:candidates) { if (LOG.isDebugEnabled()) @@ -151,12 +154,43 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore { SessionData sd = load(candidate); - if (sd == null || sd.isExpiredAt(now)) + //if the session no longer exists + if (sd == null) { expired.add(candidate); if (LOG.isDebugEnabled()) - LOG.debug("Is null {} is expired {}", (sd==null), (sd !=null)); - } + LOG.debug("Session {} does not exist in infinispan", candidate); + } + else + { + if (_context.getWorkerName().equals(sd.getLastNode())) + { + //we are its manager, add it to the expired set if it is expired now + if ((sd.getExpiry() > 0 ) && sd.getExpiry() <= now) + { + expired.add(candidate); + if (LOG.isDebugEnabled()) + LOG.debug("Session {} managed by {} is expired", candidate, _context.getWorkerName()); + } + } + else + { + //if we are not the session's manager, only expire it iff: + // this is our first expiryCheck and the session expired a long time ago + //or + //the session expired at least one graceperiod ago + if (_lastExpiryCheckTime <=0) + { + if ((sd.getExpiry() > 0 ) && sd.getExpiry() < (now - (1000L * (3 * _gracePeriodSec)))) + expired.add(candidate); + } + else + { + if ((sd.getExpiry() > 0 ) && sd.getExpiry() < (now - (1000L * _gracePeriodSec))) + expired.add(candidate); + } + } + } } catch (Exception e) { @@ -193,6 +227,11 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore } + /** + * @param id + * @param context + * @return + */ public static String getCacheKey (String id, SessionContext context) { return context.getCanonicalContextPath()+"_"+context.getVhost()+"_"+id; @@ -224,11 +263,17 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore + /** + * @param sec the infinispan-specific idle timeout in sec or 0 if not set + */ public void setInfinispanIdleTimeoutSec (int sec) { _infinispanIdleTimeoutSec = sec; } + /** + * @return + */ public int getInfinispanIdleTimeoutSec () { return _infinispanIdleTimeoutSec; diff --git a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java index 329c72c5162..b3cd1f793b7 100644 --- a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java +++ b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java @@ -153,8 +153,6 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore private DBCollection _dbSessions; - private long _gracePeriodMs = 1000L * 60 * 60; //default grace period is 1hr - public void setDBCollection (DBCollection collection) { _dbSessions = collection; @@ -169,24 +167,7 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore return _dbSessions; } - /** - * @return - */ - public int getGracePeriodSec () - { - return (int)(_gracePeriodMs == 0L? 0 : _gracePeriodMs/1000L); - } - - /** - * @param sec - */ - public void setGracePeriodSec (int sec) - { - if (sec < 0) - _gracePeriodMs = 0; - else - _gracePeriodMs = sec * 1000L; - } + /** * @see org.eclipse.jetty.server.session.SessionDataStore#load(org.eclipse.jetty.server.session.SessionKey) @@ -342,12 +323,14 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set) */ @Override - public Set getExpired(Set candidates) + public Set doGetExpired(Set candidates, int expiryTimeoutSec) { - long upperBound = System.currentTimeMillis(); + long now = System.currentTimeMillis(); + long upperBound = now; Set expiredSessions = new HashSet<>(); - //firstly ask mongo to verify if these candidate ids have expired + //firstly ask mongo to verify if these candidate ids have expired - all of + //these candidates will be for our node BasicDBObject query = new BasicDBObject(); query.put(__ID,new BasicDBObject("$in", candidates)); query.put(__EXPIRY, new BasicDBObject("$gt", 0)); @@ -369,9 +352,13 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore if (verifiedExpiredSessions != null) verifiedExpiredSessions.close(); } - - //now ask mongo to find sessions that expired a while ago - upperBound = upperBound - (3 * _gracePeriodMs); + //now ask mongo to find sessions last managed by other nodes that expired a while ago + //if this is our first expiry check, make sure that we only grab really old sessions + if (_lastExpiryCheckTime <= 0) + upperBound = (now - (3*(1000L * _gracePeriodSec))); + else + upperBound = _lastExpiryCheckTime - (1000L * _gracePeriodSec); + query.clear(); query.put(__EXPIRY, new BasicDBObject("$gt", 0)); query.put(__EXPIRY, new BasicDBObject("$lt", upperBound)); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java index ec9e90f3a59..d7bfee1f831 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java @@ -20,6 +20,8 @@ package org.eclipse.jetty.server.session; +import java.util.Set; + import org.eclipse.jetty.util.component.AbstractLifeCycle; /** @@ -30,6 +32,8 @@ import org.eclipse.jetty.util.component.AbstractLifeCycle; public abstract class AbstractSessionDataStore extends AbstractLifeCycle implements SessionDataStore { protected SessionContext _context; //context associated with this session data store + protected int _gracePeriodSec = 60 * 60; //default of 1hr + protected long _lastExpiryCheckTime = 0; //last time in ms that getExpired was called /** @@ -43,6 +47,16 @@ public abstract class AbstractSessionDataStore extends AbstractLifeCycle impleme public abstract void doStore(String id, SessionData data, long lastSaveTime) throws Exception; + /** + * Implemented by subclasses to resolve which sessions this node + * should attempt to expire. + * + * @param candidates the ids of sessions the SessionStore thinks has expired + * @param scavengePeriodSec the period in sec of the scavenge cycle checks + * @return the reconciled set of session ids that this node should attempt to expire + * @throws Exception + */ + public abstract Set doGetExpired (Set candidates, int scavengePeriodSec); /** @@ -81,6 +95,25 @@ public abstract class AbstractSessionDataStore extends AbstractLifeCycle impleme + /** + * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set, int) + */ + @Override + public Set getExpired(Set candidates, int scavengePeriodSec) + { + try + { + return doGetExpired (candidates, scavengePeriodSec); + } + finally + { + _lastExpiryCheckTime = System.currentTimeMillis(); + } + } + + + + /** * @see org.eclipse.jetty.server.session.SessionDataStore#newSessionData(java.lang.String, long, long, long, long) */ @@ -110,7 +143,29 @@ public abstract class AbstractSessionDataStore extends AbstractLifeCycle impleme super.doStart(); } - + + + + + /** + * @return + */ + public int getGracePeriodSec() + { + return _gracePeriodSec; + } + + + + + /** + * @param sec + */ + public void setGracePeriodSec(int sec) + { + _gracePeriodSec = sec; + } + } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java index 4eece0bd65a..eaab680d220 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java @@ -565,7 +565,7 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements if (LOG.isDebugEnabled()) LOG.debug("SessionStore checking expiration on {}", candidates); - return _sessionDataStore.getExpired(candidates); + return _sessionDataStore.getExpired(candidates, _expiryTimeoutSec); } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/CachingSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/CachingSessionDataStore.java index fe16f6aa966..d4ff31ee570 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/CachingSessionDataStore.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/CachingSessionDataStore.java @@ -119,7 +119,7 @@ public class CachingSessionDataStore extends AbstractSessionDataStore * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set) */ @Override - public Set getExpired(Set candidates) + public Set doGetExpired(Set candidates, int expiryTimeoutSec) { // TODO Auto-generated method stub return null; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java index 6c26b6fe640..9c7a3913b48 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java @@ -114,7 +114,7 @@ public class FileSessionDataStore extends AbstractSessionDataStore * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set) */ @Override - public Set getExpired(Set candidates) + public Set doGetExpired(Set candidates, int expiryTimeoutSec) { //we don't want to open up each file and check, so just leave it up to the SessionStore //TODO as the session manager is likely to be a lazy loader, if a session is never requested, its diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionDataStore.java index a60f74d2367..fc32eaf8458 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionDataStore.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionDataStore.java @@ -58,8 +58,7 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore private int _attempts = -1; // <= 0 means unlimited attempts to load a session private boolean _deleteUnloadables = false; //true means if attempts exhausted delete the session - private long _gracePeriodMs = 1000L * 60 * 60; //default grace period is 1hr - + /** * SessionTableSchema * @@ -285,7 +284,7 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore } - public PreparedStatement getMyExpiredSessionsStatement (Connection connection, String canonicalContextPath, String vhost, long expiry) + public PreparedStatement getExpiredSessionsStatement (Connection connection, String canonicalContextPath, String vhost, long expiry) throws SQLException { if (_dbAdaptor == null) @@ -317,6 +316,42 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore return statement; } + + public PreparedStatement getMyExpiredSessionsStatement (Connection connection, SessionContext sessionContext, long expiry) + throws SQLException + { + if (_dbAdaptor == null) + throw new IllegalStateException("No DB adaptor"); + + if (sessionContext.getCanonicalContextPath() == null || "".equals(sessionContext.getCanonicalContextPath())) + { + if (_dbAdaptor.isEmptyStringNull()) + { + PreparedStatement statement = connection.prepareStatement("select "+getIdColumn()+", "+getExpiryTimeColumn()+ + " from "+getTableName()+" where "+ + getLastNodeColumn() + " = ? and "+ + getContextPathColumn()+" is null and "+ + getVirtualHostColumn()+" = ? and "+getExpiryTimeColumn()+" >0 and "+getExpiryTimeColumn()+" <= ?"); + statement.setString(1, sessionContext.getWorkerName()); + statement.setString(2, sessionContext.getVhost()); + statement.setLong(3, expiry); + return statement; + } + } + + PreparedStatement statement = connection.prepareStatement("select "+getIdColumn()+", "+getExpiryTimeColumn()+ + " from "+getTableName()+" where "+ + getLastNodeColumn()+" = ? and "+ + getContextPathColumn()+" = ? and "+ + getVirtualHostColumn()+" = ? and "+ + getExpiryTimeColumn()+" >0 and "+getExpiryTimeColumn()+" <= ?"); + + statement.setString(1, sessionContext.getWorkerName()); + statement.setString(2, sessionContext.getCanonicalContextPath()); + statement.setString(3, sessionContext.getVhost()); + statement.setLong(4, expiry); + return statement; + } public PreparedStatement getAllAncientExpiredSessionsStatement (Connection connection) @@ -804,6 +839,7 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore } } + private void doUpdate (String id, SessionData data) throws Exception { @@ -854,27 +890,26 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set) */ @Override - public Set getExpired(Set candidates) + public Set doGetExpired(Set candidates, int scavengeIntervalSec) { if (LOG.isDebugEnabled()) LOG.debug("Getting expired sessions "+System.currentTimeMillis()); long now = System.currentTimeMillis(); - Set expiredSessionKeys = new HashSet<>(); try (Connection connection = _dbAdaptor.getConnection()) { connection.setAutoCommit(true); /* - * 1. Select sessions for our context that have expired + * 1. Select sessions managed by this node for our context that have expired */ long upperBound = now; if (LOG.isDebugEnabled()) - LOG.debug ("{}- Pass 1: Searching for sessions for context {} expired before {}", _context.getWorkerName(), _context.getCanonicalContextPath(), upperBound); + LOG.debug ("{}- Pass 1: Searching for sessions for context {} managed by me {} and expired before {}", _context.getCanonicalContextPath(), _context.getWorkerName(), upperBound); - try (PreparedStatement statement = _sessionTableSchema.getMyExpiredSessionsStatement(connection, _context.getCanonicalContextPath(), _context.getVhost(), upperBound)) + try (PreparedStatement statement = _sessionTableSchema.getExpiredSessionsStatement(connection, _context.getCanonicalContextPath(), _context.getVhost(), upperBound)) { try (ResultSet result = statement.executeQuery()) { @@ -889,30 +924,33 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore } /* - * 2. Select sessions for any node or context that have expired a long time ago (ie at least 3 grace periods ago) + * 2. Select sessions for any node or context that have expired + * at least 1 graceperiod since the last expiry check. If we haven't done previous expiry checks, then check + * those that have expired at least 3 graceperiod ago. */ try (PreparedStatement selectExpiredSessions = _sessionTableSchema.getAllAncientExpiredSessionsStatement(connection)) { - upperBound = now - (3 * _gracePeriodMs); - if (upperBound > 0) - { - if (LOG.isDebugEnabled()) LOG.debug("{}- Pass 2: Searching for sessions expired before {}",_context.getWorkerName(), upperBound); + if (_lastExpiryCheckTime <= 0) + upperBound = (now - (3*(1000L * _gracePeriodSec))); + else + upperBound = _lastExpiryCheckTime - (1000L * _gracePeriodSec); - selectExpiredSessions.setLong(1, upperBound); - try (ResultSet result = selectExpiredSessions.executeQuery()) + if (LOG.isDebugEnabled()) LOG.debug("{}- Pass 2: Searching for sessions expired before {}",_context.getWorkerName(), upperBound); + + selectExpiredSessions.setLong(1, upperBound); + try (ResultSet result = selectExpiredSessions.executeQuery()) + { + while (result.next()) { - while (result.next()) - { - String sessionId = result.getString(_sessionTableSchema.getIdColumn()); - String ctxtpth = result.getString(_sessionTableSchema.getContextPathColumn()); - String vh = result.getString(_sessionTableSchema.getVirtualHostColumn()); - expiredSessionKeys.add(sessionId); - if (LOG.isDebugEnabled()) LOG.debug ("{}- Found expired sessionId=",_context.getWorkerName(), sessionId); - } + String sessionId = result.getString(_sessionTableSchema.getIdColumn()); + String ctxtpth = result.getString(_sessionTableSchema.getContextPathColumn()); + String vh = result.getString(_sessionTableSchema.getVirtualHostColumn()); + expiredSessionKeys.add(sessionId); + if (LOG.isDebugEnabled()) LOG.debug ("{}- Found expired sessionId=",_context.getWorkerName(), sessionId); } } } - + Set notExpiredInDB = new HashSet<>(); for (String k: candidates) @@ -958,44 +996,48 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore LOG.warn(e); return expiredSessionKeys; //return whatever we got } - - } - public int getGracePeriodSec () - { - return (int)(_gracePeriodMs == 0L? 0 : _gracePeriodMs/1000L); } - public void setGracePeriodSec (int sec) - { - if (sec < 0) - _gracePeriodMs = 0; - else - _gracePeriodMs = sec * 1000L; - } + /** + * @param dbAdaptor + */ public void setDatabaseAdaptor (DatabaseAdaptor dbAdaptor) { checkStarted(); _dbAdaptor = dbAdaptor; } + /** + * @param schema + */ public void setSessionTableSchema (SessionTableSchema schema) { checkStarted(); _sessionTableSchema = schema; } + /** + * @param attempts + */ public void setLoadAttempts (int attempts) { checkStarted(); _attempts = attempts; } + /** + * @return + */ public int getLoadAttempts () { return _attempts; } + /** + * @param id + * @return + */ public boolean loadAttemptsExhausted (String id) { AtomicInteger i = _unloadables.get(id); @@ -1004,18 +1046,27 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore return (i.get() >= _attempts); } + /** + * @param delete + */ public void setDeleteUnloadableSessions (boolean delete) { checkStarted(); _deleteUnloadables = delete; } + /** + * @return true if we should delete data for sessions that we cant reconstitute + */ public boolean isDeleteUnloadableSessions () { return _deleteUnloadables; } + /** + * @param id + */ protected void incLoadAttempt (String id) { AtomicInteger i = new AtomicInteger(0); @@ -1027,6 +1078,10 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore + /** + * @param id + * @return number of attempts to load the given id + */ public int getLoadAttempts (String id) { AtomicInteger i = _unloadables.get(id); @@ -1035,15 +1090,21 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore return i.get(); } + /** + * @return how many sessions we've failed to load + */ public Set getUnloadableSessions () { return new HashSet(_unloadables.keySet()); } - - public void clearUnloadableSessions() - { - _unloadables.clear(); - } + + /** + * + */ + public void clearUnloadableSessions() + { + _unloadables.clear(); + } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionDataStore.java index 78f4004102e..18730723459 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionDataStore.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionDataStore.java @@ -72,7 +72,7 @@ public class NullSessionDataStore extends AbstractSessionDataStore * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set) */ @Override - public Set getExpired(Set candidates) + public Set doGetExpired(Set candidates, int expiryTimeoutSec) { return candidates; //whatever is suggested we accept } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java index 3f05b08aca1..b96b4773a97 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java @@ -433,4 +433,23 @@ public class SessionData implements Serializable return (getExpiry() < time); } + + /** + * @see java.lang.Object#toString() + */ + public String toString() + { + StringBuilder builder = new StringBuilder(); + builder.append("id="+_id); + builder.append(", contextpath="+_contextPath); + builder.append(", vhost="+_vhost); + builder.append(", accessed="+_accessed); + builder.append(", lastaccessed="+_lastAccessed); + builder.append(", created="+_created); + builder.append(", cookieset="+_cookieSet); + builder.append(", lastnode="+_lastNode); + builder.append(", expiry="+_expiry); + builder.append(", maxinactive="+_maxInactiveMs); + return builder.toString(); + } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionDataStore.java index 367d6f549d7..80ea9836553 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionDataStore.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionDataStore.java @@ -95,7 +95,7 @@ public interface SessionDataStore extends LifeCycle * SessionDataStore * @return set of session ids */ - public Set getExpired (Set candidates); + public Set getExpired (Set candidates, int scavengePeriodSec); diff --git a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionInvalidateAndCreateTest.java b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionInvalidateAndCreateTest.java index 4e98e91f87e..b1c1d4286cc 100644 --- a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionInvalidateAndCreateTest.java +++ b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionInvalidateAndCreateTest.java @@ -139,7 +139,7 @@ public abstract class AbstractSessionInvalidateAndCreateTest assertTrue(listener.destroys.contains("session1")); assertTrue(listener.destroys.contains("session2")); //session2's HttpSessionBindingListener should have been called when it was scavenged - assertTrue(servlet.unbound); + assertTrue(servlet.listener.unbound); } finally { @@ -151,24 +151,33 @@ public abstract class AbstractSessionInvalidateAndCreateTest server.stop(); } } + + public static class Foo implements Serializable + { + public boolean bar = false; + + public boolean getBar() { return bar;}; + } - public static class TestServlet extends HttpServlet + public static class MySessionBindingListener implements HttpSessionBindingListener, Serializable { private boolean unbound = false; - public class MySessionBindingListener implements HttpSessionBindingListener, Serializable + public void valueUnbound(HttpSessionBindingEvent event) + { + unbound = true; + } + + public void valueBound(HttpSessionBindingEvent event) { - public void valueUnbound(HttpSessionBindingEvent event) - { - unbound = true; - } - - public void valueBound(HttpSessionBindingEvent event) - { - - } } + } + + public static class TestServlet extends HttpServlet + { + public MySessionBindingListener listener = new MySessionBindingListener(); + @Override protected void doGet(HttpServletRequest request, HttpServletResponse httpServletResponse) throws ServletException, IOException @@ -190,7 +199,7 @@ public abstract class AbstractSessionInvalidateAndCreateTest //now make a new session session = request.getSession(true); session.setAttribute("identity", "session2"); - session.setAttribute("listener", new MySessionBindingListener()); + session.setAttribute("listener", listener); } else fail("Session already missing"); From d48cfcdb62d237add90369b45b15969406306816 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sat, 5 Mar 2016 17:50:56 +0100 Subject: [PATCH 05/21] Issue #397 Multipart EOF handling read to EOF when reading multipart. --- .../eclipse/jetty/util/MultiPartInputStreamParser.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartInputStreamParser.java b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartInputStreamParser.java index 23437475d88..6b82fc1f715 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartInputStreamParser.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartInputStreamParser.java @@ -764,11 +764,15 @@ public class MultiPartInputStreamParser } finally { - part.close(); } } - if (!lastPart) + if (lastPart) + { + while(line!=null) + line=((ReadLineInputStream)_in).readLine(); + } + else throw new IOException("Incomplete parts"); } catch (Exception e) From 815bc54f5d23e028e9ed1fac0b80dcd892778851 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 7 Mar 2016 10:34:49 +0100 Subject: [PATCH 06/21] Improved failure reporting. --- .../util/MultiPartContentProviderTest.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartContentProviderTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartContentProviderTest.java index ad59b41cdac..0f9d8280f76 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartContentProviderTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/util/MultiPartContentProviderTest.java @@ -185,11 +185,9 @@ public class MultiPartContentProviderTest extends AbstractHttpClientServerTest .content(multiPart) .send(result -> { - if (result.isSucceeded()) - { - Assert.assertEquals(200, result.getResponse().getStatus()); - responseLatch.countDown(); - } + Assert.assertTrue(String.valueOf(result.getFailure()), result.isSucceeded()); + Assert.assertEquals(200, result.getResponse().getStatus()); + responseLatch.countDown(); }); // Wait until the request has been sent. @@ -408,11 +406,9 @@ public class MultiPartContentProviderTest extends AbstractHttpClientServerTest .content(multiPart) .send(result -> { - if (result.isSucceeded()) - { - Assert.assertEquals(200, result.getResponse().getStatus()); - responseLatch.countDown(); - } + Assert.assertTrue(String.valueOf(result.getFailure()), result.isSucceeded()); + Assert.assertEquals(200, result.getResponse().getStatus()); + responseLatch.countDown(); }); // Wait until the request has been sent. From d36e5864dbd4e7044a601be3f4d9b62b3a9cc110 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 7 Mar 2016 15:20:12 +0100 Subject: [PATCH 07/21] Issue #377 (HttpClient - No supported cipher suites leads to stuck requests) Fixed by rethrowing the exception thrown by onOpen() so that the SelectorManager can act appropriately. --- .../client/HostnameVerificationTest.java | 3 +- .../jetty/client/HttpClientTLSTest.java | 183 ++++++++++++++++++ .../org/eclipse/jetty/io/SelectorManager.java | 23 +-- .../eclipse/jetty/io/ssl/SslConnection.java | 5 +- 4 files changed, 200 insertions(+), 14 deletions(-) create mode 100644 jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HostnameVerificationTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HostnameVerificationTest.java index 2b70e3b1dec..0fee488928c 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HostnameVerificationTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HostnameVerificationTest.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.nio.channels.ClosedChannelException; import java.security.cert.CertificateException; import java.util.concurrent.ExecutionException; + import javax.net.ssl.SSLHandshakeException; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; @@ -124,7 +125,7 @@ public class HostnameVerificationTest if (cause instanceof SSLHandshakeException) Assert.assertThat(cause.getCause().getCause(), Matchers.instanceOf(CertificateException.class)); else - Assert.assertThat(cause.getCause(), Matchers.instanceOf(ClosedChannelException.class)); + Assert.assertThat(cause, Matchers.instanceOf(ClosedChannelException.class)); } } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java new file mode 100644 index 00000000000..43559aa2184 --- /dev/null +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java @@ -0,0 +1,183 @@ +// +// ======================================================================== +// Copyright (c) 1995-2016 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.client; + +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; + +import org.eclipse.jetty.http.HttpScheme; +import org.eclipse.jetty.server.Handler; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.junit.After; +import org.junit.Assert; +import org.junit.Test; + +public class HttpClientTLSTest +{ + private Server server; + private ServerConnector connector; + private HttpClient client; + + private void startServer(SslContextFactory sslContextFactory, Handler handler) throws Exception + { + QueuedThreadPool serverThreads = new QueuedThreadPool(); + serverThreads.setName("server"); + server = new Server(serverThreads); + + connector = new ServerConnector(server, sslContextFactory); + server.addConnector(connector); + + server.setHandler(handler); + server.start(); + } + + private void startClient(SslContextFactory sslContextFactory) throws Exception + { + QueuedThreadPool clientThreads = new QueuedThreadPool(); + clientThreads.setName("client"); + client = new HttpClient(sslContextFactory); + client.setExecutor(clientThreads); + client.start(); + } + + private SslContextFactory createSslContextFactory() + { + SslContextFactory sslContextFactory = new SslContextFactory(); + sslContextFactory.setEndpointIdentificationAlgorithm(""); + sslContextFactory.setKeyStorePath("src/test/resources/keystore.jks"); + sslContextFactory.setKeyStorePassword("storepwd"); + sslContextFactory.setTrustStorePath("src/test/resources/truststore.jks"); + sslContextFactory.setTrustStorePassword("storepwd"); + return sslContextFactory; + } + + @After + public void dispose() throws Exception + { + if (client != null) + client.stop(); + if (server != null) + server.stop(); + } + + @Test + public void testNoCommonTLSProtocol() throws Exception + { + SslContextFactory serverTLSFactory = createSslContextFactory(); + serverTLSFactory.setIncludeProtocols("TLSv1.2"); + startServer(serverTLSFactory, new EmptyServerHandler()); + + SslContextFactory clientTLSFactory = createSslContextFactory(); + clientTLSFactory.setIncludeProtocols("TLSv1.1"); + startClient(clientTLSFactory); + + try + { + client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .timeout(5, TimeUnit.SECONDS) + .send(); + Assert.fail(); + } + catch (ExecutionException x) + { + // Expected. + } + } + + @Test + public void testNoCommonTLSCiphers() throws Exception + { + SslContextFactory serverTLSFactory = createSslContextFactory(); + serverTLSFactory.setIncludeCipherSuites("TLS_RSA_WITH_AES_128_CBC_SHA"); + startServer(serverTLSFactory, new EmptyServerHandler()); + + SslContextFactory clientTLSFactory = createSslContextFactory(); + clientTLSFactory.setExcludeCipherSuites(".*_SHA$"); + startClient(clientTLSFactory); + + try + { + client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .timeout(5, TimeUnit.SECONDS) + .send(); + Assert.fail(); + } + catch (ExecutionException x) + { + // Expected. + } + } + + @Test + public void testMismatchBetweenTLSProtocolAndTLSCiphersOnServer() throws Exception + { + SslContextFactory serverTLSFactory = createSslContextFactory(); + // TLS 1.1 protocol, but only TLS 1.2 ciphers. + serverTLSFactory.setIncludeProtocols("TLSv1.1"); + serverTLSFactory.setIncludeCipherSuites("TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"); + startServer(serverTLSFactory, new EmptyServerHandler()); + + SslContextFactory clientTLSFactory = createSslContextFactory(); + startClient(clientTLSFactory); + + try + { + client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .timeout(5, TimeUnit.SECONDS) + .send(); + Assert.fail(); + } + catch (ExecutionException x) + { + // Expected. + } + } + + @Test + public void testMismatchBetweenTLSProtocolAndTLSCiphersOnClient() throws Exception + { + SslContextFactory serverTLSFactory = createSslContextFactory(); + startServer(serverTLSFactory, new EmptyServerHandler()); + + SslContextFactory clientTLSFactory = createSslContextFactory(); + // TLS 1.1 protocol, but only TLS 1.2 ciphers. + clientTLSFactory.setIncludeProtocols("TLSv1.1"); + clientTLSFactory.setIncludeCipherSuites("TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"); + startClient(clientTLSFactory); + + try + { + client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .timeout(5, TimeUnit.SECONDS) + .send(); + Assert.fail(); + } + catch (ExecutionException x) + { + // Expected. + } + } +} diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java index 47f188a3af3..26a18c3d3b3 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java @@ -62,14 +62,14 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa public static final int DEFAULT_CONNECT_TIMEOUT = 15000; protected static final Logger LOG = Log.getLogger(SelectorManager.class); private final static boolean __submitKeyUpdates = Boolean.valueOf(System.getProperty(SUBMIT_KEY_UPDATES, "true")); - + private final Executor executor; private final Scheduler scheduler; private final ManagedSelector[] _selectors; private long _connectTimeout = DEFAULT_CONNECT_TIMEOUT; private long _selectorIndex; private int _priorityDelta; - + protected SelectorManager(Executor executor, Scheduler scheduler) { this(executor, scheduler, (Runtime.getRuntime().availableProcessors() + 1) / 2); @@ -149,7 +149,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa } } } - + /** * Executes the given task in a different thread. * @@ -217,13 +217,13 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa final ManagedSelector selector = chooseSelector(); selector.submit(selector.new Accept(channel, attachment)); } - + /** *

Registers a server channel for accept operations. * When a {@link SocketChannel} is accepted from the given {@link ServerSocketChannel} * then the {@link #accepted(SocketChannel)} method is called, which must be * overridden by a derivation of this class to handle the accepted channel - * + * * @param server the server channel to register */ public void acceptor(ServerSocketChannel server) @@ -231,7 +231,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa final ManagedSelector selector = chooseSelector(); selector.submit(selector.new Acceptor(server)); } - + /** * Callback method when a channel is accepted from the {@link ServerSocketChannel} * passed to {@link #acceptor(ServerSocketChannel)}. @@ -315,6 +315,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa LOG.warn("Exception while notifying connection " + connection, x); else LOG.debug("Exception while notifying connection " + connection, x); + throw x; } } @@ -439,8 +440,8 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa /** * Submit a task to update a selector key. If the System property {@link SelectorManager#SUBMIT_KEY_UPDATES} - * is set true (default is false), the task is passed to {@link #submit(Runnable)}. Otherwise it is run immediately and the selector - * woken up if need be. + * is set true (default is false), the task is passed to {@link #submit(Runnable)}. Otherwise it is run immediately and the selector + * woken up if need be. * @param update the update to a key */ public void updateKey(Runnable update) @@ -460,7 +461,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa wakeup(); } } - + /** *

Submits a change to be executed in the selector thread.

*

Changes may be submitted from any thread, and the selector thread woken up @@ -587,7 +588,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa _state.set(State.CHANGES); continue; default: - throw new IllegalStateException(); + throw new IllegalStateException(); } } // Must check first for SELECT and *then* for WAKEUP @@ -696,7 +697,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa connect.failed(x); } } - + private void processAccept(SelectionKey key) { ServerSocketChannel server = (ServerSocketChannel)key.channel(); diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index dc20bf7bbac..c68cc06c48f 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -23,6 +23,7 @@ import java.nio.ByteBuffer; import java.nio.channels.ClosedChannelException; import java.util.Arrays; import java.util.concurrent.Executor; + import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLEngineResult; import javax.net.ssl.SSLEngineResult.HandshakeStatus; @@ -663,7 +664,7 @@ public class SslConnection extends AbstractConnection } catch (Exception e) { - getEndPoint().close(); + close(); throw e; } finally @@ -742,7 +743,7 @@ public class SslConnection extends AbstractConnection { BufferUtil.flipToFlush(_encryptedOutput, pos); } - + if (DEBUG) LOG.debug("{} wrap {}", SslConnection.this, wrapResult); if (wrapResult.bytesConsumed()>0) From 3b0f577aed8b41a97b756047875b6f7f0f319e74 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 7 Mar 2016 16:40:34 +0100 Subject: [PATCH 08/21] Removed DEBUG logging during tests. --- .../src/test/resources/jetty-logging.properties | 3 --- 1 file changed, 3 deletions(-) diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/resources/jetty-logging.properties b/jetty-websocket/javax-websocket-server-impl/src/test/resources/jetty-logging.properties index 9681d9168b4..c5a50f62391 100644 --- a/jetty-websocket/javax-websocket-server-impl/src/test/resources/jetty-logging.properties +++ b/jetty-websocket/javax-websocket-server-impl/src/test/resources/jetty-logging.properties @@ -6,9 +6,6 @@ org.eclipse.jetty.LEVEL=WARN # org.eclipse.jetty.websocket.LEVEL=WARN # org.eclipse.jetty.websocket.common.io.LEVEL=DEBUG -org.eclipse.jetty.websocket.common.WebSocketSession.LEVEL=DEBUG -org.eclipse.jetty.websocket.jsr356.LEVEL=DEBUG - ### Show state changes on BrowserDebugTool # -- LEAVE THIS AT DEBUG LEVEL -- org.eclipse.jetty.websocket.jsr356.server.browser.LEVEL=DEBUG From 8af356bc0d4b8fec572311e4cd319270a3ed68da Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 7 Mar 2016 17:00:52 +0100 Subject: [PATCH 09/21] Issue #305 (NPE when notifying the session listener if the channel is closed before a session has been opened) Added guard against NPE. It was needed since the connection may not be present if the connect operation failed. --- .../http/HttpClientTransportOverHTTP2.java | 4 ++- .../jetty/http/client/AbstractTest.java | 2 +- .../jetty/http/client/HttpClientTest.java | 31 +++++++++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2.java index a68eabf3795..844be1a5e37 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpClientTransportOverHTTP2.java @@ -199,7 +199,9 @@ public class HttpClientTransportOverHTTP2 extends ContainerLifeCycle implements @Override public void onFailure(Session session, Throwable failure) { - connection.close(failure); + HttpConnectionOverHTTP2 c = connection; + if (c != null) + c.close(failure); } } } diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/AbstractTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/AbstractTest.java index a47a8618d0c..974762bdcd2 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/AbstractTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/AbstractTest.java @@ -154,7 +154,7 @@ public abstract class AbstractTest return result.toArray(new ConnectionFactory[result.size()]); } - private HttpClientTransport provideClientTransport(Transport transport) + protected HttpClientTransport provideClientTransport(Transport transport) { switch (transport) { diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java index 415bbe3700d..b0f3d177351 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java @@ -20,6 +20,7 @@ package org.eclipse.jetty.http.client; import java.io.IOException; import java.io.InterruptedIOException; +import java.util.EnumSet; import java.util.Random; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; @@ -39,7 +40,10 @@ import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http2.FlowControlStrategy; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.AbstractHandler; +import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.junit.Assert; +import org.junit.Assume; import org.junit.Test; public class HttpClientTest extends AbstractTest @@ -285,6 +289,33 @@ public class HttpClientTest extends AbstractTest Assert.assertEquals(response.getStatus(), 200); } + @Test(expected = ExecutionException.class) + public void testClientCannotValidateServerCertificate() throws Exception + { + // Only run this test for transports over TLS. + Assume.assumeTrue(EnumSet.of(Transport.HTTPS, Transport.H2).contains(transport)); + + startServer(new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + baseRequest.setHandled(true); + } + }); + + // Use a default SslContextFactory, requests should fail because the server certificate is unknown. + client = newHttpClient(provideClientTransport(transport), new SslContextFactory()); + QueuedThreadPool clientThreads = new QueuedThreadPool(); + clientThreads.setName("client"); + client.setExecutor(clientThreads); + client.start(); + + client.newRequest(newURI()) + .timeout(5, TimeUnit.SECONDS) + .send(); + } + private void sleep(long time) throws IOException { try From da6be3ed7a80e834b0adba524a1a07d586948000 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 7 Mar 2016 10:24:57 -0700 Subject: [PATCH 10/21] Fixing javadoc --- .../session/GCloudSessionDataStore.java | 43 +++++----- .../gcloud/session/GCloudSessionManager.java | 22 ++--- .../org/eclipse/jetty/http/HttpContent.java | 2 +- .../java/org/eclipse/jetty/http/HttpURI.java | 7 +- .../jetty/http/pathmap/PathSpecGroup.java | 3 +- .../InfinispanSessionDataStore.java | 8 +- .../InfinispanSessionIdManager.java | 10 +-- .../infinispan/InfinispanSessionManager.java | 4 +- .../jetty/nosql/NoSqlSessionDataStore.java | 9 -- .../nosql/mongodb/MongoSessionDataStore.java | 24 +++--- .../nosql/mongodb/MongoSessionManager.java | 10 +-- .../jetty/server/AbstractNCSARequestLog.java | 3 + .../org/eclipse/jetty/server/PushBuilder.java | 2 +- .../org/eclipse/jetty/server/Request.java | 2 +- .../eclipse/jetty/server/SessionManager.java | 8 +- .../jetty/server/handler/ErrorHandler.java | 2 +- .../session/AbstractSessionDataStore.java | 24 +----- .../session/AbstractSessionIdManager.java | 7 +- .../session/AbstractSessionInspector.java | 10 +-- .../server/session/AbstractSessionStore.java | 32 +++---- .../session/CachingSessionDataStore.java | 2 +- .../jetty/server/session/ExpiryInspector.java | 6 +- .../server/session/FileSessionDataStore.java | 6 +- .../server/session/FileSessionManager.java | 2 +- .../server/session/HashSessionIdManager.java | 2 +- .../jetty/server/session/IdleInspector.java | 2 +- .../server/session/JDBCSessionDataStore.java | 36 +------- .../server/session/JDBCSessionManager.java | 4 +- .../server/session/NullSessionDataStore.java | 2 +- .../session/PeriodicSessionInspector.java | 7 +- .../eclipse/jetty/server/session/Session.java | 41 ++------- .../jetty/server/session/SessionContext.java | 6 +- .../jetty/server/session/SessionData.java | 84 +------------------ .../server/session/SessionDataStore.java | 17 ++-- .../jetty/server/session/SessionManager.java | 12 +-- .../server/session/StalePeriodStrategy.java | 6 +- .../server/session/SessionCookieTest.java | 26 ++---- pom.xml | 5 +- 38 files changed, 139 insertions(+), 359 deletions(-) diff --git a/jetty-gcloud/jetty-gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionDataStore.java b/jetty-gcloud/jetty-gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionDataStore.java index 836c357b99b..eec6f303a64 100644 --- a/jetty-gcloud/jetty-gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionDataStore.java +++ b/jetty-gcloud/jetty-gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionDataStore.java @@ -19,21 +19,6 @@ package org.eclipse.jetty.gcloud.session; -import java.io.ByteArrayOutputStream; -import java.io.ObjectOutputStream; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.atomic.AtomicReference; - -import org.eclipse.jetty.server.session.AbstractSessionDataStore; -import org.eclipse.jetty.server.session.SessionContext; -import org.eclipse.jetty.server.session.SessionData; -import org.eclipse.jetty.util.ClassLoadingObjectInputStream; -import org.eclipse.jetty.util.StringUtil; -import org.eclipse.jetty.util.log.Log; -import org.eclipse.jetty.util.log.Logger; - import com.google.gcloud.datastore.Blob; import com.google.gcloud.datastore.Datastore; import com.google.gcloud.datastore.DatastoreFactory; @@ -50,6 +35,21 @@ import com.google.gcloud.datastore.StructuredQuery.Projection; import com.google.gcloud.datastore.StructuredQuery.ProjectionEntityQueryBuilder; import com.google.gcloud.datastore.StructuredQuery.PropertyFilter; +import java.io.ByteArrayOutputStream; +import java.io.ObjectOutputStream; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; + +import org.eclipse.jetty.server.session.AbstractSessionDataStore; +import org.eclipse.jetty.server.session.SessionContext; +import org.eclipse.jetty.server.session.SessionData; +import org.eclipse.jetty.util.ClassLoadingObjectInputStream; +import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; + /** * GCloudSessionDataStore * @@ -182,7 +182,7 @@ public class GCloudSessionDataStore extends AbstractSessionDataStore } /** - * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set) + * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(Set, int) */ @Override public Set doGetExpired(Set candidates, int expiryTimeoutSec) @@ -296,8 +296,9 @@ public class GCloudSessionDataStore extends AbstractSessionDataStore * * * - * @param session - * @return + * @param id the id + * @param context the session context + * @return the key */ private Key makeKey (String id, SessionContext context) { @@ -308,9 +309,9 @@ public class GCloudSessionDataStore extends AbstractSessionDataStore /** * Generate a gcloud datastore Entity from SessionData - * @param session - * @param key - * @return + * @param session the session data + * @param key the key + * @return the entity * @throws Exception */ private Entity entityFromSession (SessionData session, Key key) throws Exception diff --git a/jetty-gcloud/jetty-gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionManager.java b/jetty-gcloud/jetty-gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionManager.java index dd50b9412eb..03c778f3672 100644 --- a/jetty-gcloud/jetty-gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionManager.java +++ b/jetty-gcloud/jetty-gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionManager.java @@ -18,11 +18,6 @@ package org.eclipse.jetty.gcloud.session; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.locks.ReentrantLock; - -import javax.servlet.http.HttpServletRequest; - import org.eclipse.jetty.server.session.AbstractSessionStore; import org.eclipse.jetty.server.session.MemorySessionStore; import org.eclipse.jetty.server.session.SessionManager; @@ -49,9 +44,7 @@ public class GCloudSessionManager extends SessionManager -/* - - *//** + /* * * Session * * Representation of a session in local memory. @@ -75,7 +68,8 @@ public class GCloudSessionManager extends SessionManager - *//** + */ + /* * * Called on entry to the session. * * @see org.eclipse.jetty.server.session.AbstractSession#access(long) @@ -122,7 +116,7 @@ public class GCloudSessionManager extends SessionManager } - *//** + *//* * * Exit from session * @see org.eclipse.jetty.server.session.AbstractSession#complete() *//* @@ -174,7 +168,7 @@ public class GCloudSessionManager extends SessionManager } } - *//** Test if the session is stale + *//* * Test if the session is stale * @param atTime * @return *//* @@ -184,7 +178,7 @@ public class GCloudSessionManager extends SessionManager } - *//** + *//* * * Reload the session from the cluster. If the node that * last managed the session from the cluster is ourself, * then the session does not need refreshing. @@ -295,7 +289,7 @@ public class GCloudSessionManager extends SessionManager /** * Start the session manager. * - * @see org.eclipse.jetty.server.session.AbstractSessionManager#doStart() + * @see org.eclipse.jetty.server.session.SessionManager#doStart() */ @Override public void doStart() throws Exception @@ -308,7 +302,7 @@ public class GCloudSessionManager extends SessionManager /** * Stop the session manager. * - * @see org.eclipse.jetty.server.session.AbstractSessionManager#doStop() + * @see org.eclipse.jetty.server.session.SessionManager#doStop() */ @Override public void doStop() throws Exception diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpContent.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpContent.java index 50871afede3..e5a059c39a3 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpContent.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpContent.java @@ -73,7 +73,7 @@ public interface HttpContent * @param maxBuffer The maximum buffer to allocated for this request. For cached content, a larger buffer may have * previously been allocated and returned by the {@link HttpContent#getDirectBuffer()} or {@link HttpContent#getIndirectBuffer()} calls. * @return A {@link HttpContent} - * @throws IOException + * @throws IOException if unable to get content */ HttpContent getContent(String path,int maxBuffer) throws IOException; } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java index 411ca29d101..ec54cc5f19c 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java @@ -177,9 +177,10 @@ public class HttpURI } /* ------------------------------------------------------------ */ - /** Parse according to https://tools.ietf.org/html/rfc7230#section-5.3 - * @param method - * @param uri + /** + * Parse according to https://tools.ietf.org/html/rfc7230#section-5.3 + * @param method the request method + * @param uri the request uri */ public void parseRequestTarget(String method,String uri) { diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpecGroup.java b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpecGroup.java index e03a035de10..d45ffc28458 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpecGroup.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpecGroup.java @@ -62,8 +62,7 @@ public enum PathSpecGroup MIDDLE_GLOB, /** * For path specs that have a hardcoded prefix and a trailing wildcard glob. - *

- * + * * 

      *   "/downloads/*"          - servlet spec
      *   "/api/*"                - servlet spec
diff --git a/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionDataStore.java b/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionDataStore.java
index 64c693a2084..62e98eb79bf 100644
--- a/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionDataStore.java
+++ b/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionDataStore.java
@@ -82,7 +82,7 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore
     
     
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#load(org.eclipse.jetty.server.session.SessionKey)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#load(String)
      */
     @Override
     public SessionData load(String id) throws Exception
@@ -120,7 +120,7 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore
     }
 
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#delete(org.eclipse.jetty.server.session.SessionKey)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#delete(String)
      */
     @Override
     public boolean delete(String id) throws Exception
@@ -131,7 +131,7 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore
     }
 
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(Set, int)
      */
     @Override
     public Set doGetExpired(Set candidates, int expiryTimeoutSec)
@@ -202,7 +202,7 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore
     }
 
     /** 
-     * @see org.eclipse.jetty.server.session.AbstractSessionDataStore#doStore(org.eclipse.jetty.server.session.SessionKey, org.eclipse.jetty.server.session.SessionData, long)
+     * @see org.eclipse.jetty.server.session.AbstractSessionDataStore#doStore(String, SessionData, long)
      */
     @Override
     public void doStore(String id, SessionData data, long lastSaveTime) throws Exception
diff --git a/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionIdManager.java b/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionIdManager.java
index 22efa61748a..7ba9c75d63d 100644
--- a/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionIdManager.java
+++ b/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionIdManager.java
@@ -21,17 +21,9 @@ package org.eclipse.jetty.session.infinispan;
 import java.util.Random;
 import java.util.concurrent.TimeUnit;
 
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpSession;
-
-import org.eclipse.jetty.server.Handler;
 import org.eclipse.jetty.server.Server;
-
-import org.eclipse.jetty.server.handler.ContextHandler;
 import org.eclipse.jetty.server.session.AbstractSessionIdManager;
 import org.eclipse.jetty.server.session.Session;
-import org.eclipse.jetty.server.session.SessionHandler;
-import org.eclipse.jetty.server.session.SessionManager;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
 import org.infinispan.commons.api.BasicCache;
@@ -270,7 +262,7 @@ public class InfinispanSessionIdManager extends AbstractSessionIdManager
     }
 
     /** 
-     * @see org.eclipse.jetty.server.SessionIdManager#useId(java.lang.String)
+     * @see org.eclipse.jetty.server.SessionIdManager#useId(Session)
      */
     @Override
     public void useId(Session session)
diff --git a/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionManager.java b/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionManager.java
index 47900962e9c..25bec8dfab8 100644
--- a/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionManager.java
+++ b/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionManager.java
@@ -51,7 +51,7 @@ public class InfinispanSessionManager extends SessionManager
     /**
      * Start the session manager.
      *
-     * @see org.eclipse.jetty.server.session.AbstractSessionManager#doStart()
+     * @see org.eclipse.jetty.server.session.SessionManager#doStart()
      */
     @Override
     public void doStart() throws Exception
@@ -69,7 +69,7 @@ public class InfinispanSessionManager extends SessionManager
     /**
      * Stop the session manager.
      *
-     * @see org.eclipse.jetty.server.session.AbstractSessionManager#doStop()
+     * @see org.eclipse.jetty.server.session.SessionManager#doStop()
      */
     @Override
     public void doStop() throws Exception
diff --git a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/NoSqlSessionDataStore.java b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/NoSqlSessionDataStore.java
index acea527b6d4..7bdb6281c17 100644
--- a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/NoSqlSessionDataStore.java
+++ b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/NoSqlSessionDataStore.java
@@ -40,15 +40,6 @@ public abstract class NoSqlSessionDataStore extends AbstractSessionDataStore
         private Set _dirtyAttributes = new HashSet();
         
 
-        /**
-         * @param id
-         * @param cpath
-         * @param vhost
-         * @param created
-         * @param accessed
-         * @param lastAccessed
-         * @param maxInactiveMs
-         */
         public NoSqlSessionData(String id, String cpath, String vhost, long created, long accessed, long lastAccessed, long maxInactiveMs)
         {
             super(id, cpath, vhost, created, accessed, lastAccessed, maxInactiveMs);
diff --git a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java
index b3cd1f793b7..f43e4f463a5 100644
--- a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java
+++ b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStore.java
@@ -19,6 +19,12 @@
 
 package org.eclipse.jetty.nosql.mongodb;
 
+import com.mongodb.BasicDBObject;
+import com.mongodb.DBCollection;
+import com.mongodb.DBCursor;
+import com.mongodb.DBObject;
+import com.mongodb.WriteConcern;
+
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -36,13 +42,6 @@ import org.eclipse.jetty.util.ClassLoadingObjectInputStream;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
 
-import com.mongodb.BasicDBObject;
-import com.mongodb.DBCollection;
-import com.mongodb.DBCursor;
-import com.mongodb.DBObject;
-import com.mongodb.WriteConcern;
-import com.mongodb.WriteResult;
-
 /**
  * MongoSessionDataStore
  *
@@ -159,9 +158,6 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore
     }
     
     
-    /**
-     * @return
-     */
     public DBCollection getDBCollection ()
     {
         return _dbSessions;
@@ -170,7 +166,7 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore
   
     
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#load(org.eclipse.jetty.server.session.SessionKey)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#load(String)
      */
     @Override
     public SessionData load(String id) throws Exception
@@ -262,7 +258,7 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore
     }
 
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#delete(org.eclipse.jetty.server.session.SessionKey)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#delete(String)
      */
     @Override
     public boolean delete(String id) throws Exception
@@ -320,7 +316,7 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore
     }
 
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(Set, int)
      */
     @Override
     public Set doGetExpired(Set candidates, int expiryTimeoutSec)
@@ -384,7 +380,7 @@ public class MongoSessionDataStore extends NoSqlSessionDataStore
     }
 
     /** 
-     * @see org.eclipse.jetty.server.session.AbstractSessionDataStore#doStore(org.eclipse.jetty.server.session.SessionKey, org.eclipse.jetty.server.session.SessionData, long)
+     * @see org.eclipse.jetty.server.session.AbstractSessionDataStore#doStore(String, SessionData, long) 
      */
     @Override
     public void doStore(String id, SessionData data, long lastSaveTime) throws Exception
diff --git a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionManager.java b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionManager.java
index 4e813b1d007..5fc15d0ff03 100644
--- a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionManager.java
+++ b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionManager.java
@@ -18,21 +18,20 @@
 
 package org.eclipse.jetty.nosql.mongodb;
 
+import com.mongodb.DBCollection;
+import com.mongodb.MongoException;
+
 import java.net.UnknownHostException;
 
 import org.eclipse.jetty.server.SessionIdManager;
 import org.eclipse.jetty.server.session.AbstractSessionStore;
 import org.eclipse.jetty.server.session.MemorySessionStore;
-import org.eclipse.jetty.server.session.SessionDataStore;
 import org.eclipse.jetty.server.session.SessionManager;
 import org.eclipse.jetty.util.annotation.ManagedAttribute;
 import org.eclipse.jetty.util.annotation.ManagedObject;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
 
-import com.mongodb.DBCollection;
-import com.mongodb.MongoException;
-
 
 /**
  * MongoSessionManager
@@ -116,9 +115,6 @@ public class MongoSessionManager extends SessionManager
     }
 
     /* ------------------------------------------------------------ */
-    /**
-     * @see org.eclipse.jetty.server.session.AbstractSessionManager#setSessionIdManager(org.eclipse.jetty.server.SessionIdManager)
-     */
     @Override
     public void setSessionIdManager(SessionIdManager metaManager)
     {
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractNCSARequestLog.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractNCSARequestLog.java
index 0df61d8f739..9b0f8672e71 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractNCSARequestLog.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractNCSARequestLog.java
@@ -222,6 +222,9 @@ public abstract class AbstractNCSARequestLog extends AbstractLifeCycle implement
     }
 
     /**
+     * @param request request object
+     * @param b StringBuilder to write to
+     * @throws IOException if unable to append extended log
      * @deprecated override {@link #logExtended(StringBuilder, Request, Response)} instead
      */
     @Deprecated
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilder.java b/jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilder.java
index 6300a09f2db..8771f3b29e5 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilder.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/PushBuilder.java
@@ -179,7 +179,7 @@ public interface PushBuilder
      * Set the URI path to be used for the push.  The path may start
      * with "/" in which case it is treated as an absolute path,
      * otherwise it is relative to the context path of the associated
-     * request.  There is no path default and {@link #path(String)} must
+     * request.  There is no path default and path(String) must
      * be called before every call to {@link #push()}.  If a query
      * string is present in the argument {@code path}, its contents must
      * be merged with the contents previously passed to {@link
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 55ec1899cc9..e8eb91c6f6c 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
@@ -2036,7 +2036,7 @@ public class Request implements HttpServletRequest
 
     /* ------------------------------------------------------------ */
     /**
-     * @return True if this is the first call of {@link #takeNewContext()} since the last
+     * @return True if this is the first call of takeNewContext() since the last
      *         {@link #setContext(org.eclipse.jetty.server.handler.ContextHandler.Context)} call.
      */
     public boolean takeNewContext()
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/SessionManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/SessionManager.java
index 1dd7bbc6853..071ed64340a 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/SessionManager.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/SessionManager.java
@@ -32,15 +32,9 @@ import org.eclipse.jetty.server.session.SessionHandler;
 import org.eclipse.jetty.server.session.SessionStore;
 import org.eclipse.jetty.util.component.LifeCycle;
 
-/* --------------------------------------------------------------------- */
 /**
  * Session Manager.
  * The API required to manage sessions for a servlet context.
- *
- */
-
-/* ------------------------------------------------------------ */
-/**
  */
 public interface SessionManager extends LifeCycle
 {
@@ -313,7 +307,7 @@ public interface SessionManager extends LifeCycle
     
     /**
      * Get the session store for this manager
-     * @return
+     * @return the session store
      */
     public SessionStore getSessionStore();
     
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
index 9d64b769c26..018c5f201db 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
@@ -49,7 +49,7 @@ import org.eclipse.jetty.util.log.Logger;
 /* ------------------------------------------------------------ */
 /** Handler for Error pages
  * An ErrorHandler is registered with {@link ContextHandler#setErrorHandler(ErrorHandler)} or
- * {@link Server#setErrorHandler(ErrorHandler).
+ * {@link Server#setErrorHandler(ErrorHandler)}.
  * It is called by the HttpResponse.sendError method to write a error page via {@link #handle(String, Request, HttpServletRequest, HttpServletResponse)}
  * or via {@link #badMessageError(int, String, HttpFields)} for bad requests for which a dispatch cannot be done.
  *
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java
index d7bfee1f831..afa542f0fed 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java
@@ -42,7 +42,7 @@ public abstract class AbstractSessionDataStore extends AbstractLifeCycle impleme
      * @param id identity of session to store
      * @param data info of the session
      * @param lastSaveTime time of previous save or 0 if never saved
-     * @throws Exception
+     * @throws Exception if unable to store data
      */
     public abstract void doStore(String id, SessionData data, long lastSaveTime) throws Exception;
 
@@ -54,7 +54,6 @@ public abstract class AbstractSessionDataStore extends AbstractLifeCycle impleme
      * @param candidates the ids of sessions the SessionStore thinks has expired
      * @param scavengePeriodSec the period in sec of the scavenge cycle checks
      * @return the reconciled set of session ids that this node should attempt to expire
-     * @throws Exception
      */
     public abstract Set doGetExpired (Set candidates, int scavengePeriodSec);
 
@@ -123,18 +122,12 @@ public abstract class AbstractSessionDataStore extends AbstractLifeCycle impleme
         return new SessionData(id, _context.getCanonicalContextPath(), _context.getVhost(), created, accessed, lastAccessed, maxInactiveMs);
     }
  
-    /**
-     * @throws IllegalStateException
-     */
     protected void checkStarted () throws IllegalStateException
     {
         if (isStarted())
             throw new IllegalStateException("Already started");
     }
 
-
-
-
     @Override
     protected void doStart() throws Exception
     {
@@ -144,28 +137,13 @@ public abstract class AbstractSessionDataStore extends AbstractLifeCycle impleme
         super.doStart();
     }
 
-
-
-
-    /**
-     * @return
-     */
     public int getGracePeriodSec()
     {
         return _gracePeriodSec;
     }
 
-
-
-
-    /**
-     * @param sec
-     */
     public void setGracePeriodSec(int sec)
     {
         _gracePeriodSec = sec;
     }
-
-    
-    
 }
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionIdManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionIdManager.java
index 39b477cb1a4..5710059861f 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionIdManager.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionIdManager.java
@@ -100,7 +100,7 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme
     
     /* ------------------------------------------------------------ */
     /**
-     * @param period inspector of sessions 
+     * @param inspector inspector of sessions
      */
     public void setSessionInspector (PeriodicSessionInspector inspector)
     {
@@ -219,7 +219,7 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme
 
     /* ------------------------------------------------------------ */
     /**
-     * @param seedTerm
+     * @param seedTerm the seed for RNG
      * @return a new unique session id
      */
     public String newSessionId(long seedTerm)
@@ -399,9 +399,6 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme
     }
 
     /* ------------------------------------------------------------ */
-    /**
-     * @param id
-     */
     public void invalidateAll (String id)
     {
         //take the id out of the list of known sessionids for this node
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionInspector.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionInspector.java
index 45aceaef47c..b9163da897f 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionInspector.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionInspector.java
@@ -21,15 +21,15 @@ package org.eclipse.jetty.server.session;
 
 /**
  * AbstractInspector
- *
- *
  */
 public abstract class AbstractSessionInspector implements SessionInspector
 {
     /**
-     * <0 means never inspect
-     * 0 means always inspect
-     * >0 means inspect at that interval
+     * 
    
+     *     
  • <0 means never inspect
    • 
+     *     
  • 0 means always inspect
    • 
+     *     
  • >0 means inspect at that interval
    • 
+     * 

      */
     protected int _timeoutSec = -1;
     
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java
index eaab680d220..8139a36f92c 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java
@@ -21,7 +21,6 @@ package org.eclipse.jetty.server.session;
 
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.List;
 import java.util.Set;
 import java.util.stream.Stream;
 
@@ -56,7 +55,7 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
 
     /**
      * Create a new Session object from session data
-     * @param data
+     * @param data the session data
      * @return a new Session object
      */
     public abstract Session newSession (SessionData data);
@@ -84,9 +83,9 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
     
     /**
      * Replace the mapping from id to oldValue with newValue
-     * @param id
-     * @param oldValue
-     * @param newValue
+     * @param id the id
+     * @param oldValue the old value
+     * @param newValue the new value
      * @return true if replacement was done
      */
     public abstract boolean doReplace (String id, Session oldValue, Session newValue);
@@ -95,7 +94,7 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
     
     /**
      * Check to see if the session exists in the store
-     * @param id
+     * @param id the id
      * @return true if the Session object exists in the session store
      */
     public abstract boolean doExists (String id);
@@ -104,7 +103,7 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
     
     /**
      * Remove the session with this identity from the store
-     * @param id
+     * @param id the id
      * @return true if removed false otherwise
      */
     public abstract Session doDelete (String id);
@@ -113,14 +112,12 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
     
     /**
      * PlaceHolder
-     *
-     *
      */
     protected class PlaceHolderSession extends Session
     {
 
         /**
-         * @param data
+         * @param data the session data
          */
         public PlaceHolderSession(SessionData data)
         {
@@ -139,7 +136,7 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
     
     
     /**
-     * @param manager
+     * @param manager the SessionManager
      */
     public void setSessionManager (SessionManager manager)
     {
@@ -216,7 +213,7 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
     }
 
     /**
-     * @param sessionDataStore
+     * @param sessionDataStore the session datastore
      */
     public void setSessionDataStore(SessionDataStore sessionDataStore)
     {
@@ -402,7 +399,7 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
     /**
      * Load the info for the session from the session data store
      * 
-     * @param id
+     * @param id the id
      * @return a Session object filled with data or null if the session doesn't exist
      * @throws Exception
      */
@@ -555,7 +552,7 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
 
 
     /** 
-     * @see org.eclipse.jetty.server.session.SessionStore#checkExpiry(java.util.Set)
+     * @see org.eclipse.jetty.server.session.SessionStore#checkExpiration(Set)
      */
     @Override
     public Set checkExpiration(Set candidates)
@@ -695,23 +692,16 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements
     }
     
     
-    /**
-     * @param passivateOnComplete
-     */
     public void setPassivateOnComplete (boolean passivateOnComplete)
     {
         _passivateOnComplete = passivateOnComplete;
     }
     
     
-    /**
-     * @return
-     */
     public boolean isPassivateOnComplete ()
     {
         return _passivateOnComplete;
     }
-    
 
     /** 
      * @see org.eclipse.jetty.server.session.SessionStore#newSession(javax.servlet.http.HttpServletRequest, java.lang.String, long, long)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/CachingSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/CachingSessionDataStore.java
index d4ff31ee570..5248fd3be9a 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/CachingSessionDataStore.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/CachingSessionDataStore.java
@@ -116,7 +116,7 @@ public class CachingSessionDataStore extends AbstractSessionDataStore
     }
 
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(Set, int)
      */
     @Override
     public Set doGetExpired(Set candidates, int expiryTimeoutSec)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/ExpiryInspector.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/ExpiryInspector.java
index 6ed53f7eae5..3d3614b59b0 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/ExpiryInspector.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/ExpiryInspector.java
@@ -42,8 +42,8 @@ public class ExpiryInspector extends AbstractSessionInspector
     
     
     /**
-     * @param sessionStore
-     * @param idManager
+     * @param sessionStore the session store
+     * @param idManager the session id manager
      */
     public ExpiryInspector (AbstractSessionStore sessionStore, SessionIdManager idManager)
     {
@@ -52,7 +52,7 @@ public class ExpiryInspector extends AbstractSessionInspector
     }
     
     /** 
-     * @see org.eclipse.jetty.server.session.SessionInspector#inspect(org.eclipse.jetty.server.session.SessionStore, org.eclipse.jetty.server.session.Session)
+     * @see org.eclipse.jetty.server.session.SessionInspector#inspect(Session)
      */
     @Override
     public void inspect(Session s)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java
index 9c7a3913b48..1b58d645ad9 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java
@@ -111,7 +111,7 @@ public class FileSessionDataStore extends AbstractSessionDataStore
 
 
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(Set, int)
      */
     @Override
     public Set doGetExpired(Set candidates, int expiryTimeoutSec)
@@ -258,7 +258,7 @@ public class FileSessionDataStore extends AbstractSessionDataStore
 
     /**
      * @param id identity of session
-     * @return
+     * @return the filename of the session data store
      */
     private String getFileName (String id)
     {
@@ -268,7 +268,7 @@ public class FileSessionDataStore extends AbstractSessionDataStore
 
     /**
      * @param is inputstream containing session data
-     * @return
+     * @return the session data
      * @throws Exception
      */
     private SessionData load (InputStream is)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionManager.java
index 090841b5c18..1f79b33baa4 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionManager.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionManager.java
@@ -54,7 +54,7 @@ public class FileSessionManager extends SessionManager
     
     /**
      * Get the SessionDataStore to configure it
-     * @return
+     * @return the session datastore
      */
     public FileSessionDataStore getSessionDataStore()
     {
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionIdManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionIdManager.java
index 6bb37a2273a..a6948dc5e7f 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionIdManager.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionIdManager.java
@@ -32,7 +32,7 @@ import org.eclipse.jetty.util.ConcurrentHashSet;
 public class HashSessionIdManager extends AbstractSessionIdManager
 {
     /**
-     * @param server
+     * @param server the server
      */
     public HashSessionIdManager(Server server)
     {
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/IdleInspector.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/IdleInspector.java
index dc3915ade69..bac6787f716 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/IdleInspector.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/IdleInspector.java
@@ -39,7 +39,7 @@ public class IdleInspector extends AbstractSessionInspector
     protected AbstractSessionStore _sessionStore;
     
     /**
-     * @param sessionStore
+     * @param sessionStore the session store
      */
     public IdleInspector (AbstractSessionStore sessionStore)
     {
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionDataStore.java
index fc32eaf8458..cb67d5de33d 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionDataStore.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionDataStore.java
@@ -518,10 +518,7 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore
         
         /**
          * Set up the tables in the database
-         * @throws SQLException
-         */
-        /**
-         * @throws SQLException
+         * @throws SQLException if unable to prepare tables
          */
         public void prepareTables()
         throws SQLException
@@ -782,7 +779,7 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore
 
 
     /** 
-     * @see org.eclipse.jetty.server.session.AbstractSessionDataStore#doStore(java.lang.String, org.eclipse.jetty.server.session.SessionData, boolean)
+     * @see org.eclipse.jetty.server.session.AbstractSessionDataStore#doStore(String, SessionData, long)
      */
     @Override
     public void doStore(String id, SessionData data, long lastSaveTime) throws Exception
@@ -887,7 +884,7 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore
 
 
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(Set, int)
      */
     @Override
     public Set doGetExpired(Set candidates, int scavengeIntervalSec)
@@ -999,45 +996,29 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore
     }
     
     
-    /**
-     * @param dbAdaptor
-     */
     public void setDatabaseAdaptor (DatabaseAdaptor dbAdaptor)
     {
         checkStarted();
         _dbAdaptor = dbAdaptor;
     }
     
-    /**
-     * @param schema
-     */
     public void setSessionTableSchema (SessionTableSchema schema)
     {
         checkStarted();
         _sessionTableSchema = schema;
     }
 
-    /**
-     * @param attempts
-     */
     public void setLoadAttempts (int attempts)
     {
         checkStarted();
         _attempts = attempts;
     }
 
-    /**
-     * @return
-     */
     public int getLoadAttempts ()
     {
         return _attempts;
     }
     
-    /**
-     * @param id
-     * @return
-     */
     public boolean loadAttemptsExhausted (String id)
     {
         AtomicInteger i = _unloadables.get(id);
@@ -1046,9 +1027,6 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore
         return (i.get() >= _attempts);
     }
     
-    /**
-     * @param delete
-     */
     public void setDeleteUnloadableSessions (boolean delete)
     {
         checkStarted();
@@ -1064,9 +1042,6 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore
     }
     
     
-    /**
-     * @param id
-     */
     protected void incLoadAttempt (String id)
     {
         AtomicInteger i = new AtomicInteger(0);
@@ -1079,7 +1054,7 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore
   
     
     /**
-     * @param id
+     * @param id the id
      * @return number of attempts to load the given id
      */
     public int getLoadAttempts (String id)
@@ -1107,9 +1082,6 @@ public class JDBCSessionDataStore extends AbstractSessionDataStore
     }
 
 
-
-
-
    /** 
     * @see org.eclipse.jetty.server.session.SessionDataStore#isPassivating()
     */
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java
index 43018a7a2bf..83b6c9ee600 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java
@@ -55,7 +55,7 @@ public class JDBCSessionManager extends SessionManager
     
     /**
      * Get the db adaptor to configure jdbc settings
-     * @return
+     * @return the database adaptor
      */
     public DatabaseAdaptor getDatabaseAdaptor()
     {
@@ -64,7 +64,7 @@ public class JDBCSessionManager extends SessionManager
     
     /**
      * Get the SessionDataStore to configure it
-     * @return
+     * @return the session data store
      */
     public JDBCSessionDataStore getSessionDataStore ()
     {
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionDataStore.java
index 18730723459..9b0e1144cfd 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionDataStore.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionDataStore.java
@@ -69,7 +69,7 @@ public class NullSessionDataStore extends AbstractSessionDataStore
 
  
     /** 
-     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(java.util.Set)
+     * @see org.eclipse.jetty.server.session.SessionDataStore#getExpired(Set, int)
      */
     @Override
     public Set doGetExpired(Set candidates, int expiryTimeoutSec)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/PeriodicSessionInspector.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/PeriodicSessionInspector.java
index 63807a62bf6..afc40674d30 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/PeriodicSessionInspector.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/PeriodicSessionInspector.java
@@ -19,7 +19,6 @@
 
 package org.eclipse.jetty.server.session;
 
-import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jetty.server.SessionIdManager;
@@ -77,7 +76,7 @@ public class PeriodicSessionInspector extends AbstractLifeCycle
     
     /**
      * SessionIdManager associated with this scavenger
-     * @param sessionIdManager
+     * @param sessionIdManager the session id manager
      */
     public void setSessionIdManager (SessionIdManager sessionIdManager)
     {
@@ -137,7 +136,7 @@ public class PeriodicSessionInspector extends AbstractLifeCycle
     
     /**
      * Set the period between scavenge cycles
-     * @param sec
+     * @param sec the interval (in seconds)
      */
     public void setIntervalSec (long sec)
     {
@@ -176,7 +175,7 @@ public class PeriodicSessionInspector extends AbstractLifeCycle
     /**
      * Get the period between inspection cycles.
      * 
-     * @return
+     * @return the interval (in seconds)
      */
     public long getIntervalSec ()
     {
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java
index add98ac4852..b46df8efb9f 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java
@@ -90,8 +90,8 @@ public class Session implements SessionManager.SessionIf
     /**
      * Create a new session
      * 
-     * @param request
-     * @param data
+     * @param request the request the session should be based on
+     * @param data the session data
      */
     public Session (HttpServletRequest request, SessionData data)
     {
@@ -104,7 +104,7 @@ public class Session implements SessionManager.SessionIf
     
     /**
      * Re-create an existing session
-     * @param data
+     * @param data the session data
      */
     public Session (SessionData data)
     {
@@ -116,7 +116,7 @@ public class Session implements SessionManager.SessionIf
      * Should call this method with a lock held if you want to
      * make decision on what to do with the session
      * 
-     * @return
+     * @return the number of active requests for this session
      */
     public long getRequests()
     {
@@ -206,18 +206,12 @@ public class Session implements SessionManager.SessionIf
     
     
     /* ------------------------------------------------------------ */
-    /**
-     * @param nodename
-     */
     public void setLastNode (String nodename)
     {
         _sessionData.setLastNode(nodename);
     }
     
     /* ------------------------------------------------------------ */
-    /**
-     * @return
-     */
     public String getLastNode ()
     {
         return _sessionData.getLastNode();
@@ -479,9 +473,6 @@ public class Session implements SessionManager.SessionIf
     
     
     /* ------------------------------------------------------------- */
-    /**
-     * @throws IllegalStateException
-     */
     protected void checkLocked ()
     throws IllegalStateException
     {
@@ -640,9 +631,6 @@ public class Session implements SessionManager.SessionIf
     }
 
     /* ------------------------------------------------------------ */
-    /**
-     * @param request
-     */
     public void renewId(HttpServletRequest request)
     {
         if (_manager == null)
@@ -663,7 +651,7 @@ public class Session implements SessionManager.SessionIf
        
     
     /* ------------------------------------------------------------- */
-    /** Swap the id on a session from old to new, keeping the object
+    /* * Swap the id on a session from old to new, keeping the object
      * the same.
      * 
      * @param oldId
@@ -739,22 +727,16 @@ public class Session implements SessionManager.SessionIf
             LOG.warn(e);
         }
     }
-    
- 
-    
+
     /* ------------------------------------------------------------- */
     /** Grab the lock on the session
-     * @return
+     * @return the lock
      */
     public Lock lock ()
     {
         return _lock.lock();
     }
 
-
-
-
-
     /* ------------------------------------------------------------- */
     protected void doInvalidate() throws IllegalStateException
     {
@@ -863,21 +845,12 @@ public class Session implements SessionManager.SessionIf
         _passivationState = PassivationState.ACTIVE;
     }
 
-
-    /**
-     * @return
-     */
     public boolean isActive ()
     {
         checkLocked();
         return _passivationState == PassivationState.ACTIVE;
     }
 
-
-
-    /**
-     * @return
-     */
     public boolean isPassivated ()
     {
         checkLocked();
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionContext.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionContext.java
index 14cd396f0b0..10c9bf71831 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionContext.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionContext.java
@@ -78,7 +78,7 @@ public class SessionContext
     /**
      * Run a runnable in the context (with context classloader set) if
      * there is one, otherwise just run it.
-     * @param r
+     * @param r the runnable
      */
     public void run (Runnable r)
     {
@@ -120,8 +120,8 @@ public class SessionContext
     /**
      * Make an acceptable name from a context path.
      *
-     * @param path
-     * @return
+     * @param path the path to normalize/fix
+     * @return the clean/acceptable form of the path
      */
     private String canonicalize (String path)
     {
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java
index b96b4773a97..0dc66ab09de 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java
@@ -62,16 +62,6 @@ public class SessionData implements Serializable
     protected boolean _dirty;
     protected long _lastSaved; //time in msec since last save
     
-
-    /**
-     * @param id
-     * @param cpath
-     * @param vhost
-     * @param created
-     * @param accessed
-     * @param lastAccessed
-     * @param maxInactiveMs
-     */
     public SessionData (String id, String cpath, String vhost, long created, long accessed, long lastAccessed, long maxInactiveMs)
     {
         _id = id;
@@ -123,10 +113,6 @@ public class SessionData implements Serializable
     }
 
 
-
-    /**
-     * @param lastSaved
-     */
     public void setLastSaved(long lastSaved)
     {
         _lastSaved = lastSaved;
@@ -141,16 +127,13 @@ public class SessionData implements Serializable
         return _dirty;
     }
 
-    /**
-     * @param dirty
-     */
     public void setDirty(boolean dirty)
     {
         _dirty = dirty;
     }
     
     /**
-     * @param name
+     * @param name the name of the attribute
      * @return the value of the attribute named
      */
     public Object getAttribute (String name)
@@ -166,11 +149,6 @@ public class SessionData implements Serializable
         return _attributes.keySet();
     }
     
-    /**
-     * @param name
-     * @param value
-     * @return
-     */
     public Object setAttribute (String name, Object value)
     {
         Object old = (value==null?_attributes.remove(name):_attributes.put(name,value));
@@ -181,20 +159,11 @@ public class SessionData implements Serializable
        return old;
     }
     
-    
-    /**
-     * @param name
-     */
     public void setDirty (String name)
     {
         setDirty (true);
     }
     
-    
-    
-    /**
-     * @param attributes
-     */
     public void putAllAttributes (Map attributes)
     {
         _attributes.putAll(attributes);
@@ -224,9 +193,6 @@ public class SessionData implements Serializable
         return _id;
     }
 
-    /**
-     * @param id
-     */
     public void setId(String id)
     {
         _id = id;
@@ -240,9 +206,6 @@ public class SessionData implements Serializable
         return _contextPath;
     }
 
-    /**
-     * @param contextPath
-     */
     public void setContextPath(String contextPath)
     {
         _contextPath = contextPath;
@@ -256,9 +219,6 @@ public class SessionData implements Serializable
         return _vhost;
     }
 
-    /**
-     * @param vhost
-     */
     public void setVhost(String vhost)
     {
         _vhost = vhost;
@@ -272,9 +232,6 @@ public class SessionData implements Serializable
         return _lastNode;
     }
 
-    /**
-     * @param lastNode
-     */
     public void setLastNode(String lastNode)
     {
         _lastNode = lastNode;
@@ -288,25 +245,16 @@ public class SessionData implements Serializable
         return _expiry;
     }
 
-    /**
-     * @param expiry
-     */
     public void setExpiry(long expiry)
     {
         _expiry = expiry;
     }
 
-    /**
-     * @return
-     */
     public long getCreated()
     {
         return _created;
     }
 
-    /**
-     * @param created
-     */
     public void setCreated(long created)
     {
         _created = created;
@@ -320,9 +268,6 @@ public class SessionData implements Serializable
         return _cookieSet;
     }
 
-    /**
-     * @param cookieSet
-     */
     public void setCookieSet(long cookieSet)
     {
         _cookieSet = cookieSet;
@@ -336,9 +281,6 @@ public class SessionData implements Serializable
         return _accessed;
     }
 
-    /**
-     * @param accessed
-     */
     public void setAccessed(long accessed)
     {
         _accessed = accessed;
@@ -352,35 +294,21 @@ public class SessionData implements Serializable
         return _lastAccessed;
     }
 
-    /**
-     * @param lastAccessed
-     */
     public void setLastAccessed(long lastAccessed)
     {
         _lastAccessed = lastAccessed;
     }
 
-
-    /**
-     * @return
-     */
     public long getMaxInactiveMs()
     {
         return _maxInactiveMs;
     }
 
-    /**
-     * @param maxInactive
-     */
     public void setMaxInactiveMs(long maxInactive)
     {
         _maxInactiveMs = maxInactive;
     }
 
-    /**
-     * @param out
-     * @throws IOException
-     */
     private void writeObject(java.io.ObjectOutputStream out) throws IOException
     {  
         out.writeUTF(_id); //session id
@@ -398,11 +326,6 @@ public class SessionData implements Serializable
         out.writeObject(_attributes);
     }
     
-    /**
-     * @param in
-     * @throws IOException
-     * @throws ClassNotFoundException
-     */
     private void readObject(java.io.ObjectInputStream in) throws IOException, ClassNotFoundException
     {
         _id = in.readUTF();
@@ -419,11 +342,6 @@ public class SessionData implements Serializable
         _attributes = (ConcurrentHashMap)in.readObject();
     }
     
-    
-    /**
-     * @param time
-     * @return
-     */
     public boolean isExpiredAt (long time)
     {
         if (LOG.isDebugEnabled())
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionDataStore.java
index 80ea9836553..b7ffd1edd95 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionDataStore.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionDataStore.java
@@ -46,18 +46,18 @@ public interface SessionDataStore extends LifeCycle
      * Read in session data from storage
      * @param id identity of session to load
      * @return the SessionData matching the id
-     * @throws Exception
+     * @throws Exception if unable to load session data
      */
     public SessionData load (String id) throws Exception;
     
     
     /**
      * Create a new SessionData 
-     * @param id
-     * @param created
-     * @param accessed
-     * @param lastAccessed
-     * @param maxInactiveMs
+     * @param id the id
+     * @param created the timestamp when created
+     * @param accessed the timestamp when accessed
+     * @param lastAccessed the timestamp when last accessed
+     * @param maxInactiveMs the max inactive time in milliseconds
      * @return  a new SessionData object
      */
     public SessionData newSessionData (String id, long created, long accessed, long lastAccessed, long maxInactiveMs);
@@ -69,7 +69,7 @@ public interface SessionDataStore extends LifeCycle
      * Write out session data to storage
      * @param id identity of session to store
      * @param data info of session to store
-     * @throws Exception
+     * @throws Exception if unable to write session data
      */
     public void store (String id, SessionData data) throws Exception;
     
@@ -79,7 +79,7 @@ public interface SessionDataStore extends LifeCycle
      * Delete session data from storage
      * @param id identity of session to delete
      * @return true if the session was deleted from the permanent store
-     * @throws Exception
+     * @throws Exception if unable to delete session data
      */
     public boolean delete (String id) throws Exception;
     
@@ -93,6 +93,7 @@ public interface SessionDataStore extends LifeCycle
      * @param candidates if provided, these are keys of sessions that
      * the SessionStore thinks has expired and should be verified by the
      * SessionDataStore
+     * @param scavengePeriodSec the time to attempt scavenge (in seconds)
      * @return set of session ids
      */
     public Set getExpired (Set candidates, int scavengePeriodSec);
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionManager.java
index 543c61e2fcf..e0210616498 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionManager.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionManager.java
@@ -19,8 +19,6 @@
 
 package org.eclipse.jetty.server.session;
 
-import static java.lang.Math.round;
-
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -53,7 +51,8 @@ import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
 import org.eclipse.jetty.util.statistic.CounterStatistic;
 import org.eclipse.jetty.util.statistic.SampleStatistic;
-import org.eclipse.jetty.util.thread.Locker.Lock;
+
+import static java.lang.Math.round;
 
 
 
@@ -766,7 +765,7 @@ public class SessionManager extends ContainerLifeCycle implements org.eclipse.je
 
     /* ------------------------------------------------------------ */
     /**
-     * @return
+     * @return the session store
      */
     public SessionStore getSessionStore ()
     {
@@ -1006,7 +1005,7 @@ public class SessionManager extends ContainerLifeCycle implements org.eclipse.je
      * Called either when a session has expired, or the app has
      * invalidated it.
      * 
-     * @param id
+     * @param id the id to invalidate
      */
     public void invalidate (String id)
     {
@@ -1032,9 +1031,6 @@ public class SessionManager extends ContainerLifeCycle implements org.eclipse.je
 
     
     /* ------------------------------------------------------------ */
-    /**
-     * @return
-     */
     public void inspect ()
     {
         //don't attempt to scavenge if we are shutting down
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/StalePeriodStrategy.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/StalePeriodStrategy.java
index 6dd9756c693..7de75b846db 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/StalePeriodStrategy.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/StalePeriodStrategy.java
@@ -55,10 +55,6 @@ public class StalePeriodStrategy implements StalenessStrategy
             
     }
     
-    
-    /**
-     * @return
-     */
     public long getStaleSec ()
     {
         return (_staleMs<=0?0L:_staleMs/1000L);
@@ -67,7 +63,7 @@ public class StalePeriodStrategy implements StalenessStrategy
     /**
      * The amount of time in seconds that a session can be held
      * in memory without being refreshed from the cluster.
-     * @param sec
+     * @param sec the time in seconds
      */
     public void setStaleSec (long sec)
     {
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/session/SessionCookieTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/session/SessionCookieTest.java
index e9b6b82039d..f7ab4636360 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/session/SessionCookieTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/session/SessionCookieTest.java
@@ -18,22 +18,18 @@
 
 package org.eclipse.jetty.server.session;
 
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
-import java.util.Enumeration;
-import java.util.Map;
-import java.util.Set;
 import java.util.stream.Stream;
 
 import javax.servlet.SessionCookieConfig;
 import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpSession;
 
 import org.eclipse.jetty.http.HttpCookie;
 import org.eclipse.jetty.server.Server;
 import org.junit.Test;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
 /**
  * SessionCookieTest
  */
@@ -46,7 +42,7 @@ public class SessionCookieTest
     {
 
         /** 
-         * @see org.eclipse.jetty.server.session.SessionStore#newSession(org.eclipse.jetty.server.session.SessionKey, long, long, long, long)
+         * @see org.eclipse.jetty.server.session.SessionStore#newSession(HttpServletRequest, String, long, long)
          */
         @Override
         public Session newSession(HttpServletRequest request, String key, long time, long maxInactiveMs)
@@ -76,7 +72,7 @@ public class SessionCookieTest
         }
 
         /** 
-         * @see org.eclipse.jetty.server.session.AbstractSessionStore#doGet(org.eclipse.jetty.server.session.SessionKey)
+         * @see org.eclipse.jetty.server.session.AbstractSessionStore#doGet(String)
          */
         @Override
         public Session doGet(String key)
@@ -86,7 +82,7 @@ public class SessionCookieTest
         }
 
         /** 
-         * @see org.eclipse.jetty.server.session.AbstractSessionStore#doPutIfAbsent(org.eclipse.jetty.server.session.SessionKey, org.eclipse.jetty.server.session.Session)
+         * @see org.eclipse.jetty.server.session.AbstractSessionStore#doPutIfAbsent(String, Session)
          */
         @Override
         public Session doPutIfAbsent(String key, Session session)
@@ -95,7 +91,7 @@ public class SessionCookieTest
         }
 
         /** 
-         * @see org.eclipse.jetty.server.session.AbstractSessionStore#doExists(org.eclipse.jetty.server.session.SessionKey)
+         * @see org.eclipse.jetty.server.session.AbstractSessionStore#doExists(String)
          */
         @Override
         public boolean doExists(String key)
@@ -105,7 +101,7 @@ public class SessionCookieTest
         }
 
         /** 
-         * @see org.eclipse.jetty.server.session.AbstractSessionStore#doDelete(org.eclipse.jetty.server.session.SessionKey)
+         * @see org.eclipse.jetty.server.session.AbstractSessionStore#doDelete(String)
          */
         @Override
         public Session doDelete(String key)
@@ -140,10 +136,6 @@ public class SessionCookieTest
     
     public class MockSessionIdManager extends AbstractSessionIdManager
     {
-
-        /**
-         * @param server
-         */
         public MockSessionIdManager(Server server)
         {
             super(server);
@@ -175,7 +167,7 @@ public class SessionCookieTest
         }
 
         /** 
-         * @see org.eclipse.jetty.server.SessionIdManager#useId(java.lang.String)
+         * @see org.eclipse.jetty.server.SessionIdManager#useId(Session)
          */
         @Override
         public void useId(Session session)
diff --git a/pom.xml b/pom.xml
index 4198564f590..de39a4da86a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -368,15 +368,16 @@
         
           org.apache.maven.plugins
           maven-javadoc-plugin
+          2.8
           
             true
-            false
+            true
             true
+            protected
             com.acme.*;org.slf4j.*;org.mortbay.*
             
               http://docs.oracle.com/javase/8/docs/api/
               http://docs.oracle.com/javaee/7/api/
-              http://junit.org/javadoc/latest/
               http://download.eclipse.org/jetty/stable-9/apidocs/
             
             

From cc9a19775b4afe2eb7cb2927098e8c5ab08a2bb4 Mon Sep 17 00:00:00 2001
From: Joakim Erdfelt 
Date: Mon, 7 Mar 2016 16:25:54 -0700
Subject: [PATCH 11/21] Issue #343 - ensure release deployment of
 test-jetty-webapp:war and test-proxy-webapp:war

---
 tests/test-webapps/pom.xml                   | 2 +-
 tests/test-webapps/test-jetty-webapp/pom.xml | 8 ++++++++
 tests/test-webapps/test-proxy-webapp/pom.xml | 8 ++++++++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/tests/test-webapps/pom.xml b/tests/test-webapps/pom.xml
index 90bda2a00d0..7b94dcfd8b2 100644
--- a/tests/test-webapps/pom.xml
+++ b/tests/test-webapps/pom.xml
@@ -37,7 +37,7 @@
           
           true
         
-          
+      
     
   
   
diff --git a/tests/test-webapps/test-jetty-webapp/pom.xml b/tests/test-webapps/test-jetty-webapp/pom.xml
index 4568dbcd62b..28b0ad58e04 100644
--- a/tests/test-webapps/test-jetty-webapp/pom.xml
+++ b/tests/test-webapps/test-jetty-webapp/pom.xml
@@ -148,6 +148,14 @@
           
         
       
+      
+        org.apache.maven.plugins
+        maven-deploy-plugin
+        
+          
+          false
+        
+      
     
   
   
diff --git a/tests/test-webapps/test-proxy-webapp/pom.xml b/tests/test-webapps/test-proxy-webapp/pom.xml
index e26ac325fea..5943b7b9e24 100644
--- a/tests/test-webapps/test-proxy-webapp/pom.xml
+++ b/tests/test-webapps/test-proxy-webapp/pom.xml
@@ -41,6 +41,14 @@
           
         
       
+      
+        org.apache.maven.plugins
+        maven-deploy-plugin
+        
+          
+          false
+        
+      
     
   
   

From 2cc50ed92522e28232865189be487a0b21f93b62 Mon Sep 17 00:00:00 2001
From: Joakim Erdfelt 
Date: Mon, 7 Mar 2016 16:32:36 -0700
Subject: [PATCH 12/21] Issue #316 - Add *.chm mimetype mapping

---
 .../src/main/resources/org/eclipse/jetty/http/mime.properties    | 1 +
 1 file changed, 1 insertion(+)

diff --git a/jetty-http/src/main/resources/org/eclipse/jetty/http/mime.properties b/jetty-http/src/main/resources/org/eclipse/jetty/http/mime.properties
index 654454c7a14..302cbd0a0b5 100644
--- a/jetty-http/src/main/resources/org/eclipse/jetty/http/mime.properties
+++ b/jetty-http/src/main/resources/org/eclipse/jetty/http/mime.properties
@@ -12,6 +12,7 @@ bcpio=application/x-bcpio
 bin=application/octet-stream
 cab=application/x-cabinet
 cdf=application/x-netcdf
+chm=application/vnd.ms-htmlhelp
 class=application/java-vm
 cpio=application/x-cpio
 cpt=application/mac-compactpro

From 07d54224c74b6e9784e2bd9a7bca723d4d5c4426 Mon Sep 17 00:00:00 2001
From: Joakim Erdfelt 
Date: Mon, 7 Mar 2016 17:12:05 -0700
Subject: [PATCH 13/21] Issue #405 - adding testcase for problematic HttpURI
 parsing of path params

---
 .../eclipse/jetty/http/HttpURIParseTest.java  | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURIParseTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURIParseTest.java
index d67a3b70d10..189e09fe8ea 100644
--- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURIParseTest.java
+++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURIParseTest.java
@@ -19,22 +19,24 @@
 
 package org.eclipse.jetty.http;
 
-import static org.hamcrest.Matchers.*;
-import static org.junit.Assert.*;
-import static org.junit.Assume.*;
-
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.Arrays;
 import java.util.List;
 
-import org.eclipse.jetty.util.URIUtil;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 import org.junit.runners.Parameterized.Parameter;
 import org.junit.runners.Parameterized.Parameters;
 
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assume.assumeNoException;
+import static org.junit.Assume.assumeNotNull;
+
 
 @RunWith(Parameterized.class)
 public class HttpURIParseTest
@@ -69,7 +71,8 @@ public class HttpURIParseTest
         {"/path/info;param#fragment",null,null,null,"/path/info;param","param",null,"fragment"},
         {"/path/info;param?query",null,null,null,"/path/info;param","param","query",null},
         {"/path/info;param?query#fragment",null,null,null,"/path/info;param","param","query","fragment"},
-        
+        // FIXME: {"/path/info;a=b/foo;c=d",null,null,null,"/data/info;a=b/foo;c=d","foo=bar1",null,null},
+
         // Protocol Less (aka scheme-less) URIs
         {"//host/path/info",null,"host",null,"/path/info",null,null,null},
         {"//user@host/path/info",null,"host",null,"/path/info",null,null,null},
@@ -151,14 +154,14 @@ public class HttpURIParseTest
         // Interpreted as relative path of "*" (no host/port/scheme/query/fragment)
         {"*",null,null,null,"*",null, null,null},
 
-        // Path detection Tests (seen from JSP/JSTL and  use
+        // Path detection Tests (seen from JSP/JSTL and  use)
         {"http://host:8080/path/info?q1=v1&q2=v2","http","host","8080","/path/info",null,"q1=v1&q2=v2",null},
         {"/path/info?q1=v1&q2=v2",null,null,null,"/path/info",null,"q1=v1&q2=v2",null},
         {"/info?q1=v1&q2=v2",null,null,null,"/info",null,"q1=v1&q2=v2",null},
         {"info?q1=v1&q2=v2",null,null,null,"info",null,"q1=v1&q2=v2",null},
         {"info;q1=v1?q2=v2",null,null,null,"info;q1=v1","q1=v1","q2=v2",null},
         
-        // Path-less, query only (seen from JSP/JSTL and  use
+        // Path-less, query only (seen from JSP/JSTL and  use)
         {"?q1=v1&q2=v2",null,null,null,"",null,"q1=v1&q2=v2",null}
         };
         

From 87ea5ac0395902730e8a59af5f28c9cb95b3f12c Mon Sep 17 00:00:00 2001
From: Simone Bordet 
Date: Tue, 8 Mar 2016 11:37:05 +0100
Subject: [PATCH 14/21] Fixed tests.

---
 .../jetty/client/ssl/SslBytesServerTest.java  | 31 ++++----
 .../http2/alpn/tests/ALPNNegotiationTest.java | 13 ++--
 .../server/ssl/SslConnectionFactoryTest.java  | 71 +++++++++++--------
 3 files changed, 66 insertions(+), 49 deletions(-)

diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesServerTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesServerTest.java
index 0e0f48b714b..9c10f7bc1ac 100644
--- a/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesServerTest.java
+++ b/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesServerTest.java
@@ -18,8 +18,6 @@
 
 package org.eclipse.jetty.client.ssl;
 
-import static org.hamcrest.Matchers.nullValue;
-
 import java.io.BufferedReader;
 import java.io.EOFException;
 import java.io.File;
@@ -368,7 +366,7 @@ public class SslBytesServerTest extends SslBytesTest
         System.arraycopy(doneBytes, 0, chunk, recordBytes.length, doneBytes.length);
         System.arraycopy(closeRecordBytes, 0, chunk, recordBytes.length + doneBytes.length, closeRecordBytes.length);
         proxy.flushToServer(0, chunk);
-        
+
         // Close the raw socket
         proxy.flushToServer(null);
 
@@ -380,7 +378,7 @@ public class SslBytesServerTest extends SslBytesTest
             Assert.assertEquals(Type.ALERT,record.getType());
             record = proxy.readFromServer();
         }
-        
+
         Assert.assertNull(record);
 
         // Check that we did not spin
@@ -488,7 +486,7 @@ public class SslBytesServerTest extends SslBytesTest
         if (record!=null)
         {
             Assert.assertEquals(record.getType(),Type.ALERT);
-            
+
             // Now should be a raw close
             record = proxy.readFromServer();
             Assert.assertNull(String.valueOf(record), record);
@@ -784,7 +782,7 @@ public class SslBytesServerTest extends SslBytesTest
         if (record!=null)
         {
             Assert.assertEquals(record.getType(),Type.ALERT);
-            
+
             // Now should be a raw close
             record = proxy.readFromServer();
             Assert.assertNull(String.valueOf(record), record);
@@ -846,7 +844,7 @@ public class SslBytesServerTest extends SslBytesTest
         if (record!=null)
         {
             Assert.assertEquals(record.getType(),Type.ALERT);
-            
+
             // Now should be a raw close
             record = proxy.readFromServer();
             Assert.assertNull(String.valueOf(record), record);
@@ -921,7 +919,7 @@ public class SslBytesServerTest extends SslBytesTest
         if (record!=null)
         {
             Assert.assertEquals(record.getType(),Type.ALERT);
-            
+
             // Now should be a raw close
             record = proxy.readFromServer();
             Assert.assertNull(String.valueOf(record), record);
@@ -983,7 +981,7 @@ public class SslBytesServerTest extends SslBytesTest
         if (record!=null)
         {
             Assert.assertEquals(record.getType(),Type.ALERT);
-            
+
             // Now should be a raw close
             record = proxy.readFromServer();
             Assert.assertNull(String.valueOf(record), record);
@@ -1040,7 +1038,7 @@ public class SslBytesServerTest extends SslBytesTest
         if (record!=null)
         {
             Assert.assertEquals(record.getType(),Type.ALERT);
-            
+
             // Now should be a raw close
             record = proxy.readFromServer();
             Assert.assertNull(String.valueOf(record), record);
@@ -1060,7 +1058,7 @@ public class SslBytesServerTest extends SslBytesTest
     {
         // Don't run on Windows (buggy JVM)
         Assume.assumeTrue(!OS.IS_WINDOWS);
-        
+
         final SSLSocket client = newClient();
 
         SimpleProxy.AutomaticFlow automaticProxyFlow = proxy.startAutomaticFlow();
@@ -1121,7 +1119,7 @@ public class SslBytesServerTest extends SslBytesTest
     {
         // Don't run on Windows (buggy JVM)
         Assume.assumeTrue(!OS.IS_WINDOWS);
-        
+
         final SSLSocket client = newClient();
 
         SimpleProxy.AutomaticFlow automaticProxyFlow = proxy.startAutomaticFlow();
@@ -1247,7 +1245,7 @@ public class SslBytesServerTest extends SslBytesTest
         if (record!=null)
         {
             Assert.assertEquals(record.getType(),Type.ALERT);
-            
+
             // Now should be a raw close
             record = proxy.readFromServer();
             Assert.assertNull(String.valueOf(record), record);
@@ -1862,8 +1860,11 @@ public class SslBytesServerTest extends SslBytesTest
         // Instead of passing the Client Hello, we simulate plain text was passed in
         proxy.flushToServer(0, "GET / HTTP/1.1\r\n".getBytes(StandardCharsets.UTF_8));
 
-        // We expect that the server closes the connection immediately
+        // We expect that the server sends an alert message and closes.
         TLSRecord record = proxy.readFromServer();
+        Assert.assertNotNull(record);
+        Assert.assertEquals(TLSRecord.Type.ALERT, record.getType());
+        record = proxy.readFromServer();
         Assert.assertNull(String.valueOf(record), record);
 
         // Check that we did not spin
@@ -1982,6 +1983,6 @@ public class SslBytesServerTest extends SslBytesTest
             Assert.assertEquals(record.getType(),Type.ALERT);
             record = proxy.readFromServer();
         }
-        Assert.assertThat(record,nullValue());
+        Assert.assertThat(record, Matchers.nullValue());
     }
 }
diff --git a/jetty-http2/http2-alpn-tests/src/test/java/org/eclipse/jetty/http2/alpn/tests/ALPNNegotiationTest.java b/jetty-http2/http2-alpn-tests/src/test/java/org/eclipse/jetty/http2/alpn/tests/ALPNNegotiationTest.java
index baac1bf3ebf..752471a2559 100644
--- a/jetty-http2/http2-alpn-tests/src/test/java/org/eclipse/jetty/http2/alpn/tests/ALPNNegotiationTest.java
+++ b/jetty-http2/http2-alpn-tests/src/test/java/org/eclipse/jetty/http2/alpn/tests/ALPNNegotiationTest.java
@@ -91,12 +91,17 @@ public class ALPNNegotiationTest extends AbstractALPNTest
             Assert.assertTrue(read > 0);
             // Cannot decrypt, as the SSLEngine has been already closed
 
-            // Now if we read more, we should either read the TLS Close Alert, or directly -1
+            // Now if we read more, we should read a TLS Alert.
             encrypted.clear();
             read = channel.read(encrypted);
-            // Sending a TLS Close Alert during handshake results in an exception when
-            // unwrapping that the server react to by closing the connection abruptly.
-            Assert.assertTrue(read < 0);
+            if (read > 0)
+            {
+                encrypted.flip();
+                // TLS Alert message type == 21.
+                Assert.assertEquals(21, encrypted.get() & 0xFF);
+                encrypted.clear();
+                Assert.assertEquals(-1, channel.read(encrypted));
+            }
         }
     }
 
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslConnectionFactoryTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslConnectionFactoryTest.java
index 9f0e43358fd..d88076645d0 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslConnectionFactoryTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/SslConnectionFactoryTest.java
@@ -21,6 +21,7 @@ package org.eclipse.jetty.server.ssl;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.Socket;
 import java.nio.charset.StandardCharsets;
@@ -59,11 +60,11 @@ import org.junit.Before;
 import org.junit.Test;
 
 public class SslConnectionFactoryTest
-{        
+{
     Server _server;
     ServerConnector _connector;
     int _port;
-    
+
     @Before
     public void before() throws Exception
     {
@@ -83,7 +84,7 @@ public class SslConnectionFactoryTest
         HttpConfiguration https_config = new HttpConfiguration(http_config);
         https_config.addCustomizer(new SecureRequestCustomizer());
 
-        
+
         SslContextFactory sslContextFactory = new SslContextFactory();
         sslContextFactory.setKeyStorePath(keystoreFile.getAbsolutePath());
         sslContextFactory.setKeyStorePassword("OBF:1vny1zlo1x8e1vnw1vn61x8g1zlu1vn4");
@@ -96,7 +97,7 @@ public class SslConnectionFactoryTest
         https.setIdleTimeout(30000);
 
         _server.addConnector(https);
-        
+
         _server.setHandler(new AbstractHandler()
         {
             @Override
@@ -107,30 +108,30 @@ public class SslConnectionFactoryTest
                 response.flushBuffer();
             }
         });
-        
+
         _server.start();
-        _port=https.getLocalPort();   
+        _port=https.getLocalPort();
     }
-    
+
     @After
     public void after() throws Exception
     {
         _server.stop();
         _server=null;
     }
-    
+
     @Test
     public void testConnect() throws Exception
     {
-        String response= getResponse("127.0.0.1",null);        
+        String response= getResponse("127.0.0.1",null);
         Assert.assertThat(response,Matchers.containsString("host=127.0.0.1"));
     }
-    
+
     @Test
     public void testSNIConnect() throws Exception
     {
         String response;
-        
+
         response= getResponse("localhost","localhost","jetty.eclipse.org");
         Assert.assertThat(response,Matchers.containsString("host=localhost"));
     }
@@ -151,22 +152,32 @@ public class SslConnectionFactoryTest
         {
             out.write("Rubbish".getBytes());
             out.flush();
-            
-            Assert.assertThat(socket.getInputStream().read(),Matchers.equalTo(-1));
+
+            socket.setSoTimeout(1000);
+            InputStream input = socket.getInputStream();
+            int read = input.read();
+            // TLS Alert message type == 21.
+            Assert.assertThat(read, Matchers.equalTo(21));
+            int reads = 0;
+            while (read >= 0)
+            {
+                read = input.read();
+                ++reads;
+            }
+            Assert.assertThat(reads, Matchers.lessThan(32));
         }
-        
     }
-    
+
     private String getResponse(String sniHost,String reqHost, String cn) throws Exception
     {
         SslContextFactory clientContextFactory = new SslContextFactory(true);
         clientContextFactory.start();
         SSLSocketFactory factory = clientContextFactory.getSslContext().getSocketFactory();
-        
+
         SSLSocket sslSocket = (SSLSocket)factory.createSocket("127.0.0.1", _port);
 
         if (cn!=null)
-        {        
+        {
             SNIHostName serverName = new SNIHostName(sniHost);
             List serverNames = new ArrayList<>();
             serverNames.add(serverName);
@@ -177,35 +188,35 @@ public class SslConnectionFactoryTest
         }
         sslSocket.startHandshake();
 
-        
+
         if (cn!=null)
-        {                                        
+        {
             X509Certificate cert = ((X509Certificate)sslSocket.getSession().getPeerCertificates()[0]);
-            
+
             Assert.assertThat(cert.getSubjectX500Principal().getName("CANONICAL"), Matchers.startsWith("cn="+cn));
         }
 
         sslSocket.getOutputStream().write(("GET /ctx/path HTTP/1.0\r\nHost: "+reqHost+":"+_port+"\r\n\r\n").getBytes(StandardCharsets.ISO_8859_1));
         String response = IO.toString(sslSocket.getInputStream());
-        
+
         sslSocket.close();
         clientContextFactory.stop();
         return response;
     }
-    
+
 
     @Test
     public void testSocketCustomization() throws Exception
     {
         final Queue history = new ConcurrentArrayQueue<>();
-        
+
         _connector.addBean(new SocketCustomizationListener()
         {
             @Override
             protected void customize(Socket socket, Class connection, boolean ssl)
             {
                 history.add("customize connector "+connection+","+ssl);
-            }            
+            }
         });
 
         _connector.getBean(SslConnectionFactory.class).addBean(new SocketCustomizationListener()
@@ -214,26 +225,26 @@ public class SslConnectionFactoryTest
             protected void customize(Socket socket, Class connection, boolean ssl)
             {
                 history.add("customize ssl "+connection+","+ssl);
-            }            
+            }
         });
-        
+
         _connector.getBean(HttpConnectionFactory.class).addBean(new SocketCustomizationListener()
         {
             @Override
             protected void customize(Socket socket, Class connection, boolean ssl)
             {
                 history.add("customize http "+connection+","+ssl);
-            }            
+            }
         });
 
-        String response= getResponse("127.0.0.1",null);        
+        String response= getResponse("127.0.0.1",null);
         Assert.assertThat(response,Matchers.containsString("host=127.0.0.1"));
-        
+
         Assert.assertEquals("customize connector class org.eclipse.jetty.io.ssl.SslConnection,false",history.poll());
         Assert.assertEquals("customize ssl class org.eclipse.jetty.io.ssl.SslConnection,false",history.poll());
         Assert.assertEquals("customize connector class org.eclipse.jetty.server.HttpConnection,true",history.poll());
         Assert.assertEquals("customize http class org.eclipse.jetty.server.HttpConnection,true",history.poll());
         Assert.assertEquals(0,history.size());
     }
-    
+
 }

From 18b689b820b5ee9ae9240b6a3f00c5a93ef9894f Mon Sep 17 00:00:00 2001
From: Simone Bordet 
Date: Tue, 8 Mar 2016 11:32:17 +0100
Subject: [PATCH 15/21] Code cleanups.

---
 .../jetty/client/ssl/SslBytesTest.java        | 91 +++++++++----------
 1 file changed, 42 insertions(+), 49 deletions(-)

diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesTest.java
index e217d4f8978..7e5490eb6bd 100644
--- a/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesTest.java
+++ b/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/SslBytesTest.java
@@ -28,7 +28,6 @@ import java.net.Socket;
 import java.net.SocketTimeoutException;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.concurrent.Callable;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
@@ -49,16 +48,16 @@ public abstract class SslBytesTest
 
     public static class TLSRecord
     {
-        private final SslBytesServerTest.TLSRecord.Type type;
+        private final Type type;
         private final byte[] bytes;
 
-        public TLSRecord(SslBytesServerTest.TLSRecord.Type type, byte[] bytes)
+        public TLSRecord(Type type, byte[] bytes)
         {
             this.type = type;
             this.bytes = bytes;
         }
 
-        public SslBytesServerTest.TLSRecord.Type getType()
+        public Type getType()
         {
             return type;
         }
@@ -80,15 +79,15 @@ public abstract class SslBytesTest
 
             private int code;
 
-            private Type(int code)
+            Type(int code)
             {
                 this.code = code;
-                SslBytesServerTest.TLSRecord.Type.Mapper.codes.put(this.code, this);
+                Mapper.codes.put(this.code, this);
             }
 
-            public static SslBytesServerTest.TLSRecord.Type from(int code)
+            public static Type from(int code)
             {
-                SslBytesServerTest.TLSRecord.Type result = SslBytesServerTest.TLSRecord.Type.Mapper.codes.get(code);
+                Type result = Mapper.codes.get(code);
                 if (result == null)
                     throw new IllegalArgumentException("Invalid TLSRecord.Type " + code);
                 return result;
@@ -96,7 +95,7 @@ public abstract class SslBytesTest
 
             private static class Mapper
             {
-                private static final Map codes = new HashMap<>();
+                private static final Map codes = new HashMap<>();
             }
         }
     }
@@ -218,7 +217,7 @@ public abstract class SslBytesTest
             }
         }
 
-        private TLSRecord read(SslBytesServerTest.TLSRecord.Type type, InputStream input, byte[] bytes, int offset, int length) throws IOException
+        private TLSRecord read(TLSRecord.Type type, InputStream input, byte[] bytes, int offset, int length) throws IOException
         {
             while (length > 0)
             {
@@ -291,57 +290,51 @@ public abstract class SslBytesTest
             }
         }
 
-        public SslBytesServerTest.SimpleProxy.AutomaticFlow startAutomaticFlow() throws InterruptedException
+        public SslBytesTest.SimpleProxy.AutomaticFlow startAutomaticFlow() throws InterruptedException
         {
             final CountDownLatch startLatch = new CountDownLatch(2);
             final CountDownLatch stopLatch = new CountDownLatch(2);
-            Future clientToServer = threadPool.submit(new Callable()
+            Future clientToServer = threadPool.submit(() ->
             {
-                public Object call() throws Exception
+                startLatch.countDown();
+                logger.debug("Automatic flow C --> S started");
+                try
                 {
-                    startLatch.countDown();
-                    logger.debug("Automatic flow C --> S started");
-                    try
+                    while (true)
                     {
-                        while (true)
-                        {
-                            flushToServer(readFromClient(), 0);
-                        }
-                    }
-                    catch (InterruptedIOException x)
-                    {
-                        return null;
-                    }
-                    finally
-                    {
-                        stopLatch.countDown();
-                        logger.debug("Automatic flow C --> S finished");
+                        flushToServer(readFromClient(), 0);
                     }
                 }
-            });
-            Future serverToClient = threadPool.submit(new Callable()
-            {
-                public Object call() throws Exception
+                catch (InterruptedIOException x)
                 {
-                    startLatch.countDown();
-                    logger.debug("Automatic flow C <-- S started");
-                    try
+                    return null;
+                }
+                finally
+                {
+                    stopLatch.countDown();
+                    logger.debug("Automatic flow C --> S finished");
+                }
+            });
+            Future serverToClient = threadPool.submit(() ->
+            {
+                startLatch.countDown();
+                logger.debug("Automatic flow C <-- S started");
+                try
+                {
+                    while (true)
                     {
-                        while (true)
-                        {
-                            flushToClient(readFromServer());
-                        }
-                    }
-                    catch (InterruptedIOException x)
-                    {
-                        return null;
-                    }
-                    finally
-                    {
-                        stopLatch.countDown();
-                        logger.debug("Automatic flow C <-- S finished");
+                        flushToClient(readFromServer());
                     }
                 }
+                catch (InterruptedIOException x)
+                {
+                    return null;
+                }
+                finally
+                {
+                    stopLatch.countDown();
+                    logger.debug("Automatic flow C <-- S finished");
+                }
             });
             Assert.assertTrue(startLatch.await(5, TimeUnit.SECONDS));
             return new SslBytesServerTest.SimpleProxy.AutomaticFlow(stopLatch, clientToServer, serverToClient);

From 6ac2c4a7011e9aedd3bb34b48670c9b5aa705070 Mon Sep 17 00:00:00 2001
From: Simone Bordet 
Date: Tue, 8 Mar 2016 12:46:10 +0100
Subject: [PATCH 16/21] Code cleanups.

---
 .../eclipse/jetty/server/ShutdownMonitor.java | 87 ++++++++-----------
 1 file changed, 38 insertions(+), 49 deletions(-)

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java
index 7fbd20a4e01..6d973127c5b 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java
@@ -28,7 +28,6 @@ import java.net.ServerSocket;
 import java.net.Socket;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
-import java.util.Properties;
 import java.util.Set;
 import java.util.concurrent.CopyOnWriteArraySet;
 
@@ -39,19 +38,20 @@ import org.eclipse.jetty.util.thread.ShutdownThread;
 /**
  * Shutdown/Stop Monitor thread.
  * 

- * This thread listens on the host/port specified by the STOP.HOST/STOP.PORT system parameter (defaults to 127.0.0.1/-1 for not listening) for 
- * request authenticated with the key given by the STOP.KEY system parameter (defaults to "eclipse") for admin requests.
+ * This thread listens on the host/port specified by the STOP.HOST/STOP.PORT
+ * system parameter (defaults to 127.0.0.1/-1 for not listening) for request
+ * authenticated with the key given by the STOP.KEY system parameter
+ * (defaults to "eclipse") for admin requests.
  * 

- * If the stop port is set to zero, then a random port is assigned and the port number is printed to stdout.
+ * If the stop port is set to zero, then a random port is assigned and the
+ * port number is printed to stdout.
  * 

  * Commands "stop" and "status" are currently supported.
  */
-public class ShutdownMonitor 
+public class ShutdownMonitor
 {
-    private final Set _lifeCycles = new CopyOnWriteArraySet();
-    
     // Implementation of safe lazy init, using Initialization on Demand Holder technique.
-    static class Holder
+    private static class Holder
     {
         static ShutdownMonitor instance = new ShutdownMonitor();
     }
@@ -60,34 +60,26 @@ public class ShutdownMonitor
     {
         return Holder.instance;
     }
-    
-    /* ------------------------------------------------------------ */
+
     public static synchronized void register(LifeCycle... lifeCycles)
     {
         getInstance()._lifeCycles.addAll(Arrays.asList(lifeCycles));
     }
 
-   
-    /* ------------------------------------------------------------ */
     public static synchronized void deregister(LifeCycle lifeCycle)
     {
         getInstance()._lifeCycles.remove(lifeCycle);
     }
-    
-    /* ------------------------------------------------------------ */
+
     public static synchronized boolean isRegistered(LifeCycle lifeCycle)
     {
         return getInstance()._lifeCycles.contains(lifeCycle);
     }
-    
-    /* ------------------------------------------------------------ */
+
     /**
-     * ShutdownMonitorRunnable
-     *
      * Thread for listening to STOP.PORT for command to stop Jetty.
      * If ShowndownMonitor.exitVm is true, then Sytem.exit will also be
      * called after the stop.
-     *
      */
     private class ShutdownMonitorRunnable implements Runnable
     {
@@ -95,7 +87,7 @@ public class ShutdownMonitor
         {
             startListenSocket();
         }
-        
+
         @Override
         public void run()
         {
@@ -127,7 +119,7 @@ public class ShutdownMonitor
                     {
                         //Stop the lifecycles, only if they are registered with the ShutdownThread, only destroying if vm is exiting
                         debug("Issuing stop...");
-                        
+
                         for (LifeCycle l:_lifeCycles)
                         {
                             try
@@ -136,7 +128,7 @@ public class ShutdownMonitor
                                 {
                                     l.stop();
                                 }
-                                
+
                                 if ((l instanceof Destroyable) && exitVm)
                                     ((Destroyable)l).destroy();
                             }
@@ -166,7 +158,7 @@ public class ShutdownMonitor
                     else if ("forcestop".equalsIgnoreCase(cmd))
                     {
                         debug("Issuing force stop...");
-                        
+
                         //Ensure that objects are stopped, destroyed only if vm is forcibly exiting
                         stopLifeCycles(exitVm);
 
@@ -179,7 +171,7 @@ public class ShutdownMonitor
 
                         //Stop the output and close the monitor socket
                         stopOutput(socket);
-                        
+
                         //Honour any pre-setup config to stop the jvm when this command is given
                         if (exitVm)
                         {
@@ -193,17 +185,17 @@ public class ShutdownMonitor
                         debug("Issuing stop and exit...");
                         //Make sure that objects registered with the shutdown thread will be stopped
                         stopLifeCycles(true);
-                        
+
                         //Stop accepting any more input
                         stopInput(socket);
 
                         // Reply to client
-                        debug("Informing client that we are stopped.");                       
-                        informClient(out, "Stopped\r\n");              
+                        debug("Informing client that we are stopped.");
+                        informClient(out, "Stopped\r\n");
 
                         //Stop the output and close the monitor socket
                         stopOutput(socket);
-                        
+
                         debug("Killing JVM");
                         System.exit(0);
                     }
@@ -230,25 +222,25 @@ public class ShutdownMonitor
                 }
             }
         }
-        
+
         public void stopInput (Socket socket)
         {
             //Stop accepting any more input
             close(serverSocket);
             serverSocket = null;
             //Shutdown input from client
-            shutdownInput(socket);  
+            shutdownInput(socket);
         }
-        
+
         public void stopOutput (Socket socket) throws IOException
         {
             socket.shutdownOutput();
             close(socket);
-            socket = null;                        
+            socket = null;
             debug("Shutting down monitor");
             serverSocket = null;
         }
-        
+
         public void informClient (OutputStream out, String message) throws IOException
         {
             out.write(message.getBytes(StandardCharsets.UTF_8));
@@ -258,7 +250,7 @@ public class ShutdownMonitor
         /**
          * Stop the registered lifecycles, optionally
          * calling destroy on them.
-         * 
+         *
          * @param destroy true if {@link Destroyable}'s should also be destroyed.
          */
         public void stopLifeCycles (boolean destroy)
@@ -271,7 +263,7 @@ public class ShutdownMonitor
                     {
                         l.stop();
                     }
-                    
+
                     if ((l instanceof Destroyable) && destroy)
                         ((Destroyable)l).destroy();
                 }
@@ -285,7 +277,7 @@ public class ShutdownMonitor
         public void startListenSocket()
         {
             if (port < 0)
-            {            
+            {
                 if (DEBUG)
                     System.err.println("ShutdownMonitor not in use (port < 0): " + port);
                 return;
@@ -326,7 +318,8 @@ public class ShutdownMonitor
         }
 
     }
-    
+
+    private final Set _lifeCycles = new CopyOnWriteArraySet();
     private boolean DEBUG;
     private String host;
     private int port;
@@ -388,23 +381,22 @@ public class ShutdownMonitor
         }
     }
 
-    
+
     private void shutdownInput(Socket socket)
     {
         if (socket == null)
             return;
-        
+
         try
         {
             socket.shutdownInput();
-        }   
+        }
         catch (IOException ignore)
         {
             debug(ignore);
         }
     }
-    
-    
+
     private void debug(String format, Object... args)
     {
         if (DEBUG)
@@ -441,7 +433,6 @@ public class ShutdownMonitor
         return exitVm;
     }
 
-
     public void setDebug(boolean flag)
     {
         this.DEBUG = flag;
@@ -489,7 +480,7 @@ public class ShutdownMonitor
     protected void start() throws Exception
     {
         Thread t = null;
-        
+
         synchronized (this)
         {
             if (thread != null && thread.isAlive())
@@ -498,18 +489,17 @@ public class ShutdownMonitor
                     System.err.printf("ShutdownMonitorThread already started");
                 return; // cannot start it again
             }
-         
+
             thread = new Thread(new ShutdownMonitorRunnable());
             thread.setDaemon(true);
             thread.setName("ShutdownMonitor");
             t = thread;
         }
-         
+
         if (t != null)
             t.start();
     }
 
-
     protected boolean isAlive ()
     {
         boolean result = false;
@@ -519,8 +509,7 @@ public class ShutdownMonitor
         }
         return result;
     }
-    
- 
+
     @Override
     public String toString()
     {

From 87f011d98ba6c5671981cd1b63e7e5088f2ca34f Mon Sep 17 00:00:00 2001
From: Simone Bordet 
Date: Tue, 8 Mar 2016 14:29:49 +0100
Subject: [PATCH 17/21] Issue #402 (Don't use Thread.isAlive() in
 ShutdownMonitor).

Refactored completely ShutdownMonitor, fixing synchronization, race
conditions, cleaning up code, deleting unnecessary code, etc.
---
 .../eclipse/jetty/server/ShutdownMonitor.java | 617 ++++++++----------
 .../jetty/server/ShutdownMonitorTest.java     | 206 +++---
 2 files changed, 368 insertions(+), 455 deletions(-)

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java
index 6d973127c5b..f6f3f1c526b 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java
@@ -27,9 +27,12 @@ import java.net.InetSocketAddress;
 import java.net.ServerSocket;
 import java.net.Socket;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Set;
-import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.function.Predicate;
 
 import org.eclipse.jetty.util.component.Destroyable;
 import org.eclipse.jetty.util.component.LifeCycle;
@@ -61,275 +64,31 @@ public class ShutdownMonitor
         return Holder.instance;
     }
 
-    public static synchronized void register(LifeCycle... lifeCycles)
+    public static void register(LifeCycle... lifeCycles)
     {
-        getInstance()._lifeCycles.addAll(Arrays.asList(lifeCycles));
+        getInstance().addLifeCycles(lifeCycles);
     }
 
-    public static synchronized void deregister(LifeCycle lifeCycle)
+    public static void deregister(LifeCycle lifeCycle)
     {
-        getInstance()._lifeCycles.remove(lifeCycle);
+        getInstance().removeLifeCycle(lifeCycle);
     }
 
-    public static synchronized boolean isRegistered(LifeCycle lifeCycle)
+    public static boolean isRegistered(LifeCycle lifeCycle)
     {
-        return getInstance()._lifeCycles.contains(lifeCycle);
+        return getInstance().containsLifeCycle(lifeCycle);
     }
 
-    /**
-     * Thread for listening to STOP.PORT for command to stop Jetty.
-     * If ShowndownMonitor.exitVm is true, then Sytem.exit will also be
-     * called after the stop.
-     */
-    private class ShutdownMonitorRunnable implements Runnable
-    {
-        public ShutdownMonitorRunnable()
-        {
-            startListenSocket();
-        }
-
-        @Override
-        public void run()
-        {
-            if (serverSocket == null)
-            {
-                return;
-            }
-
-            while (serverSocket != null)
-            {
-                Socket socket = null;
-                try
-                {
-                    socket = serverSocket.accept();
-
-                    LineNumberReader lin = new LineNumberReader(new InputStreamReader(socket.getInputStream()));
-                    String receivedKey = lin.readLine();
-                    if (!key.equals(receivedKey))
-                    {
-                        System.err.println("Ignoring command with incorrect key");
-                        continue;
-                    }
-
-                    OutputStream out = socket.getOutputStream();
-
-                    String cmd = lin.readLine();
-                    debug("command=%s",cmd);
-                    if ("stop".equalsIgnoreCase(cmd)) //historic, for backward compatibility
-                    {
-                        //Stop the lifecycles, only if they are registered with the ShutdownThread, only destroying if vm is exiting
-                        debug("Issuing stop...");
-
-                        for (LifeCycle l:_lifeCycles)
-                        {
-                            try
-                            {
-                                if (l.isStarted() && ShutdownThread.isRegistered(l))
-                                {
-                                    l.stop();
-                                }
-
-                                if ((l instanceof Destroyable) && exitVm)
-                                    ((Destroyable)l).destroy();
-                            }
-                            catch (Exception e)
-                            {
-                                debug(e);
-                            }
-                        }
-
-                        //Stop accepting any more commands
-                        stopInput(socket);
-
-                        // Reply to client
-                        debug("Informing client that we are stopped.");
-                        informClient(out, "Stopped\r\n");
-
-                        //Stop the output and close the monitor socket
-                        stopOutput(socket);
-
-                        if (exitVm)
-                        {
-                            // Kill JVM
-                            debug("Killing JVM");
-                            System.exit(0);
-                        }
-                    }
-                    else if ("forcestop".equalsIgnoreCase(cmd))
-                    {
-                        debug("Issuing force stop...");
-
-                        //Ensure that objects are stopped, destroyed only if vm is forcibly exiting
-                        stopLifeCycles(exitVm);
-
-                        //Stop accepting any more commands
-                        stopInput(socket);
-
-                        // Reply to client
-                        debug("Informing client that we are stopped.");
-                        informClient(out, "Stopped\r\n");
-
-                        //Stop the output and close the monitor socket
-                        stopOutput(socket);
-
-                        //Honour any pre-setup config to stop the jvm when this command is given
-                        if (exitVm)
-                        {
-                            // Kill JVM
-                            debug("Killing JVM");
-                            System.exit(0);
-                        }
-                    }
-                    else if ("stopexit".equalsIgnoreCase(cmd))
-                    {
-                        debug("Issuing stop and exit...");
-                        //Make sure that objects registered with the shutdown thread will be stopped
-                        stopLifeCycles(true);
-
-                        //Stop accepting any more input
-                        stopInput(socket);
-
-                        // Reply to client
-                        debug("Informing client that we are stopped.");
-                        informClient(out, "Stopped\r\n");
-
-                        //Stop the output and close the monitor socket
-                        stopOutput(socket);
-
-                        debug("Killing JVM");
-                        System.exit(0);
-                    }
-                    else if ("exit".equalsIgnoreCase(cmd))
-                    {
-                        debug("Killing JVM");
-                        System.exit(0);
-                    }
-                    else if ("status".equalsIgnoreCase(cmd))
-                    {
-                        // Reply to client
-                        informClient(out, "OK\r\n");
-                    }
-                }
-                catch (Exception e)
-                {
-                    debug(e);
-                    System.err.println(e.toString());
-                }
-                finally
-                {
-                    close(socket);
-                    socket = null;
-                }
-            }
-        }
-
-        public void stopInput (Socket socket)
-        {
-            //Stop accepting any more input
-            close(serverSocket);
-            serverSocket = null;
-            //Shutdown input from client
-            shutdownInput(socket);
-        }
-
-        public void stopOutput (Socket socket) throws IOException
-        {
-            socket.shutdownOutput();
-            close(socket);
-            socket = null;
-            debug("Shutting down monitor");
-            serverSocket = null;
-        }
-
-        public void informClient (OutputStream out, String message) throws IOException
-        {
-            out.write(message.getBytes(StandardCharsets.UTF_8));
-            out.flush();
-        }
-
-        /**
-         * Stop the registered lifecycles, optionally
-         * calling destroy on them.
-         *
-         * @param destroy true if {@link Destroyable}'s should also be destroyed.
-         */
-        public void stopLifeCycles (boolean destroy)
-        {
-            for (LifeCycle l:_lifeCycles)
-            {
-                try
-                {
-                    if (l.isStarted())
-                    {
-                        l.stop();
-                    }
-
-                    if ((l instanceof Destroyable) && destroy)
-                        ((Destroyable)l).destroy();
-                }
-                catch (Exception e)
-                {
-                    debug(e);
-                }
-            }
-        }
-
-        public void startListenSocket()
-        {
-            if (port < 0)
-            {
-                if (DEBUG)
-                    System.err.println("ShutdownMonitor not in use (port < 0): " + port);
-                return;
-            }
-
-            try
-            {
-                serverSocket = new ServerSocket();
-                serverSocket.setReuseAddress(true);
-                serverSocket.bind(new InetSocketAddress(InetAddress.getByName(host), port), 1);
-                if (port == 0)
-                {
-                    // server assigned port in use
-                    port = serverSocket.getLocalPort();
-                    System.out.printf("STOP.PORT=%d%n",port);
-                }
-
-                if (key == null)
-                {
-                    // create random key
-                    key = Long.toString((long)(Long.MAX_VALUE * Math.random() + this.hashCode() + System.currentTimeMillis()),36);
-                    System.out.printf("STOP.KEY=%s%n",key);
-                }
-            }
-            catch (Exception e)
-            {
-                debug(e);
-                System.err.println("Error binding monitor port " + port + ": " + e.toString());
-                serverSocket = null;
-            }
-            finally
-            {
-                // establish the port and key that are in use
-                debug("STOP.PORT=%d",port);
-                debug("STOP.KEY=%s",key);
-                debug("%s",serverSocket);
-            }
-        }
-
-    }
-
-    private final Set _lifeCycles = new CopyOnWriteArraySet();
-    private boolean DEBUG;
-    private String host;
+    private final Set _lifeCycles = new LinkedHashSet<>();
+    private boolean debug;
+    private final String host;
     private int port;
     private String key;
     private boolean exitVm;
-    private ServerSocket serverSocket;
-    private Thread thread;
+    private boolean alive;
 
     /**
-     * Create a ShutdownMonitor using configuration from the System properties.
+     * Creates a ShutdownMonitor using configuration from the System properties.
      * 

      * STOP.PORT = the port to listen on (empty, null, or values less than 0 disable the stop ability)

      * STOP.KEY = the magic key/passphrase to allow the stop (defaults to "eclipse")

@@ -338,104 +97,76 @@ public class ShutdownMonitor
      */
     private ShutdownMonitor()
     {
-        this.DEBUG = System.getProperty("DEBUG") != null;
-
-        // Use values passed thru via /jetty-start/
-        this.host = System.getProperty("STOP.HOST","127.0.0.1");
-        this.port = Integer.parseInt(System.getProperty("STOP.PORT","-1"));
-        this.key = System.getProperty("STOP.KEY",null);
+        this.debug = System.getProperty("DEBUG") != null;
+        this.host = System.getProperty("STOP.HOST", "127.0.0.1");
+        this.port = Integer.parseInt(System.getProperty("STOP.PORT", "-1"));
+        this.key = System.getProperty("STOP.KEY", null);
         this.exitVm = true;
     }
 
-    private void close(ServerSocket server)
+    private void addLifeCycles(LifeCycle... lifeCycles)
     {
-        if (server == null)
+        synchronized (this)
         {
-            return;
-        }
-
-        try
-        {
-            server.close();
-        }
-        catch (IOException ignore)
-        {
-            debug(ignore);
+            _lifeCycles.addAll(Arrays.asList(lifeCycles));
         }
     }
 
-    private void close(Socket socket)
+    private void removeLifeCycle(LifeCycle lifeCycle)
     {
-        if (socket == null)
+        synchronized (this)
         {
-            return;
-        }
-
-        try
-        {
-            socket.close();
-        }
-        catch (IOException ignore)
-        {
-            debug(ignore);
+            _lifeCycles.remove(lifeCycle);
         }
     }
 
-
-    private void shutdownInput(Socket socket)
+    private boolean containsLifeCycle(LifeCycle lifeCycle)
     {
-        if (socket == null)
-            return;
-
-        try
+        synchronized (this)
         {
-            socket.shutdownInput();
-        }
-        catch (IOException ignore)
-        {
-            debug(ignore);
+            return _lifeCycles.contains(lifeCycle);
         }
     }
 
     private void debug(String format, Object... args)
     {
-        if (DEBUG)
-        {
-            System.err.printf("[ShutdownMonitor] " + format + "%n",args);
-        }
+        if (debug)
+            System.err.printf("[ShutdownMonitor] " + format + "%n", args);
     }
 
     private void debug(Throwable t)
     {
-        if (DEBUG)
-        {
+        if (debug)
             t.printStackTrace(System.err);
-        }
     }
 
     public String getKey()
     {
-        return key;
+        synchronized (this)
+        {
+            return key;
+        }
     }
 
     public int getPort()
     {
-        return port;
-    }
-
-    public ServerSocket getServerSocket()
-    {
-        return serverSocket;
+        synchronized (this)
+        {
+            return port;
+        }
     }
 
     public boolean isExitVm()
     {
-        return exitVm;
+        synchronized (this)
+        {
+            return exitVm;
+        }
     }
 
     public void setDebug(boolean flag)
     {
-        this.DEBUG = flag;
+        this.debug = flag;
     }
 
     /**
@@ -445,10 +176,8 @@ public class ShutdownMonitor
     {
         synchronized (this)
         {
-            if (thread != null && thread.isAlive())
-            {
-                throw new IllegalStateException("ShutdownMonitorThread already started");
-            }
+            if (alive)
+                throw new IllegalStateException("ShutdownMonitor already started");
             this.exitVm = exitVm;
         }
     }
@@ -457,10 +186,8 @@ public class ShutdownMonitor
     {
         synchronized (this)
         {
-            if (thread != null && thread.isAlive())
-            {
-                throw new IllegalStateException("ShutdownMonitorThread already started");
-            }
+            if (alive)
+                throw new IllegalStateException("ShutdownMonitor already started");
             this.key = key;
         }
     }
@@ -469,50 +196,254 @@ public class ShutdownMonitor
     {
         synchronized (this)
         {
-            if (thread != null && thread.isAlive())
-            {
-                throw new IllegalStateException("ShutdownMonitorThread already started");
-            }
+            if (alive)
+                throw new IllegalStateException("ShutdownMonitor already started");
             this.port = port;
         }
     }
 
     protected void start() throws Exception
     {
-        Thread t = null;
-
         synchronized (this)
         {
-            if (thread != null && thread.isAlive())
+            if (alive)
             {
-                if (DEBUG)
-                    System.err.printf("ShutdownMonitorThread already started");
+                debug("Already started");
                 return; // cannot start it again
             }
-
-            thread = new Thread(new ShutdownMonitorRunnable());
-            thread.setDaemon(true);
-            thread.setName("ShutdownMonitor");
-            t = thread;
+            ServerSocket serverSocket = listen();
+            if (serverSocket != null)
+            {
+                alive = true;
+                Thread thread = new Thread(new ShutdownMonitorRunnable(serverSocket));
+                thread.setDaemon(true);
+                thread.setName("ShutdownMonitor");
+                thread.start();
+            }
         }
-
-        if (t != null)
-            t.start();
     }
 
-    protected boolean isAlive ()
+    private void stop()
     {
-        boolean result = false;
         synchronized (this)
         {
-            result = (thread != null && thread.isAlive());
+            alive = false;
+            notifyAll();
+        }
+    }
+
+    // For test purposes only.
+    void await() throws InterruptedException
+    {
+        synchronized (this)
+        {
+            while (alive)
+            {
+                wait();
+            }
+        }
+    }
+
+    protected boolean isAlive()
+    {
+        synchronized (this)
+        {
+            return alive;
+        }
+    }
+
+
+    private ServerSocket listen()
+    {
+        int port = getPort();
+        if (port < 0)
+        {
+            debug("Not enabled (port < 0): %d", port);
+            return null;
+        }
+
+        String key = getKey();
+        try
+        {
+            ServerSocket serverSocket = new ServerSocket();
+            serverSocket.setReuseAddress(true);
+            serverSocket.bind(new InetSocketAddress(InetAddress.getByName(host), port));
+            if (port == 0)
+            {
+                port = serverSocket.getLocalPort();
+                System.out.printf("STOP.PORT=%d%n", port);
+                setPort(port);
+            }
+
+            if (key == null)
+            {
+                key = Long.toString((long)(Long.MAX_VALUE * Math.random() + this.hashCode() + System.currentTimeMillis()), 36);
+                System.out.printf("STOP.KEY=%s%n", key);
+                setKey(key);
+            }
+
+            return serverSocket;
+        }
+        catch (Throwable x)
+        {
+            debug(x);
+            System.err.println("Error binding ShutdownMonitor to port " + port + ": " + x.toString());
+            return null;
+        }
+        finally
+        {
+            // establish the port and key that are in use
+            debug("STOP.PORT=%d", port);
+            debug("STOP.KEY=%s", key);
         }
-        return result;
     }
 
     @Override
     public String toString()
     {
-        return String.format("%s[port=%d]",this.getClass().getName(),port);
+        return String.format("%s[port=%d,alive=%b]", this.getClass().getName(), getPort(), isAlive());
+    }
+
+    /**
+     * Thread for listening to STOP.PORT for command to stop Jetty.
+     * If ShutdownMonitor.exitVm is true, then System.exit will also be
+     * called after the stop.
+     */
+    private class ShutdownMonitorRunnable implements Runnable
+    {
+        private final ServerSocket serverSocket;
+
+        private ShutdownMonitorRunnable(ServerSocket serverSocket)
+        {
+            this.serverSocket = serverSocket;
+        }
+
+        @Override
+        public void run()
+        {
+            debug("Started");
+            try
+            {
+                String key = getKey();
+                while (true)
+                {
+                    try (Socket socket = serverSocket.accept())
+                    {
+                        LineNumberReader reader = new LineNumberReader(new InputStreamReader(socket.getInputStream()));
+                        String receivedKey = reader.readLine();
+                        if (!key.equals(receivedKey))
+                        {
+                            debug("Ignoring command with incorrect key: %s", receivedKey);
+                            continue;
+                        }
+
+                        String cmd = reader.readLine();
+                        debug("command=%s", cmd);
+                        OutputStream out = socket.getOutputStream();
+                        boolean exitVm = isExitVm();
+
+                        if ("stop".equalsIgnoreCase(cmd)) //historic, for backward compatibility
+                        {
+                            //Stop the lifecycles, only if they are registered with the ShutdownThread, only destroying if vm is exiting
+                            debug("Performing stop command");
+                            stopLifeCycles(ShutdownThread::isRegistered, exitVm);
+
+                            // Reply to client
+                            debug("Informing client that we are stopped");
+                            informClient(out, "Stopped\r\n");
+
+                            if (!exitVm)
+                                break;
+
+                            // Kill JVM
+                            debug("Killing JVM");
+                            System.exit(0);
+                        }
+                        else if ("forcestop".equalsIgnoreCase(cmd))
+                        {
+                            debug("Performing forced stop command");
+                            stopLifeCycles(l -> true, exitVm);
+
+                            // Reply to client
+                            debug("Informing client that we are stopped");
+                            informClient(out, "Stopped\r\n");
+
+                            if (!exitVm)
+                                break;
+
+                            // Kill JVM
+                            debug("Killing JVM");
+                            System.exit(0);
+                        }
+                        else if ("stopexit".equalsIgnoreCase(cmd))
+                        {
+                            debug("Performing stop and exit commands");
+                            stopLifeCycles(ShutdownThread::isRegistered, true);
+
+                            // Reply to client
+                            debug("Informing client that we are stopped");
+                            informClient(out, "Stopped\r\n");
+
+                            debug("Killing JVM");
+                            System.exit(0);
+                        }
+                        else if ("exit".equalsIgnoreCase(cmd))
+                        {
+                            debug("Killing JVM");
+                            System.exit(0);
+                        }
+                        else if ("status".equalsIgnoreCase(cmd))
+                        {
+                            // Reply to client
+                            informClient(out, "OK\r\n");
+                        }
+                    }
+                    catch (Throwable x)
+                    {
+                        debug(x);
+                    }
+                }
+            }
+            catch (Throwable x)
+            {
+                debug(x);
+            }
+            finally
+            {
+                stop();
+                debug("Stopped");
+            }
+        }
+
+        private void informClient(OutputStream out, String message) throws IOException
+        {
+            out.write(message.getBytes(StandardCharsets.UTF_8));
+            out.flush();
+        }
+
+        private void stopLifeCycles(Predicate predicate, boolean destroy)
+        {
+            List lifeCycles = new ArrayList<>();
+            synchronized (this)
+            {
+                lifeCycles.addAll(_lifeCycles);
+            }
+
+            for (LifeCycle l : lifeCycles)
+            {
+                try
+                {
+                    if (l.isStarted() && predicate.test(l))
+                        l.stop();
+
+                    if ((l instanceof Destroyable) && destroy)
+                        ((Destroyable)l).destroy();
+                }
+                catch (Throwable x)
+                {
+                    debug(x);
+                }
+            }
+        }
     }
 }
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ShutdownMonitorTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ShutdownMonitorTest.java
index 3a40ec3a83e..fadd4c9e888 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/ShutdownMonitorTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ShutdownMonitorTest.java
@@ -18,196 +18,151 @@
 
 package org.eclipse.jetty.server;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-
 import java.io.InputStreamReader;
 import java.io.LineNumberReader;
 import java.io.OutputStream;
 import java.net.InetAddress;
 import java.net.Socket;
-import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jetty.util.thread.ShutdownThread;
 import org.junit.Test;
 
-/**
- * ShutdownMonitorTest
- */
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
 public class ShutdownMonitorTest
 {
-    public class TestableServer extends Server
-    {
-        boolean destroyed = false;
-        boolean stopped = false;
-        @Override
-        protected void doStop() throws Exception
-        {
-            stopped = true;
-            super.doStop();
-        }
-        @Override
-        public void destroy()
-        {
-            destroyed = true;
-            super.destroy();
-        }
-        @Override
-        protected void doStart() throws Exception
-        {
-            stopped = false;
-            destroyed  = false;
-            super.doStart();
-        }    
-    }
-    
-    
     @Test
     public void testShutdownMonitor() throws Exception
     {
-        // test port and key assignment
-        ShutdownMonitor.getInstance().setPort(0);
-        ShutdownMonitor.getInstance().setExitVm(false);
-        ShutdownMonitor.getInstance().start();
-        String key = ShutdownMonitor.getInstance().getKey();
-        int port = ShutdownMonitor.getInstance().getPort();
+        ShutdownMonitor monitor = ShutdownMonitor.getInstance();
+        monitor.setDebug(true);
+        monitor.setPort(0);
+        monitor.setExitVm(false);
+        monitor.start();
+        String key = monitor.getKey();
+        int port = monitor.getPort();
 
         // try starting a 2nd time (should be ignored)
-        ShutdownMonitor.getInstance().start();
+        monitor.start();
 
-        stop("stop", port,key,true);
-        assertTrue(!ShutdownMonitor.getInstance().isAlive());
+        stop("stop", port, key, true);
+        monitor.await();
+        assertTrue(!monitor.isAlive());
 
         // should be able to change port and key because it is stopped
-        ShutdownMonitor.getInstance().setPort(0);
-        ShutdownMonitor.getInstance().setKey("foo");
-        ShutdownMonitor.getInstance().start();
+        monitor.setPort(0);
+        String newKey = "foo";
+        monitor.setKey(newKey);
+        monitor.start();
 
-        key = ShutdownMonitor.getInstance().getKey();
-        port = ShutdownMonitor.getInstance().getPort();
-        assertTrue(ShutdownMonitor.getInstance().isAlive());
+        key = monitor.getKey();
+        assertEquals(newKey, key);
+        port = monitor.getPort();
+        assertTrue(monitor.isAlive());
 
-        stop("stop", port,key,true);
-        assertTrue(!ShutdownMonitor.getInstance().isAlive());
+        stop("stop", port, key, true);
+        monitor.await();
+        assertTrue(!monitor.isAlive());
     }
-    
-    
+
     @Test
     public void testForceStopCommand() throws Exception
     {
-        //create a testable Server with stop(), destroy() overridden to instrument
-        //start server
-        //call "forcestop" and check that server stopped but not destroyed
-        // test port and key assignment
-        System.setProperty("DEBUG", "true");
-        ShutdownMonitor.getInstance().setPort(0);
+        ShutdownMonitor monitor = ShutdownMonitor.getInstance();
+        monitor.setPort(0);
         TestableServer server = new TestableServer();
         server.start();
-       
+
         //shouldn't be registered for shutdown on jvm
         assertTrue(!ShutdownThread.isRegistered(server));
         assertTrue(ShutdownMonitor.isRegistered(server));
-        
-        String key = ShutdownMonitor.getInstance().getKey();
-        int port = ShutdownMonitor.getInstance().getPort();
-        
-        stop("forcestop", port,key,true);
-        
-        assertTrue(!ShutdownMonitor.getInstance().isAlive());
+
+        String key = monitor.getKey();
+        int port = monitor.getPort();
+
+        stop("forcestop", port, key, true);
+        monitor.await();
+
+        assertTrue(!monitor.isAlive());
         assertTrue(server.stopped);
         assertTrue(!server.destroyed);
         assertTrue(!ShutdownThread.isRegistered(server));
         assertTrue(!ShutdownMonitor.isRegistered(server));
     }
-    
+
     @Test
     public void testOldStopCommandWithStopOnShutdownTrue() throws Exception
     {
-        
-        //create a testable Server with stop(), destroy() overridden to instrument
-        //call server.setStopAtShudown(true);
-        //start server
-        //call "stop" and check that server stopped but not destroyed
-        
-        //stop server
-        
-        //call server.setStopAtShutdown(false);
-        //start server
-        //call "stop" and check that the server is not stopped and not destroyed
-        System.setProperty("DEBUG", "true");
-        ShutdownMonitor.getInstance().setExitVm(false);
-      
-        ShutdownMonitor.getInstance().setPort(0);
+        ShutdownMonitor monitor = ShutdownMonitor.getInstance();
+        monitor.setExitVm(false);
+
+        monitor.setPort(0);
         TestableServer server = new TestableServer();
         server.setStopAtShutdown(true);
         server.start();
-        
+
         //should be registered for shutdown on exit
         assertTrue(ShutdownThread.isRegistered(server));
         assertTrue(ShutdownMonitor.isRegistered(server));
-        
-        String key = ShutdownMonitor.getInstance().getKey();
-        int port = ShutdownMonitor.getInstance().getPort();
-        
+
+        String key = monitor.getKey();
+        int port = monitor.getPort();
+
         stop("stop", port, key, true);
-        assertTrue(!ShutdownMonitor.getInstance().isAlive());
+        monitor.await();
+
+        assertTrue(!monitor.isAlive());
         assertTrue(server.stopped);
         assertTrue(!server.destroyed);
         assertTrue(!ShutdownThread.isRegistered(server));
         assertTrue(!ShutdownMonitor.isRegistered(server));
     }
-    
+
     @Test
     public void testOldStopCommandWithStopOnShutdownFalse() throws Exception
     {
-        //change so stopatshutdown is false, so stop does nothing in this case (as exitVm is false otherwise we couldn't run test)
-        ShutdownMonitor.getInstance().setExitVm(false);
-        System.setProperty("DEBUG", "true");
-        ShutdownMonitor.getInstance().setPort(0);
+        ShutdownMonitor monitor = ShutdownMonitor.getInstance();
+        monitor.setExitVm(false);
+        monitor.setPort(0);
         TestableServer server = new TestableServer();
         server.setStopAtShutdown(false);
         server.start();
-        
+
         assertTrue(!ShutdownThread.isRegistered(server));
         assertTrue(ShutdownMonitor.isRegistered(server));
-        
-        String key = ShutdownMonitor.getInstance().getKey();
-        int port = ShutdownMonitor.getInstance().getPort();
-        
-        stop ("stop", port, key, true);
-        assertTrue(!ShutdownMonitor.getInstance().isAlive());
+
+        String key = monitor.getKey();
+        int port = monitor.getPort();
+
+        stop("stop", port, key, true);
+        monitor.await();
+
+        assertTrue(!monitor.isAlive());
         assertTrue(!server.stopped);
         assertTrue(!server.destroyed);
         assertTrue(!ShutdownThread.isRegistered(server));
         assertTrue(ShutdownMonitor.isRegistered(server));
     }
-    
-    
-  
 
     public void stop(String command, int port, String key, boolean check) throws Exception
     {
-        System.out.printf("Attempting to send "+command+" to localhost:%d (%b)%n",port,check);
-        try (Socket s = new Socket(InetAddress.getByName("127.0.0.1"),port))
+        System.out.printf("Attempting to send " + command + " to localhost:%d (%b)%n", port, check);
+        try (Socket s = new Socket(InetAddress.getByName("127.0.0.1"), port))
         {
             // send stop command
             try (OutputStream out = s.getOutputStream())
             {
-                out.write((key + "\r\n"+command+"\r\n").getBytes());
+                out.write((key + "\r\n" + command + "\r\n").getBytes());
                 out.flush();
 
                 if (check)
                 {
-                    // wait a little
-                    TimeUnit.MILLISECONDS.sleep(600);
-
                     // check for stop confirmation
                     LineNumberReader lin = new LineNumberReader(new InputStreamReader(s.getInputStream()));
                     String response;
                     if ((response = lin.readLine()) != null)
-                    {
-                        assertEquals("Stopped",response);
-                    }
+                        assertEquals("Stopped", response);
                     else
                         throw new IllegalStateException("No stop confirmation");
                 }
@@ -215,4 +170,31 @@ public class ShutdownMonitorTest
         }
     }
 
+    public class TestableServer extends Server
+    {
+        boolean destroyed = false;
+        boolean stopped = false;
+
+        @Override
+        protected void doStop() throws Exception
+        {
+            stopped = true;
+            super.doStop();
+        }
+
+        @Override
+        public void destroy()
+        {
+            destroyed = true;
+            super.destroy();
+        }
+
+        @Override
+        protected void doStart() throws Exception
+        {
+            stopped = false;
+            destroyed = false;
+            super.doStart();
+        }
+    }
 }

From 710949dbb7c70cb730ac567f67bf6e1eb278a363 Mon Sep 17 00:00:00 2001
From: Simone Bordet 
Date: Tue, 8 Mar 2016 22:05:10 +0100
Subject: [PATCH 18/21] Issue #406 (GzipHandler: allow to override the Vary
 response header)

Added an overridable getVaryField() method that, if returns null,
disables the send of the Vary header.
---
 .../server/handler/gzip/GzipHandler.java      | 79 ++++++++---------
 .../gzip/GzipHttpOutputInterceptor.java       | 85 ++++++++++---------
 2 files changed, 85 insertions(+), 79 deletions(-)

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java
index 5200eff13b6..6c0a584211e 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java
@@ -18,8 +18,6 @@
 
 package org.eclipse.jetty.server.handler.gzip;
 
-import static org.eclipse.jetty.http.GzipHttpContent.ETAG_GZIP_QUOTE;
-
 import java.io.File;
 import java.io.IOException;
 import java.util.Set;
@@ -47,9 +45,8 @@ import org.eclipse.jetty.util.URIUtil;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
 
-
 /**
- * A Handler that can dynamically GZIP compress responses.   Unlike 
+ * A Handler that can dynamically GZIP compress responses.   Unlike
  * previous and 3rd party GzipFilters, this mechanism works with asynchronously
  * generated responses and does not need to wrap the response or it's output
  * stream.  Instead it uses the efficient {@link org.eclipse.jetty.server.HttpOutput.Interceptor} mechanism.
@@ -69,22 +66,22 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
     private int _compressionLevel=Deflater.DEFAULT_COMPRESSION;
     private boolean _checkGzExists = true;
     private boolean _syncFlush = false;
-    
+
     // non-static, as other GzipHandler instances may have different configurations
-    private final ThreadLocal _deflater = new ThreadLocal();
+    private final ThreadLocal _deflater = new ThreadLocal<>();
 
     private final IncludeExclude _agentPatterns=new IncludeExclude<>(RegexSet.class);
     private final IncludeExclude _methods = new IncludeExclude<>();
     private final IncludeExclude _paths = new IncludeExclude<>(PathSpecSet.class);
     private final IncludeExclude _mimeTypes = new IncludeExclude<>();
-    
+
     private HttpField _vary;
 
 
     /* ------------------------------------------------------------ */
     /**
      * Instantiates a new gzip handler.
-     * The excluded Mime Types are initialized to common known 
+     * The excluded Mime Types are initialized to common known
      * images, audio, video and other already compressed types.
      * The included methods is initialized to GET.
      * The excluded agent patterns are set to exclude MSIE 6.0
@@ -107,7 +104,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
         _mimeTypes.exclude("application/bzip2");
         _mimeTypes.exclude("application/x-rar-compressed");
         LOG.debug("{} mime types {}",this,_mimeTypes);
-        
+
         _agentPatterns.exclude(".*MSIE 6.0.*");
     }
 
@@ -145,7 +142,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
 
     /* ------------------------------------------------------------ */
     /**
-     * @param pathspecs Path specs (as per servlet spec) to exclude. If a 
+     * @param pathspecs Path specs (as per servlet spec) to exclude. If a
      * ServletContext is available, the paths are relative to the context path,
      * otherwise they are absolute.
      * For backward compatibility the pathspecs may be comma separated strings, but this
@@ -165,7 +162,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
     {
         _agentPatterns.include(patterns);
     }
-    
+
     /* ------------------------------------------------------------ */
     /**
      * @param methods The methods to include in compression
@@ -214,7 +211,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
     /* ------------------------------------------------------------ */
     /**
      * Add path specs to include. Inclusion takes precedence over exclusion.
-     * @param pathspecs Path specs (as per servlet spec) to include. If a 
+     * @param pathspecs Path specs (as per servlet spec) to include. If a
      * ServletContext is available, the paths are relative to the context path,
      * otherwise they are absolute
      * For backward compatibility the pathspecs may be comma separated strings, but this
@@ -225,7 +222,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
         for (String p : pathspecs)
             _paths.include(StringUtil.csvSplit(p));
     }
-    
+
     /* ------------------------------------------------------------ */
     @Override
     protected void doStart() throws Exception
@@ -245,7 +242,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
     {
         return _compressionLevel;
     }
-    
+
     /* ------------------------------------------------------------ */
     @Override
     public Deflater getDeflater(Request request, long content_length)
@@ -256,7 +253,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
             LOG.debug("{} excluded user agent {}",this,request);
             return null;
         }
-        
+
         if (content_length>=0 && content_length<_minGzipSize)
         {
             LOG.debug("{} excluded minGzipSize {}",this,request);
@@ -281,16 +278,16 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
                 return null;
             }
         }
-        
+
         Deflater df = _deflater.get();
         if (df==null)
-            df=new Deflater(_compressionLevel,true);        
+            df=new Deflater(_compressionLevel,true);
         else
             _deflater.set(null);
-        
+
         return df;
     }
-    
+
     /* ------------------------------------------------------------ */
     public String[] getExcludedAgentPatterns()
     {
@@ -325,7 +322,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
         Set includes=_agentPatterns.getIncluded();
         return includes.toArray(new String[includes.size()]);
     }
-    
+
     /* ------------------------------------------------------------ */
     public String[] getIncludedMethods()
     {
@@ -365,6 +362,11 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
         return _minGzipSize;
     }
 
+    protected HttpField getVaryField()
+    {
+        return _vary;
+    }
+
     /* ------------------------------------------------------------ */
     /**
      * @see org.eclipse.jetty.server.handler.HandlerWrapper#handle(java.lang.String, org.eclipse.jetty.server.Request, javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse)
@@ -375,8 +377,8 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
         ServletContext context = baseRequest.getServletContext();
         String path = context==null?baseRequest.getRequestURI():URIUtil.addPaths(baseRequest.getServletPath(),baseRequest.getPathInfo());
         LOG.debug("{} handle {} in {}",this,baseRequest,context);
-        
-        HttpOutput out = baseRequest.getResponse().getHttpOutput();   
+
+        HttpOutput out = baseRequest.getResponse().getHttpOutput();
         // Are we already being gzipped?
         HttpOutput.Interceptor interceptor = out.getInterceptor();
         while (interceptor!=null)
@@ -389,7 +391,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
             }
             interceptor=interceptor.getNextInterceptor();
         }
-        
+
         // If not a supported method - no Vary because no matter what client, this URI is always excluded
         if (!_methods.matches(baseRequest.getMethod()))
         {
@@ -397,7 +399,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
             _handler.handle(target,baseRequest, request, response);
             return;
         }
-        
+
         // If not a supported URI- no Vary because no matter what client, this URI is always excluded
         // Use pathInfo because this is be
         if (!isPathGzipable(path))
@@ -420,7 +422,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
                 return;
             }
         }
-        
+
         if (_checkGzExists && context!=null)
         {
             String realpath=request.getServletContext().getRealPath(path);
@@ -436,25 +438,26 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
                 }
             }
         }
-        
+
         // Special handling for etags
-        String etag = baseRequest.getHttpFields().get(HttpHeader.IF_NONE_MATCH); 
+        String etag = baseRequest.getHttpFields().get(HttpHeader.IF_NONE_MATCH);
         if (etag!=null)
         {
-            int i=etag.indexOf(ETAG_GZIP_QUOTE);
+            int i=etag.indexOf(GzipHttpContent.ETAG_GZIP_QUOTE);
             if (i>0)
             {
                 while (i>=0)
                 {
                     etag=etag.substring(0,i)+etag.substring(i+GzipHttpContent.ETAG_GZIP.length());
-                    i=etag.indexOf(ETAG_GZIP_QUOTE,i);
+                    i=etag.indexOf(GzipHttpContent.ETAG_GZIP_QUOTE,i);
                 }
                 baseRequest.getHttpFields().put(new HttpField(HttpHeader.IF_NONE_MATCH,etag));
             }
         }
 
         // install interceptor and handle
-        out.setInterceptor(new GzipHttpOutputInterceptor(this,_vary,baseRequest.getHttpChannel(),out.getInterceptor(),_syncFlush));
+        out.setInterceptor(new GzipHttpOutputInterceptor(this,getVaryField(),baseRequest.getHttpChannel(),out.getInterceptor(),isSyncFlush()));
+
         if (_handler!=null)
             _handler.handle(target,baseRequest, request, response);
     }
@@ -470,7 +473,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
     {
         if (ua == null)
             return false;
-        
+
         return _agentPatterns.matches(ua);
     }
 
@@ -483,7 +486,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
 
     /* ------------------------------------------------------------ */
     /**
-     * Checks to see if the path is included or not excluded 
+     * Checks to see if the path is included or not excluded
      *
      * @param requestURI
      *            the request uri
@@ -493,7 +496,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
     {
         if (requestURI == null)
             return true;
-        
+
         return _paths.matches(requestURI);
     }
 
@@ -515,7 +518,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
     {
         _checkGzExists = checkGzExists;
     }
-    
+
     /* ------------------------------------------------------------ */
     /**
      * @param compressionLevel  The compression level to use to initialize {@link Deflater#setLevel(int)}
@@ -558,7 +561,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
 
     /* ------------------------------------------------------------ */
     /**
-     * @param pathspecs Path specs (as per servlet spec) to exclude. If a 
+     * @param pathspecs Path specs (as per servlet spec) to exclude. If a
      * ServletContext is available, the paths are relative to the context path,
      * otherwise they are absolute.
      */
@@ -577,7 +580,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
         _agentPatterns.getIncluded().clear();
         addIncludedAgentPatterns(patterns);
     }
-    
+
     /* ------------------------------------------------------------ */
     /**
      * @param methods The methods to include in compression
@@ -587,7 +590,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
         _methods.getIncluded().clear();
         _methods.include(methods);
     }
-    
+
     /* ------------------------------------------------------------ */
     /**
      * Set included mime types. Inclusion takes precedence over
@@ -603,7 +606,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
     /* ------------------------------------------------------------ */
     /**
      * Set the path specs to include. Inclusion takes precedence over exclusion.
-     * @param pathspecs Path specs (as per servlet spec) to include. If a 
+     * @param pathspecs Path specs (as per servlet spec) to include. If a
      * ServletContext is available, the paths are relative to the context path,
      * otherwise they are absolute
      */
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java
index f717c2656af..17a1e25fb46 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHttpOutputInterceptor.java
@@ -32,6 +32,7 @@ import org.eclipse.jetty.http.MimeTypes;
 import org.eclipse.jetty.http.PreEncodedHttpField;
 import org.eclipse.jetty.server.HttpChannel;
 import org.eclipse.jetty.server.HttpOutput;
+import org.eclipse.jetty.server.Response;
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.IteratingNestedCallback;
@@ -46,7 +47,7 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
 
     public final static HttpField VARY_ACCEPT_ENCODING_USER_AGENT=new PreEncodedHttpField(HttpHeader.VARY,HttpHeader.ACCEPT_ENCODING+", "+HttpHeader.USER_AGENT);
     public final static HttpField VARY_ACCEPT_ENCODING=new PreEncodedHttpField(HttpHeader.VARY,HttpHeader.ACCEPT_ENCODING.asString());
-    
+
     private enum GZState {  MIGHT_COMPRESS, NOT_COMPRESSING, COMMITTING, COMPRESSING, FINISHED};
     private final AtomicReference _state = new AtomicReference<>(GZState.MIGHT_COMPRESS);
     private final CRC32 _crc = new CRC32();
@@ -57,7 +58,7 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
     private final HttpField _vary;
     private final int _bufferSize;
     private final boolean _syncFlush;
-    
+
     private Deflater _deflater;
     private ByteBuffer _buffer;
 
@@ -65,12 +66,12 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
     {
         this(factory,VARY_ACCEPT_ENCODING_USER_AGENT,channel.getHttpConfiguration().getOutputBufferSize(),channel,next,syncFlush);
     }
-    
+
     public GzipHttpOutputInterceptor(GzipFactory factory, HttpField vary, HttpChannel channel, HttpOutput.Interceptor next,boolean syncFlush)
     {
         this(factory,vary,channel.getHttpConfiguration().getOutputBufferSize(),channel,next,syncFlush);
     }
-    
+
     public GzipHttpOutputInterceptor(GzipFactory factory, HttpField vary, int bufferSize, HttpChannel channel, HttpOutput.Interceptor next,boolean syncFlush)
     {
         _factory=factory;
@@ -85,14 +86,14 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
     {
         return _interceptor;
     }
-    
+
     @Override
     public boolean isOptimizedForDirectBuffers()
     {
         return false; // No point as deflator is in user space.
     }
-    
-    
+
+
     @Override
     public void write(ByteBuffer content, boolean complete, Callback callback)
     {
@@ -101,11 +102,11 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
             case MIGHT_COMPRESS:
                 commit(content,complete,callback);
                 break;
-                
+
             case NOT_COMPRESSING:
                 _interceptor.write(content, complete, callback);
                 return;
-                
+
             case COMMITTING:
                 callback.failed(new WritePendingException());
                 break;
@@ -124,21 +125,21 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
     {
         int i=_buffer.limit();
         _buffer.limit(i+8);
-        
+
         int v=(int)_crc.getValue();
         _buffer.put(i++,(byte)(v & 0xFF));
         _buffer.put(i++,(byte)((v>>>8) & 0xFF));
         _buffer.put(i++,(byte)((v>>>16) & 0xFF));
         _buffer.put(i++,(byte)((v>>>24) & 0xFF));
-        
+
         v=_deflater.getTotalIn();
         _buffer.put(i++,(byte)(v & 0xFF));
         _buffer.put(i++,(byte)((v>>>8) & 0xFF));
         _buffer.put(i++,(byte)((v>>>16) & 0xFF));
         _buffer.put(i++,(byte)((v>>>24) & 0xFF));
     }
-    
-    
+
+
     private void gzip(ByteBuffer content, boolean complete, final Callback callback)
     {
         if (content.hasRemaining() || complete)
@@ -150,7 +151,8 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
     protected void commit(ByteBuffer content, boolean complete, Callback callback)
     {
         // Are we excluding because of status?
-        int sc = _channel.getResponse().getStatus();
+        Response response = _channel.getResponse();
+        int sc = response.getStatus();
         if (sc>0 && (sc<200 || sc==204 || sc==205 || sc>=300))
         {
             LOG.debug("{} exclude by status {}",this,sc);
@@ -158,9 +160,9 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
             _interceptor.write(content, complete, callback);
             return;
         }
-        
+
         // Are we excluding because of mime-type?
-        String ct = _channel.getResponse().getContentType();
+        String ct = response.getContentType();
         if (ct!=null)
         {
             ct=MimeTypes.getContentTypeWithoutCharset(ct);
@@ -172,9 +174,9 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
                 return;
             }
         }
-        
+
         // Has the Content-Encoding header already been set?
-        String ce=_channel.getResponse().getHeader("Content-Encoding");
+        String ce=response.getHeader("Content-Encoding");
         if (ce != null)
         {
             LOG.debug("{} exclude by content-encoding {}",this,ce);
@@ -182,20 +184,21 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
             _interceptor.write(content, complete, callback);
             return;
         }
-        
+
         // Are we the thread that commits?
         if (_state.compareAndSet(GZState.MIGHT_COMPRESS,GZState.COMMITTING))
         {
             // We are varying the response due to accept encoding header.
-            HttpFields fields = _channel.getResponse().getHttpFields();
-            fields.add(_vary);
+            HttpFields fields = response.getHttpFields();
+            if (_vary != null)
+                fields.add(_vary);
 
-            long content_length = _channel.getResponse().getContentLength();
+            long content_length = response.getContentLength();
             if (content_length<0 && complete)
                 content_length=content.remaining();
-            
+
             _deflater = _factory.getDeflater(_channel.getRequest(),content_length);
-            
+
             if (_deflater==null)
             {
                 LOG.debug("{} exclude no deflater",this);
@@ -210,7 +213,7 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
             BufferUtil.fill(_buffer,GZIP_HEADER,0,GZIP_HEADER.length);
 
             // Adjust headers
-            _channel.getResponse().setContentLength(-1);
+            response.setContentLength(-1);
             String etag=fields.get(HttpHeader.ETAG);
             if (etag!=null)
             {
@@ -218,10 +221,10 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
                 etag=(etag.charAt(end)=='"')?etag.substring(0,end)+GzipHttpContent.ETAG_GZIP+'"':etag+GzipHttpContent.ETAG_GZIP;
                 fields.put(HttpHeader.ETAG,etag);
             }
-            
+
             LOG.debug("{} compressing {}",this,_deflater);
             _state.set(GZState.COMPRESSING);
-            
+
             gzip(content,complete,callback);
         }
         else
@@ -268,14 +271,14 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
             }
         }
     }
-    
+
     public boolean mightCompress()
     {
         return _state.get()==GZState.MIGHT_COMPRESS;
     }
-    
+
     private class GzipBufferCB extends IteratingNestedCallback
-    {        
+    {
         private ByteBuffer _copy;
         private final ByteBuffer _content;
         private final boolean _last;
@@ -291,11 +294,11 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
         {
             if (_deflater==null)
                 return Action.SUCCEEDED;
-                
+
             if (_deflater.needsInput())
-            {                
+            {
                 if (BufferUtil.isEmpty(_content))
-                {                    
+                {
                     if (_deflater.finished())
                     {
                         _factory.recycle(_deflater);
@@ -309,12 +312,12 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
                         }
                         return Action.SUCCEEDED;
                     }
-                    
+
                     if (!_last)
                     {
                         return Action.SUCCEEDED;
                     }
-                    
+
                     _deflater.finish();
                 }
                 else if (_content.hasArray())
@@ -323,9 +326,9 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
                     int off=_content.arrayOffset()+_content.position();
                     int len=_content.remaining();
                     BufferUtil.clear(_content);
-                    
+
                     _crc.update(array,off,len);
-                    _deflater.setInput(array,off,len);                
+                    _deflater.setInput(array,off,len);
                     if (_last)
                         _deflater.finish();
                 }
@@ -338,13 +341,13 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
                     BufferUtil.flipToFlush(_copy,0);
                     if (took==0)
                         throw new IllegalStateException();
-                   
+
                     byte[] array=_copy.array();
                     int off=_copy.arrayOffset()+_copy.position();
                     int len=_copy.remaining();
 
                     _crc.update(array,off,len);
-                    _deflater.setInput(array,off,len);                
+                    _deflater.setInput(array,off,len);
                     if (_last && BufferUtil.isEmpty(_content))
                         _deflater.finish();
                 }
@@ -359,10 +362,10 @@ public class GzipHttpOutputInterceptor implements HttpOutput.Interceptor
                 _buffer.limit(_buffer.limit()+produced);
             }
             boolean finished=_deflater.finished();
-            
+
             if (finished)
                 addTrailer();
-                
+
             _interceptor.write(_buffer,finished,this);
             return Action.SCHEDULED;
         }

From 195a02fdaaeae47512ea1ee1e3849c8e0fe137c2 Mon Sep 17 00:00:00 2001
From: Joakim Erdfelt 
Date: Tue, 8 Mar 2016 14:27:38 -0700
Subject: [PATCH 19/21] Issue #85 - Expose TLS protocol used for connection in
 SecureRequestCustomizer

---
 .../jetty/server/SecureRequestCustomizer.java       | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/SecureRequestCustomizer.java b/jetty-server/src/main/java/org/eclipse/jetty/server/SecureRequestCustomizer.java
index dc6cf79d5d1..bf36446511a 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/SecureRequestCustomizer.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/SecureRequestCustomizer.java
@@ -48,6 +48,8 @@ public class SecureRequestCustomizer implements HttpConfiguration.Customizer
      */
     public static final String CACHED_INFO_ATTR = CachedInfo.class.getName();
 
+    private String sslSessionAttribute = "org.eclipse.jetty.servlet.request.ssl_session";
+
     @Override
     public void customize(Connector connector, HttpConfiguration channelConfig, Request request)
     {
@@ -118,12 +120,23 @@ public class SecureRequestCustomizer implements HttpConfiguration.Customizer
             request.setAttribute("javax.servlet.request.cipher_suite",cipherSuite);
             request.setAttribute("javax.servlet.request.key_size",keySize);
             request.setAttribute("javax.servlet.request.ssl_session_id", idStr);
+            request.setAttribute(getSslSessionAttribute(), sslSession);
         }
         catch (Exception e)
         {
             LOG.warn(Log.EXCEPTION,e);
         }
     }
+    
+    public void setSslSessionAttribute(String attribute)
+    {
+        this.sslSessionAttribute = attribute;
+    }
+
+    public String getSslSessionAttribute()
+    {
+        return sslSessionAttribute;
+    }
 
     @Override
     public String toString()

From 98448262eac41178d39624e61aca0c3e4c2492a4 Mon Sep 17 00:00:00 2001
From: Joakim Erdfelt 
Date: Tue, 8 Mar 2016 15:14:54 -0700
Subject: [PATCH 20/21] Making servlet context base resource discovery more
 robust

---
 .../server/browser/JsrBrowserDebugTool.java      | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserDebugTool.java b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserDebugTool.java
index ce8d230bbb6..ac5f8410f61 100644
--- a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserDebugTool.java
+++ b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserDebugTool.java
@@ -18,6 +18,11 @@
 
 package org.eclipse.jetty.websocket.jsr356.server.browser;
 
+import java.net.MalformedURLException;
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.util.Objects;
+
 import javax.servlet.ServletException;
 import javax.websocket.DeploymentException;
 
@@ -30,6 +35,7 @@ import org.eclipse.jetty.servlet.ServletContextHandler;
 import org.eclipse.jetty.servlet.ServletHolder;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
+import org.eclipse.jetty.util.resource.Resource;
 import org.eclipse.jetty.websocket.jsr356.server.ServerContainer;
 import org.eclipse.jetty.websocket.jsr356.server.deploy.WebSocketServerContainerInitializer;
 
@@ -78,7 +84,7 @@ public class JsrBrowserDebugTool
         server.join();
     }
 
-    private void setupServer(int port) throws DeploymentException, ServletException
+    private void setupServer(int port) throws DeploymentException, ServletException, URISyntaxException, MalformedURLException
     {
         server = new Server();
         
@@ -89,10 +95,16 @@ public class JsrBrowserDebugTool
         connector.setPort(port);
         server.addConnector(connector);
 
+        String resourcePath = "/jsr-browser-debug-tool/index.html";
+        URL urlStatics = JsrBrowserDebugTool.class.getResource(resourcePath);
+        Objects.requireNonNull(urlStatics,"Unable to find " + resourcePath + " in classpath");
+        String urlBase = urlStatics.toURI().toASCIIString().replaceFirst("/[^/]*$","/");
+
         ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS);
         context.setContextPath("/");
+        context.setBaseResource(Resource.newResource(urlBase));
+
         ServletHolder holder = context.addServlet(DefaultServlet.class,"/");
-        holder.setInitParameter("resourceBase","src/test/resources/jsr-browser-debug-tool");
         holder.setInitParameter("dirAllowed","true");
         server.setHandler(context);
 

From db4b920ae4d2227b9bc73b90d8c973256c334dab Mon Sep 17 00:00:00 2001
From: Joakim Erdfelt 
Date: Tue, 8 Mar 2016 15:15:21 -0700
Subject: [PATCH 21/21] Issue #407 - JSR356 Server WebSocket Sessions no longer
 being tracked

---
 .../jetty/websocket/jsr356/server/ServerContainer.java   | 2 ++
 .../jsr356/server/browser/JsrBrowserSocket.java          | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/ServerContainer.java b/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/ServerContainer.java
index 8d114c37e05..b21344a1163 100644
--- a/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/ServerContainer.java
+++ b/jetty-websocket/javax-websocket-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/ServerContainer.java
@@ -245,12 +245,14 @@ public class ServerContainer extends ClientContainer implements javax.websocket.
     @Override
     public void onSessionClosed(WebSocketSession session)
     {
+        super.onSessionClosed(session);
         webSocketServerFactory.onSessionClosed(session);
     }
 
     @Override
     public void onSessionOpened(WebSocketSession session)
     {
+        super.onSessionOpened(session);
         webSocketServerFactory.onSessionOpened(session);
     }
 }
diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserSocket.java b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserSocket.java
index 3b17bdb3c0e..d724913577a 100644
--- a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserSocket.java
+++ b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/browser/JsrBrowserSocket.java
@@ -23,6 +23,7 @@ import java.text.SimpleDateFormat;
 import java.util.Calendar;
 import java.util.Locale;
 import java.util.Random;
+import java.util.Set;
 
 import javax.websocket.CloseReason;
 import javax.websocket.OnClose;
@@ -129,6 +130,14 @@ public class JsrBrowserSocket
                     {
                         writeMessage("Client Sec-WebSocket-Extensions: " + this.requestedExtensions);
                     }
+
+                    Set openSessions = session.getOpenSessions();
+                    writeMessage("OpenSessions.size() = " + openSessions.size());
+                    int i = 0;
+                    for (Session open : openSessions)
+                    {
+                        writeMessage("  OpenSession[%d] = %s", i++, open);
+                    }
                     break;
                 }
                 case "many":