Merge remote-tracking branch 'origin/jetty-9.4.x'

jetty-documentation/src/main/asciidoc/development/maven/jetty-maven-plugin.adoc

@@ -116,7 +116,7 @@ soLinger;;
 The socket linger time.
 +
 You could instead configure the connectors in a standard link:#jetty-xml-config[jetty xml config file] and put its location into the `jettyXml` parameter.
-Note that since jetty-9.0 it is no longer possible to configure a link:#maven-config-https[https connector] directly in the pom.xml: you need to link:#maven-config-https[use jetty xml config files to do it].
+Note that since Jetty 9.0 it is no longer possible to configure a link:#maven-config-https[https connector] directly in the pom.xml: you need to link:#maven-config-https[use jetty xml config files to do it].
 jettyXml::
 Optional.
 A comma separated list of locations of Jetty xml files to apply in addition to any plugin configuration parameters.
@@ -356,7 +356,7 @@ Any changes you make are immediately reflected in the running instance of Jetty,
 
 ____
 [NOTE]
-As of jetty-9.4.7, when using jetty:run in a multi-module build, it is no longer necessary to build each of the modules that form dependencies of the webapp first.
+As of Jetty 9.4.7, when using jetty:run in a multi-module build, it is no longer necessary to build each of the modules that form dependencies of the webapp first.
 Thus, if your webapp depends on other modules in your project and they are present in the reactor at the same time, jetty will use their compiled classes rather than their jar files from your local maven repository.
 ____
 
@@ -675,11 +675,11 @@ ____
 [[jetty-run-distro-goal]]
 ==== jetty:run-distro
 
-Introduced in jetty-9.4.8, this goal allows you to execute your unassembled webapp in a local distribution of jetty.
+Introduced in Jetty 9.4.8, this goal allows you to execute your unassembled webapp in a local distribution of Jetty.
 This can be useful if your webapp requires a highly customized environment in which to run.
 If your webapp is designed to run in the jetty distribution in production, then this goal is the closest approximation to that environment.
 
-Similar to the jetty:run-forked goal, this goal will fork a child process in which to execute your webapp in the distro.
+Similar to the `jetty:run-forked` goal, this goal will fork a child process in which to execute your webapp in the distro.
 
 ===== Configuration
 
@@ -719,7 +719,7 @@ Only applicable if `waitForChild` is `false`.
 
 ____
 [NOTE]
-Use the `modules` parameter to configure the jetty distribution appropriately rather than using jetty artifacts as `plugin dependencies`.
+Use the `modules` parameter to configure the Jetty distribution appropriately rather than using jetty artifacts as `plugin dependencies`.
 ____
 
 The following `jetty:run` parameters are *NOT* applicable to this goal:
@@ -772,6 +772,7 @@ Here's an example of using the configuration parameters:
       <modules>
        <module>apache-jsp</module>
        <module>apache-jstl</module>
+       <module>jmx</module>
       </modules>
       <jettyProperties>
         <jettyProperty>jetty.server.dumpAfterStart=true</jettyProperty>
@@ -788,6 +789,13 @@ Here's an example of using the configuration parameters:
 
 ----
 
+____
+[NOTE]
+When defining modules for this goal, use the standard link:#startup-modules[Jetty module] names and not the name of the related Jetty sub-project.
+For example, in the configuration above support for JMX is configured by adding the `jmx` Jetty module and not the `jetty-jmx` sub-project.
+____
+
+
 To deploy your unassembled web app to jetty running as a local distribution:
 
 [source, screen, subs="{sub-order}"]

jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java

@@ -314,9 +314,10 @@ public class HttpParser
     }
 
     /* ------------------------------------------------------------------------------- */
-    protected String legacyString(String orig, String cached)
+    protected String caseInsensitiveHeader(String orig, String normative)
     {                   
-        return (_compliance!=LEGACY || orig.equals(cached) || complianceViolation(RFC2616,"case sensitive"))?cached:orig;
+        return (_compliance!=LEGACY || orig.equals(normative) || complianceViolation(RFC2616,"https://tools.ietf.org/html/rfc2616#section-4.2 case sensitive header: "+orig))
+                ?normative:orig;
     }
     
     /* ------------------------------------------------------------------------------- */
@@ -629,9 +630,27 @@ public class HttpParser
                     {
                         _length=_string.length();
                         _methodString=takeString();
+
+                        // TODO #1966 This cache lookup is case insensitive when it should be case sensitive by RFC2616, RFC7230
                         HttpMethod method=HttpMethod.CACHE.get(_methodString);
                         if (method!=null)
-                            _methodString=legacyString(_methodString,method.asString());
+                        {
+                            switch(_compliance)
+                            {
+                                case LEGACY:
+                                    // Legacy correctly allows case sensitive header;
+                                    break;
+
+                                case RFC2616:
+                                case RFC7230:
+                                    if (!method.asString().equals(_methodString) && _complianceHandler!=null)
+                                        _complianceHandler.onComplianceViolation(_compliance,HttpCompliance.LEGACY,
+                                                "https://tools.ietf.org/html/rfc7230#section-3.1.1 case insensitive method "+_methodString);
+                                    // TODO Good to used cached version for faster equals checking, but breaks case sensitivity because cache is insensitive
+                                    _methodString = method.asString();  
+                                    break;
+                            }                       
+                        }
                         setState(State.SPACE1);
                     }
                     else if (b < SPACE)
@@ -732,7 +751,7 @@ public class HttpParser
                     else if (b < HttpTokens.SPACE && b>=0)
                     {
                         // HTTP/0.9
-                        if (complianceViolation(RFC7230,"HTTP/0.9"))
+                        if (complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#appendix-A.2 HTTP/0.9"))
                             throw new BadMessageException("HTTP/0.9 not supported");
                         handle=_requestHandler.startRequest(_methodString,_uri.toString(), HttpVersion.HTTP_0_9);
                         setState(State.END);
@@ -799,7 +818,7 @@ public class HttpParser
                         else
                         {
                             // HTTP/0.9
-                            if (complianceViolation(RFC7230,"HTTP/0.9"))
+                            if (complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#appendix-A.2 HTTP/0.9"))
                                 throw new BadMessageException("HTTP/0.9 not supported");
 
                             handle=_requestHandler.startRequest(_methodString,_uri.toString(), HttpVersion.HTTP_0_9);
@@ -917,7 +936,7 @@ public class HttpParser
                         _host=true;
                         if (!(_field instanceof HostPortHttpField) && _valueString!=null && !_valueString.isEmpty())
                         {
-                            _field=new HostPortHttpField(_header,legacyString(_headerString,_header.asString()),_valueString);
+                            _field=new HostPortHttpField(_header,caseInsensitiveHeader(_headerString,_header.asString()),_valueString);
                             add_to_connection_trie=_fieldCache!=null;
                         }
                       break;
@@ -946,7 +965,7 @@ public class HttpParser
                 if (add_to_connection_trie && !_fieldCache.isFull() && _header!=null && _valueString!=null)
                 {
                     if (_field==null)
-                        _field=new HttpField(_header,legacyString(_headerString,_header.asString()),_valueString);
+                        _field=new HttpField(_header,caseInsensitiveHeader(_headerString,_header.asString()),_valueString);
                     _fieldCache.put(_field);
                 }
             }
@@ -1014,7 +1033,7 @@ public class HttpParser
                         case HttpTokens.SPACE:
                         case HttpTokens.TAB:
                         {
-                            if (complianceViolation(RFC7230,"header folding"))
+                            if (complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#section-3.2.4 folding"))
                                 throw new BadMessageException(HttpStatus.BAD_REQUEST_400,"Header Folding");
 
                             // header value without name - continuation?
@@ -1137,13 +1156,13 @@ public class HttpParser
                                     {
                                         // Have to get the fields exactly from the buffer to match case
                                         String fn=field.getName();
-                                        n=legacyString(BufferUtil.toString(buffer,buffer.position()-1,fn.length(),StandardCharsets.US_ASCII),fn);
+                                        n=caseInsensitiveHeader(BufferUtil.toString(buffer,buffer.position()-1,fn.length(),StandardCharsets.US_ASCII),fn);
                                         String fv=field.getValue();
                                         if (fv==null)
                                             v=null;
                                         else
                                         {
-                                            v=legacyString(BufferUtil.toString(buffer,buffer.position()+fn.length()+1,fv.length(),StandardCharsets.ISO_8859_1),fv);
+                                            v=caseInsensitiveHeader(BufferUtil.toString(buffer,buffer.position()+fn.length()+1,fv.length(),StandardCharsets.ISO_8859_1),fv);
                                             field=new HttpField(field.getHeader(),n,v);
                                         }
                                     }
@@ -1236,7 +1255,7 @@ public class HttpParser
                         break;
                     }
                     
-                    if (b==HttpTokens.LINE_FEED && !complianceViolation(RFC7230,"name only header"))
+                    if (b==HttpTokens.LINE_FEED && !complianceViolation(RFC7230,"https://tools.ietf.org/html/rfc7230#section-3.2 No colon"))
                     {
                         if (_headerString==null)
                         {

jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java

@@ -350,7 +350,7 @@ public class HttpParserTest
     }
 
     @Test
-    public void testNoColon2616() throws Exception
+    public void testNoColonLegacy() throws Exception
     {
         ByteBuffer buffer = BufferUtil.toBuffer(
                 "GET / HTTP/1.0\r\n" +
@@ -360,7 +360,7 @@ public class HttpParserTest
                         "\r\n");
 
         HttpParser.RequestHandler handler = new Handler();
-        HttpParser parser = new HttpParser(handler,HttpCompliance.RFC2616);
+        HttpParser parser = new HttpParser(handler,HttpCompliance.LEGACY);
         parseAll(parser, buffer);
 
         Assert.assertTrue(_headerCompleted);
@@ -375,7 +375,7 @@ public class HttpParserTest
         Assert.assertEquals("Other", _hdr[2]);
         Assert.assertEquals("value", _val[2]);
         Assert.assertEquals(2, _headers);
-        Assert.assertThat(_complianceViolation, Matchers.containsString("name only"));
+        Assert.assertThat(_complianceViolation, Matchers.containsString("No colon"));
     }
     
     @Test
@@ -674,10 +674,42 @@ public class HttpParserTest
     }
 
     @Test
-    public void testCaseInsensitive() throws Exception
+    public void testCaseSensitiveMethod() throws Exception
     {
         ByteBuffer buffer = BufferUtil.toBuffer(
-                "get / http/1.0\r\n" +
+                "gEt / http/1.0\r\n" +
+                "Host: localhost\r\n" +
+                "Connection: close\r\n" +
+                "\r\n");
+        HttpParser.RequestHandler handler = new Handler();
+        HttpParser parser = new HttpParser(handler, -1, HttpCompliance.RFC7230);
+        parseAll(parser, buffer);
+        Assert.assertNull(_bad);
+        Assert.assertEquals("GET", _methodOrVersion);
+        Assert.assertThat(_complianceViolation, Matchers.containsString("case insensitive method gEt"));
+    }
+
+    @Test
+    public void testCaseSensitiveMethodLegacy() throws Exception
+    {
+        ByteBuffer buffer = BufferUtil.toBuffer(
+                "gEt / http/1.0\r\n" +
+                "Host: localhost\r\n" +
+                "Connection: close\r\n" +
+                "\r\n");
+        HttpParser.RequestHandler handler = new Handler();
+        HttpParser parser = new HttpParser(handler, -1, HttpCompliance.LEGACY);
+        parseAll(parser, buffer);
+        Assert.assertNull(_bad);
+        Assert.assertEquals("gEt", _methodOrVersion);
+        Assert.assertNull(_complianceViolation);
+    }
+
+    @Test
+    public void testCaseInsensitiveHeader() throws Exception
+    {
+        ByteBuffer buffer = BufferUtil.toBuffer(
+                "GET / http/1.0\r\n" +
                         "HOST: localhost\r\n" +
                         "cOnNeCtIoN: ClOsE\r\n" +
                         "\r\n");
@@ -697,10 +729,10 @@ public class HttpParserTest
     }
 
     @Test
-    public void testCaseSensitiveLegacy() throws Exception
+    public void testCaseInSensitiveHeaderLegacy() throws Exception
     {
         ByteBuffer buffer = BufferUtil.toBuffer(
-                "gEt / http/1.0\r\n" +
+                "GET / http/1.0\r\n" +
                         "HOST: localhost\r\n" +
                         "cOnNeCtIoN: ClOsE\r\n" +
                         "\r\n");
@@ -708,7 +740,7 @@ public class HttpParserTest
         HttpParser parser = new HttpParser(handler, -1, HttpCompliance.LEGACY);
         parseAll(parser, buffer);
         Assert.assertNull(_bad);
-        Assert.assertEquals("gEt", _methodOrVersion);
+        Assert.assertEquals("GET", _methodOrVersion);
         Assert.assertEquals("/", _uriOrStatus);
         Assert.assertEquals("HTTP/1.0", _versionOrReason);
         Assert.assertEquals("HOST", _hdr[0]);

jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java

@@ -36,6 +36,7 @@ import java.util.Queue;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executor;
+import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -148,7 +149,20 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable
             selector.wakeup();
     }
 
-    private Runnable processConnect(SelectionKey key, final Connect connect)
+    private void execute(Runnable task)
+    {
+        try
+        {
+            _selectorManager.execute(task);
+        }
+        catch (RejectedExecutionException x)
+        {
+            if (task instanceof Closeable)
+                closeNoExceptions((Closeable)task);
+        }
+    }
+
+    private void processConnect(SelectionKey key, final Connect connect)
     {
         SelectableChannel channel = key.channel();
         try
@@ -162,7 +176,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable
                 if (connect.timeout.cancel())
                 {
                     key.interestOps(0);
-                    return new CreateEndPoint(channel, key)
+                    execute(new CreateEndPoint(channel, key)
                     {
                         @Override
                         protected void failed(Throwable failure)
@@ -170,7 +184,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable
                             super.failed(failure);
                             connect.failed(failure);
                         }
-                    };
+                    });
                 }
                 else
                 {
@@ -185,7 +199,6 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable
         catch (Throwable x)
         {
             connect.failed(x);
-            return null;
         }
     }
 
@@ -217,7 +230,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable
 
     public void destroyEndPoint(final EndPoint endPoint)
     {
-        submit(new DestroyEndPoint(endPoint));
+        execute(new DestroyEndPoint(endPoint));
     }
 
     private int getActionSize()
@@ -424,9 +437,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable
                         }
                         else if (key.isConnectable())
                         {
-                            Runnable task = processConnect(key, (Connect)attachment);
+                            processConnect(key, (Connect)attachment);
-                            if (task != null)
-                                return task;
                         }
                         else
                         {
@@ -621,7 +632,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable
             try
             {
                 final SelectionKey key = channel.register(_selector, 0, attachment);
-                submit(new CreateEndPoint(channel, key));
+                execute(new CreateEndPoint(channel, key));
             }
             catch (Throwable x)
             {
@@ -631,7 +642,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable
         }
     }
 
-    private class CreateEndPoint implements Runnable, Invocable, Closeable
+    private class CreateEndPoint implements Runnable, Closeable
     {
         private final SelectableChannel channel;
         private final SelectionKey key;
@@ -823,7 +834,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable
         }
     }
 
-    private class DestroyEndPoint implements Runnable, Invocable, Closeable
+    private class DestroyEndPoint implements Runnable, Closeable
     {
         private final EndPoint endPoint;
 

jetty-server/src/main/java/org/eclipse/jetty/server/AbstractNCSARequestLog.java

@@ -127,8 +127,9 @@ public abstract class AbstractNCSARequestLog extends AbstractLifeCycle implement
 
             buf.append(addr);
             buf.append(" - ");
-            Authentication authentication = request.getAuthentication();
+            
-            append(buf,(authentication instanceof Authentication.User)?((Authentication.User)authentication).getUserIdentity().getUserPrincipal().getName():null);
+            String auth = getAuthentication(request);
+            append(buf,auth==null?"-":auth);
 
             buf.append(" [");
             if (_logDateCache != null)
@@ -221,6 +222,23 @@ public abstract class AbstractNCSARequestLog extends AbstractLifeCycle implement
         }
     }
     
+    /**
+     * Extract the user authentication
+     * @param request The request to extract from
+     * @return The string to log for authenticated user.
+     */
+    protected String getAuthentication(Request request)
+    {
+        Authentication authentication = request.getAuthentication();
+        
+        if (authentication instanceof Authentication.User)
+            return ((Authentication.User)authentication).getUserIdentity().getUserPrincipal().getName();
+        
+        // TODO extract the user name if it is Authentication.Deferred and return as '?username'
+        
+        return null;
+    }
+
     /**
      * Writes extended request and response information to the output stream.
      *

jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java

@@ -318,8 +318,13 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
                 {
                     case TERMINATED:
                     case WAIT:
+                        // break loop without calling unhandle
                         break loop;
                         
+                    case NOOP:
+                        // do nothing other than call unhandle
+                        break;
+
                     case DISPATCH:
                     {
                         if (!_request.hasMetaData())

jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java

@@ -73,6 +73,7 @@ public class HttpChannelState
      */
     public enum Action
     {
+        NOOP,             // No action 
         DISPATCH,         // handle a normal request dispatch
         ASYNC_DISPATCH,   // handle an async request dispatch
         ERROR_DISPATCH,   // handle a normal error
@@ -243,6 +244,8 @@ public class HttpChannelState
                         case IDLE:
                         case REGISTERED:
                             break;
+                        default:
+                            throw new IllegalStateException(getStatusStringLocked());
                     }
 
                     if (_asyncWritePossible)
@@ -269,14 +272,13 @@ public class HttpChannelState
                         case STARTED:
                         case EXPIRING:
                         case ERRORING:
-                            return Action.WAIT;
+                            _state=State.ASYNC_WAIT;
+                            return Action.NOOP;
                         case NOT_ASYNC:
-                            break;
                         default:
                             throw new IllegalStateException(getStatusStringLocked());
                     }
 
-                    return Action.WAIT;
 
                 case ASYNC_ERROR:
                     return Action.ASYNC_ERROR;
@@ -408,6 +410,7 @@ public class HttpChannelState
                 case DISPATCHED:
                 case ASYNC_IO:
                 case ASYNC_ERROR:
+                case ASYNC_WAIT:
                     break;
 
                 default:

jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java

@@ -46,6 +46,11 @@ import javax.servlet.SessionTrackingMode;
 import javax.servlet.descriptor.JspConfigDescriptor;
 import javax.servlet.descriptor.JspPropertyGroupDescriptor;
 import javax.servlet.descriptor.TaglibDescriptor;
+import javax.servlet.http.HttpSessionActivationListener;
+import javax.servlet.http.HttpSessionAttributeListener;
+import javax.servlet.http.HttpSessionBindingListener;
+import javax.servlet.http.HttpSessionIdListener;
+import javax.servlet.http.HttpSessionListener;
 
 import org.eclipse.jetty.security.ConstraintAware;
 import org.eclipse.jetty.security.ConstraintMapping;
@@ -178,6 +183,27 @@ public class ServletContextHandler extends ContextHandler
             ((HandlerCollection)parent).addHandler(this);
     }
 
+    /* ------------------------------------------------------------ */
+    /** Add EventListener
+     * Adds an EventListener to the list. @see org.eclipse.jetty.server.handler.ContextHandler#addEventListener().
+     * Also adds any listeners that are session related to the SessionHandler.
+     * @param listener the listener to add
+     */
+    @Override
+    public void addEventListener(EventListener listener)
+    {
+        super.addEventListener(listener);
+        if ((listener instanceof HttpSessionActivationListener)
+            || (listener instanceof HttpSessionAttributeListener)
+            || (listener instanceof HttpSessionBindingListener)
+            || (listener instanceof HttpSessionListener)
+            || (listener instanceof HttpSessionIdListener))
+        {
+            if (_sessionHandler!=null)
+                _sessionHandler.addEventListener(listener);
+        }
+    }
+    
     @Override
     public void setHandler(Handler handler)
     {

jetty-servlet/src/test/java/org/eclipse/jetty/servlet/AsyncServletIOTest.java

@@ -22,7 +22,6 @@ import static java.nio.charset.StandardCharsets.ISO_8859_1;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
-import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.startsWith;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
@@ -41,6 +40,9 @@ import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+import java.util.function.UnaryOperator;
 
 import javax.servlet.AsyncContext;
 import javax.servlet.AsyncEvent;
@@ -63,6 +65,7 @@ import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
+import org.eclipse.jetty.util.thread.QueuedThreadPool;
 import org.hamcrest.Matchers;
 import org.junit.After;
 import org.junit.Assert;
@@ -77,14 +80,19 @@ public class AsyncServletIOTest
     protected AsyncIOServlet2 _servlet2=new AsyncIOServlet2();
     protected AsyncIOServlet3 _servlet3=new AsyncIOServlet3();
     protected AsyncIOServlet4 _servlet4=new AsyncIOServlet4();
+    protected StolenAsyncReadServlet _servletStolenAsyncRead=new StolenAsyncReadServlet();
     protected int _port;
-    protected Server _server = new Server();
+    protected WrappingQTP _wQTP;
+    protected Server _server;
     protected ServletHandler _servletHandler;
     protected ServerConnector _connector;
 
     @Before
     public void setUp() throws Exception
     {
+        _wQTP = new WrappingQTP();
+        _server = new Server(_wQTP);
+        
         HttpConfiguration http_config = new HttpConfiguration();
         http_config.setOutputBufferSize(4096);
         _connector = new ServerConnector(_server,new HttpConnectionFactory(http_config));
@@ -113,6 +121,10 @@ public class AsyncServletIOTest
         holder4.setAsyncSupported(true);
         _servletHandler.addServletWithMapping(holder4,"/path4/*");
         
+        ServletHolder holder5=new ServletHolder(_servletStolenAsyncRead);
+        holder5.setAsyncSupported(true);
+        _servletHandler.addServletWithMapping(holder5,"/stolen/*");
+        
         _server.start();
         _port=_connector.getLocalPort();
 
@@ -788,4 +800,178 @@ public class AsyncServletIOTest
     }
     
 
+    @Test
+    public void testStolenAsyncRead() throws Exception
+    {
+        StringBuilder request = new StringBuilder(512);
+        request.append("POST /ctx/stolen/info HTTP/1.1\r\n")
+        .append("Host: localhost\r\n")
+        .append("Content-Type: text/plain\r\n")
+        .append("Content-Length: 2\r\n")
+        .append("\r\n")
+        .append("1");
+        int port=_port;
+        try (Socket socket = new Socket("localhost",port))
+        {
+            socket.setSoTimeout(10000);
+            OutputStream out = socket.getOutputStream();
+            out.write(request.toString().getBytes(ISO_8859_1));
+            out.flush();
+            
+            // wait until server is ready
+            _servletStolenAsyncRead.ready.await();
+            final CountDownLatch wait = new CountDownLatch(1);
+            
+            // Stop any dispatches until we want them
+            UnaryOperator<Runnable> old = _wQTP.wrapper.getAndSet(r->
+                ()->
+                {
+                    try
+                    {
+                        wait.await();
+                        r.run();
+                    }
+                    catch (InterruptedException e)
+                    {
+                        e.printStackTrace();
+                    }
+                }
+            );
+
+            // We are an unrelated thread, let's mess with the input stream
+            ServletInputStream sin = _servletStolenAsyncRead.listener.in;
+            sin.setReadListener(_servletStolenAsyncRead.listener);
+            // thread should be dispatched to handle, but held by our wQTP wait.
+            
+            // Let's steal our read
+            Assert.assertTrue(sin.isReady());
+            Assert.assertThat(sin.read(),Matchers.is((int)'1'));
+            Assert.assertFalse(sin.isReady());
+            
+            // let the ODA call go
+            _wQTP.wrapper.set(old);
+            wait.countDown();
+            
+            // ODA should not be called
+            Assert.assertFalse(_servletStolenAsyncRead.oda.await(500,TimeUnit.MILLISECONDS));
+
+            // Send some more data
+            out.write((int)'2');
+            out.flush();
+
+            // ODA should now be called!!
+            Assert.assertTrue(_servletStolenAsyncRead.oda.await(500,TimeUnit.MILLISECONDS));
+            
+            // We can not read some more
+            Assert.assertTrue(sin.isReady());
+            Assert.assertThat(sin.read(),Matchers.is((int)'2'));
+
+            // read EOF
+            Assert.assertTrue(sin.isReady());
+            Assert.assertThat(sin.read(),Matchers.is(-1));
+            
+            BufferedReader in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
+            
+            // response line
+            String line = in.readLine();
+            LOG.debug("response-line: "+line);
+            Assert.assertThat(line,startsWith("HTTP/1.1 200 OK"));
+            
+            // Skip headers
+            while (line!=null)
+            {
+                line = in.readLine();
+                LOG.debug("header-line: "+line);
+                if (line.length()==0)
+                    break;
+            }
+        }
+
+        assertTrue(_servletStolenAsyncRead.completed.await(5, TimeUnit.SECONDS));     
+    }
+
+    @SuppressWarnings("serial")
+    public class StolenAsyncReadServlet extends HttpServlet
+    {
+        public CountDownLatch ready = new CountDownLatch(1);
+        public CountDownLatch oda = new CountDownLatch(1);
+        public CountDownLatch completed = new CountDownLatch(1);
+        public volatile StealingListener listener;
+        
+        @Override
+        public void doPost(final HttpServletRequest request, final HttpServletResponse response) throws IOException
+        {
+            listener = new StealingListener(request);
+            ready.countDown();
+        }
+
+        public class StealingListener implements ReadListener, AsyncListener
+        {
+            final HttpServletRequest request;
+            final ServletInputStream in;
+            final AsyncContext asyncContext;
+
+            StealingListener(HttpServletRequest request) throws IOException
+            {
+                asyncContext = request.startAsync();
+                asyncContext.setTimeout(10000L);
+                asyncContext.addListener(this);
+                this.request=request;
+                in = request.getInputStream();
+            }
+
+            @Override
+            public void onDataAvailable()
+            {
+                oda.countDown();
+            }
+
+            @Override
+            public void onAllDataRead() throws IOException
+            {
+                asyncContext.complete();
+            }
+
+            @Override
+            public void onError(final Throwable t)
+            {
+                t.printStackTrace();
+                asyncContext.complete();
+            }
+
+            @Override
+            public void onComplete(final AsyncEvent event)
+            {
+                completed.countDown();
+            }
+
+            @Override
+            public void onTimeout(final AsyncEvent event)
+            {
+                asyncContext.complete();
+            }
+
+            @Override
+            public void onError(final AsyncEvent event)
+            {
+                asyncContext.complete();
+            }
+
+            @Override
+            public void onStartAsync(AsyncEvent event)
+            {
+            }
+        }
+    }
+    
+    private class WrappingQTP extends QueuedThreadPool
+    {
+        AtomicReference<UnaryOperator<Runnable>> wrapper = new AtomicReference<>(UnaryOperator.identity());
+        
+        @Override
+        public void execute(Runnable job)
+        {
+            super.execute(wrapper.get().apply(job));
+        }
+    }
 }

jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ComplianceViolations2616Test.java

@@ -147,8 +147,7 @@ public class ComplianceViolations2616Test
 
         String response = connector.getResponse(req1.toString());
         assertThat("Response status", response, containsString("HTTP/1.1 200 OK"));
-        assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: name only header"));
+        assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: https://tools.ietf.org/html/rfc7230#section-3.2 No colon"));   
-
         assertThat("Response body", response, containsString("[Name] = []"));
     }
 
@@ -165,8 +164,7 @@ public class ComplianceViolations2616Test
 
         String response = connector.getResponse(req1.toString());
         assertThat("Response status", response, containsString("HTTP/1.1 200"));
-        assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: name only header"));
+        assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: https://tools.ietf.org/html/rfc7230#section-3.2 No colon"));        
-
         assertThat("Response body", response, containsString("[Name] = []"));
     }
 
@@ -184,8 +182,7 @@ public class ComplianceViolations2616Test
 
         String response = connector.getResponse(req1.toString());
         assertThat("Response status", response, containsString("HTTP/1.1 200"));
-        assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: header folding"));
+        assertThat("Response headers", response, containsString("X-Http-Violation-0: RFC2616<RFC7230: https://tools.ietf.org/html/rfc7230#section-3.2.4 folding"));        
-
         assertThat("Response body", response, containsString("[Name] = [Some Value]"));
     }
 }

jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java

@@ -22,6 +22,7 @@ import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
@@ -38,6 +39,10 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSessionAttributeListener;
+import javax.servlet.http.HttpSessionBindingEvent;
+import javax.servlet.http.HttpSessionEvent;
+import javax.servlet.http.HttpSessionListener;
 
 import org.eclipse.jetty.security.ConstraintSecurityHandler;
 import org.eclipse.jetty.security.SecurityHandler;
@@ -68,6 +73,57 @@ public class ServletContextHandlerTest
 
     private static final AtomicInteger __testServlets = new AtomicInteger();
     
+    public static class MySessionHandler extends SessionHandler
+    {
+        public void checkSessionListeners (int size)
+        {
+            assertNotNull(_sessionListeners);
+            assertEquals(size, _sessionListeners.size());
+        }
+        
+        public void checkSessionAttributeListeners(int size)
+        {
+            assertNotNull(_sessionAttributeListeners);
+            assertEquals(size, _sessionAttributeListeners.size());
+        }
+        
+        public void checkSessionIdListeners(int size)
+        {
+            assertNotNull(_sessionIdListeners);
+            assertEquals(size, _sessionIdListeners.size());
+        }
+    }
+    
+    public static class MyTestSessionListener implements HttpSessionAttributeListener, HttpSessionListener
+    {
+
+        @Override
+        public void sessionCreated(HttpSessionEvent se)
+        {
+        }
+
+        @Override
+        public void sessionDestroyed(HttpSessionEvent se)
+        {
+        }
+
+        @Override
+        public void attributeAdded(HttpSessionBindingEvent event)
+        {
+        }
+
+        @Override
+        public void attributeRemoved(HttpSessionBindingEvent event)
+        {
+        }
+
+        @Override
+        public void attributeReplaced(HttpSessionBindingEvent event)
+        {            
+        }
+        
+    }
+    
     @Before
     public void createServer()
     {
@@ -85,6 +141,24 @@ public class ServletContextHandlerTest
         _server.join();
     }
     
+    @Test
+    public void testAddSessionListener() throws Exception
+    {
+        ContextHandlerCollection contexts = new ContextHandlerCollection();
+        _server.setHandler(contexts);
+
+        ServletContextHandler root = new ServletContextHandler(contexts,"/",ServletContextHandler.SESSIONS);
+
+        MySessionHandler sessions = new MySessionHandler(); 
+        root.setSessionHandler(sessions);
+        assertNotNull(sessions);
+        
+        root.addEventListener(new MyTestSessionListener());
+        sessions.checkSessionAttributeListeners(1);
+        sessions.checkSessionIdListeners(0);
+        sessions.checkSessionListeners(1);
+    }
+
     @Test
     public void testFindContainer() throws Exception
     {

jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java

@@ -1098,25 +1098,7 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL
         super.setEventListeners(eventListeners);
     }
 
-    /* ------------------------------------------------------------ */
+
-    /** Add EventListener
-     * Convenience method that calls {@link #setEventListeners(EventListener[])}
-     * @param listener the listener to add
-     */
-    @Override
-    public void addEventListener(EventListener listener)
-    {
-        super.addEventListener(listener);
-        if ((listener instanceof HttpSessionActivationListener)
-            || (listener instanceof HttpSessionAttributeListener)
-            || (listener instanceof HttpSessionBindingListener)
-            || (listener instanceof HttpSessionListener)
-            || (listener instanceof HttpSessionIdListener))
-        {
-            if (_sessionHandler!=null)
-                _sessionHandler.addEventListener(listener);
-        }
-    }
     
     @Override
     public void removeEventListener(EventListener listener)

jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java

@@ -30,6 +30,7 @@ import java.io.IOException;
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.List;
 import java.util.stream.Collectors;
 
@@ -40,6 +41,8 @@ import javax.servlet.ServletContextListener;
 import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpSessionEvent;
+import javax.servlet.http.HttpSessionListener;
 
 import org.eclipse.jetty.server.LocalConnector;
 import org.eclipse.jetty.server.Server;
@@ -58,6 +61,44 @@ import org.junit.Test;
 
 public class WebAppContextTest
 {
+    public class MySessionListener implements HttpSessionListener
+    {
+
+        @Override
+        public void sessionCreated(HttpSessionEvent se)
+        {
+            // TODO Auto-generated method stub
+            
+        }
+
+        @Override
+        public void sessionDestroyed(HttpSessionEvent se)
+        {
+            // TODO Auto-generated method stub
+            
+        }
+        
+    }
+    
+    @Test
+    public void testSessionListeners ()
+    throws Exception
+    {
+        Server server = new Server();
+
+        WebAppContext wac = new WebAppContext();
+
+        wac.setServer(server);
+        server.setHandler(wac);
+        wac.addEventListener(new MySessionListener());
+
+        Collection<MySessionListener> listeners = wac.getSessionHandler().getBeans(org.eclipse.jetty.webapp.WebAppContextTest.MySessionListener.class);
+        assertNotNull(listeners);
+        assertEquals(1, listeners.size());
+    }
+    
+    
+    
     @Test
     public void testConfigurationClassesFromDefault ()
     {